search

sqfmi / Watchy

Watchy - An Open Source E-Ink Smartwatch
http://www.sqfmi.com
MIT License
2k stars 334 forks source link

Set GMT offset from weatherdata #188

Closed shtrom closed 1 year ago

shtrom commented 1 year ago

This extends the recent changes to get DST from the weather data to also persist the information in gmtOffset, so the gmtOffset can also be determined automatically.

sqfmi commented 1 year ago

Great idea to show the GMT offset in the syncNTP screen! I think we don't want to persist the GMT offset though because that will change throughout the year due to daylight savings (the weather API [timezone] accounts for that.

It might also be a good to have an option that allows the user to choose between the hardcoded GMT offset, or pull from timezone data using the weather API.

shtrom commented 1 year ago

With the current code on master, manually triggering the NTP sync would reset the offset to the hardcoded one, regardless of what was obtained from the weather data. The DST offset will get updated, if needed, every time the weather API is queried.

With this change, the weather data offset becomes the offset for both automatic and manual updates. This is why a new RTC_DATA variable is needed to store the offset beyond what is hardcoded in the watchySettings. This allows to get all NTP syncs, automatic or manual, to behave consistently, which I think is desirable.

I see your point about letting the hardcoded offset override the weather data (noting that there's no UI setting for it in the watch menu at the moment, and it needs a recompile). Maybe the tradeoff would be to use the hardcoded offset if present and valid in the watchySettings, and otherwise use the weather data to set it. We could just set it to something invalid by default, say INT_MAX, or maybe just -1 for ease of detection (but that's a bit iffy).

Let me know what you think, and I'll update the code.

shtrom commented 1 year ago

(Unrelated, I pushed a fixup that restores the syncNTP-after-weather I previously deleted, as I now realise it's now beneficial to do it every so often to avoid the RTC drifting too far.)

shtrom commented 1 year ago

@sqfmi What do you think about my previous suggestion?

[a] tradeoff would be to use the hardcoded offset if present and valid in the watchySettings, and otherwise use the weather data to set it. We could just set it to something invalid by default, say INT_MAX, or maybe just -1 for ease of detection (but that's a bit iffy).

sqfmi commented 1 year ago

Thanks, merged!