Do not trigger when numeric_state is true at startup (#46424)

homeassistant/components/homeassistant/triggers/numeric_state.py

@@ -73,7 +73,7 @@ async def async_attach_trigger(
     template.attach(hass, time_delta)
     value_template = config.get(CONF_VALUE_TEMPLATE)
     unsub_track_same = {}
-    entities_triggered = set()
+    armed_entities = set()
     period: dict = {}
     attribute = config.get(CONF_ATTRIBUTE)
     job = HassJob(action)
@@ -100,20 +100,22 @@ async def async_attach_trigger(
 
     @callback
     def check_numeric_state(entity_id, from_s, to_s):
-        """Return True if criteria are now met."""
+        """Return whether the criteria are met, raise ConditionError if unknown."""
-        try:
         return condition.async_numeric_state(
-                hass,
+            hass, to_s, below, above, value_template, variables(entity_id), attribute
-                to_s,
+        )
-                below,
+
-                above,
+    # Each entity that starts outside the range is already armed (ready to fire).
-                value_template,
+    for entity_id in entity_ids:
-                variables(entity_id),
+        try:
-                attribute,
+            if not check_numeric_state(entity_id, None, entity_id):
+                armed_entities.add(entity_id)
+        except exceptions.ConditionError as ex:
+            _LOGGER.warning(
+                "Error initializing 'numeric_state' trigger for '%s': %s",
+                automation_info["name"],
+                ex,
             )
-        except exceptions.ConditionError as err:
-            _LOGGER.warning("%s", err)
-            return False
 
     @callback
     def state_automation_listener(event):
@@ -142,12 +144,27 @@ async def async_attach_trigger(
                 to_s.context,
             )
 
+        @callback
+        def check_numeric_state_no_raise(entity_id, from_s, to_s):
+            """Return True if the criteria are now met, False otherwise."""
+            try:
+                return check_numeric_state(entity_id, from_s, to_s)
+            except exceptions.ConditionError:
+                # This is an internal same-state listener so we just drop the
+                # error. The same error will be reached and logged by the
+                # primary async_track_state_change_event() listener.
+                return False
+
+        try:
             matching = check_numeric_state(entity_id, from_s, to_s)
+        except exceptions.ConditionError as ex:
+            _LOGGER.warning("Error in '%s' trigger: %s", automation_info["name"], ex)
+            return
 
         if not matching:
-            entities_triggered.discard(entity_id)
+            armed_entities.add(entity_id)
-        elif entity_id not in entities_triggered:
+        elif entity_id in armed_entities:
-            entities_triggered.add(entity_id)
+            armed_entities.discard(entity_id)
 
             if time_delta:
                 try:
@@ -160,7 +177,6 @@ async def async_attach_trigger(
                         automation_info["name"],
                         ex,
                     )
-                    entities_triggered.discard(entity_id)
                     return
 
                 unsub_track_same[entity_id] = async_track_same_state(
@@ -168,7 +184,7 @@ async def async_attach_trigger(
                     period[entity_id],
                     call_action,
                     entity_ids=entity_id,
-                    async_check_same_func=check_numeric_state,
+                    async_check_same_func=check_numeric_state_no_raise,
                 )
             else:
                 call_action()

tests/components/cover/test_device_trigger.py

@@ -542,8 +542,12 @@ async def test_if_fires_on_position(hass, calls):
             ]
         },
     )
+    hass.states.async_set(ent.entity_id, STATE_OPEN, attributes={"current_position": 1})
     hass.states.async_set(
-        ent.entity_id, STATE_CLOSED, attributes={"current_position": 50}
+        ent.entity_id, STATE_CLOSED, attributes={"current_position": 95}
+    )
+    hass.states.async_set(
+        ent.entity_id, STATE_OPEN, attributes={"current_position": 50}
     )
     await hass.async_block_till_done()
     assert len(calls) == 3
@@ -551,8 +555,8 @@ async def test_if_fires_on_position(hass, calls):
         [calls[0].data["some"], calls[1].data["some"], calls[2].data["some"]]
     ) == sorted(
         [
-            "is_pos_gt_45_lt_90 - device - cover.set_position_cover - open - closed - None",
+            "is_pos_gt_45_lt_90 - device - cover.set_position_cover - closed - open - None",
-            "is_pos_lt_90 - device - cover.set_position_cover - open - closed - None",
+            "is_pos_lt_90 - device - cover.set_position_cover - closed - open - None",
             "is_pos_gt_45 - device - cover.set_position_cover - open - closed - None",
         ]
     )
