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

Fix: noon and midnight adjustment #980

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

AndreasBackx
Copy link

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 setting min_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:

0. event_name='solar_midnight'
0. event_time_formatted=datetime.datetime(2024, 4, 16, 23, 59, 42)
1. event_name='sunrise'
1. event_time_formatted=datetime.datetime(2024, 4, 17, 4, 59, 32, 723716)
2. event_name='solar_noon'
2. event_time_formatted=datetime.datetime(2024, 4, 17, 11, 59, 48)
3. event_name='sunset'
3. event_time_formatted=datetime.datetime(2024, 4, 17, 19, 0, 56, 58566)
4. event_name='solar_midnight'
4. event_time_formatted=datetime.datetime(2024, 4, 17, 23, 59, 29)
5. event_name='sunrise'
5. event_time_formatted=datetime.datetime(2024, 4, 18, 4, 57, 25, 655122)
6. event_name='solar_noon'
6. event_time_formatted=datetime.datetime(2024, 4, 18, 11, 59, 35)
7. event_name='sunset'
7. event_time_formatted=datetime.datetime(2024, 4, 18, 19, 2, 36, 688279)
8. event_name='solar_midnight'
8. event_time_formatted=datetime.datetime(2024, 4, 18, 23, 59, 16)
9. event_name='sunrise'
9. event_time_formatted=datetime.datetime(2024, 4, 19, 4, 55, 19, 432385)
10. event_name='solar_noon'
10. event_time_formatted=datetime.datetime(2024, 4, 19, 11, 59, 21)
11. event_name='sunset'
11. event_time_formatted=datetime.datetime(2024, 4, 19, 19, 4, 17, 265827)

You can see that it always follows the same pattern cyclically: solar_midnight, sunrise, solar_noon, sunset, and so on. When changing for example min_sunset_time and max_sunset_time even though they don't affect the eventually calculated sunset time, it will give the following incorrect result:

0. event_name='solar_midnight'
0. event_time_formatted=datetime.datetime(2024, 4, 17, 0, 0, 14, 391141)
1. event_name='sunrise'
1. event_time_formatted=datetime.datetime(2024, 4, 17, 4, 59, 32, 723716)
2. event_name='solar_noon'
2. event_time_formatted=datetime.datetime(2024, 4, 17, 12, 0, 14, 391141)
3. event_name='sunset'
3. event_time_formatted=datetime.datetime(2024, 4, 17, 19, 0, 56, 58566)
4. event_name='solar_midnight'
4. event_time_formatted=datetime.datetime(2024, 4, 18, 0, 0, 1, 171700)
5. event_name='sunrise'
5. event_time_formatted=datetime.datetime(2024, 4, 18, 4, 57, 25, 655122)
6. event_name='solar_noon'
6. event_time_formatted=datetime.datetime(2024, 4, 18, 12, 0, 1, 171700)
7. event_name='sunset'
7. event_time_formatted=datetime.datetime(2024, 4, 18, 19, 2, 36, 688279)
8. event_name='sunrise'
8. event_time_formatted=datetime.datetime(2024, 4, 19, 4, 55, 19, 432385)
9. event_name='solar_noon'
9. event_time_formatted=datetime.datetime(2024, 4, 19, 11, 59, 48, 349106)
10. event_name='sunset'
10. event_time_formatted=datetime.datetime(2024, 4, 19, 19, 4, 17, 265827)
11. event_name='solar_midnight'
11. event_time_formatted=datetime.datetime(2024, 4, 19, 23, 59, 48, 349106)

You can see that it incorrectly added a day to the last solar midnight leading to sunset -> sunrise instead of sunset -> 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:

0. event_name='solar_midnight'
0. event_time_formatted=datetime.datetime(2024, 4, 16, 20, 58, 45, 941434)
1. event_name='sunrise'
1. event_time_formatted=datetime.datetime(2024, 4, 17, 4, 59, 32, 723716)
2. event_name='solar_noon'
2. event_time_formatted=datetime.datetime(2024, 4, 17, 11, 59, 48)
3. event_name='sunset'
3. event_time_formatted=datetime.datetime(2024, 4, 17, 16, 0)
4. event_name='solar_midnight'
4. event_time_formatted=datetime.datetime(2024, 4, 17, 20, 56, 52, 311721)
5. event_name='sunrise'
5. event_time_formatted=datetime.datetime(2024, 4, 18, 4, 57, 25, 655122)
6. event_name='solar_noon'
6. event_time_formatted=datetime.datetime(2024, 4, 18, 11, 59, 35)
7. event_name='sunset'
7. event_time_formatted=datetime.datetime(2024, 4, 18, 16, 0)
8. event_name='solar_midnight'
8. event_time_formatted=datetime.datetime(2024, 4, 18, 20, 54, 58, 734173)
9. event_name='sunrise'
9. event_time_formatted=datetime.datetime(2024, 4, 19, 4, 55, 19, 432385)
10. event_name='solar_noon'
10. event_time_formatted=datetime.datetime(2024, 4, 19, 11, 59, 21)
11. event_name='sunset'
11. event_time_formatted=datetime.datetime(2024, 4, 19, 16, 0)

I primarily tested this with sunset and midnight, though it should work the same for sunrise and noon.

@AndreasBackx
Copy link
Author

I'm not entirely sure why test_light_settings in test_switch.py started to fail. It's a bit hard to follow the code there as I'm not familar with Home Assistant's code nor the testing code here. @basnijholt, if you have some time you might know what the issue is. Though no rush, thank you!

@AndreasBackx
Copy link
Author

AndreasBackx commented Aug 31, 2024

@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.

@basnijholt
Copy link
Owner

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!

@AndreasBackx
Copy link
Author

@basnijholt

I verified that 2 tests have pre-existing failures, those are:

  • test_switch.py::test_proactive_adaptation_with_separate_commands
  • test_light_switch_in_specific_area
    • I was able to fix this one easily, see the trivial change in mock_area_registry.

Though I did break the test test_switch.py::test_light_settings. However after spending quite some time trying to debug this, I cannot figure out why assert_expected_color_temp is called. It seems to assure that the color temperature does not change, though I'm not sure why it shouldn't change. If you could help share your experience with the codebase, we could hopefully figure out why it's failing so I can fix it.

At the same time, I fixed the Dockerfile to build again because the paths were incorrect and it would say the tests folder from CMD could not be found. Additionally, I've made it so docker build handles caching better and so every single change does not trigger reinstalling all Python dependencies.

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

Successfully merging this pull request may close these issues.

Adaptive lighting always sets light to 1%
2 participants