SimpleWeatherService: Add sunrise and sunset data

[vkareh]

This receives sunrise and sunset data from the weather service and adapts weather icons to represent the correct time of day (sun or moon).

It requires a new version of the weather service data byte array.

• Companion patch to Gadgetbridge: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/4011
• Companion patch to InfiniSim: SimpleWeatherService: Add sunrise and sunset data InfiniSim#152

[github-actions]

Build size and comparison to main:

Section	Size	Difference
text	383496B	716B
data	944B	0B
bss	22640B	8B

Run in InfiniEmu

[NeroBurner]

Woult it be enough to have the sunset and sunrise in minutes resolution. If so we could define the sunset sunrise timestamps to be in minutes since midnight. Then we only need log2(24*60)=10.5 bits. So we could only introduce two 16 bit uints instead of two 64bit numbers. Saving 96 bits of memory per message

What do you think?

[vkareh]

I like this idea. A quick/naïve implementation would be to truncate the timestamp that Gadgetbridge sends, and we pad it when calculating the actual time, but then the resolution becomes arbitrary.

What you're suggesting instead is Gadgetbridge essentially doing something like

-long sunriseLocal = weatherSpec.sunRise + Calendar.getOffset("UTC").getTimeInMillis()) / 1000L;
+int sunriseLocal = (weatherSpec.sunRise + Calendar.getOffset("UTC").getTimeInMillis()) / 1000L) / 60;

(or whatever the type conversion looks like)

I'll give that a try and see what happens.

[vkareh]

Ah, nevermind - I see what you're suggesting. Diving the timestamp doesn't really change the size. Math is hard 🤦

[NeroBurner]

I don't like how in the forecast the sun symbol changes to a moon just because it is night when I look at the forecast. I expect to see sun symbols in the forecast independently of the current time of day

[NeroBurner]

To test with InfiniSim with CurrentWeather with the new version 1 ble-packages you can use https://github.com/InfiniTimeOrg/InfiniSim/tree/weather_send_v1

[NeroBurner]

much better now, thanks. Current weather has moon, and forecast has sun. I like it

[vkareh]

Once this is merged, I'll let the Gadgetbridge folks know so that they can merge the changes on their side (they're waiting for this PR to be merged first).

Let me know if there's any change requests here.

[mark9064]

Looks great!

[mark9064]

What about if there is no sunrise or sunset e.g midnight sun at northern latitudes?

[vkareh]

Great question. This data would be sent from a companion and/or weather app, so it all depends how the apps do this calculation.
Likely it'd be empty (which would cast to 0) or have a negative number. Both cases would lead to the watch showing as daytime.

[mark9064]

Do we want to support the case where it's always night (e.g polar winter). If it gets complex and messy it might not be worth implementing

[vkareh]

I don't think it's worth implementing. It would require some knowledge of why the sunrise/sunset data is not reported (or reported "differently"), rather than assuming the meaning of an empty/0, which I think is beyond the scope of this app.

Also in the case of a polar winter, worst case here is that weather is reported with a sun icon rather than a moon icon, which is the current behavior anyway (until this is merged at least), so it'd be a no-op in that scenario. 😛

[mark9064]

OK cool in that case the only suggestion I have is that the protocol specification should be clear about the expected value when there is no sunrise or sunset, and also when it's unknown.

I would suggest u16 max and then counting backwards (e.g 65535 for unknown, 65534 for no sunset/sunrise event that day) as that allows more flexibility if we want to care more about this later on.
Since there are less than 2^15 minutes in a day, it may also be worth making the field signed in this case as then it's just -1 / -2 which are easier to work with

With this protocol the implementation here could just bail on any negative and assume it's day to avoid the complex edge cases

[jmlich]

It would be nice to show the sunset/sunrise time to the user in the weather app. PineTime is primarily a clock, so its main purpose is to display the time—in this case, the time of sunset. From my perspective, it’s interesting to know how much time is left until sunset.

I have implemented support for the protocol changes introduced here also in Amazfish.
piggz/harbour-amazfish#526

[JF002]

Thank you very much for creating and maintaining this PR (in InfiniTime, InfiniSim AND Gadgetbridge!) @vkareh !

Could you confirm that these change do not break the compatibility between InfiniTime and companion apps, thanks to the version field (meaning that InfiniTime with new API will work with companion app with old API, and that InfiniTime with old API will work with companion apps with now API) ? I just want to be sure we won't break the weather functionality until companion apps have the opportunity to catch up.

[vkareh]

