From 29c30861bf5441b20ecbb476dd0037aac738d89a Mon Sep 17 00:00:00 2001 From: Jan Harkes Date: Thu, 7 Apr 2016 15:19:28 -0400 Subject: [PATCH] Be flexible in what we accept for script.delay configuration. (#1738) Accept delay configuration even when someone forgets to indent the time specification. Also removed 'weeks' and 'microseconds' from acceptable delay values. There is a new homeassistant release every 2 weeks and running scripts are not persisting across restarts. And there is still the option of using (weeks*7) days if the long delay is really necessary. And if someone really depends on microsecond delay precision we are unlikely to be able to provide this accuracy, even milliseconds is suspect for that matter but will at least allow us to specify some subsecond delay. --- homeassistant/components/script.py | 41 ++++++++++++++-------- homeassistant/helpers/config_validation.py | 1 + tests/components/test_script.py | 39 ++++++++++++++++++++ 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/homeassistant/components/script.py b/homeassistant/components/script.py index 6a07665d372..3dde6f1690e 100644 --- a/homeassistant/components/script.py +++ b/homeassistant/components/script.py @@ -67,20 +67,29 @@ def _alias_stripper(validator): return validate -_DELAY_SCHEMA = { - vol.Required(CONF_DELAY): vol.All({ - CONF_ALIAS: cv.string, - 'days': vol.All(vol.Coerce(int), vol.Range(min=0)), - 'seconds': vol.All(vol.Coerce(int), vol.Range(min=0)), - 'microseconds': vol.All(vol.Coerce(int), vol.Range(min=0)), - 'milliseconds': vol.All(vol.Coerce(int), vol.Range(min=0)), - 'minutes': vol.All(vol.Coerce(int), vol.Range(min=0)), - 'hours': vol.All(vol.Coerce(int), vol.Range(min=0)), - 'weeks': vol.All(vol.Coerce(int), vol.Range(min=0)), - }, cv.has_at_least_one_key( - 'days', 'seconds', 'microseconds', 'milliseconds', 'minutes', 'hours', - 'weeks')) -} +_TIMESPEC = vol.Schema({ + 'days': cv.positive_int, + 'hours': cv.positive_int, + 'minutes': cv.positive_int, + 'seconds': cv.positive_int, + 'milliseconds': cv.positive_int, +}) +_TIMESPEC_REQ = cv.has_at_least_one_key( + 'days', 'hours', 'minutes', 'seconds', 'milliseconds', +) + +_DELAY_SCHEMA = vol.Any( + vol.Schema({ + vol.Required(CONF_DELAY): vol.All(_TIMESPEC.extend({ + vol.Optional(CONF_ALIAS): cv.string + }), _TIMESPEC_REQ) + }), + # Alternative format in case people forgot to indent after 'delay:' + vol.All(_TIMESPEC.extend({ + vol.Required(CONF_DELAY): None, + vol.Optional(CONF_ALIAS): cv.string, + }), _TIMESPEC_REQ) +) _EVENT_SCHEMA = cv.EVENT_SCHEMA.extend({ CONF_ALIAS: cv.string, @@ -234,7 +243,9 @@ class Script(ToggleEntity): self._listener = None self.turn_on() - delay = timedelta(**action[CONF_DELAY]) + timespec = action[CONF_DELAY] or action.copy() + timespec.pop(CONF_DELAY, None) + delay = timedelta(**timespec) self._listener = track_point_in_utc_time( self.hass, script_delay, date_util.utcnow() + delay) self._cur = cur + 1 diff --git a/homeassistant/helpers/config_validation.py b/homeassistant/helpers/config_validation.py index ef363d2b392..44a05f926ff 100644 --- a/homeassistant/helpers/config_validation.py +++ b/homeassistant/helpers/config_validation.py @@ -16,6 +16,7 @@ from homeassistant.util import slugify # Home Assistant types byte = vol.All(vol.Coerce(int), vol.Range(min=0, max=255)) small_float = vol.All(vol.Coerce(float), vol.Range(min=0, max=1)) +positive_int = vol.All(vol.Coerce(int), vol.Range(min=0)) latitude = vol.All(vol.Coerce(float), vol.Range(min=-90, max=90), msg='invalid latitude') longitude = vol.All(vol.Coerce(float), vol.Range(min=-180, max=180), diff --git a/tests/components/test_script.py b/tests/components/test_script.py index 74b3ce28af6..4f912dc77a0 100644 --- a/tests/components/test_script.py +++ b/tests/components/test_script.py @@ -206,6 +206,45 @@ class TestScript(unittest.TestCase): self.assertEqual(2, len(calls)) + def test_alt_delay(self): + """Test alternative delay config format.""" + event = 'test_event' + calls = [] + + def record_event(event): + """Add recorded event to set.""" + calls.append(event) + + self.hass.bus.listen(event, record_event) + + assert _setup_component(self.hass, 'script', { + 'script': { + 'test': { + 'sequence': [{ + 'event': event, + }, { + 'delay': None, + 'seconds': 5 + }, { + 'event': event, + }] + } + } + }) + + script.turn_on(self.hass, ENTITY_ID) + self.hass.pool.block_till_done() + + self.assertTrue(script.is_on(self.hass, ENTITY_ID)) + self.assertEqual(1, len(calls)) + + future = dt_util.utcnow() + timedelta(seconds=5) + fire_time_changed(self.hass, future) + self.hass.pool.block_till_done() + + self.assertFalse(script.is_on(self.hass, ENTITY_ID)) + self.assertEqual(2, len(calls)) + def test_cancel_while_delay(self): """Test the cancelling while the delay is present.""" event = 'test_event'