diff --git a/custom_components/adaptive_lighting/switch.py b/custom_components/adaptive_lighting/switch.py index 3923e056..be716113 100644 --- a/custom_components/adaptive_lighting/switch.py +++ b/custom_components/adaptive_lighting/switch.py @@ -2704,11 +2704,50 @@ class AdaptiveLightingManager: ) return changed_attributes + def _member_turn_on_explains_group_turn_on(self, entity_id: str) -> bool: + """Check if a light group turned on because a member light was turned on. + + When a parent light group is turned off and then a child light is turned on + (e.g., by a motion sensor), the group turns back on as a side effect. + This method detects that scenario by checking if any member has a turn_on_event + that happened after the group's on→off event. + + See: https://github.com/basnijholt/adaptive-lighting/issues/1378 + """ + state = self.hass.states.get(entity_id) + if state is None or not _is_light_group(state): + return False + + on_to_off_event = self.on_to_off_event.get(entity_id) + member_lights: list[str] = state.attributes.get("entity_id", []) + + for member in member_lights: + member_turn_on = self.turn_on_event.get(member) + if member_turn_on is None: + continue + # Only count if member was turned on AFTER the group was turned off + if on_to_off_event is None or ( + member_turn_on.time_fired > on_to_off_event.time_fired + ): + _LOGGER.debug( + "Light group '%s' turned on because member '%s' was turned on", + entity_id, + member, + ) + return True + + return False + def _off_to_on_state_event_is_from_turn_on( self, entity_id: str, off_to_on_event: Event[EventStateChangedData], ) -> bool: + """Check if an off→on state change was triggered by a light.turn_on call. + + For light groups, also checks if any member's turn_on explains the group's + turn-on (see _member_turn_on_explains_group_turn_on). + """ # Adaptive Lighting should never turn on lights itself if is_our_context(off_to_on_event.context) and not is_our_context( off_to_on_event.context, @@ -2723,11 +2762,19 @@ class AdaptiveLightingManager: off_to_on_event.context.id, off_to_on_event, ) - turn_on_event: Event | None = self.turn_on_event.get(entity_id) - id_off_to_on = off_to_on_event.context.id - return turn_on_event is not None and id_off_to_on == turn_on_event.context.id - async def just_turned_off( # noqa: PLR0911 + # Check if this entity was directly turned on via service call + turn_on_event = self.turn_on_event.get(entity_id) + if ( + turn_on_event is not None + and off_to_on_event.context.id == turn_on_event.context.id + ): + return True + + # For light groups: check if a member's turn_on explains the group's turn-on + return self._member_turn_on_explains_group_turn_on(entity_id) + + async def just_turned_off( # noqa: PLR0911, PLR0912 self, entity_id: str, ) -> bool: @@ -2755,6 +2802,18 @@ class AdaptiveLightingManager: return False if off_to_on_event.context.id == on_to_off_event.context.id: + # For light groups: check if a member's turn_on explains the group's turn_on. + # If so, this is NOT a polling artifact - it's a legitimate turn-on. + # HA may reuse the turn_off context for the group's state change when a + # member is turned on, which would incorrectly trigger this check. + # See: https://github.com/basnijholt/adaptive-lighting/issues/1378 + if self._member_turn_on_explains_group_turn_on(entity_id): + _LOGGER.debug( + "just_turned_off: Context IDs match for '%s' but a member light was " + "turned on, so this is a legitimate turn-on, not a polling artifact.", + entity_id, + ) + return False _LOGGER.debug( "just_turned_off: 'on' → 'off' state change has the same context.id as the" " 'off' → 'on' state change for '%s'. This is probably a false positive.", @@ -2770,6 +2829,9 @@ class AdaptiveLightingManager: else: transition = None + # Check if the off→on state change was triggered by a light.turn_on call. + # For light groups, this also checks if a member's turn_on explains the + # group's turn-on (handles cases where context IDs don't match). if self._off_to_on_state_event_is_from_turn_on(entity_id, off_to_on_event): is_toggle = off_to_on_event == self.toggle_event.get(entity_id) from_service = "light.toggle" if is_toggle else "light.turn_on" diff --git a/tests/test_switch.py b/tests/test_switch.py index 5ba3cb60..c0a0e71f 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -2914,3 +2914,345 @@ async def test_adapt_only_on_bare_turn_on_respects_pause_changed_mode(hass, inte f"With take_over_control_mode=PAUSE_CHANGED and only brightness marked " f"as manually controlled, color_temp should still be adapted." ) + + +async def test_off_to_on_state_event_is_from_turn_on_detects_group_members(hass): + """Test that _off_to_on_state_event_is_from_turn_on detects member turn_on events. + + Regression test for https://github.com/basnijholt/adaptive-lighting/issues/1378 + + When a parent light group is turned off and then child lights are turned on + via motion sensor (or other automation), Home Assistant may reuse the old + turn_off context ID for the parent group's off→on state change. + + The fix uses causality-based detection: if any member light has a turn_on_event + that happened after the group's on→off event, the group's turn-on is valid. + + This test directly tests _off_to_on_state_event_is_from_turn_on() with mocked + events to verify the causality detection works for light groups. + """ + from homeassistant.core import Event + + # Setup lights with a group (light_group contains light_4 and light_5) + lights = await setup_lights(hass, with_group=True) + entity_ids = [light.entity_id for light in lights[:3]] + entity_ids.append("light.light_group") + + _, switch = await setup_switch( + hass, + { + CONF_LIGHTS: entity_ids, + CONF_SUNRISE_TIME: datetime.time(SUNRISE.hour), + CONF_SUNSET_TIME: datetime.time(SUNSET.hour), + CONF_INITIAL_TRANSITION: 0, + CONF_TRANSITION: 0, + }, + ) + await hass.async_block_till_done() + manager = switch.manager + + light_group = "light.light_group" + member_light = "light.light_4" + + # Verify the group is expanded and members are tracked + assert member_light in switch.lights, "Group members should be in switch.lights" + + # 1. Simulate a member light being turned on by a motion sensor + # This stores turn_on_event for the member + member_turn_on_context = Context(id="motion_sensor_turn_on") + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: member_light}, + blocking=True, + context=member_turn_on_context, + ) + await hass.async_block_till_done() + + # Verify turn_on_event was stored for the member + assert ( + manager.turn_on_event.get(member_light) is not None + ), "turn_on_event should be stored for member light" + + # 2. Create an off→on event for the GROUP + # This simulates the group turning on because a member was turned on + group_off_to_on_context = Context(id="group_state_change") + off_to_on_event = Event( + event_type=EVENT_STATE_CHANGED, + data={ + ATTR_ENTITY_ID: light_group, + "old_state": State(light_group, STATE_OFF), + "new_state": State(light_group, STATE_ON), + }, + context=group_off_to_on_context, + ) + + # 3. Call _off_to_on_state_event_is_from_turn_on for the GROUP + # It should return True because member light was turned on + result = manager._off_to_on_state_event_is_from_turn_on( + light_group, + off_to_on_event, + ) + + # Before the fix: this would return False (no turn_on for group itself) + # After the fix: should return True because member light was turned on + assert result is True, ( + "_off_to_on_state_event_is_from_turn_on should return True when a group member " + "was turned on, even if no turn_on event exists for the group itself. " + "The member's turn_on explains why the group turned on." + ) + + +async def test_off_to_on_state_event_is_from_turn_on_respects_timing(hass): + """Test that member turn_on must happen after group's on→off to count. + + This ensures we don't incorrectly attribute a group's turn-on to an old + member turn_on event that happened before the group was turned off. + """ + from homeassistant.core import Event + + # Setup lights with a group + lights = await setup_lights(hass, with_group=True) + entity_ids = [light.entity_id for light in lights[:3]] + entity_ids.append("light.light_group") + + _, switch = await setup_switch( + hass, + { + CONF_LIGHTS: entity_ids, + CONF_SUNRISE_TIME: datetime.time(SUNRISE.hour), + CONF_SUNSET_TIME: datetime.time(SUNSET.hour), + CONF_INITIAL_TRANSITION: 0, + CONF_TRANSITION: 0, + }, + ) + await hass.async_block_till_done() + manager = switch.manager + + light_group = "light.light_group" + member_light = "light.light_4" + + # 1. Turn on member first (stores turn_on_event with early timestamp) + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: member_light}, + blocking=True, + ) + await hass.async_block_till_done() + member_turn_on_event = manager.turn_on_event.get(member_light) + assert member_turn_on_event is not None + + # 2. Simulate the group being turned off AFTER the member turn_on + # Create a fake on_to_off_event with a timestamp after the member turn_on + await asyncio.sleep(0.01) # Small delay to ensure different timestamps + group_turn_off_context = Context(id="group_turn_off") + group_on_to_off_event = Event( + event_type=EVENT_STATE_CHANGED, + data={ + ATTR_ENTITY_ID: light_group, + "old_state": State(light_group, STATE_ON), + "new_state": State(light_group, STATE_OFF), + }, + context=group_turn_off_context, + ) + manager.on_to_off_event[light_group] = group_on_to_off_event + + # 3. Create an off→on event for the group + off_to_on_event = Event( + event_type=EVENT_STATE_CHANGED, + data={ + ATTR_ENTITY_ID: light_group, + "old_state": State(light_group, STATE_OFF), + "new_state": State(light_group, STATE_ON), + }, + context=Context(id="group_turn_on"), + ) + + # 4. The member's turn_on happened BEFORE the group's on→off, + # so it should NOT explain the group's turn-on + result = manager._off_to_on_state_event_is_from_turn_on( + light_group, + off_to_on_event, + ) + + assert result is False, ( + "_off_to_on_state_event_is_from_turn_on should return False when the member's " + "turn_on happened BEFORE the group's on→off event. The old turn_on event " + "cannot explain a new turn-on after the group was turned off." + ) + + +async def test_just_turned_off_polling_artifact_still_detected(hass): + """Test that polling artifacts are still correctly detected as false positives. + + This is a companion test to test_just_turned_off_context_reuse_with_light_groups. + It verifies that when context IDs match and NO member light was turned on, + the off→on is still correctly treated as a polling artifact (false positive). + + This ensures the fix for #1378 doesn't break the original polling artifact detection. + """ + from homeassistant.core import Event + + switch, _ = await setup_lights_and_switch(hass) + manager = switch.manager + + # 1. Turn on the light first + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: ENTITY_LIGHT_1}, + blocking=True, + ) + await hass.async_block_till_done() + assert hass.states.get(ENTITY_LIGHT_1).state == STATE_ON + + # 2. Turn off the light (stores on_to_off_event) + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_OFF, + {ATTR_ENTITY_ID: ENTITY_LIGHT_1}, + blocking=True, + ) + await hass.async_block_till_done() + assert hass.states.get(ENTITY_LIGHT_1).state == STATE_OFF + + on_to_off_event = manager.on_to_off_event.get(ENTITY_LIGHT_1) + assert on_to_off_event is not None + + # 3. Create an off→on event with the SAME context as turn_off + # This simulates a polling artifact (HA briefly seeing ON during turn_off transition) + # NOTE: We do NOT turn on any lights, so no turn_on_event is stored + off_to_on_event = Event( + event_type=EVENT_STATE_CHANGED, + data={ + ATTR_ENTITY_ID: ENTITY_LIGHT_1, + "old_state": State(ENTITY_LIGHT_1, STATE_OFF), + "new_state": State(ENTITY_LIGHT_1, STATE_ON), + }, + context=on_to_off_event.context, # SAME context as turn_off + ) + manager.off_to_on_event[ENTITY_LIGHT_1] = off_to_on_event + + # 4. Call just_turned_off - it should return True (block adaptation) + # because this is a polling artifact (no turn_on event, matching context IDs) + result = await manager.just_turned_off(ENTITY_LIGHT_1) + + assert result is True, ( + "just_turned_off should return True (block adaptation) when context IDs match " + "and no turn_on event explains the off→on. This is a polling artifact." + ) + + +async def test_just_turned_off_context_reuse_with_light_groups(hass): + """Test that light groups adapt even when HA reuses the turn_off context ID. + + Regression test for https://github.com/basnijholt/adaptive-lighting/issues/1378 + + This is the CRITICAL test case that was missing. It tests the actual scenario: + 1. A light group is turned off (stores on_to_off_event with context A) + 2. A member light is turned on by automation (stores turn_on_event with context B) + 3. The group turns back on as a side effect + 4. Home Assistant reuses the turn_off context (off_to_on_event has context A!) + 5. just_turned_off() is called - it should return False (allow adaptation) + + The bug: The context ID match check at line 2804 returns True early, + blocking adaptation before the member turn_on check at line 2824 is reached. + """ + from homeassistant.core import Context, Event + + # Setup lights with a group (light_group contains light_4 and light_5) + lights = await setup_lights(hass, with_group=True) + entity_ids = [light.entity_id for light in lights[:3]] + entity_ids.append("light.light_group") + + _, switch = await setup_switch( + hass, + { + CONF_LIGHTS: entity_ids, + CONF_SUNRISE_TIME: datetime.time(SUNRISE.hour), + CONF_SUNSET_TIME: datetime.time(SUNSET.hour), + CONF_INITIAL_TRANSITION: 0, + CONF_TRANSITION: 0, + }, + ) + await hass.async_block_till_done() + manager = switch.manager + + light_group = "light.light_group" + member_light = "light.light_4" + + # Verify the group is expanded and members are tracked + assert member_light in switch.lights, "Group members should be in switch.lights" + + # 1. Simulate the group being turned off first + # This creates the on_to_off_event with a specific context + group_turn_off_context = Context(id="group_turn_off_context") + group_on_to_off_event = Event( + event_type=EVENT_STATE_CHANGED, + data={ + ATTR_ENTITY_ID: light_group, + "old_state": State(light_group, STATE_ON), + "new_state": State(light_group, STATE_OFF), + }, + context=group_turn_off_context, + ) + manager.on_to_off_event[light_group] = group_on_to_off_event + + # 2. Small delay to ensure member turn_on happens AFTER group turn_off + await asyncio.sleep(0.01) + + # 3. Simulate a member light being turned on by a motion sensor + # This stores turn_on_event for the member with a DIFFERENT context + member_turn_on_context = Context(id="motion_sensor_turn_on") + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: member_light}, + blocking=True, + context=member_turn_on_context, + ) + await hass.async_block_till_done() + + # Verify turn_on_event was stored for the member + assert ( + manager.turn_on_event.get(member_light) is not None + ), "turn_on_event should be stored for member light" + + # Verify the member's turn_on happened after the group's on_to_off + assert ( + manager.turn_on_event[member_light].time_fired + > group_on_to_off_event.time_fired + ), "Member turn_on should happen after group on_to_off" + + # 4. Create an off→on event for the GROUP with the SAME context as turn_off + # This simulates HA reusing the turn_off context ID (the bug scenario!) + off_to_on_event = Event( + event_type=EVENT_STATE_CHANGED, + data={ + ATTR_ENTITY_ID: light_group, + "old_state": State(light_group, STATE_OFF), + "new_state": State(light_group, STATE_ON), + }, + context=group_turn_off_context, # SAME context as turn_off - this is the key! + ) + manager.off_to_on_event[light_group] = off_to_on_event + + # Verify context IDs match (this is what triggers the bug) + assert ( + off_to_on_event.context.id == group_on_to_off_event.context.id + ), "Context IDs should match to simulate the HA context reuse scenario" + + # 5. Call just_turned_off - it should return False (allow adaptation) + # because a member light was turned on after the group was turned off. + # + # BUG: Currently returns True because the context ID match check + # at line 2804 returns early, before the member check at line 2824. + result = await manager.just_turned_off(light_group) + + assert result is False, ( + "just_turned_off should return False (allow adaptation) for light groups " + "when a member was turned on after the group was turned off, EVEN IF " + "the context IDs match due to HA's context reuse behavior. " + "The member's turn_on_event explains why the group turned on." + )