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: Add support for conditions #1904

Closed
wants to merge 1 commit into from

Conversation

vkareh
Copy link
Contributor

@vkareh vkareh commented Nov 3, 2023

Use OpenWeatherMap conditions to determine the correct condition icon. This is more accurate than attempting to calculate the condition based on precipitation and cloud cover, which often yields the wrong condition.

Companion PR to Gadgetbridge: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3407

Copy link

github-actions bot commented Nov 3, 2023

Build size and comparison to main:

Section Size Difference
text 378096B 448B
data 940B 0B
bss 63492B 0B

@JF002
Copy link
Collaborator

JF002 commented Nov 11, 2023

Thanks for your contribution @vkareh !
I'm currently in the process of trying re-design the weather service to fix its current issues and to make the integration easier in companion apps, and using icon ID or condition ID was also one of my ideas!

I'll probably reach out to the Gadgetbridge community to get some feedback about this re-design !

@FintasticMan FintasticMan added enhancement Enhancement to an existing app/feature weather Bugs and PRs related to Weather labels Nov 16, 2023
@vkareh vkareh force-pushed the weather-conditions branch from 91e1b4b to f26ae5d Compare November 23, 2023 18:17
@JF002
Copy link
Collaborator

JF002 commented Nov 26, 2023

I posted a comment on the PR on Gadgetbridge, but received no answer so far... Since I'm redesigning the whole weather feature, I was hoping I could get in touch with Gadgetbridge devs to have their feedback..

@vkareh
Copy link
Contributor Author

vkareh commented Nov 27, 2023

@JF002: right - I think the Gadgetbridge community hangs out in a matrix server, but not sure (I don't really use matrix).

From what I can see in the GB sourcecode, they already use icon IDs for weather conditions based on owm: https://codeberg.org/Freeyourgadget/Gadgetbridge/src/branch/master/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/model/Weather.java#L336-L363

Changing to this model for InfiniTime is not a stretch, and in fact could make the entire weather service a lot leaner.

Use OpenWeatherMap conditions to determine the correct condition icon.
This is more accurate than attempting to calculate the condition based
on precipitation and cloud cover, which often yields the wrong
condition.

Companion PR to Gadgetbridge: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/3407
@JF002
Copy link
Collaborator

JF002 commented Dec 23, 2023

Thanks for your work, @vkareh. However, I decided to rework the weather service so we can fix the issues more easily. The new implementation also supports icons for the weather condition. Let me know what you think of it :)
I'm closing this one in favor of #1924.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing app/feature weather Bugs and PRs related to Weather
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants