From 621aff9d418fd7f768f14acbb95a15ebb74e7a4d Mon Sep 17 00:00:00 2001 From: Erik Date: Thu, 23 Apr 2026 13:34:13 +0200 Subject: [PATCH 1/4] Add state tracking to EntityConditionBase --- homeassistant/helpers/condition.py | 125 +++++++- homeassistant/helpers/target.py | 16 +- tests/helpers/test_condition.py | 457 +++++++++++++++++++++++++++++ tests/helpers/test_target.py | 118 ++++++++ 4 files changed, 710 insertions(+), 6 deletions(-) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index adaea77fc31147..ada1707300cb0b 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -94,7 +94,12 @@ NumericThresholdType, TargetSelector, ) -from .target import TargetSelection, async_extract_referenced_entity_ids +from .target import ( + TargetSelection, + TargetStateChangedData, + async_extract_referenced_entity_ids, + async_track_target_selector_state_change_event, +) from .template import Template, render_complex from .trace import ( TraceElement, @@ -443,6 +448,7 @@ def __init__(self, hass: HomeAssistant, config: ConditionConfig) -> None: if TYPE_CHECKING: assert config.target assert config.options + self._target_config = config.target self._target_selection = TargetSelection(config.target) self._behavior = config.options[ATTR_BEHAVIOR] self._duration: timedelta | None = config.options.get(CONF_FOR) @@ -450,11 +456,97 @@ def __init__(self, hass: HomeAssistant, config: ConditionConfig) -> None: self._matcher = self._check_any_match_state elif self._behavior == BEHAVIOR_ALL: self._matcher = self._check_all_match_state + self._on_unload: list[Callable[[], None]] = [] + self._valid_since: dict[str, datetime] = {} def entity_filter(self, entities: set[str]) -> set[str]: """Filter entities matching any of the domain specs.""" return filter_by_domain_specs(self._hass, self._domain_specs, entities) + @property + def _needs_duration_tracking(self) -> bool: + """Whether this condition needs active state change tracking for duration. + + Conditions that are true for a single main state value can use + state.last_changed directly. Conditions that track attributes or + match multiple states need active tracking because last_changed + does not capture those transitions. + """ + return True + + def _prime_valid_since(self, entity_id: str) -> None: + """Prime _valid_since for an entity already in a valid state. + + For state-based conditions (value_source is None), last_changed + accurately reflects when the state changed to the current value. + For attribute-based conditions, last_changed only tracks main state + changes, so we use last_updated which is bumped on any update + (state or attributes). This is conservative - the tracked attribute + may have held its value longer - but it's the best we can do + to avoid false positives. + """ + if ( + (_state := self._hass.states.get(entity_id)) is not None + and _state.state not in (STATE_UNAVAILABLE, STATE_UNKNOWN) + and self.is_valid_state(_state) + ): + domain_spec = self._domain_specs[_state.domain] + if domain_spec.value_source is None: + self._valid_since[entity_id] = _state.last_changed + else: + self._valid_since[entity_id] = _state.last_updated + + @override + async def async_setup(self) -> None: + """Set up state tracking for duration-based conditions.""" + await super().async_setup() + if not self._duration or not self._needs_duration_tracking: + return + + @callback + def _state_change_listener( + data: TargetStateChangedData, + ) -> None: + """Track when entities enter or leave a valid state.""" + event = data.state_change_event + entity_id = event.data["entity_id"] + to_state = event.data["new_state"] + + if ( + to_state is not None + and to_state.state not in (STATE_UNAVAILABLE, STATE_UNKNOWN) + and self.is_valid_state(to_state) + ): + if entity_id not in self._valid_since: + # Prime from the state object — this handles both + # genuine transitions and newly tracked entities + self._prime_valid_since(entity_id) + else: + self._valid_since.pop(entity_id, None) + + @callback + def _on_entities_update(added: set[str], removed: set[str]) -> None: + """Handle changes to the tracked entity set.""" + for entity_id in added: + self._prime_valid_since(entity_id) + for entity_id in removed: + self._valid_since.pop(entity_id, None) + + unsub = async_track_target_selector_state_change_event( + self._hass, + self._target_config, + _state_change_listener, + self.entity_filter, + _on_entities_update, + ) + self._on_unload.append(unsub) + + def async_unload(self) -> None: + """Unsubscribe from listeners.""" + for cb in self._on_unload: + cb() + self._on_unload.clear() + def _get_tracked_value(self, entity_state: State) -> Any: """Get the tracked value from a state based on the DomainSpec.""" domain_spec = self._domain_specs[entity_state.domain] @@ -471,9 +563,16 @@ def _check_any_match_state(self, states: list[State]) -> bool: if not self._duration: # Skip duration check if duration is not specified or 0 return any(self.is_valid_state(state) for state in states) - duration = dt_util.utcnow() - self._duration + cutoff = dt_util.utcnow() - self._duration + if not self._needs_duration_tracking: + return any( + self.is_valid_state(state) and state.last_changed <= cutoff + for state in states + ) return any( - self.is_valid_state(state) and duration > state.last_changed + self.is_valid_state(state) + and (valid_since := self._valid_since.get(state.entity_id)) is not None + and valid_since <= cutoff for state in states ) @@ -482,9 +581,16 @@ def _check_all_match_state(self, states: list[State]) -> bool: if not self._duration: # Skip duration check if duration is not specified or 0 return all(self.is_valid_state(state) for state in states) - duration = dt_util.utcnow() - self._duration + cutoff = dt_util.utcnow() - self._duration + if not self._needs_duration_tracking: + return all( + self.is_valid_state(state) and state.last_changed <= cutoff + for state in states + ) return all( - self.is_valid_state(state) and duration > state.last_changed + self.is_valid_state(state) + and (valid_since := self._valid_since.get(state.entity_id)) is not None + and valid_since <= cutoff for state in states ) @@ -511,6 +617,15 @@ class EntityStateConditionBase(EntityConditionBase): _states: set[str | bool] + @property + def _needs_duration_tracking(self) -> bool: + """Single-state conditions with no attribute tracking can use last_changed.""" + if len(self._states) != 1: + return True + return any( + spec.value_source is not None for spec in self._domain_specs.values() + ) + def is_valid_state(self, entity_state: State) -> bool: """Check if the state matches the expected state(s).""" return self._get_tracked_value(entity_state) in self._states diff --git a/homeassistant/helpers/target.py b/homeassistant/helpers/target.py index 334b7147e01b0c..d8de7d0b1d216c 100644 --- a/homeassistant/helpers/target.py +++ b/homeassistant/helpers/target.py @@ -345,14 +345,25 @@ def __init__( target_selection: TargetSelection, action: Callable[[TargetStateChangedData], Any], entity_filter: Callable[[set[str]], set[str]], + on_entities_update: Callable[[set[str], set[str]], None] | None = None, ) -> None: """Initialize the state change tracker.""" super().__init__(hass, target_selection, entity_filter) self._action = action + self._on_entities_update = on_entities_update self._state_change_unsub: CALLBACK_TYPE | None = None + self._tracked_entities: set[str] = set() def _handle_entities_update(self, tracked_entities: set[str]) -> None: """Handle the tracked entities.""" + previous_entities = self._tracked_entities + self._tracked_entities = tracked_entities + + if self._on_entities_update is not None: + added = tracked_entities - previous_entities + removed = previous_entities - tracked_entities + if added or removed: + self._on_entities_update(added, removed) @callback def state_change_listener(event: Event[EventStateChangedData]) -> None: @@ -380,6 +391,7 @@ def async_track_target_selector_state_change_event( target_selector_config: ConfigType, action: Callable[[TargetStateChangedData], Any], entity_filter: Callable[[set[str]], set[str]] = lambda x: x, + on_entities_update: Callable[[set[str], set[str]], None] | None = None, ) -> CALLBACK_TYPE: """Track state changes for entities referenced directly or indirectly in a target selector.""" target_selection = TargetSelection(target_selector_config) @@ -387,5 +399,7 @@ def async_track_target_selector_state_change_event( raise HomeAssistantError( f"Target selector {target_selector_config} does not have any selectors defined" ) - tracker = TargetStateChangeTracker(hass, target_selection, action, entity_filter) + tracker = TargetStateChangeTracker( + hass, target_selection, action, entity_filter, on_entities_update + ) return tracker.async_setup() diff --git a/tests/helpers/test_condition.py b/tests/helpers/test_condition.py index 63f1cf6dfd1d6e..ae1be32aa7c52c 100644 --- a/tests/helpers/test_condition.py +++ b/tests/helpers/test_condition.py @@ -2,6 +2,7 @@ from collections.abc import Mapping from contextlib import AbstractContextManager, nullcontext as does_not_raise +from dataclasses import dataclass, field from datetime import timedelta import io from typing import Any @@ -22,6 +23,7 @@ from homeassistant.components.system_health import DOMAIN as SYSTEM_HEALTH_DOMAIN from homeassistant.const import ( ATTR_DEVICE_CLASS, + ATTR_LABEL_ID, ATTR_UNIT_OF_MEASUREMENT, CONF_CONDITION, CONF_DEVICE_ID, @@ -42,6 +44,7 @@ condition, config_validation as cv, entity_registry as er, + label_registry as lr, trace, ) from homeassistant.helpers.automation import ( @@ -4386,3 +4389,457 @@ def _async_check(self, **kwargs: Any) -> bool: # to immediately call __del__. checker.__del__() # pylint: disable=unnecessary-dunder-call unload_mock.assert_not_called() + + +_ATTR_DOMAIN_SPECS: Mapping[str, DomainSpec] = { + "test": DomainSpec(value_source="test_attr") +} + + +async def _setup_attr_state_condition( + hass: HomeAssistant, + entity_ids: str | list[str], + states: str | bool | set[str | bool], + condition_options: dict[str, Any] | None = None, +) -> condition.ConditionChecker: + """Set up an attribute-based state condition and return the checker.""" + condition_cls = make_entity_state_condition( + _ATTR_DOMAIN_SPECS, + states, + support_duration=True, + ) + + async def async_get_conditions( + hass: HomeAssistant, + ) -> dict[str, type[Condition]]: + return {"_": condition_cls} + + mock_integration(hass, MockModule("test")) + mock_platform( + hass, "test.condition", Mock(async_get_conditions=async_get_conditions) + ) + + if isinstance(entity_ids, str): + entity_ids = [entity_ids] + + config: dict[str, Any] = { + CONF_CONDITION: "test", + CONF_TARGET: {CONF_ENTITY_ID: entity_ids}, + CONF_OPTIONS: condition_options or {}, + } + + config = await async_validate_condition_config(hass, config) + test = await condition.async_from_config(hass, config) + assert test is not None + return test + + +async def test_state_condition_attr_duration_not_met( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test attribute-based condition with duration: not met yet.""" + test = await _setup_attr_state_condition( + hass, + entity_ids="test.entity_1", + states={True}, + condition_options={CONF_FOR: {"seconds": 10}}, + ) + + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + # Just set — duration not met + assert test(hass) is False + + freezer.tick(timedelta(seconds=5)) + assert test(hass) is False + + +async def test_state_condition_attr_duration_met( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test attribute-based condition with duration: met after waiting.""" + test = await _setup_attr_state_condition( + hass, + entity_ids="test.entity_1", + states={True}, + condition_options={CONF_FOR: {"seconds": 10}}, + ) + + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + freezer.tick(timedelta(seconds=11)) + assert test(hass) is True + + +async def test_state_condition_attr_duration_reset_on_attr_change( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test attribute-based condition: timer resets when attribute changes. + + This is the key difference from state-based duration: the tracked value + is in an attribute, so state.last_changed does not capture it. The + _valid_since tracking in async_setup handles this correctly. + """ + test = await _setup_attr_state_condition( + hass, + entity_ids="test.entity_1", + states={True}, + condition_options={CONF_FOR: {"seconds": 10}}, + ) + + # Set attribute to True + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + # After 8s, change attribute to False (state stays the same) + freezer.tick(timedelta(seconds=8)) + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": False}) + await hass.async_block_till_done() + + # Set attribute back to True + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + # 5s after re-set — not enough (timer was reset) + freezer.tick(timedelta(seconds=5)) + assert test(hass) is False + + # 6 more seconds (11 from re-set) — now met + freezer.tick(timedelta(seconds=6)) + assert test(hass) is True + + +@pytest.mark.parametrize( + ("behavior", "one_match_expected"), + [(BEHAVIOR_ANY, True), (BEHAVIOR_ALL, False)], +) +async def test_state_condition_attr_duration_behavior( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + behavior: str, + one_match_expected: bool, +) -> None: + """Test attribute-based condition with duration and behavior any/all.""" + test = await _setup_attr_state_condition( + hass, + entity_ids=["test.entity_1", "test.entity_2"], + states={True}, + condition_options={ATTR_BEHAVIOR: behavior, CONF_FOR: {"seconds": 10}}, + ) + + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True}) + hass.states.async_set("test.entity_2", STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + # Both matching but duration not met + assert test(hass) is False + + # Advance past duration — both matching long enough + freezer.tick(timedelta(seconds=11)) + assert test(hass) is True + + # Change entity_2 attribute — only one matching for duration + hass.states.async_set("test.entity_2", STATE_ON, {"test_attr": False}) + await hass.async_block_till_done() + assert test(hass) is one_match_expected + + +@dataclass +class _AttrInitStep: + """A state update step before the condition is created.""" + + state: str + attrs: dict[str, Any] = field(default_factory=dict) + delay_before: int = 0 + + +@pytest.mark.parametrize( + ("steps", "duration", "initially_met"), + [ + # Attribute set to valid 10s ago, no further changes → met (10 >= 5) + ( + [_AttrInitStep(STATE_ON, {"test_attr": True})], + 10, + True, + ), + # Attribute set to valid 3s ago → not met (3 < 5) + ( + [_AttrInitStep(STATE_ON, {"test_attr": True})], + 3, + False, + ), + # Attribute set to valid, then main state changes 2s later + # (attribute stays valid). last_updated is bumped by the state change, + # so the effective duration is only 2s from the second update → not met + ( + [ + _AttrInitStep(STATE_ON, {"test_attr": True}), + _AttrInitStep(STATE_OFF, {"test_attr": True}, delay_before=8), + ], + 2, + False, + ), + # Same as above but enough time after the state change → met + ( + [ + _AttrInitStep(STATE_ON, {"test_attr": True}), + _AttrInitStep(STATE_OFF, {"test_attr": True}, delay_before=2), + ], + 8, + True, + ), + # Attribute was invalid, then set to valid 4s ago → not met (4 < 5) + ( + [ + _AttrInitStep(STATE_ON, {"test_attr": False}), + _AttrInitStep(STATE_ON, {"test_attr": True}, delay_before=6), + ], + 4, + False, + ), + # Attribute was invalid, then set to valid 6s ago → met (6 >= 5) + ( + [ + _AttrInitStep(STATE_ON, {"test_attr": False}), + _AttrInitStep(STATE_ON, {"test_attr": True}, delay_before=4), + ], + 6, + True, + ), + # Attribute valid → invalid → valid 3s ago → not met (3 < 5) + ( + [ + _AttrInitStep(STATE_ON, {"test_attr": True}), + _AttrInitStep(STATE_ON, {"test_attr": False}, delay_before=5), + _AttrInitStep(STATE_ON, {"test_attr": True}, delay_before=2), + ], + 3, + False, + ), + ], + ids=[ + "valid_long_enough", + "valid_too_short", + "state_change_bumps_last_updated_not_met", + "state_change_bumps_last_updated_met", + "invalid_then_valid_not_met", + "invalid_then_valid_met", + "valid_invalid_valid_not_met", + ], +) +async def test_state_condition_attr_duration_initial_state( + hass: HomeAssistant, + freezer: FrozenDateTimeFactory, + steps: list[_AttrInitStep], + duration: int, + initially_met: bool, +) -> None: + """Test attribute-based condition initialization from existing state. + + The condition uses last_updated (not last_changed) to determine how long + an attribute-based condition has been true. This is conservative: when + the main state changes but the tracked attribute stays the same, + last_updated is bumped and the effective duration resets. + """ + for step in steps: + freezer.tick(timedelta(seconds=step.delay_before)) + hass.states.async_set("test.entity_1", step.state, step.attrs) + await hass.async_block_till_done() + + freezer.tick(timedelta(seconds=duration)) + test = await _setup_attr_state_condition( + hass, + entity_ids="test.entity_1", + states={True}, + condition_options={CONF_FOR: {"seconds": 5}}, + ) + + assert test(hass) is initially_met + + +async def _setup_attr_state_condition_with_target( + hass: HomeAssistant, + target: dict[str, Any], + states: str | bool | set[str | bool], + condition_options: dict[str, Any] | None = None, +) -> condition.ConditionChecker: + """Set up an attribute-based state condition with a custom target.""" + condition_cls = make_entity_state_condition( + _ATTR_DOMAIN_SPECS, + states, + support_duration=True, + ) + + async def async_get_conditions( + hass: HomeAssistant, + ) -> dict[str, type[Condition]]: + return {"_": condition_cls} + + mock_integration(hass, MockModule("test")) + mock_platform( + hass, "test.condition", Mock(async_get_conditions=async_get_conditions) + ) + + config: dict[str, Any] = { + CONF_CONDITION: "test", + CONF_TARGET: target, + CONF_OPTIONS: condition_options or {}, + } + + config = await async_validate_condition_config(hass, config) + test = await condition.async_from_config(hass, config) + assert test is not None + return test + + +async def test_state_condition_attr_duration_entity_added_to_target( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test that _valid_since is primed when an entity is added to the tracked set. + + When targeting by label, adding a label to an entity should make it + tracked, and if it's already in a valid state, its duration should be + primed from the state timestamps. + """ + label_reg = lr.async_get(hass) + label = label_reg.async_create("Test Duration") + + entity_reg = er.async_get(hass) + entry = entity_reg.async_get_or_create( + domain="test", platform="test", unique_id="duration_add" + ) + + # Entity starts valid but without the label + hass.states.async_set(entry.entity_id, STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + # Create condition targeting the label + test = await _setup_attr_state_condition_with_target( + hass, + target={ATTR_LABEL_ID: label.label_id}, + states={True}, + condition_options={CONF_FOR: {"seconds": 5}}, + ) + + # No entities have the label yet — condition has no entities to check, + # behavior "any" with no matching entities returns False + assert test(hass) is False + + # Add the label to the entity — entity is already in valid state + freezer.tick(timedelta(seconds=1)) + entity_reg.async_update_entity(entry.entity_id, labels={label.label_id}) + await hass.async_block_till_done() + + # Just added — duration not met yet + assert test(hass) is False + + # Wait past the duration from when entity was last_updated + freezer.tick(timedelta(seconds=5)) + assert test(hass) is True + + +async def test_state_condition_attr_duration_entity_removed_from_target( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test that _valid_since is evicted when an entity is removed from the tracked set.""" + label_reg = lr.async_get(hass) + label = label_reg.async_create("Test Duration Remove") + + entity_reg = er.async_get(hass) + entry1 = entity_reg.async_get_or_create( + domain="test", platform="test", unique_id="duration_remove_1" + ) + entry2 = entity_reg.async_get_or_create( + domain="test", platform="test", unique_id="duration_remove_2" + ) + # Both entities start with the label + entity_reg.async_update_entity(entry1.entity_id, labels={label.label_id}) + entity_reg.async_update_entity(entry2.entity_id, labels={label.label_id}) + + # Both entities in valid state + hass.states.async_set(entry1.entity_id, STATE_ON, {"test_attr": True}) + hass.states.async_set(entry2.entity_id, STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + test = await _setup_attr_state_condition_with_target( + hass, + target={ATTR_LABEL_ID: label.label_id}, + states={True}, + condition_options={ + ATTR_BEHAVIOR: BEHAVIOR_ALL, + CONF_FOR: {"seconds": 5}, + }, + ) + + # Wait past duration — both valid + freezer.tick(timedelta(seconds=6)) + assert test(hass) is True + + # Remove label from entry2 + entity_reg.async_update_entity(entry2.entity_id, labels=set()) + await hass.async_block_till_done() + + # Condition should still be True — only entry1 is tracked now, and it's valid + assert test(hass) is True + + # Now remove label from entry1 too + entity_reg.async_update_entity(entry1.entity_id, labels=set()) + await hass.async_block_till_done() + + # No entities tracked — "all" with empty set is vacuously True + assert test(hass) is True + + # Change entry1 to invalid state and re-add its label + hass.states.async_set(entry1.entity_id, STATE_ON, {"test_attr": False}) + await hass.async_block_till_done() + entity_reg.async_update_entity(entry1.entity_id, labels={label.label_id}) + await hass.async_block_till_done() + + # entry1 is now tracked again but invalid — "all" fails + freezer.tick(timedelta(seconds=10)) + assert test(hass) is False + + +async def test_state_condition_attr_duration_entity_added_then_state_changes( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test that a newly added entity's state changes are properly tracked.""" + label_reg = lr.async_get(hass) + label = label_reg.async_create("Test Duration Track") + + entity_reg = er.async_get(hass) + entry = entity_reg.async_get_or_create( + domain="test", platform="test", unique_id="duration_track" + ) + + # Entity starts in invalid state + hass.states.async_set(entry.entity_id, STATE_ON, {"test_attr": False}) + await hass.async_block_till_done() + + # Create condition targeting the label + test = await _setup_attr_state_condition_with_target( + hass, + target={ATTR_LABEL_ID: label.label_id}, + states={True}, + condition_options={CONF_FOR: {"seconds": 5}}, + ) + + # Add the label — entity is invalid, so no priming + entity_reg.async_update_entity(entry.entity_id, labels={label.label_id}) + await hass.async_block_till_done() + assert test(hass) is False + + # Now change to valid state + freezer.tick(timedelta(seconds=1)) + hass.states.async_set(entry.entity_id, STATE_ON, {"test_attr": True}) + await hass.async_block_till_done() + + # Just became valid — not long enough + freezer.tick(timedelta(seconds=3)) + assert test(hass) is False + + # Now past the duration + freezer.tick(timedelta(seconds=3)) + assert test(hass) is True diff --git a/tests/helpers/test_target.py b/tests/helpers/test_target.py index 92a8a0e2ee2a36..c66a865f5fadba 100644 --- a/tests/helpers/test_target.py +++ b/tests/helpers/test_target.py @@ -762,3 +762,121 @@ async def set_states_and_check_events( ) unsub() + + +async def test_async_track_target_selector_state_change_event_on_entities_update( + hass: HomeAssistant, +) -> None: + """Test on_entities_update callback reports added and removed entities.""" + entity_updates: list[tuple[set[str], set[str]]] = [] + + @callback + def state_change_callback(event: target.TargetStateChangedData) -> None: + """Handle state change events.""" + + @callback + def on_entities_update(added: set[str], removed: set[str]) -> None: + """Track entity set changes.""" + entity_updates.append((added, removed)) + + config_entry = MockConfigEntry(domain="test") + config_entry.add_to_hass(hass) + + entity_reg = er.async_get(hass) + label_reg = lr.async_get(hass) + label = label_reg.async_create("Track Test") + + entity_a = entity_reg.async_get_or_create( + domain="light", platform="test", unique_id="track_a" + ) + entity_b = entity_reg.async_get_or_create( + domain="light", platform="test", unique_id="track_b" + ) + + # entity_a starts with the label + entity_reg.async_update_entity(entity_a.entity_id, labels={label.label_id}) + + hass.states.async_set(entity_a.entity_id, STATE_ON) + hass.states.async_set(entity_b.entity_id, STATE_ON) + await hass.async_block_till_done() + + unsub = target.async_track_target_selector_state_change_event( + hass, + {ATTR_LABEL_ID: label.label_id}, + state_change_callback, + on_entities_update=on_entities_update, + ) + + # Initial setup fires on_entities_update with all entities as "added" + assert len(entity_updates) == 1 + assert entity_updates[-1] == ({entity_a.entity_id}, set()) + entity_updates.clear() + + # Add label to entity_b → added + entity_reg.async_update_entity(entity_b.entity_id, labels={label.label_id}) + await hass.async_block_till_done() + + assert len(entity_updates) == 1 + assert entity_updates[-1] == ({entity_b.entity_id}, set()) + entity_updates.clear() + + # Remove label from entity_a → removed + entity_reg.async_update_entity(entity_a.entity_id, labels=set()) + await hass.async_block_till_done() + + assert len(entity_updates) == 1 + assert entity_updates[-1] == (set(), {entity_a.entity_id}) + entity_updates.clear() + + # Remove label from entity_b → removed + entity_reg.async_update_entity(entity_b.entity_id, labels=set()) + await hass.async_block_till_done() + + assert len(entity_updates) == 1 + assert entity_updates[-1] == (set(), {entity_b.entity_id}) + entity_updates.clear() + + # Re-add both labels at once — entity_a first, then entity_b + entity_reg.async_update_entity(entity_a.entity_id, labels={label.label_id}) + await hass.async_block_till_done() + entity_reg.async_update_entity(entity_b.entity_id, labels={label.label_id}) + await hass.async_block_till_done() + + assert len(entity_updates) == 2 + assert entity_updates[0] == ({entity_a.entity_id}, set()) + assert entity_updates[1] == ({entity_b.entity_id}, set()) + entity_updates.clear() + + # After unsubscribing, no more callbacks + unsub() + entity_reg.async_update_entity(entity_a.entity_id, labels=set()) + await hass.async_block_till_done() + assert len(entity_updates) == 0 + + +async def test_async_track_target_selector_no_on_entities_update( + hass: HomeAssistant, +) -> None: + """Test that on_entities_update is optional and defaults to no callback.""" + events: list[target.TargetStateChangedData] = [] + + @callback + def state_change_callback(event: target.TargetStateChangedData) -> None: + events.append(event) + + entity_id = "light.test_no_callback" + hass.states.async_set(entity_id, STATE_ON) + await hass.async_block_till_done() + + # No on_entities_update — should work without errors + unsub = target.async_track_target_selector_state_change_event( + hass, + {ATTR_ENTITY_ID: entity_id}, + state_change_callback, + ) + + hass.states.async_set(entity_id, STATE_OFF) + await hass.async_block_till_done() + assert len(events) == 1 + + unsub() From a8b9e8075cff1d79e95fe57d5c4fed6a026a73da Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 24 Apr 2026 08:41:33 +0200 Subject: [PATCH 2/4] Add missing super call to EntityConditionBase.async_unload --- homeassistant/helpers/condition.py | 1 + 1 file changed, 1 insertion(+) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index ada1707300cb0b..610751771c7956 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -543,6 +543,7 @@ def _on_entities_update(added: set[str], removed: set[str]) -> None: def async_unload(self) -> None: """Unsubscribe from listeners.""" + super().async_unload() for cb in self._on_unload: cb() self._on_unload.clear() From a425bd056d685a488a60e554281047a90de0e66f Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 28 Apr 2026 08:42:01 +0200 Subject: [PATCH 3/4] Address review comments --- homeassistant/helpers/condition.py | 48 +++++++++++++++--------------- tests/helpers/test_condition.py | 31 +++++++++++++++++++ 2 files changed, 55 insertions(+), 24 deletions(-) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index b531e163316965..a64afe8affe9ca 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -463,7 +463,7 @@ def __init__(self, hass: HomeAssistant, config: ConditionConfig) -> None: if TYPE_CHECKING: assert config.target assert config.options - self._target_config = config.target + self._target = config.target self._target_selection = TargetSelection(config.target) self._behavior = config.options[ATTR_BEHAVIOR] self._duration: timedelta | None = config.options.get(CONF_FOR) @@ -489,27 +489,36 @@ def _needs_duration_tracking(self) -> bool: """ return True - def _prime_valid_since(self, entity_id: str) -> None: - """Prime _valid_since for an entity already in a valid state. + def _update_valid_since(self, entity_id: str, _state: State | None) -> None: + """Update _valid_since tracking for an entity based on its current state. + + If the entity is in a valid state and not already tracked, records when + the condition became true. If the entity is not in a valid state, removes + it from tracking. For state-based conditions (value_source is None), last_changed accurately reflects when the state changed to the current value. For attribute-based conditions, last_changed only tracks main state changes, so we use last_updated which is bumped on any update - (state or attributes). This is conservative - the tracked attribute - may have held its value longer - but it's the best we can do + (state or attributes). This is conservative — the tracked attribute + may have held its value longer — but it's the best we can do to avoid false positives. """ if ( - (_state := self._hass.states.get(entity_id)) is not None + _state is not None and _state.state not in (STATE_UNAVAILABLE, STATE_UNKNOWN) and self.is_valid_state(_state) ): - domain_spec = self._domain_specs[_state.domain] - if domain_spec.value_source is None: - self._valid_since[entity_id] = _state.last_changed - else: - self._valid_since[entity_id] = _state.last_updated + # Only record the time if not already tracked, to avoid + # resetting the duration on unrelated state/attribute updates. + if entity_id not in self._valid_since: + domain_spec = self._domain_specs[_state.domain] + if domain_spec.value_source is None: + self._valid_since[entity_id] = _state.last_changed + else: + self._valid_since[entity_id] = _state.last_updated + else: + self._valid_since.pop(entity_id, None) @override async def async_setup(self) -> None: @@ -527,35 +536,26 @@ def _state_change_listener( entity_id = event.data["entity_id"] to_state = event.data["new_state"] - if ( - to_state is not None - and to_state.state not in (STATE_UNAVAILABLE, STATE_UNKNOWN) - and self.is_valid_state(to_state) - ): - if entity_id not in self._valid_since: - # Prime from the state object — this handles both - # genuine transitions and newly tracked entities - self._prime_valid_since(entity_id) - else: - self._valid_since.pop(entity_id, None) + self._update_valid_since(entity_id, to_state) @callback def _on_entities_update(added: set[str], removed: set[str]) -> None: """Handle changes to the tracked entity set.""" for entity_id in added: - self._prime_valid_since(entity_id) + self._update_valid_since(entity_id, self._hass.states.get(entity_id)) for entity_id in removed: self._valid_since.pop(entity_id, None) unsub = async_track_target_selector_state_change_event( self._hass, - self._target_config, + self._target, _state_change_listener, self.entity_filter, _on_entities_update, ) self._on_unload.append(unsub) + @override def async_unload(self) -> None: """Unsubscribe from listeners.""" super().async_unload() diff --git a/tests/helpers/test_condition.py b/tests/helpers/test_condition.py index 036df715f88bd3..13fcf455ae75f4 100644 --- a/tests/helpers/test_condition.py +++ b/tests/helpers/test_condition.py @@ -5008,3 +5008,34 @@ async def test_state_condition_attr_duration_entity_added_then_state_changes( # Now past the duration freezer.tick(timedelta(seconds=3)) assert test(hass) is True + + +async def test_state_condition_attr_duration_unrelated_attr_update( + hass: HomeAssistant, freezer: FrozenDateTimeFactory +) -> None: + """Test that unrelated attribute updates don't reset the duration timer. + + When the tracked attribute stays valid but another attribute changes, + _update_valid_since must not overwrite the existing timestamp. + """ + test = await _setup_attr_state_condition( + hass, + entity_ids="test.entity_1", + states={True}, + condition_options={CONF_FOR: {"seconds": 10}}, + ) + + # Set tracked attribute to True + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True, "other": "a"}) + await hass.async_block_till_done() + + # After 6s, change an unrelated attribute (tracked attr stays True) + freezer.tick(timedelta(seconds=6)) + hass.states.async_set("test.entity_1", STATE_ON, {"test_attr": True, "other": "b"}) + await hass.async_block_till_done() + + # After 5 more seconds (11 total from initial set), the duration + # should be met — the unrelated attribute change must NOT have + # reset the timer. + freezer.tick(timedelta(seconds=5)) + assert test(hass) is True From 58c1a2264896a548caa47e4949914c5367235ec3 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 28 Apr 2026 08:50:15 +0200 Subject: [PATCH 4/4] Adjust docstring --- homeassistant/helpers/condition.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index a64afe8affe9ca..e431875f229e94 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -482,10 +482,12 @@ def entity_filter(self, entities: set[str]) -> set[str]: def _needs_duration_tracking(self) -> bool: """Whether this condition needs active state change tracking for duration. - Conditions that are true for a single main state value can use - state.last_changed directly. Conditions that track attributes or - match multiple states need active tracking because last_changed - does not capture those transitions. + The base implementation intentionally defaults to always tracking + duration and should be overridden by subclasses that can safely use + state.last_changed directly. For example, conditions that are true + for a single main state value may not need active tracking, while + conditions that track attributes or match multiple states do because + last_changed does not capture those transitions. """ return True