diff --git a/homeassistant/components/automation/time.py b/homeassistant/components/automation/time.py index 17b0c989f16..559832eee80 100644 --- a/homeassistant/components/automation/time.py +++ b/homeassistant/components/automation/time.py @@ -27,8 +27,7 @@ def trigger(hass, config, action): if CONF_AFTER in config: after = dt_util.parse_time_str(config[CONF_AFTER]) if after is None: - _LOGGER.error( - 'Received invalid after value: %s', config[CONF_AFTER]) + _error_time(config[CONF_AFTER], CONF_AFTER) return False hours, minutes, seconds = after.hour, after.minute, after.second elif (CONF_HOURS in config or CONF_MINUTES in config @@ -63,27 +62,27 @@ def if_action(hass, config): CONF_BEFORE, CONF_AFTER, CONF_WEEKDAY) return None + if before is not None: + before = dt_util.parse_time_str(before) + if before is None: + _error_time(before, CONF_BEFORE) + return None + + if after is not None: + after = dt_util.parse_time_str(after) + if after is None: + _error_time(after, CONF_AFTER) + return None + def time_if(): """ Validate time based if-condition """ now = dt_util.now() - if before is not None: - time = dt_util.parse_time_str(before) - if time is None: + if before is not None and now > now.replace(hour=before.hour, + minute=before.minute): return False - before_point = now.replace(hour=time.hour, minute=time.minute) - - if now > before_point: - return False - - if after is not None: - time = dt_util.parse_time_str(after) - if time is None: - return False - - after_point = now.replace(hour=time.hour, minute=time.minute) - - if now < after_point: + if after is not None and now < now.replace(hour=after.hour, + minute=after.minute): return False if weekday is not None: @@ -96,3 +95,11 @@ def if_action(hass, config): return True return time_if + + +def _error_time(value, key): + """ Helper method to print error. """ + _LOGGER.error( + "Received invalid value for '%s': %s", key, value) + if isinstance(value, int): + _LOGGER.error('Make sure you wrap time values in quotes') diff --git a/tests/components/automation/test_time.py b/tests/components/automation/test_time.py index f7187592c66..95997bfec42 100644 --- a/tests/components/automation/test_time.py +++ b/tests/components/automation/test_time.py @@ -285,9 +285,9 @@ class TestAutomationTime(unittest.TestCase): automation.DOMAIN: { 'trigger': { 'platform': 'time', - 'hours': 0, - 'minutes': 0, - 'seconds': 0, + 'hours': 1, + 'minutes': 2, + 'seconds': 3, }, 'action': { 'execute_service': 'test.automation' @@ -296,7 +296,7 @@ class TestAutomationTime(unittest.TestCase): })) fire_time_changed(self.hass, dt_util.utcnow().replace( - hour=0, minute=0, second=0)) + hour=1, minute=2, second=3)) self.hass.pool.block_till_done() self.assertEqual(1, len(self.calls)) @@ -320,6 +320,30 @@ class TestAutomationTime(unittest.TestCase): self.hass.pool.block_till_done() self.assertEqual(1, len(self.calls)) + @patch('homeassistant.components.automation.time._LOGGER.error') + def test_if_not_fires_using_wrong_after(self, mock_error): + """ YAML translates time values to total seconds. This should break the + before rule. """ + self.assertTrue(automation.setup(self.hass, { + automation.DOMAIN: { + 'trigger': { + 'platform': 'time', + 'after': 3605, + # Total seconds. Hour = 3600 second + }, + 'action': { + 'execute_service': 'test.automation' + } + } + })) + + fire_time_changed(self.hass, dt_util.utcnow().replace( + hour=1, minute=0, second=5)) + + self.hass.pool.block_till_done() + self.assertEqual(0, len(self.calls)) + self.assertEqual(2, mock_error.call_count) + def test_if_action_before(self): automation.setup(self.hass, { automation.DOMAIN: {