diff --git a/homeassistant/components/homeassistant/triggers/numeric_state.py b/homeassistant/components/homeassistant/triggers/numeric_state.py index 4f406de23ca..16b3fb97475 100644 --- a/homeassistant/components/homeassistant/triggers/numeric_state.py +++ b/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.""" + return condition.async_numeric_state( + hass, to_s, below, above, value_template, variables(entity_id), attribute + ) + + # Each entity that starts outside the range is already armed (ready to fire). + for entity_id in entity_ids: try: - return condition.async_numeric_state( - hass, - to_s, - below, - above, - value_template, - variables(entity_id), - 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, ) - matching = check_numeric_state(entity_id, from_s, to_s) + @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) - elif entity_id not in entities_triggered: - entities_triggered.add(entity_id) + armed_entities.add(entity_id) + elif entity_id in armed_entities: + 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() diff --git a/tests/components/cover/test_device_trigger.py b/tests/components/cover/test_device_trigger.py index ab054ad8223..e8bb3cdc8df 100644 --- a/tests/components/cover/test_device_trigger.py +++ b/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_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 - 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_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 - closed - open - None", "is_pos_gt_45 - device - cover.set_position_cover - open - closed - None", ] ) diff --git a/tests/components/homeassistant/triggers/test_numeric_state.py b/tests/components/homeassistant/triggers/test_numeric_state.py index 85dc68c770d..831e20b78a1 100644 --- a/tests/components/homeassistant/triggers/test_numeric_state.py +++ b/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", ( diff --git a/tests/components/sensor/test_device_trigger.py b/tests/components/sensor/test_device_trigger.py index c39b4597632..d5755ac3288 100644 --- a/tests/components/sensor/test_device_trigger.py +++ b/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" )