InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.64k stars 902 forks source link

Improve weather service #1890

Closed FintasticMan closed 6 months ago

FintasticMan commented 8 months ago

This pull request adds a number of changes to WeatherService and WeatherData that improve its functionality and stability. These changes are partially based on @faxe1008's work in #1822, @JF002's work in #1860 and my work in #1882.

The change I made to #1822 is to not use a template function for the general GetCurrent function, but rather let the GetCurrent* functions do the cast themselves. This reduces the number of functions created in total.

One of the big improvements is the addition of an event ID. This ID will allow companion apps to update values they have already sent with new information, allowing them to operate statelessly. This is a BREAKING CHANGE to the WeatherService API, meaning that companion apps will need to update to add an EventID field to the CBOR data for every event that they send. If this PR gets approved, I will make prepare patches for all the companion apps that implement WeatherService to make the change smoother.

I propose that we create a list of standardised IDs for events that most companion apps will implement, which will allow the user to switch companion apps and have the weather data update seamlessly. Companion apps will then also be able to create their own IDs outside of the standard ones that won't be affected by other companions.

One change I would still like to make is to move away from using std::strings and switch to using std::array, to reduce the uncertainty of how long the string might be.

@JF002 @Avamander @faxe1008 could you please give some feedback on these changes.

Fixes #1786, #1788 and #1877.

github-actions[bot] commented 8 months ago
Build size and comparison to main: Section Size Difference
text 377568B 768B
data 940B 0B
bss 63420B 0B
JF002 commented 6 months ago

Thanks to everyone involved in this PR. I'm closing it in favor to #1924 since I decided to rework the weather service so we can fix the issues more easily.