Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated: Time & Weather (based on #2395) #2465

Merged
merged 10 commits into from
May 22, 2024

Conversation

kaffolder7
Copy link
Contributor

This PR is based on @sudeepban's initial work in PR #2395.

Changes / additions

  • Adds additional API's (e.g. OpenWeather, OpenWeather One Call API 3.0, Tomorrow.io, & Open-Meteo)
  • Removes AccuWeather API, as there was a legal issue documented by @rohansingh in this Tidbyt forum post
  • Adds additional weather metrics and animates through them based on what is selected via app schema. (Default is still set to displaying only 'wind speed'. More weather metrics may be enabled via schema toggles.)

Preview

Initial app load state (before API key is entered):
CleanShot 2024-05-14 at 23 23 16@2x
Preview when a single weather metric is enabled (e.g. 'wind speed'):
CleanShot 2024-05-14 at 23 29 51@2x
Preview when multiple weather metrics are enabled (e.g. 'wind speed', 'humidity', 'dew point', 'cloud coverage', & 'pressure'):
CleanShot 2024-05-14 at 23 32 29

sudeepban and others added 4 commits April 15, 2024 21:11
Added additional weather APIs for user to choose between (AccuWeather, OpenWeather API 2.5, OpenWeather One Call API 3.0, Tomorrow.io, & Open-Meteo). Also added additional weather metrics and ability to animate through them based on what is selected. (Default is still set to wind speed)
According to the forum comment left here by @rohan (https://discuss.tidbyt.com/t/weather-app/5115/40), we are not able to leverage AccuWeather’s API per any Tidbyt community application. Therefore I am removing this.
@kaffolder7 kaffolder7 requested a review from matslina as a code owner May 15, 2024 03:40
@tidbyt
Copy link

tidbyt bot commented May 15, 2024

⚠️ The automated review process is experimental and likely has bugs. Please bear with us as we iron out the kinks and enable you to ship changes at high velocity 🚀

All Set 🎉

This change is ready to go! To merge your change, simply comment with a :shipit: (:shipit:) emoji on this pull request and our robots will take it from there.

Manual Review Required

Hang tight! A Tidbyt engineer will be by shortly to review your change. Here is what they will be looking for:

Test Details
App Dir All files are in a single app directory
🟡 Modules Usage of http.star requires review
Original Author The original author matches the PR author

@kaffolder7
Copy link
Contributor Author

kaffolder7 commented May 15, 2024

@sudeepban – What are your thoughts on merging this PR in w/ yours? I added additional weather API's (since we cannot use AccuWeather) and added additional functionality as described in the PR's description. Hoping you're ok with how I've expanded upon your initial PR to add to your Time & Temp app. I'm sure some things could be improved, but hopefully this is a decent start. 😉

Also, was wondering if maybe it makes sense to rename this app to be "Time & Weather" or "Time & Current Weather Conditions", or something else to indicate that this update now displays more than just the current time and temperature.

If you're comfortable with these changes, then we can try and get @matslina to approve. 🙂

@sudeepban
Copy link
Contributor

@kaffolder7 that's very cool, nice work! I am definitely supportive of this and am glad to have you carry the torch and hopefully find a way to get this out with @matslina's approval. Appreciate the effort!

@sudeepban
Copy link
Contributor

Also no strong feelings about the name of the app, agree that Time & Temp seems limiting and am fine with a change there.

Confirmed w/ original app author (@sudeepban) and permission given in PR comment.
@kaffolder7 kaffolder7 changed the title Updated: Time & Temp (based on #2395) Updated: Time & Weather (based on #2395) May 15, 2024
@kaffolder7
Copy link
Contributor Author

Also no strong feelings about the name of the app, agree that Time & Temp seems limiting and am fine with a change there.

Thanks for your kind feedback @sudeepban and approval. Let's see if @matslina can get around to reviewing and hopefully accepting/merging this into main so more users can access this on their Tidbyts. :)

I also went ahead and changed the name of the app to better-reflect the functionality of this app.

kaffolder7 and others added 4 commits May 15, 2024 16:27
- Added: National Weather Service (NWS) & Weatherbit APIs
- Cleaned up various items
- Adjusted layout to better align within Tidbyt screen
nMgfdF+AGMzsmgD6BFiZq7TbbynZLVR3Pz6z7poaVKKT1rLJ+MsYAGKWVKuIhuN3
AAAAAElFTkSuQmCC
""",
"foggy.png": """
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can try running exiftool -all= on assets like these to trim down their size by removing EXIF data. On this particular one, it drops from 2.8k to 212 bytes.

@matslina matslina merged commit a1b3663 into tidbyt:main May 22, 2024
2 checks passed
@tidbyt tidbyt locked as resolved and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants