From ea6ca9252c8db82b03d33ee2b66c80bb2ab1c33b Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Thu, 30 Nov 2017 21:03:52 +0100 Subject: [PATCH] Bugfix trigger state with multible entities (#10857) * Bugfix trigger state with multible entities * Fix numeric state * fix lint * fix dict * fix unsub * fix logic * fix name * fix new logic * add test for state * add numeric state test for unsub * add test for multible entities * Update numeric_state.py * Update numeric_state.py * Update state.py * Fix logic for triple match * Add clear to numeric state * clear for state trigger --- .../components/automation/numeric_state.py | 21 ++--- homeassistant/components/automation/state.py | 11 ++- .../automation/test_numeric_state.py | 77 +++++++++++++++++++ tests/components/automation/test_state.py | 41 ++++++++++ 4 files changed, 134 insertions(+), 16 deletions(-) diff --git a/homeassistant/components/automation/numeric_state.py b/homeassistant/components/automation/numeric_state.py index d5cdc9ffd83..b59271f25e5 100644 --- a/homeassistant/components/automation/numeric_state.py +++ b/homeassistant/components/automation/numeric_state.py @@ -37,8 +37,8 @@ def async_trigger(hass, config, action): above = config.get(CONF_ABOVE) time_delta = config.get(CONF_FOR) value_template = config.get(CONF_VALUE_TEMPLATE) - async_remove_track_same = None - already_triggered = False + unsub_track_same = {} + entities_triggered = set() if value_template is not None: value_template.hass = hass @@ -63,8 +63,6 @@ def async_trigger(hass, config, action): @callback def state_automation_listener(entity, from_s, to_s): """Listen for state changes and calls action.""" - nonlocal already_triggered, async_remove_track_same - @callback def call_action(): """Call action with right context.""" @@ -81,16 +79,18 @@ def async_trigger(hass, config, action): matching = check_numeric_state(entity, from_s, to_s) - if matching and not already_triggered: + if not matching: + entities_triggered.discard(entity) + elif entity not in entities_triggered: + entities_triggered.add(entity) + if time_delta: - async_remove_track_same = async_track_same_state( + unsub_track_same[entity] = async_track_same_state( hass, time_delta, call_action, entity_ids=entity_id, async_check_same_func=check_numeric_state) else: call_action() - already_triggered = matching - unsub = async_track_state_change( hass, entity_id, state_automation_listener) @@ -98,7 +98,8 @@ def async_trigger(hass, config, action): def async_remove(): """Remove state listeners async.""" unsub() - if async_remove_track_same: - async_remove_track_same() # pylint: disable=not-callable + for async_remove in unsub_track_same.values(): + async_remove() + unsub_track_same.clear() return async_remove diff --git a/homeassistant/components/automation/state.py b/homeassistant/components/automation/state.py index 7ed44761be8..e4d096d35fd 100644 --- a/homeassistant/components/automation/state.py +++ b/homeassistant/components/automation/state.py @@ -35,13 +35,11 @@ def async_trigger(hass, config, action): to_state = config.get(CONF_TO, MATCH_ALL) time_delta = config.get(CONF_FOR) match_all = (from_state == MATCH_ALL and to_state == MATCH_ALL) - async_remove_track_same = None + unsub_track_same = {} @callback def state_automation_listener(entity, from_s, to_s): """Listen for state changes and calls action.""" - nonlocal async_remove_track_same - @callback def call_action(): """Call action with right context.""" @@ -64,7 +62,7 @@ def async_trigger(hass, config, action): call_action() return - async_remove_track_same = async_track_same_state( + unsub_track_same[entity] = async_track_same_state( hass, time_delta, call_action, lambda _, _2, to_state: to_state.state == to_s.state, entity_ids=entity_id) @@ -76,7 +74,8 @@ def async_trigger(hass, config, action): def async_remove(): """Remove state listeners async.""" unsub() - if async_remove_track_same: - async_remove_track_same() # pylint: disable=not-callable + for async_remove in unsub_track_same.values(): + async_remove() + unsub_track_same.clear() return async_remove diff --git a/tests/components/automation/test_numeric_state.py b/tests/components/automation/test_numeric_state.py index 35841baa930..58cfd2cbd70 100644 --- a/tests/components/automation/test_numeric_state.py +++ b/tests/components/automation/test_numeric_state.py @@ -84,6 +84,36 @@ class TestAutomationNumericState(unittest.TestCase): self.hass.block_till_done() self.assertEqual(1, len(self.calls)) + def test_if_fires_on_entities_change_over_to_below(self): + """"Test the firing with changed entities.""" + self.hass.states.set('test.entity_1', 11) + self.hass.states.set('test.entity_2', 11) + self.hass.block_till_done() + + assert setup_component(self.hass, automation.DOMAIN, { + automation.DOMAIN: { + 'trigger': { + 'platform': 'numeric_state', + 'entity_id': [ + 'test.entity_1', + 'test.entity_2', + ], + 'below': 10, + }, + 'action': { + 'service': 'test.automation' + } + } + }) + + # 9 is below 10 + self.hass.states.set('test.entity_1', 9) + self.hass.block_till_done() + self.assertEqual(1, len(self.calls)) + self.hass.states.set('test.entity_2', 9) + self.hass.block_till_done() + self.assertEqual(2, len(self.calls)) + def test_if_not_fires_on_entity_change_below_to_below(self): """"Test the firing with changed entity.""" self.hass.states.set('test.entity', 11) @@ -112,6 +142,11 @@ class TestAutomationNumericState(unittest.TestCase): self.hass.block_till_done() self.assertEqual(1, len(self.calls)) + # still below so should not fire again + self.hass.states.set('test.entity', 3) + self.hass.block_till_done() + self.assertEqual(1, len(self.calls)) + def test_if_not_below_fires_on_entity_change_to_equal(self): """"Test the firing with changed entity.""" self.hass.states.set('test.entity', 11) @@ -701,6 +736,48 @@ class TestAutomationNumericState(unittest.TestCase): self.hass.block_till_done() self.assertEqual(0, len(self.calls)) + def test_if_not_fires_on_entities_change_with_for_afte_stop(self): + """Test for not firing on entities change with for after stop.""" + assert setup_component(self.hass, automation.DOMAIN, { + automation.DOMAIN: { + 'trigger': { + 'platform': 'numeric_state', + 'entity_id': [ + 'test.entity_1', + 'test.entity_2', + ], + 'above': 8, + 'below': 12, + 'for': { + 'seconds': 5 + }, + }, + 'action': { + 'service': 'test.automation' + } + } + }) + + self.hass.states.set('test.entity_1', 9) + self.hass.states.set('test.entity_2', 9) + self.hass.block_till_done() + fire_time_changed(self.hass, dt_util.utcnow() + timedelta(seconds=10)) + self.hass.block_till_done() + self.assertEqual(2, len(self.calls)) + + self.hass.states.set('test.entity_1', 15) + self.hass.states.set('test.entity_2', 15) + self.hass.block_till_done() + self.hass.states.set('test.entity_1', 9) + self.hass.states.set('test.entity_2', 9) + self.hass.block_till_done() + automation.turn_off(self.hass) + self.hass.block_till_done() + + fire_time_changed(self.hass, dt_util.utcnow() + timedelta(seconds=10)) + self.hass.block_till_done() + self.assertEqual(2, len(self.calls)) + def test_if_fires_on_entity_change_with_for_attribute_change(self): """Test for firing on entity change with for and attribute change.""" assert setup_component(self.hass, automation.DOMAIN, { diff --git a/tests/components/automation/test_state.py b/tests/components/automation/test_state.py index 1f245d1cf5c..b1ee0841e2d 100644 --- a/tests/components/automation/test_state.py +++ b/tests/components/automation/test_state.py @@ -334,6 +334,47 @@ class TestAutomationState(unittest.TestCase): self.hass.block_till_done() self.assertEqual(0, len(self.calls)) + def test_if_not_fires_on_entities_change_with_for_after_stop(self): + """Test for not firing on entity change with for after stop trigger.""" + assert setup_component(self.hass, automation.DOMAIN, { + automation.DOMAIN: { + 'trigger': { + 'platform': 'state', + 'entity_id': [ + 'test.entity_1', + 'test.entity_2', + ], + 'to': 'world', + 'for': { + 'seconds': 5 + }, + }, + 'action': { + 'service': 'test.automation' + } + } + }) + + self.hass.states.set('test.entity_1', 'world') + self.hass.states.set('test.entity_2', 'world') + self.hass.block_till_done() + fire_time_changed(self.hass, dt_util.utcnow() + timedelta(seconds=10)) + self.hass.block_till_done() + self.assertEqual(2, len(self.calls)) + + self.hass.states.set('test.entity_1', 'world_no') + self.hass.states.set('test.entity_2', 'world_no') + self.hass.block_till_done() + self.hass.states.set('test.entity_1', 'world') + self.hass.states.set('test.entity_2', 'world') + self.hass.block_till_done() + automation.turn_off(self.hass) + self.hass.block_till_done() + + fire_time_changed(self.hass, dt_util.utcnow() + timedelta(seconds=10)) + self.hass.block_till_done() + self.assertEqual(2, len(self.calls)) + def test_if_fires_on_entity_change_with_for_attribute_change(self): """Test for firing on entity change with for and attribute change.""" assert setup_component(self.hass, automation.DOMAIN, {