fixed DST handling for cron time triggers

Author: craigbarratt

custom_components/pyscript/trigger.py

@@ -13,6 +13,7 @@
 
 from homeassistant.core import Context
 from homeassistant.helpers import sun
+from homeassistant.util import dt as dt_util
 
 from .const import LOGGER_PATH
 from .eval import AstEval, EvalFunc, EvalFuncVar
@@ -368,15 +369,15 @@ async def wait_until(
             if startup_time is None:
                 startup_time = now
             if time_trigger is not None:
-                time_next = cls.timer_trigger_next(time_trigger, now, startup_time)
+                time_next, time_next_adj = cls.timer_trigger_next(time_trigger, now, startup_time)
                 _LOGGER.debug(
                     "trigger %s wait_until time_next = %s, now = %s",
                     ast_ctx.name,
                     time_next,
                     now,
                 )
                 if time_next is not None:
-                    this_timeout = (time_next - now).total_seconds()
+                    this_timeout = (time_next_adj - now).total_seconds()
             if timeout is not None:
                 time_left = time0 + timeout - time.monotonic()
                 if time_left <= 0:
@@ -708,6 +709,7 @@ def timer_active_check(cls, time_spec, now, startup_time):
     def timer_trigger_next(cls, time_spec, now, startup_time):
         """Return the next trigger time based on the given time and time specification."""
         next_time = None
+        next_time_adj = None
         if not isinstance(time_spec, list):
             time_spec = [time_spec]
         for spec in time_spec:
@@ -719,9 +721,32 @@ def timer_trigger_next(cls, time_spec, now, startup_time):
                     _LOGGER.error("Invalid cron expression: %s", cron_match)
                     continue
 
-                val = croniter(cron_match.group("cron_expr"), now, dt.datetime).get_next()
+                #
+                # Handling DST changes is tricky; all times in pyscript are naive (no timezone).  This is the
+                # one part of the code where we do check timezones, in case now and next_time bracket a DST
+                # change.  We return next_time as the local time of the next trigger according to the cron
+                # spec, and next_time_adj is potentially adjusted so that (next_time_adj - now) is the correct
+                # timedelta to wait (eg: if cron is a daily trigger at 6am, next_time will always be 6am
+                # tomorrow, and next_time_adj will also by 6am, except on the day of a DST change, when it
+                # will be 5am or 7am, such that (next_time_adj - now) is 23 hours or 25 hours.
+                #
+                # We might have to fetch multiple croniter times, in case (next_time_adj - now) is non-positive
+                # after a DST change.
+                #
+                # Also, datetime doesn't correctly subtract datetimes in different timezones, so we need to compute
+                # the different in UTC.  See https://blog.ganssle.io/articles/2018/02/aware-datetime-arithmetic.html.
+                #
+                cron_iter = croniter(cron_match.group("cron_expr"), now, dt.datetime)
+                delta = None
+                while delta is None or delta.total_seconds() <= 0:
+                    val = cron_iter.get_next()
+                    delta = dt_util.as_local(val).astimezone(dt_util.UTC) - dt_util.as_local(now).astimezone(
+                        dt_util.UTC
+                    )
+
                 if next_time is None or val < next_time:
                     next_time = val
+                    next_time_adj = now + delta
 
             elif len(match1) == 3:
                 this_t = cls.parse_date_time(match1[1].strip(), 0, now, startup_time)
@@ -733,7 +758,7 @@ def timer_trigger_next(cls, time_spec, now, startup_time):
                     this_t = cls.parse_date_time(match1[1].strip(), day_offset, now, startup_time)
                 startup = now == this_t and now == startup_time
                 if (now < this_t or startup) and (next_time is None or this_t < next_time):
-                    next_time = this_t
+                    next_time_adj = next_time = this_t
 
             elif len(match2) == 5:
                 start_str, period_str = match2[1].strip(), match2[2].strip()
@@ -746,12 +771,12 @@ def timer_trigger_next(cls, time_spec, now, startup_time):
                 if match2[3] is None:
                     startup = now == start and now == startup_time
                     if (now < start or startup) and (next_time is None or start < next_time):
-                        next_time = start
+                        next_time_adj = next_time = start
                     if now >= start and not startup:
                         secs = period * (1.0 + math.floor((now - start).total_seconds() / period))
                         this_t = start + dt.timedelta(seconds=secs)
                         if now < this_t and (next_time is None or this_t < next_time):
-                            next_time = this_t
+                            next_time_adj = next_time = this_t
                     continue
                 end_str = match2[3].strip()
                 end = cls.parse_date_time(end_str, 0, now, startup_time)
@@ -761,18 +786,18 @@ def timer_trigger_next(cls, time_spec, now, startup_time):
                     end = cls.parse_date_time(end_str, day + end_offset, now, startup_time)
                     if now < start or (now == start and now == startup_time):
                         if next_time is None or start < next_time:
-                            next_time = start
+                            next_time_adj = next_time = start
                         break
                     secs = period * (1.0 + math.floor((now - start).total_seconds() / period))
                     this_t = start + dt.timedelta(seconds=secs)
                     if start <= this_t <= end:
                         if next_time is None or this_t < next_time:
-                            next_time = this_t
+                            next_time_adj = next_time = this_t
                         break
 
             else:
                 _LOGGER.warning("Can't parse %s in time_trigger check", spec)
-        return next_time
+        return next_time, next_time_adj
 
 
 class TrigInfo:
@@ -1007,15 +1032,17 @@ async def trigger_watch(self):
                     check_state_expr_on_start = False
                 else:
                     if self.time_trigger:
-                        time_next = TrigTime.timer_trigger_next(self.time_trigger, now, startup_time)
+                        time_next, time_next_adj = TrigTime.timer_trigger_next(
+                            self.time_trigger, now, startup_time
+                        )
                         _LOGGER.debug(
                             "trigger %s time_next = %s, now = %s",
                             self.name,
                             time_next,
                             now,
                         )
                         if time_next is not None:
-                            timeout = (time_next - now).total_seconds()
+                            timeout = (time_next_adj - now).total_seconds()
                     if state_trig_waiting:
                         time_left = last_state_trig_time + self.state_hold - time.monotonic()
                         if timeout is None or time_left < timeout:

docs/reference.rst

@@ -675,6 +675,25 @@ It's important that any trigger function based on ``shutdown`` runs as quickly a
 since the shutdown of HASS (or reload completion) will be stalled until your function
 completes.
 
+Note that ``period`` and ``cron`` behave differently as daylight savings changes occur.  The
+``datetime`` values are always in local time.  The ``interval`` in the ``period()`` trigger is
+always the same duration, even when it crosses a daylight savings time change.  For example, if
+the starting time is 6:00pm on some day in summer, and the interval is 1 day, the triggers will
+occur at 6:00pm in summer and 5:00pm in winter; they are always exactly 24 hours apart.  In
+contrast, if ``cron()`` specifies a daily trigger at a specific time of day, the interval between
+triggers will be 23 or 25 hours when it crosses a daylight savings time change, so that the trigger
+continues to be at the specific time of day according to the ``cron()`` specification.  For example,
+if the ``cron()`` specification is ``"0 18 * * *"`` (6:00pm every day), the triggers will occur at
+6:00pm every day, even when the time changes from summer to winter or vice versa.
+
+This can create some surprising behavior when the ``cron()`` specification is for a time that lands
+in the middle of the repeated or skipped hour when daylight savings time changes.  For example, if
+the ``cron()`` specification is ``"1 1-4 * * *"`` (1:01am, 2:01am, 3:01am, 4:01am every day), when
+the transition from summer to winter time occurs, the triggers will occur at 1:01am, 2:01am, 3:01am,
+4:01am, but the 2:01am trigger will occur 2 hours after the 1:01am trigger.  When the transition
+from winter to summer time occurs, the triggers will occur at 1:01am, 3:01am, 4:01am, and the 3:01am
+trigger will occur 1 hour after the 1:01am trigger.
+
 @event_trigger
 ^^^^^^^^^^^^^^
 

tests/test_unit_trigger.py

@@ -210,7 +210,7 @@ async def test_timer_active_check(hass, spec, now, expected):
     startup_time = dt(2019, 9, 1, 13, 0, 0, 0)
     Function.init(hass)
     TrigTime.init(hass)
-    print(f"calling timer_active_check({spec}, {now}, {startup_time})")
+    # print(f"calling timer_active_check({spec}, {now}, {startup_time})")
     out = TrigTime.timer_active_check(spec, now, startup_time)
     assert out == expected
     await Function.waiter_sync()
@@ -479,8 +479,7 @@ async def test_timer_trigger_next(hass):
         startup_time = now = dt(2019, 9, 1, 13, 0, 0, 100000)
         spec, expect_seq = test_data
         for expect in expect_seq:
-            print(f"calling timer_trigger_next({spec}, {now}, {startup_time})")
-            t_next = TrigTime.timer_trigger_next(spec, now, startup_time)
+            t_next, _ = TrigTime.timer_trigger_next(spec, now, startup_time)
             assert t_next == expect
             if t_next is None:
                 break
@@ -596,8 +595,86 @@ async def test_timer_trigger_next_month_rollover(hass):
         startup_time = now = dt(2020, 6, 30, 13, 0, 0, 100000)
         spec, expect_seq = test_data
         for expect in expect_seq:
-            t_next = TrigTime.timer_trigger_next(spec, now, startup_time)
+            t_next, _ = TrigTime.timer_trigger_next(spec, now, startup_time)
             assert t_next == expect
+            if t_next is None:
+                break
+            now = t_next + timedelta(microseconds=1)
+    await Function.waiter_sync()
+    await Function.waiter_stop()
+    await Function.reaper_stop()
+
+
+timerTriggerDSTNextTests = [
+    [
+        ["cron(0 0 * * *)"],
+        [
+            [dt(2019, 11, 4, 0, 0, 0, 0), 90000],
+            [dt(2019, 11, 5, 0, 0, 0, 0), 86400],
+            [dt(2019, 11, 6, 0, 0, 0, 0), 86400],
+        ],
+    ],
+    [
+        ["cron(0 * * * *)"],
+        [
+            [dt(2019, 11, 3, 1, 0, 0, 0), 3600],
+            [dt(2019, 11, 3, 2, 0, 0, 0), 7200],
+            [dt(2019, 11, 3, 3, 0, 0, 0), 3600],
+            [dt(2019, 11, 3, 4, 0, 0, 0), 3600],
+        ],
+    ],
+    [
+        ["cron(1 * * * *)"],
+        [
+            [dt(2019, 11, 3, 0, 1, 0, 0), 60],
+            [dt(2019, 11, 3, 1, 1, 0, 0), 3600],
+            [dt(2019, 11, 3, 2, 1, 0, 0), 7200],
+            [dt(2019, 11, 3, 3, 1, 0, 0), 3600],
+            [dt(2019, 11, 3, 4, 1, 0, 0), 3600],
+        ],
+    ],
+    [
+        ["cron(1 * 8,9,10 3 *)"],
+        [
+            dt(2020, 3, 8, 0, 1, 0, 0),
+            [dt(2020, 3, 8, 1, 1, 0, 0), 3600],
+            [dt(2020, 3, 8, 2, 1, 0, 0), 3600],
+            [dt(2020, 3, 8, 4, 1, 0, 0), 3600],
+            [dt(2020, 3, 8, 5, 1, 0, 0), 3600],
+        ],
+    ],
+]
+
+
+@pytest.mark.asyncio
+async def test_timer_trigger_dst_next(hass):
+    """Run trigger next tests."""
+    #
+    # Hardcode a location and timezone so we can check sunrise
+    # and sunset.
+    #
+    hass.config.latitude = 38
+    hass.config.longitude = -122
+    hass.config.elevation = 0
+    hass.config.time_zone = "America/Los_Angeles"
+
+    Function.init(hass)
+    TrigTime.init(hass)
+
+    for test_data in timerTriggerDSTNextTests:
+        startup_time = now = dt(2019, 11, 3, 0, 0, 0, 0)
+        spec, expect_seq = test_data
+        for expect in expect_seq:
+            t_next, t_next_adj = TrigTime.timer_trigger_next(spec, now, startup_time)
+            delta = (t_next_adj - now).total_seconds()
+            # print(f"calling timer_trigger_next({spec}, {dt_util.as_local(now)}, {startup_time}) -> {dt_util.as_local(t_next)} (delta = {delta})")
+            if isinstance(expect, list):
+                assert t_next == expect[0]
+                assert delta == expect[1]
+            else:
+                assert t_next == expect
+            if t_next is None:
+                break
             now = t_next
     await Function.waiter_sync()
     await Function.waiter_stop()