Skip to content
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

SimpleWeatherService: Add sunrise and sunset data #2100

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vkareh
Copy link
Contributor

@vkareh vkareh commented Aug 20, 2024

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.

Copy link

github-actions bot commented Aug 20, 2024

Build size and comparison to main:

Section Size Difference
text 375228B 700B
data 948B 0B
bss 63496B 8B

@NeroBurner
Copy link
Contributor

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
Copy link
Contributor Author

vkareh commented Sep 24, 2024

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
Copy link
Contributor Author

vkareh commented Sep 24, 2024

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

@NeroBurner
Copy link
Contributor

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

InfiniSim_2024-09-25_220713_forecast_with_moon

@NeroBurner
Copy link
Contributor

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
Copy link
Contributor

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

InfiniSim_2024-09-25_223344_forecast_with_moon2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants