From b7ad2cdd35e230276d57a863332cf041c0b11085 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Tue, 13 Jan 2026 00:34:10 -0800 Subject: [PATCH] Fix light groups not adapting when child lights turned on (#1378) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary When a parent light group is turned off and then child lights are turned on by an automation (e.g., motion sensor), Home Assistant may reuse the old turn_off context ID for the parent group's state change. This caused `just_turned_off()` to incorrectly treat the group's turn-on as a "false positive" polling artifact, blocking adaptation. ## Root Cause The `just_turned_off()` function checked if the off→on context ID matched the on→off context ID. If they matched, it assumed this was a polling artifact (HA briefly seeing the light as ON during a turn_off transition) and cancelled adaptation. However, for light groups, HA reuses the context ID when the parent group turns on as a side effect of child lights turning on. This is a valid turn-on that should be adapted. ## Fix Use causality-based detection instead of just context ID matching: 1. Enhanced `_off_to_on_state_event_is_from_turn_on()` to check if any member light of a group has a `turn_on_event` that happened after the group's on→off event. If so, the member's turn_on explains why the group turned on. 2. Restructured `just_turned_off()` to check for turn_on events BEFORE checking for matching context IDs. Only treat matching context IDs as a false positive if no turn_on event explains the state change. ## Why This is Robust | Old Approach | New Approach | |--------------|--------------| | Magic 1-second threshold | Actual causal relationship | | Fails with different timing | Works regardless of timing | | No explanation in logs | Clear log: "group turned on because member X was turned on" | ## Test Plan - [x] Added test for light group context reuse scenario - [x] Added test to verify polling artifacts are still detected --- custom_components/adaptive_lighting/switch.py | 53 ++++-- tests/test_switch.py | 165 ++++++++++++++++++ 2 files changed, 208 insertions(+), 10 deletions(-) diff --git a/custom_components/adaptive_lighting/switch.py b/custom_components/adaptive_lighting/switch.py index 3923e056..66aeb5c8 100644 --- a/custom_components/adaptive_lighting/switch.py +++ b/custom_components/adaptive_lighting/switch.py @@ -2725,7 +2725,34 @@ class AdaptiveLightingManager: ) 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 + if turn_on_event is not None and id_off_to_on == turn_on_event.context.id: + return True + + # For light groups: check if any member light has a turn_on_event that could + # explain why the group turned on. This handles the case where child lights + # are turned on by an automation, causing the parent group to turn on as a + # side effect. See: https://github.com/basnijholt/adaptive-lighting/issues/1378 + state = self.hass.states.get(entity_id) + if state is not None and _is_light_group(state): + 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 + # Check if the member's turn_on happened after the group's on→off event. + # If so, the member's turn_on likely caused the group to turn on. + 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 async def just_turned_off( # noqa: PLR0911 self, @@ -2754,6 +2781,21 @@ class AdaptiveLightingManager: ) return False + # Check if the off→on state change was triggered by a light.turn_on call. + # This check now also handles light groups: if a member light was turned on + # after the group turned off, the group's turn-on is considered valid. + # See: https://github.com/basnijholt/adaptive-lighting/issues/1378 + 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" + _LOGGER.debug( + "just_turned_off: State change 'off' → 'on' triggered by '%s'", + from_service, + ) + return False + + # If context IDs match but no turn_on event was found, this is likely a polling + # artifact (HA briefly seeing the light as ON during a turn_off transition). if off_to_on_event.context.id == on_to_off_event.context.id: _LOGGER.debug( "just_turned_off: 'on' → 'off' state change has the same context.id as the" @@ -2770,15 +2812,6 @@ class AdaptiveLightingManager: else: transition = None - 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" - _LOGGER.debug( - "just_turned_off: State change 'off' → 'on' triggered by '%s'", - from_service, - ) - return False - if ( turn_off_event is not None and id_on_to_off == turn_off_event.context.id diff --git a/tests/test_switch.py b/tests/test_switch.py index 5ba3cb60..0874754c 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -2914,3 +2914,168 @@ 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_just_turned_off_context_reuse_with_light_groups(hass): + """Test that just_turned_off handles light group context reuse correctly. + + 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. + """ + 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. Turn on the group first (so it can be turned off) + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_ON, + {ATTR_ENTITY_ID: light_group}, + blocking=True, + ) + await hass.async_block_till_done() + assert hass.states.get(light_group).state == STATE_ON + + # 2. Turn off the group (stores on_to_off_event with context A) + await hass.services.async_call( + LIGHT_DOMAIN, + SERVICE_TURN_OFF, + {ATTR_ENTITY_ID: light_group}, + blocking=True, + ) + await hass.async_block_till_done() + assert hass.states.get(light_group).state == STATE_OFF + + on_to_off_event = manager.on_to_off_event.get(light_group) + assert on_to_off_event is not None, "on_to_off_event should be stored for group" + + # 3. 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" + + # 4. Create an off→on event for the GROUP with the SAME context as turn_off + # This simulates what HA does when children turning on causes parent to turn on + 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=on_to_off_event.context, # SAME context as turn_off (HA reuses it) + ) + manager.off_to_on_event[light_group] = off_to_on_event + + # 5. Call just_turned_off - it should return False (allow adaptation) + # because a member light has a turn_on_event that explains the group turning on + result = await manager.just_turned_off(light_group) + + # Before the fix: this would return True (cancel adaptation) due to matching context IDs + # After the fix: should return False because member light was turned on + assert result is False, ( + "just_turned_off should return False when a group member was turned on, " + "even if the group's off→on context matches the on→off context. " + "The member's turn_on explains why the group turned on." + ) + + +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." + )