@@ -666,7 +670,13 @@ async def test_if_fires_on_tilt_position(hass, calls):
         },
     )
     hass.states.async_set(
-        ent.entity_id, STATE_CLOSED, attributes={"current_tilt_position": 50}
+        ent.entity_id, STATE_OPEN, attributes={"current_tilt_position": 1}
+    )
+    hass.states.async_set(
+        ent.entity_id, STATE_CLOSED, attributes={"current_tilt_position": 95}
+    )
+    hass.states.async_set(
+        ent.entity_id, STATE_OPEN, attributes={"current_tilt_position": 50}
     )
     await hass.async_block_till_done()
     assert len(calls) == 3
@@ -674,8 +684,8 @@ async def test_if_fires_on_tilt_position(hass, calls):
         [calls[0].data["some"], calls[1].data["some"], calls[2].data["some"]]
     ) == sorted(
         [
-            "is_pos_gt_45_lt_90 - device - cover.set_position_cover - open - closed - None",
+            "is_pos_gt_45_lt_90 - device - cover.set_position_cover - closed - open - None",
-            "is_pos_lt_90 - device - cover.set_position_cover - open - closed - None",
+            "is_pos_lt_90 - device - cover.set_position_cover - closed - open - None",
             "is_pos_gt_45 - device - cover.set_position_cover - open - closed - None",
         ]
     )

tests/components/homeassistant/triggers/test_numeric_state.py

@@ -10,7 +10,12 @@ import homeassistant.components.automation as automation
 from homeassistant.components.homeassistant.triggers import (
     numeric_state as numeric_state_trigger,
 )
-from homeassistant.const import ATTR_ENTITY_ID, ENTITY_MATCH_ALL, SERVICE_TURN_OFF
+from homeassistant.const import (
+    ATTR_ENTITY_ID,
+    ENTITY_MATCH_ALL,
+    SERVICE_TURN_OFF,
+    STATE_UNAVAILABLE,
+)
 from homeassistant.core import Context
 from homeassistant.setup import async_setup_component
 import homeassistant.util.dt as dt_util
@@ -241,7 +246,7 @@ async def test_if_not_below_fires_on_entity_change_to_equal(hass, calls, below):
 
 
 @pytest.mark.parametrize("below", (10, "input_number.value_10"))
-async def test_if_fires_on_initial_entity_below(hass, calls, below):
+async def test_if_not_fires_on_initial_entity_below(hass, calls, below):
     """Test the firing when starting with a match."""
     hass.states.async_set("test.entity", 9)
     await hass.async_block_till_done()
@@ -261,14 +266,14 @@ async def test_if_fires_on_initial_entity_below(hass, calls, below):
         },
     )
 
-    # Fire on first update even if initial state was already below
+    # Do not fire on first update when initial state was already below
     hass.states.async_set("test.entity", 8)
     await hass.async_block_till_done()
-    assert len(calls) == 1
+    assert len(calls) == 0
 
 
 @pytest.mark.parametrize("above", (10, "input_number.value_10"))
-async def test_if_fires_on_initial_entity_above(hass, calls, above):
+async def test_if_not_fires_on_initial_entity_above(hass, calls, above):
     """Test the firing when starting with a match."""
     hass.states.async_set("test.entity", 11)
     await hass.async_block_till_done()
@@ -288,10 +293,10 @@ async def test_if_fires_on_initial_entity_above(hass, calls, above):
         },
     )
 
-    # Fire on first update even if initial state was already above
+    # Do not fire on first update when initial state was already above
     hass.states.async_set("test.entity", 12)
     await hass.async_block_till_done()
-    assert len(calls) == 1
+    assert len(calls) == 0
 
 
 @pytest.mark.parametrize("above", (10, "input_number.value_10"))
@@ -320,6 +325,74 @@ async def test_if_fires_on_entity_change_above(hass, calls, above):
     assert len(calls) == 1
 
 
