Skip to content

[Weather] Add feels-like temperature display #3912

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

RKBoss6
Copy link
Contributor

@RKBoss6 RKBoss6 commented Jun 28, 2025

I did not bump the version on this commit because there are already three other PRs for weather, and I am not certain what version the app will be after all of those.

RKBoss6 added 3 commits June 28, 2025 14:24
Modified the shortcut to show feels like temp, based on @stweedo 's shortcut from their previous PR
@stweedo
Copy link
Contributor

stweedo commented Jun 28, 2025

I just checked your version of the shortcut and it was missing the “mostly cloudy” filter that shows multiple clouds on the bangle. I added your changes as well and this version contains everything. https://www.icloud.com/shortcuts/71f49664b5a44b6cb80703b73af2d8f0

@RKBoss6
Copy link
Contributor Author

RKBoss6 commented Jun 29, 2025

Alright @stweedo thanks!

@thyttan
Copy link
Collaborator

thyttan commented Jun 29, 2025

@Rengyr @\stweedo @\RKBoss6 What do you think of the merge order of these weather PRs?

It seems to me this order would make sense: #3908 #3910 #3911 #3912

Looks good to me, just one thing

Currently, while the "feels like" is contained in the gadgetbridge v2 weather data, it's not really usable without parsing/requesting forecast data as it's not in v1 data. Though it could be added to v1 in gadgetbridge to fix this issue. Otherwise the field will always have unknown value in android.

Though I might have something more to discus on my PR, so it can be done last so it doesn't block other.

Originally posted by @Rengyr in #3908 (comment)

@RKBoss6 I think we should not merge something that adds an non functional field for android users. What do you think we tweak the logic so it only shows for ios users for now?

@Rengyr
Copy link

Rengyr commented Jun 29, 2025

There might be support for "feels like temp" for android users later (and if users will use the new GadgetBridge extended weather data, which is optional setting). So best would be to limit not it based on the platform, but just check if the field exists in the weather data we get. Either we will have value or the field will be undefined, in which case we could just hide it?

@RKBoss6
Copy link
Contributor Author

RKBoss6 commented Jun 29, 2025

@Rengyr I agree. We should hide the field when not in use, or no data is available.

@RKBoss6
Copy link
Contributor Author

RKBoss6 commented Jun 29, 2025

Before merging this, a bug needs to be fix where the icon does not fully clear itself. To achieve proper clearing, do we use g.clearRect?

@thyttan
Copy link
Collaborator

thyttan commented Jun 29, 2025

Before merging this, a bug needs to be fix where the icon does not fully clear itself. To achieve proper clearing, do we use g.clearRect?

Since it uses the layout module it feels like there should be some way native to that for clearing correctly. If not maybe it should be added. I'm unfortunately not good at that module. I should learn it though...

But I'll pass the question along to @bobrippling, do you know?

{type: "txt", font: "6x8", pad: 2, valign: -1, id: "windUnit", label: "km/h"},
]},
{type: "custom", fillx: 1, height: 15, id: "uvDisplay",
render: l => weather.drawIcon(l, l.x+l.w/2, l.y+l.h/2.1, l.w/2.1-10)},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this is the icon that's not clearing? The custom layout will clear it for you if you set bgCol, then so long as you draw in the bounds I'd expect you to be alright

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.

5 participants