pnbruckner / ha-sun2

Home Assistant Sun2 Sensor
The Unlicense
230 stars 22 forks source link

Update for sensor.x_sun_phase fails #86

Closed Scoff123 closed 1 year ago

Scoff123 commented 1 year ago

After updating to the latest version of the sun2 integration, I am getting the following error in my HA logs now and again:

This error originated from a custom integration.

Logger: homeassistant.helpers.entity
Source: custom_components/sun2/helpers.py:95
Integration: sun2 (documentation, issues)
First occurred: 17 July 2023 at 13:19:00 (2 occurrences)
Last logged: 17 July 2023 at 23:02:46

Update for sensor.x_sun_phase fails
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 695, in async_update_ha_state
    await self.async_device_update()
  File "/usr/src/homeassistant/homeassistant/helpers/entity.py", line 940, in async_device_update
    await self.async_update()
  File "/config/custom_components/sun2/helpers.py", line 132, in async_update
    self._update(dt_util.now(self._loc_data.tzi))
  File "/config/custom_components/sun2/sensor.py", line 779, in _update
    self._setup_updates(cur_dttm, cur_elev)
  File "/config/custom_components/sun2/sensor.py", line 757, in _setup_updates
    self._setup_update_at_elev(elev)
  File "/config/custom_components/sun2/sensor.py", line 709, in _setup_update_at_elev
    est_dttm = get_est_dttm()
               ^^^^^^^^^^^^^^
  File "/config/custom_components/sun2/sensor.py", line 695, in get_est_dttm
    return nearest_second(
           ^^^^^^^^^^^^^^^
  File "/config/custom_components/sun2/helpers.py", line 95, in nearest_second
    return dttm.replace(microsecond=0) + timedelta(
           ^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'replace'

This is on HA 2023.7.2

pnbruckner commented 1 year ago

I think updating the integration is just a coincidence. This is apparently a bug that has existed for quite some time and only manifested itself now given your location and time of year. I.e., the time_at_elevation method in the astral package sometimes returns None when it can't determine a value, which is much more likely to happen around solar noon or solar midnight at some locations & time of year. When this happens the nearest_second function throws an AttributeError exception, but the calling code was catching the wrong type of exception (TypeError instead of AttributeError.) This may be due to a change in Python from older versions to now (i.e., Python may have changed the exact type of exception it throws. If so, that explains why my code was trying to catch TypeError. But, I can't verify, so who knows.) Anyway, I'll release a fix soon that also catches the AttributeError exception type.

Scoff123 commented 1 year ago

Ah ok I think I follow, thank you. I've enabled debug logging following this error. If any logs would be helpful, should the error get triggered again, I'll upload them here for you.

pnbruckner commented 1 year ago

@Scoff123 thanks. I did just release a beta fix (2.3.1b0). Maybe it would be good if you stay with 2.3.0 with logging enabled for a day, just to verify the problem still happens, and capture anything from the logs that would be helpful. (Although, honestly, I'm not sure there will be, if I'm right about what the problem is.) But then if you could try the fix and let me know if that solves the problem, I'd appreciate it. Again, not sure why I had the code catching the wrong type of exception, but this integration has been around for a while and we've gone through several versions of Python in that time, so maybe it was right at some time. 🤷

I'll reopen this issue until I hear from you that the fix works, at which time I'll make a full release (i.e., 2.3.1.)

Scoff123 commented 1 year ago

OK great, thank you. I'll stay on 2.3.0 with logging enabled and see of I can capture anything, although there weren't many instances of the error previously, so it looked quite sporadic. I'll report back either way in a couple of days.

pnbruckner commented 1 year ago

@Scoff123 do you happen to have the min_elevation & max_elevation sensors enabled, and/or the elevation sensor? My guess is that given your location & the time of year, your min_elevation (which happens at solar midnight) or your max_elevation (which happens at solar noon), is near one of the elevation points at which the sun phase sensor changes state (i.e., -18, -12, -6, -0.833 or 6.)

Scoff123 commented 1 year ago

@pnbruckner Yes indeed, I have elevation, min_elevation and max_elevation set up as monitored conditions:

Screenshot_20230719_163956_Home Assistant

pnbruckner commented 1 year ago

Thanks. That seems to verify my theory.

Basically, this integration determines when the sun phase sensor should change between a pair of solar midnight and solar noon. E.g., at solar midnight, it determines when the phase should change up until the next solar noon, and at solar noon it does the same for the period up until the next solar midnight. At these times (solar midnight & solar noon) it tries to determine when the sun's elevation will be at -18, -12, etc. It uses the astral package's time_at_elevation method to get a rough estimate for each of these elevations. But, if the requested elevation is near solar midnight or solar noon, often this method doesn't work and just returns the Python value None to indicate it can't determine the time. My code is supposed to handle that case, but apparently it didn't handle it properly, resulting in the uncaught exception.

So, in your recent case, the minimum elevation (i.e., the elevation at solar midnight) was about -18, so when that value was given to time_at_elevation, it returned None, and bang, the exception.

Anyway, I'm pretty sure that is the problem, and the fix is correct. I'm going to close this and fully release the fix.

THANKS!

pnbruckner commented 1 year ago

And just as a final verification, I configured a sun phase sensor for Antwerp. With 2.3.0 today it caused the noted exception, and with 2.3.1 it properly caught the exception, wrote a debug message stating that time_at_elevation returned None, then went on to calculate the correct times.

Scoff123 commented 1 year ago

Brilliant, thanks so much for the quick resolution and explanation of the cause 🙂