+async def test_if_fires_on_entity_unavailable_at_startup(hass, calls):
+    """Test the firing with changed entity at startup."""
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "trigger": {
+                    "platform": "numeric_state",
+                    "entity_id": "test.entity",
+                    "above": 10,
+                },
+                "action": {"service": "test.automation"},
+            }
+        },
+    )
+    # 11 is above 10
+    hass.states.async_set("test.entity", 11)
+    await hass.async_block_till_done()
+    assert len(calls) == 0
+
+
+async def test_if_not_fires_on_entity_unavailable(hass, calls):
+    """Test the firing with entity changing to unavailable."""
+    # set initial state
+    hass.states.async_set("test.entity", 9)
+    await hass.async_block_till_done()
+
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "trigger": {
+                    "platform": "numeric_state",
+                    "entity_id": "test.entity",
+                    "above": 10,
+                },
+                "action": {"service": "test.automation"},
+            }
+        },
+    )
+
+    # 11 is above 10
+    hass.states.async_set("test.entity", 11)
+    await hass.async_block_till_done()
+    assert len(calls) == 1
+
+    # Going to unavailable and back should not fire
+    hass.states.async_set("test.entity", STATE_UNAVAILABLE)
+    await hass.async_block_till_done()
+    assert len(calls) == 1
+    hass.states.async_set("test.entity", 11)
+    await hass.async_block_till_done()
+    assert len(calls) == 1
+
+    # Crossing threshold via unavailable should fire
+    hass.states.async_set("test.entity", 9)
+    await hass.async_block_till_done()
+    assert len(calls) == 1
+    hass.states.async_set("test.entity", STATE_UNAVAILABLE)
+    await hass.async_block_till_done()
+    assert len(calls) == 1
+    hass.states.async_set("test.entity", 11)
+    await hass.async_block_till_done()
+    assert len(calls) == 2
+
+
 @pytest.mark.parametrize("above", (10, "input_number.value_10"))
 async def test_if_fires_on_entity_change_below_to_above(hass, calls, above):
     """Test the firing with changed entity."""
@@ -1449,6 +1522,48 @@ async def test_if_fires_on_change_with_for_template_3(hass, calls, above, below)
     assert len(calls) == 1
 
 
+async def test_if_not_fires_on_error_with_for_template(hass, caplog, calls):
+    """Test for not firing on error with for template."""
+    hass.states.async_set("test.entity", 0)
+    await hass.async_block_till_done()
+
+    assert await async_setup_component(
+        hass,
+        automation.DOMAIN,
+        {
+            automation.DOMAIN: {
+                "trigger": {
+                    "platform": "numeric_state",
+                    "entity_id": "test.entity",
+                    "above": 100,
+                    "for": "00:00:05",
+                },
+                "action": {"service": "test.automation"},
+            }
+        },
+    )
+
+    hass.states.async_set("test.entity", 101)
+    await hass.async_block_till_done()
+    assert len(calls) == 0
+
+    caplog.clear()
+    caplog.set_level(logging.WARNING)
+
+    async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=3))
+    hass.states.async_set("test.entity", "unavailable")
+    await hass.async_block_till_done()
+    assert len(calls) == 0
+
+    assert len(caplog.record_tuples) == 1
+    assert caplog.record_tuples[0][1] == logging.WARNING
+
+    async_fire_time_changed(hass, dt_util.utcnow() + timedelta(seconds=3))
+    hass.states.async_set("test.entity", 101)
+    await hass.async_block_till_done()
+    assert len(calls) == 0
+
+
 @pytest.mark.parametrize(
     "above, below",
     (

tests/components/sensor/test_device_trigger.py

@@ -428,6 +428,7 @@ async def test_if_fires_on_state_change_with_for(hass, calls):
     assert hass.states.get(sensor1.entity_id).state == STATE_UNKNOWN
     assert len(calls) == 0
 
+    hass.states.async_set(sensor1.entity_id, 10)
     hass.states.async_set(sensor1.entity_id, 11)
     await hass.async_block_till_done()
     assert len(calls) == 0
@@ -437,5 +438,5 @@ async def test_if_fires_on_state_change_with_for(hass, calls):
     await hass.async_block_till_done()
     assert (
         calls[0].data["some"]
-        == f"turn_off device - {sensor1.entity_id} - unknown - 11 - 0:00:05"
+        == f"turn_off device - {sensor1.entity_id} - 10 - 11 - 0:00:05"
     )