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

Weather widget+app #340

Merged
merged 4 commits into from Apr 22, 2020
Merged

Weather widget+app #340

merged 4 commits into from Apr 22, 2020

Conversation

rigrig
Copy link
Contributor

@rigrig rigrig commented Apr 21, 2020

Based on http://forum.espruino.com/comments/15194626/, where
NebbishHacker did most of the actual work :-)

I also had to update locale, because Gadgetbridge reports temperatures in Kelvin.

rigrig added 2 commits Apr 21, 2020
Because that is what Gadgetbridge sends for the weather
Based on http://forum.espruino.com/comments/15194626/, where
NebbishHacker did most of the actual work :-)
}
}

function chooseIcon(condition) {

Choose a reason for hiding this comment

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

This method should work as long as the condition descriptions match the ones at https://openweathermap.org/weather-conditions, but it's kind of brittle and would break pretty easily if the descriptions are ever changed or localized. Long-term it would be good to see about getting the condition ID from gadgetbridge and choosing the icon based on that.

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Apr 22, 2020

That's really cool, thanks! It's very cool how it'll just work with/without the gadgetbridge widget :)

Please could you remove the locale changes though and just subtract 273.15 before calling locale.temperature?

locale works with a built-in version inside Bangle.js firmware - so if you don't install any language you just get en_GB. So changing locale functions here means that anyone without a language installed would see 290ish degrees C, while with Language it'd be ok. Not to mention the docs not being updated :)

@gfwilliams
Copy link
Member

@gfwilliams gfwilliams commented Apr 22, 2020

Thanks!

@gfwilliams gfwilliams merged commit 899eaee into espruino:master Apr 22, 2020
1 check passed
@rigrig
Copy link
Contributor Author

@rigrig rigrig commented Apr 22, 2020

Hadn't thought of that, I figured there was still time to change it because no other apps used locale.temp yet. Changed it now. (Good thing that en_GB uses °C, or this would be even more tricky)

@rigrig rigrig deleted the weather_app branch Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants