From ff6e071dff771a46ccd53c06faa809aadb164a47 Mon Sep 17 00:00:00 2001 From: Stefan Jonasson Date: Sat, 20 Feb 2016 12:48:05 +0100 Subject: [PATCH 1/4] added the for param to the conditions as well --- homeassistant/components/automation/state.py | 61 +++++++++++++------- tests/components/automation/test_state.py | 36 +++++++++++- 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/homeassistant/components/automation/state.py b/homeassistant/components/automation/state.py index 2ddc0822cc9..31d84abbe2b 100644 --- a/homeassistant/components/automation/state.py +++ b/homeassistant/components/automation/state.py @@ -24,6 +24,31 @@ CONF_STATE = "state" CONF_FOR = "for" +def get_time_config(config): + """ Helper function to extract the time specified in the config """ + if CONF_FOR in config: + hours = config[CONF_FOR].get(CONF_HOURS) + minutes = config[CONF_FOR].get(CONF_MINUTES) + seconds = config[CONF_FOR].get(CONF_SECONDS) + + if hours is None and minutes is None and seconds is None: + logging.getLogger(__name__).error( + "Received invalid value for '%s': %s", + config[CONF_FOR], CONF_FOR) + return False + + if config.get(CONF_TO) is None and config.get(CONF_STATE) is None: + logging.getLogger(__name__).error( + "For: requires a to: value'%s': %s", + config[CONF_FOR], CONF_FOR) + return False + + return timedelta(hours=(hours or 0.0), + minutes=(minutes or 0.0), + seconds=(seconds or 0.0)) + return False + + def trigger(hass, config, action): """ Listen for state changes based on `config`. """ entity_id = config.get(CONF_ENTITY_ID) @@ -42,20 +67,8 @@ def trigger(hass, config, action): return False if CONF_FOR in config: - hours = config[CONF_FOR].get(CONF_HOURS) - minutes = config[CONF_FOR].get(CONF_MINUTES) - seconds = config[CONF_FOR].get(CONF_SECONDS) - - if hours is None and minutes is None and seconds is None: - logging.getLogger(__name__).error( - "Received invalid value for '%s': %s", - config[CONF_FOR], CONF_FOR) - return False - - if config.get(CONF_TO) is None and config.get(CONF_STATE) is None: - logging.getLogger(__name__).error( - "For: requires a to: value'%s': %s", - config[CONF_FOR], CONF_FOR) + time_delta = get_time_config(config) + if time_delta is False: return False def state_automation_listener(entity, from_s, to_s): @@ -77,11 +90,7 @@ def trigger(hass, config, action): EVENT_STATE_CHANGED, for_state_listener) if CONF_FOR in config: - now = dt_util.now() - target_tm = now + timedelta( - hours=(hours or 0.0), - minutes=(minutes or 0.0), - seconds=(seconds or 0.0)) + target_tm = dt_util.now() + time_delta for_time_listener = track_point_in_time( hass, state_for_listener, target_tm) for_state_listener = track_state_change( @@ -101,6 +110,11 @@ def if_action(hass, config): entity_id = config.get(CONF_ENTITY_ID) state = config.get(CONF_STATE) + if CONF_FOR in config: + time_delta = get_time_config(config) + if time_delta is False: + return False + if entity_id is None or state is None: logging.getLogger(__name__).error( "Missing if-condition configuration key %s or %s", CONF_ENTITY_ID, @@ -111,6 +125,13 @@ def if_action(hass, config): def if_state(): """ Test if condition. """ - return hass.states.is_state(entity_id, state) + if hass.states.is_state(entity_id, state): + if CONF_FOR in config: + target_tm = dt_util.now() - time_delta + return target_tm > hass.states.get(entity_id).last_changed + else: + return True + else: + return False return if_state diff --git a/tests/components/automation/test_state.py b/tests/components/automation/test_state.py index 8475d284d3e..308b10a02bf 100644 --- a/tests/components/automation/test_state.py +++ b/tests/components/automation/test_state.py @@ -6,7 +6,7 @@ Tests state automation. """ import unittest from datetime import timedelta - +import time import homeassistant.util.dt as dt_util import homeassistant.components.automation as automation import homeassistant.components.automation.state as state @@ -423,3 +423,37 @@ class TestAutomationState(unittest.TestCase): fire_time_changed(self.hass, dt_util.utcnow() + timedelta(seconds=10)) self.hass.pool.block_till_done() self.assertEqual(1, len(self.calls)) + + def test_if_fires_on_for_condition(self): + self.hass.states.set('test.entity', 'on') + self.assertTrue(automation.setup(self.hass, { + automation.DOMAIN: { + 'trigger': { + 'platform': 'event', + 'event_type': 'test_event', + }, + 'condition': { + 'platform': 'state', + 'entity_id': 'test.entity', + 'state': 'on', + 'for': { + 'seconds': 3 + }, + }, + + 'action': { + 'service': 'test.automation' + } + } + })) + + # not enough time has passed + self.hass.bus.fire('test_event') + self.hass.pool.block_till_done() + self.assertEqual(0, len(self.calls)) + + # wait until we have passed the condition + time.sleep(4) + self.hass.bus.fire('test_event') + self.hass.pool.block_till_done() + self.assertEqual(1, len(self.calls)) From f3c95adaca4dd7bccb6892759508c5930bc560bf Mon Sep 17 00:00:00 2001 From: Stefan Jonasson Date: Sat, 20 Feb 2016 22:15:49 +0100 Subject: [PATCH 2/4] Fixed now => utcnow Fixed added time patch to unittest --- homeassistant/components/automation/state.py | 20 +++---- tests/components/automation/test_state.py | 59 +++++++++++--------- 2 files changed, 42 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/automation/state.py b/homeassistant/components/automation/state.py index 31d84abbe2b..62822f3d7e8 100644 --- a/homeassistant/components/automation/state.py +++ b/homeassistant/components/automation/state.py @@ -90,7 +90,7 @@ def trigger(hass, config, action): EVENT_STATE_CHANGED, for_state_listener) if CONF_FOR in config: - target_tm = dt_util.now() + time_delta + target_tm = dt_util.utcnow() + time_delta for_time_listener = track_point_in_time( hass, state_for_listener, target_tm) for_state_listener = track_state_change( @@ -112,7 +112,7 @@ def if_action(hass, config): if CONF_FOR in config: time_delta = get_time_config(config) - if time_delta is False: + if not time_delta: return False if entity_id is None or state is None: @@ -125,13 +125,13 @@ def if_action(hass, config): def if_state(): """ Test if condition. """ - if hass.states.is_state(entity_id, state): - if CONF_FOR in config: - target_tm = dt_util.now() - time_delta - return target_tm > hass.states.get(entity_id).last_changed - else: - return True - else: - return False + is_state = hass.states.is_state(entity_id, state) + + if CONF_FOR not in config: + return is_state + + target_tm = dt_util.utcnow() - time_delta + return (is_state and + target_tm > hass.states.get(entity_id).last_changed) return if_state diff --git a/tests/components/automation/test_state.py b/tests/components/automation/test_state.py index 308b10a02bf..cf1c49ba1ea 100644 --- a/tests/components/automation/test_state.py +++ b/tests/components/automation/test_state.py @@ -6,6 +6,7 @@ Tests state automation. """ import unittest from datetime import timedelta +from unittest.mock import patch import time import homeassistant.util.dt as dt_util import homeassistant.components.automation as automation @@ -425,35 +426,39 @@ class TestAutomationState(unittest.TestCase): self.assertEqual(1, len(self.calls)) def test_if_fires_on_for_condition(self): - self.hass.states.set('test.entity', 'on') - self.assertTrue(automation.setup(self.hass, { - automation.DOMAIN: { - 'trigger': { - 'platform': 'event', - 'event_type': 'test_event', - }, - 'condition': { - 'platform': 'state', - 'entity_id': 'test.entity', - 'state': 'on', - 'for': { - 'seconds': 3 + point1 = dt_util.utcnow() + point2 = point1 + timedelta(seconds=10) + with patch('homeassistant.core.dt_util.utcnow') as mock_utcnow: + mock_utcnow.return_value = point1 + self.hass.states.set('test.entity', 'on') + self.assertTrue(automation.setup(self.hass, { + automation.DOMAIN: { + 'trigger': { + 'platform': 'event', + 'event_type': 'test_event', + }, + 'condition': { + 'platform': 'state', + 'entity_id': 'test.entity', + 'state': 'on', + 'for': { + 'seconds': 5 + }, }, - }, - 'action': { - 'service': 'test.automation' + 'action': { + 'service': 'test.automation' + } } - } - })) + })) - # not enough time has passed - self.hass.bus.fire('test_event') - self.hass.pool.block_till_done() - self.assertEqual(0, len(self.calls)) + # not enough time has passed + self.hass.bus.fire('test_event') + self.hass.pool.block_till_done() + self.assertEqual(0, len(self.calls)) - # wait until we have passed the condition - time.sleep(4) - self.hass.bus.fire('test_event') - self.hass.pool.block_till_done() - self.assertEqual(1, len(self.calls)) + # Time travel 10 secs into the future + mock_utcnow.return_value = point2 + self.hass.bus.fire('test_event') + self.hass.pool.block_till_done() + self.assertEqual(1, len(self.calls)) From 6e7ca9505c500865dc8027206df5cad0cfbeb8b3 Mon Sep 17 00:00:00 2001 From: Stefan Jonasson Date: Sat, 20 Feb 2016 22:25:41 +0100 Subject: [PATCH 3/4] Removed unused import --- tests/components/automation/test_state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/automation/test_state.py b/tests/components/automation/test_state.py index cf1c49ba1ea..b5f0e31ddc8 100644 --- a/tests/components/automation/test_state.py +++ b/tests/components/automation/test_state.py @@ -7,7 +7,7 @@ Tests state automation. import unittest from datetime import timedelta from unittest.mock import patch -import time + import homeassistant.util.dt as dt_util import homeassistant.components.automation as automation import homeassistant.components.automation.state as state From e4485dcf3d63e18c9f188644781483487afc0203 Mon Sep 17 00:00:00 2001 From: Stefan Jonasson Date: Mon, 22 Feb 2016 09:40:27 +0100 Subject: [PATCH 4/4] Updated structure, added more tests --- homeassistant/components/automation/state.py | 70 +++++++++----------- tests/components/automation/test_state.py | 19 ++++++ 2 files changed, 52 insertions(+), 37 deletions(-) diff --git a/homeassistant/components/automation/state.py b/homeassistant/components/automation/state.py index 62822f3d7e8..b9c4164e584 100644 --- a/homeassistant/components/automation/state.py +++ b/homeassistant/components/automation/state.py @@ -26,27 +26,28 @@ CONF_FOR = "for" def get_time_config(config): """ Helper function to extract the time specified in the config """ - if CONF_FOR in config: - hours = config[CONF_FOR].get(CONF_HOURS) - minutes = config[CONF_FOR].get(CONF_MINUTES) - seconds = config[CONF_FOR].get(CONF_SECONDS) + if CONF_FOR not in config: + return None - if hours is None and minutes is None and seconds is None: - logging.getLogger(__name__).error( - "Received invalid value for '%s': %s", - config[CONF_FOR], CONF_FOR) - return False + hours = config[CONF_FOR].get(CONF_HOURS) + minutes = config[CONF_FOR].get(CONF_MINUTES) + seconds = config[CONF_FOR].get(CONF_SECONDS) - if config.get(CONF_TO) is None and config.get(CONF_STATE) is None: - logging.getLogger(__name__).error( - "For: requires a to: value'%s': %s", - config[CONF_FOR], CONF_FOR) - return False + if hours is None and minutes is None and seconds is None: + logging.getLogger(__name__).error( + "Received invalid value for '%s': %s", + config[CONF_FOR], CONF_FOR) + return None - return timedelta(hours=(hours or 0.0), - minutes=(minutes or 0.0), - seconds=(seconds or 0.0)) - return False + if config.get(CONF_TO) is None and config.get(CONF_STATE) is None: + logging.getLogger(__name__).error( + "For: requires a to: value'%s': %s", + config[CONF_FOR], CONF_FOR) + return None + + return timedelta(hours=(hours or 0.0), + minutes=(minutes or 0.0), + seconds=(seconds or 0.0)) def trigger(hass, config, action): @@ -56,20 +57,19 @@ def trigger(hass, config, action): if entity_id is None: logging.getLogger(__name__).error( "Missing trigger configuration key %s", CONF_ENTITY_ID) - return False + return None from_state = config.get(CONF_FROM, MATCH_ALL) to_state = config.get(CONF_TO) or config.get(CONF_STATE) or MATCH_ALL + time_delta = get_time_config(config) if isinstance(from_state, bool) or isinstance(to_state, bool): logging.getLogger(__name__).error( 'Config error. Surround to/from values with quotes.') - return False + return None - if CONF_FOR in config: - time_delta = get_time_config(config) - if time_delta is False: - return False + if CONF_FOR in config and time_delta is None: + return None def state_automation_listener(entity, from_s, to_s): """ Listens for state changes and calls action. """ @@ -89,7 +89,7 @@ def trigger(hass, config, action): hass.bus.remove_listener( EVENT_STATE_CHANGED, for_state_listener) - if CONF_FOR in config: + if time_delta is not None: target_tm = dt_util.utcnow() + time_delta for_time_listener = track_point_in_time( hass, state_for_listener, target_tm) @@ -110,28 +110,24 @@ def if_action(hass, config): entity_id = config.get(CONF_ENTITY_ID) state = config.get(CONF_STATE) - if CONF_FOR in config: - time_delta = get_time_config(config) - if not time_delta: - return False - if entity_id is None or state is None: logging.getLogger(__name__).error( "Missing if-condition configuration key %s or %s", CONF_ENTITY_ID, CONF_STATE) return None + time_delta = get_time_config(config) + if CONF_FOR in config and time_delta is None: + return None + state = str(state) def if_state(): """ Test if condition. """ is_state = hass.states.is_state(entity_id, state) - - if CONF_FOR not in config: - return is_state - - target_tm = dt_util.utcnow() - time_delta - return (is_state and - target_tm > hass.states.get(entity_id).last_changed) + return (time_delta is None and is_state or + time_delta is not None and + dt_util.utcnow() - time_delta > + hass.states.get(entity_id).last_changed) return if_state diff --git a/tests/components/automation/test_state.py b/tests/components/automation/test_state.py index b5f0e31ddc8..d9c458a151d 100644 --- a/tests/components/automation/test_state.py +++ b/tests/components/automation/test_state.py @@ -462,3 +462,22 @@ class TestAutomationState(unittest.TestCase): self.hass.bus.fire('test_event') self.hass.pool.block_till_done() self.assertEqual(1, len(self.calls)) + + def test_if_fails_setup_for_without_time(self): + self.assertIsNone(state.if_action( + self.hass, { + 'platform': 'state', + 'entity_id': 'test.entity', + 'state': 'on', + 'for': {}, + })) + + def test_if_fails_setup_for_without_entity(self): + self.assertIsNone(state.if_action( + self.hass, { + 'platform': 'state', + 'state': 'on', + 'for': { + 'seconds': 5 + }, + }))