Add shorter/simpler weather condition options. #2134
Conversation
|
Build size and comparison to main:
|
JustScott
force-pushed
the
add_shorter_weather_condition_options
branch
from
28be2dc to
94748eb
Compare
| case Pinetime::Controllers::SimpleWeatherService::Icons::CloudShowerHeavy: | ||
| return "Rain"; | ||
| case Pinetime::Controllers::SimpleWeatherService::Icons::CloudSunRain: | ||
| return "Drizzle"; |
There was a problem hiding this comment.
To me drizzle is quite different from rain showers, it implies fine droplets at a low intensity whereas showers can be heavy but brief (more importantly one justifies an umbrella and the other might not!). I think "Showers" might fit better or just "Rain" even
There was a problem hiding this comment.
I agree, "Drizzle" should be changed to "Rain" in my opinion. I'm not a huge fan of "Shower" for the heavier rain case as I feel like that could be misinterpreted as a light shower as well as a heavy shower... perhaps we could change the GetCondition return to "Downpour" for CloudsShowerHeavy, and make it "Pour" for GetSimpleCondition? Just a suggestion, not sure how everyone would interpret "Pour" though.
There was a problem hiding this comment.
These conditions are determined directly from the OpenWeather condition codes (which is also what Gadgetbridge uses). The actual API has a higher level of precision (light drizzle vs heavy shower and so on), but the weather apps compatible with Gadgetbridge for the most part only support a handful of the condition codes in the spec.
I would not recommend deviating from this spec, as it would compound the delta between the weather app and gadgetbridge...
There was a problem hiding this comment.
Thanks for the spec link!
I think "Showers" fits this best having looked at what code 03d is used for
It ranges from "light intensity drizzle" to " heavy intensity shower rain", and I think it'd be better to overstate rather than understate
There was a problem hiding this comment.
It's been a while since I last thought about this PR, and I'm having trouble putting myself into my original mindset. Is the consensus here changing drizzle to showers?
There was a problem hiding this comment.
I would agree with following existing codes unless there's a really good reason to reword.
JustScott
force-pushed
the
add_shorter_weather_condition_options
branch
from
94748eb to
bcc7ca7
Compare
| case Pinetime::Controllers::SimpleWeatherService::Icons::Snow: | ||
| return "Snow"; | ||
| case Pinetime::Controllers::SimpleWeatherService::Icons::Smog: | ||
| return "Mist"; |
There was a problem hiding this comment.
This ("Mist") is the only questionable one. Based on the spec it seems like it should apply to anything that opacifies the atmosphere. However, most apps (and the spec icon itself) just rely on "mist" as a catch-all, so I guess it's fine.
There was a problem hiding this comment.
Yeah I agree it's not great having to describe fog, mist, dust, smoke etc all with one word. I guess you could go for "Turbid" but that's probably overdoing it a bit... mist is probably the best catch all
vkareh
left a comment
vkareh
left a comment
There was a problem hiding this comment.
This PR looks good to me and I like the simplified descriptions (plus they match the spec), but where are we expecting to call GetSimpleCondition? Are there existing or planned use cases for it? In my mind if you want a shorter condition, you can just get the icon.
JustScott
force-pushed
the
add_shorter_weather_condition_options
branch
from
bcc7ca7 to
1b4c981
Compare
|
@JustScott sorry for taking ages to review!! If you're around to take a look at the review feedback that'd be super, hoping to get 1.16 out soon :) |
6 participants
These changes were originally created in PR #2001, but I'm spliiting them into their own PR as I believe they may be useful in other places and could be merged before the terminal watch face changes.
This change simply adds a
GetSimpleConditionoption as an alternative toGetConditionto return a shorter condition description. For example: returning 'Thunder' instead of 'Thunderstorm' as a way to save space in constrained areas without losing the original descriptions meaning.There's an example GIF here of where this would be useful.