-
Notifications
You must be signed in to change notification settings - Fork 140
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
Fix: noon and midnight adjustment #980
base: main
Are you sure you want to change the base?
Fix: noon and midnight adjustment #980
Conversation
I'm not entirely sure why |
@basnijholt would it be possible to have a look here? I observed the same issue today/yesterday as solar noon/midnight slightly differs again: https://www.timeanddate.com/sun/uk/london. |
Thanks for the reminder! All the tests seem to fail and I personally don't have time to debug it myself. If you manage to get the tests to pass I'll happily merge! |
for more information, see https://pre-commit.ci
I verified that 2 tests have pre-existing failures, those are:
Though I did break the test At the same time, I fixed the |
2506730
to
4dfed2d
Compare
Might resolve #902 and other issues related to incorrectly adjusting noon and midnight times.
I live in London and today is the day where solar midnight is very close to actual midnight. It's around 23:59. What happened this evening is that my lights suddenly went from their normal percentage to 1%. (It wasn't sleep mode.) My guess (and @Viproz's in #902) is also that the custom midnight calculation in
noon_and_midnight
is incorrect. When the custom logic is triggered by for example settingmin_sunset_time
,prev_and_next_events
returned the event types sunset, sunrise when it should have returned sunset, solar_midnight.I debugged and it's clear that when this occurs, the custom code does not fix the day properly:
Inside of
SunEvents.prev_and_next_events
it calculates yesterday's, today's, and tomorrow's events. If we log those, we get this when having not changed any values that would trigger the custom logic:You can see that it always follows the same pattern cyclically:
solar_midnight
,sunrise
,solar_noon
,sunset
, and so on. When changing for examplemin_sunset_time
andmax_sunset_time
even though they don't affect the eventually calculated sunset time, it will give the following incorrect result:You can see that it incorrectly added a day to the last solar midnight leading to
sunset
->sunrise
instead ofsunset
->solar_midnight
->sunrise
.I've fixed it so we do not need to any special calculations to figure out whether to add/remove a day. I am calculating the offset between the "adjusted offset" and "real sunset" and apply the same delta to midnight. (The same for sunrise and noon.) This leads to the above example returning the correct first example.
When we change the
sunset_max_time
to something like 4PM, we can also see that the difference is applied correctly:I primarily tested this with sunset and midnight, though it should work the same for sunrise and noon.