@JF002 Good question. With regards to Gadgetbridge, it should always works, since it's based on firmware version detection. The API is additive, so either you have sunrise data or you don't, but the preceding bits are not affected. GB only sends v1 weather data to >=0.15 InfiniTime, else it sends v0 weather. This works in both directions (old IT + new GB; old GB, new IT; etc.)

With regards to other companion apps, it depends on whether they check the firmware version:

• Apps that don't add sunrise/sunset support can continue sending version 0 indefinitely and will work with all firmware versions
• Apps that do add the feature should send version 1 only to InfiniTime >= 0.15
• Apps that send version 1 to old InfiniTime (< 0.15), the InfiniTime will silently ignore weather data.

So basically the only situation in which it would break weather functionality is if the companion app sends version 1 weather without checking what version of InfiniTime it is.

[JF002]

Thanks @vkareh, that looks good to me.

So basically the only situation in which it would break weather functionality is if the companion app sends version 1 weather without checking what version of InfiniTime it is.

Mhm you are right... This looks like something we could improve in the future : expose the version of the weather API so that companion apps have a easier way to know which version of the messages to send.

So I guess this is safe to merge this PR and the one on Gadgetbridge side.
I think there's still an open discussion here. @mark9064 do you think there's still something to add/improve before merging?

[mark9064]

Yeah basically all I wanted with that comment was to formalise the protocol for errors/exceptional cases
Idea from above

• The sunset/sunrise fields can be signed as there are (significantly) less than 2^15-1 minutes in a day
• Negative numbers for errors
• -1 for unknown
• -2 for no event on this day

So polar day would be sunrise at 0, sunset -2, and polar night sunrise -2, sunset 0
This should be relatively easily to add onto the current implementation here

@JF002 @vkareh what do you think of this proposal?

[vkareh]

@mark9064 - this could be nice, but it borders on being an edge case.

I think implementing it here is fairly easy, but I'm not sure how to accomplish this in Gadgetbridge if the weather provider doesn't support the concept of polar day/night.

[jmlich]

I suppose those corner cases could be indicated as 00:00 or 24:00 — i.e., 0 or 1440 minutes. It would be great to follow how the OpenWeather API or other APIs describe those cases.

[JF002]

I honestly don't know if there's a state of the art for handling days/night without sunrise/sunset. I've never had the opportunity to go to such place :p

But yeah, @mark9064 suggestion looks reasonable. Looking at OpenWeather API is another good suggestion from @jmlich.

I'm not used to working with the OpenWeather API, but here is how sunrise and sunset are documented :

sys.sunrise Sunrise time, unix, UTC
sys.sunset Sunset time, unix, UTC

So I guess they return an absolute epoch timestamp for the sunrise and sunset, instead of a relative one from midnight.
So, in this case, we actually have to expect that sunset/sunrise might happen in more than 24h.
But anyway, we can also add negative values for invalid values like "unknown value" so that the companion app can use them if they do not have access to sunrise/sunset time.

[vkareh]

@JF002 let me know if you want me to implement this as part of this PR. It would require changes to Gadgetbridge as well. The overall logic would probably look something like:

• if sunrise (timestamp) is 0, send -1
• else if sunrise (minutes from midnight) is between 0 and 1440, send number of minutes
• else if sunrise (minutes from midnight) is greater than 1440 minutes (meaning it's after today), send -2
• else send -1

(same logic for sunset)

[mark9064]

To flesh it out, do you mean something like:

0<=x<1440 sunrise happens this many minutes into the day
(0 day starts with sun up)
-1 unknown
-2 sun not rising today

0<x<1440 sunset happens this many minutes into the day
-1 unknown
-2 sun not setting today

So polar day would be 0,-2 and polar night -2,-2

Not permissible:
Any value below -2 or above 1439
(-1,x) where x != -1
(x,-1) where x != -1
(x,y) where x > y
(-2,x) where x >= 0: we assume the sun is down when the day starts (having sunset with no sunrise doesn't make sense)

Note: (x,-2) where x >= 0 valid, this corresponds to a day which has sunrise only. (0,x) corresponds to sunset only

Sorry to make this so long... all problems involving date and time seem to have horrible edge cases. My goal here is not to make implementation difficult but instead to have a clear protocol that handles all possibilities. The implementations of the protocol don't need to handle all the edge cases! They could choose to bail on the more complex scenarios. But if we have a clear protocol, then we don't need to upgrade it in future if we decide we do care more about those trickier cases

[mark9064]

@vkareh we'd really like to get 1.16 out shortly, do you think you'll be able to look at this soon?