Split time_pattern triggers from time trigger (#19825)

* Split interval triggers from time trigger

* Default smaller interval units to zero

* Rename interval to schedule

* Rename schedule to time_pattern

homeassistant/components/automation/time.py

@@ -13,30 +13,18 @@
 from homeassistant.helpers import config_validation as cv
 from homeassistant.helpers.event import async_track_time_change
 
-CONF_HOURS = 'hours'
-CONF_MINUTES = 'minutes'
-CONF_SECONDS = 'seconds'
-
 _LOGGER = logging.getLogger(__name__)
 
-TRIGGER_SCHEMA = vol.All(vol.Schema({
+TRIGGER_SCHEMA = vol.Schema({
     vol.Required(CONF_PLATFORM): 'time',
-    CONF_AT: cv.time,
-    CONF_HOURS: vol.Any(vol.Coerce(int), vol.Coerce(str)),
-    CONF_MINUTES: vol.Any(vol.Coerce(int), vol.Coerce(str)),
-    CONF_SECONDS: vol.Any(vol.Coerce(int), vol.Coerce(str)),
-}), cv.has_at_least_one_key(CONF_HOURS, CONF_MINUTES, CONF_SECONDS, CONF_AT))
+    vol.Required(CONF_AT): cv.time,
+})
 
 
 async def async_trigger(hass, config, action, automation_info):
     """Listen for state changes based on configuration."""
-    if CONF_AT in config:
-        at_time = config.get(CONF_AT)
-        hours, minutes, seconds = at_time.hour, at_time.minute, at_time.second
-    else:
-        hours = config.get(CONF_HOURS)
-        minutes = config.get(CONF_MINUTES)
-        seconds = config.get(CONF_SECONDS)
+    at_time = config.get(CONF_AT)
+    hours, minutes, seconds = at_time.hour, at_time.minute, at_time.second
 
     @callback
     def time_automation_listener(now):

homeassistant/components/automation/time_pattern.py

@@ -0,0 +1,53 @@
+"""
+Offer time listening automation rules.
+
+For more details about this automation rule, please refer to the documentation
+at https://home-assistant.io/docs/automation/trigger/#time-trigger
+"""
+import logging
+
+import voluptuous as vol
+
+from homeassistant.core import callback
+from homeassistant.const import CONF_PLATFORM
+from homeassistant.helpers import config_validation as cv
+from homeassistant.helpers.event import async_track_time_change
+
+CONF_HOURS = 'hours'
+CONF_MINUTES = 'minutes'
+CONF_SECONDS = 'seconds'
+
+_LOGGER = logging.getLogger(__name__)
+
+TRIGGER_SCHEMA = vol.All(vol.Schema({
+    vol.Required(CONF_PLATFORM): 'time_pattern',
+    CONF_HOURS: vol.Any(vol.Coerce(int), vol.Coerce(str)),
+    CONF_MINUTES: vol.Any(vol.Coerce(int), vol.Coerce(str)),
+    CONF_SECONDS: vol.Any(vol.Coerce(int), vol.Coerce(str)),
+}), cv.has_at_least_one_key(CONF_HOURS, CONF_MINUTES, CONF_SECONDS))
+
+
+async def async_trigger(hass, config, action, automation_info):
+    """Listen for state changes based on configuration."""
+    hours = config.get(CONF_HOURS)
+    minutes = config.get(CONF_MINUTES)
+    seconds = config.get(CONF_SECONDS)
+
+    # If larger units are specified, default the smaller units to zero
+    if minutes is None and hours is not None:
+        minutes = 0
+    if seconds is None and minutes is not None:
+        seconds = 0
+
+    @callback
+    def time_automation_listener(now):
+        """Listen for time changes and calls action."""
+        hass.async_run_job(action, {
+            'trigger': {
+                'platform': 'time_pattern',
+                'now': now,
+            },
+        })
+
+    return async_track_time_change(hass, time_automation_listener,
+                                   hour=hours, minute=minutes, second=seconds)

tests/components/automation/test_time.py

@@ -10,7 +10,6 @@
 
 from tests.common import (
     async_fire_time_changed, assert_setup_component, mock_component)
-from tests.components.automation import common
 from tests.common import async_mock_service
 
 
@@ -26,158 +25,6 @@ def setup_comp(hass):
     mock_component(hass, 'group')
 
 
-async def test_if_fires_when_hour_matches(hass, calls):
-    """Test for firing if hour is matching."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'hours': 0,
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(hour=0))
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-    await common.async_turn_off(hass)
-    await hass.async_block_till_done()
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(hour=0))
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
-async def test_if_fires_when_minute_matches(hass, calls):
-    """Test for firing if minutes are matching."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'minutes': 0,
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(minute=0))
-
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
-async def test_if_fires_when_second_matches(hass, calls):
-    """Test for firing if seconds are matching."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'seconds': 0,
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(second=0))
-
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
-async def test_if_fires_when_all_matches(hass, calls):
-    """Test for firing if everything matches."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'hours': 1,
-                'minutes': 2,
-                'seconds': 3,
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(
-        hour=1, minute=2, second=3))
-
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
-async def test_if_fires_periodic_seconds(hass, calls):
-    """Test for firing periodically every second."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'seconds': "/2",
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(
-        hour=0, minute=0, second=2))
-
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
-async def test_if_fires_periodic_minutes(hass, calls):
-    """Test for firing periodically every minute."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'minutes': "/2",
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(
-        hour=0, minute=2, second=0))
-
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
-async def test_if_fires_periodic_hours(hass, calls):
-    """Test for firing periodically every hour."""
-    assert await async_setup_component(hass, automation.DOMAIN, {
-        automation.DOMAIN: {
-            'trigger': {
-                'platform': 'time',
-                'hours': "/2",
-            },
-            'action': {
-                'service': 'test.automation'
-            }
-        }
-    })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(
-        hour=2, minute=0, second=0))
-
-    await hass.async_block_till_done()
-    assert 1 == len(calls)
-
-
 async def test_if_fires_using_at(hass, calls):
     """Test for firing at."""
     assert await async_setup_component(hass, automation.DOMAIN, {
@@ -204,27 +51,6 @@ async def test_if_fires_using_at(hass, calls):
     assert 'time - 5' == calls[0].data['some']
 
 
-async def test_if_not_working_if_no_values_in_conf_provided(hass, calls):
-    """Test for failure if no configuration."""
-    with assert_setup_component(0):
-        assert await async_setup_component(hass, automation.DOMAIN, {
-            automation.DOMAIN: {
-                'trigger': {
-                    'platform': 'time',
-                },
-                'action': {
-                    'service': 'test.automation'
-                }
-            }
-        })
-
-    async_fire_time_changed(hass, dt_util.utcnow().replace(
-        hour=5, minute=0, second=0))
-
-    await hass.async_block_till_done()
-    assert 0 == len(calls)
-
-
 async def test_if_not_fires_using_wrong_at(hass, calls):
     """YAML translates time values to total seconds.