From 23b8d9b0ee6f2bf440904ba04dcd6e6aca6541da Mon Sep 17 00:00:00 2001 From: karwosts Date: Sat, 21 Dec 2024 16:23:21 -0800 Subject: [PATCH 1/5] Fix a history stats bug when window and tracked change simultaneously --- homeassistant/components/history_stats/data.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/homeassistant/components/history_stats/data.py b/homeassistant/components/history_stats/data.py index f9b79d74cb44c..9a0d16b3bbebb 100644 --- a/homeassistant/components/history_stats/data.py +++ b/homeassistant/components/history_stats/data.py @@ -131,6 +131,18 @@ async def async_update( await self._async_history_from_db( current_period_start_timestamp, current_period_end_timestamp ) + if event and (new_state := event.data["new_state"]) is not None: + if ( + current_period_start_timestamp + <= floored_timestamp(new_state.last_changed) + <= current_period_end_timestamp + ): + self._history_current_period.append( + HistoryState( + new_state.state, new_state.last_changed.timestamp() + ) + ) + self._previous_run_before_start = False seconds_matched, match_count = self._async_compute_seconds_and_changes( From cc0bb767a9984b00516124f91486abdcdac994fa Mon Sep 17 00:00:00 2001 From: karwosts Date: Sun, 22 Dec 2024 22:07:07 -0800 Subject: [PATCH 2/5] new test WIP --- tests/components/history_stats/test_sensor.py | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/components/history_stats/test_sensor.py b/tests/components/history_stats/test_sensor.py index d60203676e6f9..398f2d3334cc1 100644 --- a/tests/components/history_stats/test_sensor.py +++ b/tests/components/history_stats/test_sensor.py @@ -1465,6 +1465,96 @@ def _fake_states(*args, **kwargs): assert hass.states.get("sensor.sensor4").state == "50.0" +async def test_state_change_during_window_rollover( + recorder_mock: Recorder, + hass: HomeAssistant, +) -> None: + """Test we startup from history and switch to watching state changes.""" + await hass.config.async_set_time_zone("UTC") + utcnow = dt_util.utcnow() + start_time = utcnow.replace(hour=23, minute=0, second=0, microsecond=0) + + def _fake_states(*args, **kwargs): + return { + "binary_sensor.state": [ + ha.State( + "binary_sensor.state", + "on", + last_changed=start_time - timedelta(hours=11), + last_updated=start_time - timedelta(hours=11), + ), + ] + } + + with ( + patch( + "homeassistant.components.recorder.history.state_changes_during_period", + _fake_states, + ), + freeze_time(start_time), + ): + await async_setup_component( + hass, + "sensor", + { + "sensor": [ + { + "platform": "history_stats", + "entity_id": "binary_sensor.state", + "name": "sensor1", + "state": "on", + "start": "{{ today_at() }}", + "end": "{{ now() }}", + "type": "time", + } + ] + }, + ) + await hass.async_block_till_done() + + await async_update_entity(hass, "sensor.sensor1") + await hass.async_block_till_done() + + assert hass.states.get("sensor.sensor1").state == "11.0" + + t1 = start_time + timedelta(minutes=30) + with freeze_time(t1): + async_fire_time_changed(hass, t1) + await hass.async_block_till_done() + + assert hass.states.get("sensor.sensor1").state == "11.5" + + t2 = t1 + timedelta(minutes=29, microseconds=300) + with freeze_time(t2): + async_fire_time_changed(hass, t2) + await hass.async_block_till_done() + + assert hass.states.get("sensor.sensor1").state == "11.98" + + t3 = t2 + timedelta(minutes=1) + with freeze_time(t3): + hass.states.async_set("binary_sensor.state", "off") + await hass.async_block_till_done() + async_fire_time_changed(hass, t3) + await hass.async_block_till_done() + + assert hass.states.get("sensor.sensor1").state == "0.0" + + t4 = t3 + timedelta(minutes=10) + with freeze_time(t4): + async_fire_time_changed(hass, t4) + await hass.async_block_till_done() + + assert hass.states.get("sensor.sensor1").state == "0.0" + + t5 = t4 + timedelta(hours=1) + with freeze_time(t5): + async_fire_time_changed(hass, t5) + await hass.async_block_till_done() + + assert hass.states.get("sensor.sensor1").state == "0.0" + + @pytest.mark.parametrize("time_zone", ["Europe/Berlin", "America/Chicago", "US/Hawaii"]) async def test_end_time_with_microseconds_zeroed( time_zone: str, From 65d911bac6e862b1e1e1d439e5aad3988b46b715 Mon Sep 17 00:00:00 2001 From: karwosts Date: Mon, 23 Dec 2024 16:43:14 +0000 Subject: [PATCH 3/5] test updates --- tests/components/history_stats/test_sensor.py | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/tests/components/history_stats/test_sensor.py b/tests/components/history_stats/test_sensor.py index 398f2d3334cc1..0859f28d8b056 100644 --- a/tests/components/history_stats/test_sensor.py +++ b/tests/components/history_stats/test_sensor.py @@ -1486,6 +1486,7 @@ def _fake_states(*args, **kwargs): ] } + # The test begins at 23:00, and queries from the database that the sensor has been on since 12:00. with ( patch( "homeassistant.components.recorder.history.state_changes_during_period", @@ -1517,6 +1518,7 @@ def _fake_states(*args, **kwargs): assert hass.states.get("sensor.sensor1").state == "11.0" + # Advance 30 minutes t1 = start_time + timedelta(minutes=30) with freeze_time(t1): async_fire_time_changed(hass, t1) @@ -1524,6 +1526,7 @@ def _fake_states(*args, **kwargs): assert hass.states.get("sensor.sensor1").state == "11.5" + # Advance 29 minutes, to record the last minute update just before midnight, just like a real system would do. t2 = t1 + timedelta(minutes=29, microseconds=300) with freeze_time(t2): async_fire_time_changed(hass, t2) @@ -1531,15 +1534,40 @@ def _fake_states(*args, **kwargs): assert hass.states.get("sensor.sensor1").state == "11.98" + # One minute has passed and the time has now rolled over into a new day, resetting the recorder window. The sensor will then query the database for updates, + # and will see that the sensor is ON starting from midnight. t3 = t2 + timedelta(minutes=1) - with freeze_time(t3): + + def _fake_states_t3(*args, **kwargs): + return { + "binary_sensor.state": [ + ha.State( + "binary_sensor.state", + "on", + last_changed=t3.replace( + hours=0, minutes=0, seconds=0, microseconds=0 + ), + last_updated=t3.replace( + hours=0, minutes=0, seconds=0, microseconds=0 + ), + ), + ] + } + + with ( + patch( + "homeassistant.components.recorder.history.state_changes_during_period", + _fake_states_t3, + ), + freeze_time(t3), + ): + # The sensor turns off around this time, before the sensor does its normal polled update. hass.states.async_set("binary_sensor.state", "off") - await hass.async_block_till_done() - async_fire_time_changed(hass, t3) - await hass.async_block_till_done() + await hass.async_block_till_done(wait_background_tasks=True) assert hass.states.get("sensor.sensor1").state == "0.0" + # More time passes, and the history stats does a polled update again. It should be 0 since the sensor has been off since midnight. t4 = t3 + timedelta(minutes=10) with freeze_time(t4): async_fire_time_changed(hass, t4) @@ -1547,6 +1575,7 @@ def _fake_states(*args, **kwargs): assert hass.states.get("sensor.sensor1").state == "0.0" + # More time passes, and the history stats does a polled update again. It should be 0 since the sensor has been off since midnight. t5 = t4 + timedelta(hours=1) with freeze_time(t5): async_fire_time_changed(hass, t5) From ecc3097f33dca048901975310bb99c050c66790b Mon Sep 17 00:00:00 2001 From: karwosts Date: Mon, 23 Dec 2024 16:55:53 +0000 Subject: [PATCH 4/5] fix test and apply suggestion --- homeassistant/components/history_stats/data.py | 8 ++------ tests/components/history_stats/test_sensor.py | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/homeassistant/components/history_stats/data.py b/homeassistant/components/history_stats/data.py index 9a0d16b3bbebb..83528b73f6f3e 100644 --- a/homeassistant/components/history_stats/data.py +++ b/homeassistant/components/history_stats/data.py @@ -118,9 +118,7 @@ async def async_update( <= current_period_end_timestamp ): self._history_current_period.append( - HistoryState( - new_state.state, new_state.last_changed.timestamp() - ) + HistoryState(new_state.state, new_state.last_changed_timestamp) ) new_data = True if not new_data and current_period_end_timestamp < now_timestamp: @@ -138,9 +136,7 @@ async def async_update( <= current_period_end_timestamp ): self._history_current_period.append( - HistoryState( - new_state.state, new_state.last_changed.timestamp() - ) + HistoryState(new_state.state, new_state.last_changed_timestamp) ) self._previous_run_before_start = False diff --git a/tests/components/history_stats/test_sensor.py b/tests/components/history_stats/test_sensor.py index 0859f28d8b056..2ddae5fb61620 100644 --- a/tests/components/history_stats/test_sensor.py +++ b/tests/components/history_stats/test_sensor.py @@ -1544,12 +1544,8 @@ def _fake_states_t3(*args, **kwargs): ha.State( "binary_sensor.state", "on", - last_changed=t3.replace( - hours=0, minutes=0, seconds=0, microseconds=0 - ), - last_updated=t3.replace( - hours=0, minutes=0, seconds=0, microseconds=0 - ), + last_changed=t3.replace(hour=0, minute=0, second=0, microsecond=0), + last_updated=t3.replace(hour=0, minute=0, second=0, microsecond=0), ), ] } From 3a1f8bd586657a3d372203f95a241d2ee1d5fd02 Mon Sep 17 00:00:00 2001 From: karwosts Date: Mon, 23 Dec 2024 17:03:01 +0000 Subject: [PATCH 5/5] simplify test --- tests/components/history_stats/test_sensor.py | 22 +++---------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/tests/components/history_stats/test_sensor.py b/tests/components/history_stats/test_sensor.py index 2ddae5fb61620..3039612d1a026 100644 --- a/tests/components/history_stats/test_sensor.py +++ b/tests/components/history_stats/test_sensor.py @@ -1469,7 +1469,7 @@ async def test_state_change_during_window_rollover( recorder_mock: Recorder, hass: HomeAssistant, ) -> None: - """Test we startup from history and switch to watching state changes.""" + """Test when the tracked sensor and the start/end window change during the same update.""" await hass.config.async_set_time_zone("UTC") utcnow = dt_util.utcnow() start_time = utcnow.replace(hour=23, minute=0, second=0, microsecond=0) @@ -1518,16 +1518,8 @@ def _fake_states(*args, **kwargs): assert hass.states.get("sensor.sensor1").state == "11.0" - # Advance 30 minutes - t1 = start_time + timedelta(minutes=30) - with freeze_time(t1): - async_fire_time_changed(hass, t1) - await hass.async_block_till_done() - - assert hass.states.get("sensor.sensor1").state == "11.5" - - # Advance 29 minutes, to record the last minute update just before midnight, just like a real system would do. - t2 = t1 + timedelta(minutes=29, microseconds=300) + # Advance 59 minutes, to record the last minute update just before midnight, just like a real system would do. + t2 = start_time + timedelta(minutes=59, microseconds=300) with freeze_time(t2): async_fire_time_changed(hass, t2) await hass.async_block_till_done() @@ -1571,14 +1563,6 @@ def _fake_states_t3(*args, **kwargs): assert hass.states.get("sensor.sensor1").state == "0.0" - # More time passes, and the history stats does a polled update again. It should be 0 since the sensor has been off since midnight. - t5 = t4 + timedelta(hours=1) - with freeze_time(t5): - async_fire_time_changed(hass, t5) - await hass.async_block_till_done() - - assert hass.states.get("sensor.sensor1").state == "0.0" - @pytest.mark.parametrize("time_zone", ["Europe/Berlin", "America/Chicago", "US/Hawaii"]) async def test_end_time_with_microseconds_zeroed(