updated state trigger state_hold_false startup logic; see #95

craigbarratt · craigbarratt · commit 59283b8f2eb7 · 2020-12-03T14:40:47.000+11:00
diff --git a/custom_components/pyscript/trigger.py b/custom_components/pyscript/trigger.py
@@ -16,8 +16,8 @@
 from .const import LOGGER_PATH
 from .eval import AstEval
 from .event import Event
-from .mqtt import Mqtt
 from .function import Function
+from .mqtt import Mqtt
 from .state import STATE_VIRTUAL_ATTRS, State
 
 _LOGGER = logging.getLogger(LOGGER_PATH + ".trigger")
@@ -173,12 +173,8 @@ async def wait_until(
         last_state_trig_time = None
         state_trig_waiting = False
         state_trig_notify_info = [None, None]
-
-        #
-        # at startup we start our state_hold_false window,
-        # although it could get updated if state_check_now is set.
-        #
-        state_false_time = time.monotonic()
+        state_false_time = None
+        check_state_expr_on_start = state_check_now or state_hold_false is not None
 
         if state_trigger is not None:
             state_trig = []
@@ -214,16 +210,16 @@ async def wait_until(
                     raise exc
 
             state_trig_ident.update(state_trig_ident_any)
-            if state_check_now and state_trig_eval:
+            if check_state_expr_on_start and state_trig_eval:
                 #
-                # check straight away to see if the condition is met (to avoid race conditions)
+                # check straight away to see if the condition is met
                 #
                 new_vars = State.notify_var_get(state_trig_ident, {})
                 state_trig_ok = await state_trig_eval.eval(new_vars)
                 exc = state_trig_eval.get_exception_obj()
                 if exc is not None:
                     raise exc
-                if state_hold_false is not None:
+                if state_hold_false is not None and not state_check_now:
                     #
                     # if state_trig_ok we wait until it is false;
                     # otherwise we consider now to be the start of the false hold time
@@ -817,17 +813,14 @@ async def trigger_watch(self):
                 Event.notify_add(self.event_trigger[0], self.notify_q)
             if self.mqtt_trigger is not None:
                 _LOGGER.debug("trigger %s adding mqtt_trigger %s", self.name, self.mqtt_trigger[0])
-                await Mqtt.notify_add(self.mqtt_trigger[0], self.notify_q)                
+                await Mqtt.notify_add(self.mqtt_trigger[0], self.notify_q)
 
             last_trig_time = None
             last_state_trig_time = None
             state_trig_waiting = False
             state_trig_notify_info = [None, None]
-            #
-            # at startup we start our state_hold_false window,
-            # although it could get updated if state_check_now is set.
-            #
-            state_false_time = time.monotonic()
+            state_false_time = None
+            check_state_expr_on_start = self.state_check_now or self.state_hold_false is not None
 
             while True:
                 timeout = None
@@ -841,7 +834,7 @@ async def trigger_watch(self):
                     notify_type = "startup"
                     notify_info = {"trigger_type": "time", "trigger_time": None}
                     self.run_on_startup = False
-                elif self.state_check_now:
+                elif check_state_expr_on_start:
                     #
                     # first time only - skip wait and check state trigger
                     #
@@ -851,7 +844,7 @@ async def trigger_watch(self):
                     else:
                         notify_vars = {}
                     notify_info = [notify_vars, {"trigger_type": notify_type}]
-                    self.state_check_now = False
+                    check_state_expr_on_start = False
                 else:
                     if self.time_trigger:
                         now = dt_now()
@@ -900,7 +893,7 @@ async def trigger_watch(self):
 
                     if not ident_any_values_changed(func_args, self.state_trig_ident_any):
                         #
-                        # if var_name not in func_args we are state_check_now
+                        # if var_name not in func_args we are check_state_expr_on_start
                         #
                         if "var_name" in func_args and not ident_values_changed(
                             func_args, self.state_trig_ident
@@ -917,23 +910,26 @@ async def trigger_watch(self):
                             if self.state_hold_false is not None:
                                 if "var_name" not in func_args:
                                     #
-                                    # this is state_check_now check
+                                    # this is check_state_expr_on_start check
                                     # if immediately true, force wait until False
                                     # otherwise start False wait now
                                     #
                                     state_false_time = None if trig_ok else time.monotonic()
-                                    continue
+                                    if not self.state_check_now:
+                                        continue
                                 if state_false_time is None:
                                     if trig_ok:
                                         #
-                                        # wasn't False, so ignore
+                                        # wasn't False, so ignore after initial check
                                         #
-                                        continue
-                                    #
-                                    # first False, so remember when it is
-                                    #
-                                    state_false_time = time.monotonic()
-                                elif trig_ok:
+                                        if "var_name" in func_args:
+                                            continue
+                                    else:
+                                        #
+                                        # first False, so remember when it is
+                                        #
+                                        state_false_time = time.monotonic()
+                                elif trig_ok and "var_name" in func_args:
                                     too_soon = time.monotonic() - state_false_time < self.state_hold_false
                                     state_false_time = None
                                     if too_soon:
diff --git a/docs/new_features.rst b/docs/new_features.rst
@@ -29,15 +29,15 @@ Planned new features post 1.0.0 include:
 
 The new features since 1.0.0 in master include:
 
-- Added ``state_hold_false=None`` optional period in seconds to ``@state_trigger()`` and ``task.wait_until()``.
-  This requires the trigger expression to be ``False`` for at least that period (including 0) before a
-  successful trigger. Setting this optional parameter makes state triggers edge triggered (ie,
-  triggers only on transition from ``False`` to ``True``), instead of the default level trigger (ie,
-  only has to evaluate to ``True``). Proposed by @tchef69 (#89).
 - Adding new decorator ``@mqtt_trigger`` by @dlashua (#98, #105).
-- All .py files below the pyscript/scripts directory are autoloaded, recursively into all subdirectories.
-  Any file name or directory starting with ``#`` is skipped, which is an in-place way of disabling
-  a specific file or directory tree (#97).
+- Added ``state_hold_false=None`` optional period in seconds to ``@state_trigger()`` and ``task.wait_until()``.
+  This requires the trigger expression to be ``False`` for at least that period (including 0) before
+  a successful trigger. Setting this optional parameter makes state triggers edge triggered (ie,
+  triggers only on transition from ``False`` to ``True``), instead of the default level trigger
+  (ie, only has to evaluate to ``True``). Proposed by @tchef69 (#89).
+- All .py files below the ``pyscript/scripts`` directory are autoloaded, recursively.  Also, any
+  file name or directory starting with ``#`` is skipped (including top-level and ``apps``), which is
+  an in-place way of disabling a specific script, app or directory tree (#97).
 - ``del`` and new function ``state.delete()` can delete state variables and state variable attributes.
 
 Bug fixes since 1.0.0 in master include:
diff --git a/docs/reference.rst b/docs/reference.rst
@@ -303,27 +303,24 @@ Optional arguments are:
   that number of seconds after the first state trigger.
 
 ``state_hold_false=None``
-  If set, the ``@state_trigger`` expression must evaluate to ``False`` for this duration in seconds
-  before the next trigger can occur. The default value of ``None`` means that triggers can occur
-  without the trigger expression having to be ``False`` between triggers. A value of ``0`` requires
-  the expression become ``False`` between triggers, but with no minimum time in that state.
-  If the expression evaluates to ``True`` during the ``state_hold_false`` period, that trigger is
-  ignored, and when the expression next is ``False`` the ``state_hold_false`` period starts over.
+  If set, the state trigger edge is triggered (triggers on a ``False`` to ``True`` transition),
+  versus the default of level triggered (triggers when ``True``). The ``@state_trigger`` expression
+  must evaluate to ``False`` for this duration in seconds before the next trigger can occur.
+  A value of ``0`` requires the expression be ``False`` before a trigger, but with no minimum
+  time in that state. If the expression evaluates to ``True`` during the ``state_hold_false`` period,
+  that trigger is ignored, and when the expression next is ``False`` the ``state_hold_false`` period
+  starts over.
 
   For example, by default the expression ``"int(sensor.temp_outside) >= 50"`` will trigger every
-  time ``sensor.temp_outside`` changes to a value that is 50 or more.  If instead
-  ``state_hold_false=0``, the trigger will only occur when ``sensor.temp_outside`` changes the first
-  time to 50 or more. It has to go back below 50 for ``state_hold_false`` seconds before a new
-  trigger can occur. To summarize, the default behavior is level triggered, and setting ``state_hold_false``
-  makes it edge triggered.
-
-  The ``state_hold_false`` period applies at startup, although the expression is not checked at
-  startup if ``state_check_now==False``. So if ``state_hold_false=0`` the first trigger after startup
-  will succeed, whether or not the expression was previously ``False``. That behavior can be changed
-  by setting ``state_check_now=True``; the expression is checked at startup, and if ``True`` the
-  trigger will not occur, and a wait for the next ``False`` will begin. So setting ``state_check_now=True``
-  and ``state_hold_false`` enforces the need for the expression to be ``False`` before the first
-  trigger.
+  time ``sensor.temp_outside`` changes to a value that is 50 or more.  If instead ``state_hold_false=0``,
+  the trigger will only occur when ``sensor.temp_outside`` changes the first time to 50 or more.
+  It has to go back below 50 for ``state_hold_false`` seconds before a new trigger can occur.
+
+  When ``state_hold_false`` is set, the state trigger expression is evaluated at startup. If ``False``
+  the ``state_hold_false`` period begins. If ``True``, a wait for the next ``False`` value begins.
+  If ``state_check_now`` is also set, the trigger will also occur at startup if the expression is
+  ``True`` at startup, while the ``state_hold_false`` logic will continue to wait until the expression
+  is ``False`` for that period before the next future trigger.
 
 All state variables in HASS have string values. So you’ll have to do comparisons against string
 values or cast the variable to an integer or float. These two examples are essentially equivalent
diff --git a/tests/test_function.py b/tests/test_function.py
@@ -420,10 +420,9 @@ def func4(trigger_type=None, event_type=None, **kwargs):
 def func4a_hold_false():
     global seq_num
 
-    while 1:
-        seq_num += 1
-        res = task.wait_until(state_trigger="int(pyscript.f4avar2) >= 10", state_hold_false=0, __test_handshake__=["pyscript.done2", seq_num], state_check_now=True)
-        pyscript.done = [seq_num, res["value"]]
+    seq_num += 1
+    res = task.wait_until(state_trigger="int(pyscript.f4avar2) >= 10", state_hold_false=0, __test_handshake__=["pyscript.done2", seq_num], state_check_now=True)
+    pyscript.done = [seq_num, res.get("value", None)]
 
 @service
 def func4b_hold_false():
@@ -442,7 +441,8 @@ def func4c_hold_false():
         # this should never trigger
         seq_num += 1
         res = task.wait_until(state_trigger="int(pyscript.f4bvar2) >= 10", state_hold_false=1000, __test_handshake__=["pyscript.done2", seq_num], state_check_now=True)
-        pyscript.done = [seq_num, res["value"]]
+        pyscript.f4bvar2 = 0
+        pyscript.done = [seq_num, res.get("value", None)]
 
 @state_trigger("pyscript.f5var1")
 @time_active("cron(* * * * *)")
@@ -842,18 +842,21 @@ def func9(var_name=None, value=None, old_value=None):
     hass.states.async_set("pyscript.f4avar2", 6)
     hass.states.async_set("pyscript.f4avar2", 15)
     assert literal_eval(await wait_until_done(notify_q)) == [seq_num, "15"]
-    # these won't retrigger
+
+    # will trigger immediately
+    seq_num += 1
+    await hass.services.async_call("pyscript", "func4a_hold_false", {})
+    assert literal_eval(await wait_until_done(notify_q)) == [seq_num, None]
+
     seq_num += 1
-    assert literal_eval(await wait_until_done(notify_q2)) == seq_num
-    hass.states.async_set("pyscript.f4avar2", 16)
-    hass.states.async_set("pyscript.f4avar2", 17)
-    hass.states.async_set("pyscript.f4avar2", 18)
     hass.states.async_set("pyscript.f4avar2", 6)
+    await hass.services.async_call("pyscript", "func4a_hold_false", {})
+    assert literal_eval(await wait_until_done(notify_q2)) == seq_num
+    hass.states.async_set("pyscript.f4avar2", 7)
+    hass.states.async_set("pyscript.f4avar2", 8)
     # this will trigger
     hass.states.async_set("pyscript.f4avar2", 19)
     assert literal_eval(await wait_until_done(notify_q)) == [seq_num, "19"]
-    seq_num += 1
-    assert literal_eval(await wait_until_done(notify_q2)) == seq_num
 
     hass.states.async_set("pyscript.f4bvar2", 6)
     seq_num += 1
@@ -875,6 +878,14 @@ def func9(var_name=None, value=None, old_value=None):
     assert literal_eval(await wait_until_done(notify_q)) == [seq_num, "20"]
     seq_num += 1
     assert literal_eval(await wait_until_done(notify_q2)) == seq_num
+    hass.states.async_set("pyscript.f4bvar2", 21)
+    hass.states.async_set("pyscript.f4bvar2", 22)
+    hass.states.async_set("pyscript.f4bvar2", 23)
+    hass.states.async_set("pyscript.f4bvar2", 1)
+    hass.states.async_set("pyscript.f4bvar2", 25)
+    assert literal_eval(await wait_until_done(notify_q)) == [seq_num, "25"]
+    seq_num += 1
+    assert literal_eval(await wait_until_done(notify_q2)) == seq_num
 
 
 async def test_trigger_closures(hass, caplog):
