From 54c525c8432c18d49b1e54bfb7220dac6dd8242b Mon Sep 17 00:00:00 2001 From: Pierre Gironde Date: Tue, 3 Nov 2026 19:14:20 +0100 Subject: [PATCH] fix: ignore stale callbacks from rescheduled timers - Add generation counters to SmartCondition and EventSensor timers. - Preserve pending changes and the retention period when timers are restarted. - Silently cancel timers that have already expired. - Add regression tests for stale callbacks and expiry during cancellation. --- smartcondition.py | 22 +++-- unit_tests/test_event_sensor.py | 79 +++++++++++++++ unit_tests/test_smartcondition_delay.py | 122 ++++++++++++++++++++++++ virtualsensors.py | 14 ++- 4 files changed, 228 insertions(+), 9 deletions(-) create mode 100644 unit_tests/test_event_sensor.py create mode 100644 unit_tests/test_smartcondition_delay.py diff --git a/smartcondition.py b/smartcondition.py index 4c6f5f5..62e589b 100644 --- a/smartcondition.py +++ b/smartcondition.py @@ -46,6 +46,7 @@ class Evaluator(): self.__state_callbacks_handle = list() self.__condition_state_change_delayed = list() self.__condition_state_change_delayed_cb_handle = None + self.__delay_generation = 0 self.__appdaemon_api = appdaemon_api self.__activated = False @@ -573,6 +574,10 @@ class Evaluator(): self.__last_evaluation_result = result def __on_condition_state_change_delayed(self, kwargs): + # An expired timer may already be queued when a newer change reschedules it. + # Ignore it before touching the current timer handle or pending changes. + if kwargs.get("generation") != self.__delay_generation: + return self.__condition_state_change_delayed_cb_handle = None if len(self.__condition_state_change_delayed) == 0: self.__log(f"Callback has been trigger but self.__condition_state_change_delayed is empty, this shouldn't happen, the callback won't be called", level = "WARNING") @@ -607,13 +612,18 @@ class Evaluator(): # self.__update_result() self.__trigger_callbacks(f"Callback trigger by {entity}({old} => {new})") else: - if self.__condition_state_change_delayed_cb_handle != None and self.__appdaemon_api.timer_running(self.__condition_state_change_delayed_cb_handle): - self.__appdaemon_api.cancel_timer(self.__condition_state_change_delayed_cb_handle) + self.__delay_generation += 1 + handle = self.__condition_state_change_delayed_cb_handle + if handle is not None: + # Expiry may already have removed the timer from the scheduler. + self.__appdaemon_api.cancel_timer(handle, silent=True) self.__condition_state_change_delayed_cb_handle = None - - self.__condition_state_change_delayed.append((entity,old,new,datetime.now())) - #self.__log("State changed :" + str(entity) + "(" + str(old) + " => " + str(new) + ") but callback delayed for " + str(self.__callback_delay) + "s") - self.__condition_state_change_delayed_cb_handle = self.__appdaemon_api.run_in(self.__on_condition_state_change_delayed, self.__callback_delay) + + self.__condition_state_change_delayed.append((entity,old,new,datetime.now())) + self.__condition_state_change_delayed_cb_handle = self.__appdaemon_api.run_in( + self.__on_condition_state_change_delayed, self.__callback_delay, + generation=self.__delay_generation, + ) def __log(self,message,**kwargs): log_string = self.__log_to_string(message) diff --git a/unit_tests/test_event_sensor.py b/unit_tests/test_event_sensor.py new file mode 100644 index 0000000..f7477b5 --- /dev/null +++ b/unit_tests/test_event_sensor.py @@ -0,0 +1,79 @@ +"""An expired timer queued before reactivation must not end the new window.""" +import ast +from pathlib import Path +from types import SimpleNamespace +import unittest +from unittest.mock import Mock + +from test_continuous_condition import FakeScheduler + +source = Path(__file__).resolve().parents[1] / 'virtualsensors.py' +node = next(node for node in ast.parse(source.read_text()).body + if isinstance(node, ast.ClassDef) and node.name == 'EventSensor') +namespace = {'VirtualSensorBase': object, 'EventHandler': Mock()} +exec(compile(ast.Module(body=[node], type_ignores=[]), str(source), 'exec'), namespace) +EventSensor = namespace['EventSensor'] + + +class EventSensorTests(unittest.TestCase): + def setUp(self): + self.scheduler = FakeScheduler() + self.state = 'off' + entity = Mock() + entity.exists.return_value = True + entity.set_state.side_effect = lambda *, state: setattr(self, 'state', state) + self.sensor = EventSensor() + self.sensor.ad_api = SimpleNamespace( + get_entity=lambda name: entity, + run_in=self.scheduler.run_in, + cancel_timer=self.scheduler.cancel_timer, + ) + self.sensor.sensor_name = 'binary_sensor.red_shelly_home' + self.sensor.yaml_block = {'retain_time': 120} + self.sensor.initialize() + + def activate(self): + self.sensor.on_activation_event('signal_strength', {}, {}) + return self.sensor.cb_handle + + def test_current_expiry_turns_sensor_off(self): + handle = self.activate() + self.assertEqual(self.state, 'on') + self.scheduler.expire(handle)() + self.assertEqual(self.state, 'off') + self.assertIsNone(self.sensor.cb_handle) + + def test_reactivation_cancels_running_timer(self): + old = self.activate() + current = self.activate() + self.assertNotIn(old, self.scheduler.timers) + self.assertIn(current, self.scheduler.timers) + self.assertEqual(self.state, 'on') + self.assertEqual(self.scheduler.warnings, []) + + def test_queued_expiry_preserves_reactivated_sensor_and_timer(self): + queued = self.scheduler.expire(self.activate()) + current = self.activate() + queued() + self.assertEqual(self.state, 'on') + self.assertEqual(self.sensor.cb_handle, current) + self.assertEqual(self.scheduler.warnings, []) + self.scheduler.expire(current)() + self.assertEqual(self.state, 'off') + self.assertIsNone(self.sensor.cb_handle) + + def test_multiple_queued_expiries_cannot_end_latest_window(self): + first = self.scheduler.expire(self.activate()) + second = self.scheduler.expire(self.activate()) + current = self.activate() + second() + first() + self.assertEqual(self.state, 'on') + self.assertEqual(self.sensor.cb_handle, current) + self.assertEqual(self.scheduler.warnings, []) + self.scheduler.expire(current)() + self.assertEqual(self.state, 'off') + + +if __name__ == '__main__': + unittest.main() diff --git a/unit_tests/test_smartcondition_delay.py b/unit_tests/test_smartcondition_delay.py new file mode 100644 index 0000000..3fbdb97 --- /dev/null +++ b/unit_tests/test_smartcondition_delay.py @@ -0,0 +1,122 @@ +"""Verify debouncing when an expired AppDaemon timer is still queued.""" +import sys +import unittest +from pathlib import Path +from unittest.mock import Mock + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from smartcondition import Evaluator + + +class FakeScheduler: + def __init__(self): + self.timers = {} + self.next_id = 0 + self.cancelled = [] + self.warnings = [] + self.expire_on_cancel = False + self.queued = [] + + def run_in(self, callback, delay, **kwargs): + self.next_id += 1 + handle = str(self.next_id) + self.timers[handle] = (callback, kwargs) + return handle + + def timer_running(self, handle): + return handle in self.timers + + def cancel_timer(self, handle, silent=False): + self.cancelled.append(handle) + if self.expire_on_cancel and handle in self.timers: + self.queued.append(self.expire(handle)) + cancelled = self.timers.pop(handle, None) is not None + if not cancelled and not silent: + self.warnings.append(handle) + return cancelled + + def expire(self, handle): + callback, kwargs = self.timers.pop(handle) + return lambda: callback(kwargs) + + +class DelayedCallbackTests(unittest.TestCase): + def setUp(self): + self.scheduler = FakeScheduler() + self.evaluator = Evaluator.__new__(Evaluator) + fields = dict( + appdaemon_api=self.scheduler, callback_delay=10, + trigger_callback_on_entity_creation=True, + condition_state_change_delayed=[], + condition_state_change_delayed_cb_handle=None, + delay_generation=0, trigger_callbacks=Mock(), log=Mock(), + ) + for name, value in fields.items(): + setattr(self.evaluator, '_Evaluator__' + name, value) + + def change(self, old, new): + self.evaluator._Evaluator__on_condition_state_change( + 'sensor.temperature', 'state', old, new, {}) + return self.evaluator._Evaluator__condition_state_change_delayed_cb_handle + + def test_queued_obsolete_callback_preserves_new_timer_and_changes(self): + old_handle = self.change('20', '21') + queued_callback = self.scheduler.expire(old_handle) + new_handle = self.change('21', '22') + queued_callback() + self.assertEqual(self.scheduler.warnings, []) + + self.assertEqual(self.evaluator._Evaluator__condition_state_change_delayed_cb_handle, + new_handle) + self.assertEqual(len(self.evaluator._Evaluator__condition_state_change_delayed), 2) + self.evaluator._Evaluator__trigger_callbacks.assert_not_called() + self.evaluator._Evaluator__log.assert_not_called() + + self.scheduler.expire(new_handle)() + self.evaluator._Evaluator__trigger_callbacks.assert_called_once() + reason = self.evaluator._Evaluator__trigger_callbacks.call_args.args[0] + self.assertIn('20 => 21', reason) + self.assertIn('21 => 22', reason) + self.assertEqual(self.evaluator._Evaluator__condition_state_change_delayed, []) + self.assertIsNone(self.evaluator._Evaluator__condition_state_change_delayed_cb_handle) + self.evaluator._Evaluator__log.assert_not_called() + + def test_obsolete_callback_after_current_callback_is_ignored(self): + queued_callback = self.scheduler.expire(self.change('20', '21')) + self.scheduler.expire(self.change('21', '22'))() + queued_callback() + self.evaluator._Evaluator__trigger_callbacks.assert_called_once() + self.evaluator._Evaluator__log.assert_not_called() + + def test_running_timer_is_cancelled_when_rescheduled(self): + old_handle = self.change('20', '21') + new_handle = self.change('21', '22') + self.assertEqual(self.scheduler.cancelled, [old_handle]) + self.assertFalse(self.scheduler.timer_running(old_handle)) + self.scheduler.expire(new_handle)() + self.evaluator._Evaluator__trigger_callbacks.assert_called_once() + + def test_expiry_during_cancellation_is_silent_and_preserves_new_batch(self): + self.change('20', '21') + self.scheduler.expire_on_cancel = True + current = self.change('21', '22') + self.assertEqual(self.scheduler.warnings, []) + self.assertEqual(len(self.scheduler.queued), 1) + self.scheduler.queued[0]() + self.evaluator._Evaluator__trigger_callbacks.assert_not_called() + self.assertEqual(self.evaluator._Evaluator__condition_state_change_delayed_cb_handle, + current) + self.assertEqual(len(self.evaluator._Evaluator__condition_state_change_delayed), 2) + self.scheduler.expire(current)() + self.evaluator._Evaluator__trigger_callbacks.assert_called_once() + self.evaluator._Evaluator__log.assert_not_called() + + def test_zero_delay_remains_immediate(self): + self.evaluator._Evaluator__callback_delay = 0 + self.change('20', '21') + self.evaluator._Evaluator__trigger_callbacks.assert_called_once() + self.assertEqual(self.scheduler.timers, {}) + + +if __name__ == '__main__': + unittest.main() diff --git a/virtualsensors.py b/virtualsensors.py index e967e3f..ff3904e 100644 --- a/virtualsensors.py +++ b/virtualsensors.py @@ -757,20 +757,28 @@ class EventSensor(VirtualSensorBase): self.retain_time = self.yaml_block['retain_time'] self.cb_handle = None + self._timer_generation = 0 self.event_handlers = [] if "activation_events" in self.yaml_block: self.event_handlers.append(EventHandler(self.ad_api, self.yaml_block["activation_events"], self.on_activation_event,logger_interface = self.logger_interface)) def on_activation_event(self, event_name, data, kwargs): + self._timer_generation += 1 self.output_sensor.set_state(state = 'on') if self.cb_handle is not None: - self.ad_api.cancel_timer(self.cb_handle) - self.cb_handle = self.ad_api.run_in(self.on_retain_time_elapsed, self.retain_time) + self.ad_api.cancel_timer(self.cb_handle, silent=True) + self.cb_handle = self.ad_api.run_in( + self.on_retain_time_elapsed, self.retain_time, + generation=self._timer_generation, + ) def on_retain_time_elapsed(self, kwargs): - self.output_sensor.set_state(state = 'off') + # Cancellation cannot retract a callback already queued by AppDaemon. + if kwargs.get('generation') != self._timer_generation: + return self.cb_handle = None + self.output_sensor.set_state(state = 'off') def generate_sensor_name(self,virtual_sensor_name): return f"binary_sensor.{virtual_sensor_name}"