mirror of
https://github.com/basnijholt/adaptive-lighting.git
synced 2026-09-16 16:54:04 +02:00
Fix light groups not adapting when child lights turned on (#1378)
## 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
This commit is contained in:
parent
eb25df01d5
commit
b7ad2cdd35
2 changed files with 208 additions and 10 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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."
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue