mirror of
https://github.com/basnijholt/adaptive-lighting.git
synced 2026-09-11 14:24:03 +02:00
fix: don't cancel adaptation when a light group turns on via a member with a reused context (#1483)
* fix: don't cancel adaptation when a light group turns on via a member with a reused context (#1378) When a member of a light group is turned on (e.g., by a motion sensor automation) while the group is off, the group turns on as a side effect, but Home Assistant may reuse the context of the earlier turn_off call for the group's state change. just_turned_off() saw matching context IDs and treated the state change as a polling artifact, cancelling adaptation. - Check whether the off->on state change comes from a light.turn_on call before the matching-context polling-artifact check, so automations that turn a light off and back on with a single (automation) context adapt correctly. - For light groups, allow adaptation when a member's turn_on event falls between the group's on->off and off->on state changes, bounded on both sides so stale member events are never treated as explanatory. - Document that integration-level groups (e.g., Zigbee2MQTT groups) should not be nested inside HA Light Groups managed by Adaptive Lighting. * fix: time-bound the same-context turn_on check instead of reordering Address review findings: - Reordering the turn_on-service check above the matching-context check reintroduced stale-event false negatives: turn_on_event entries are never cleaned up, so a 'turn_on -> delay -> turn_off(transition)' automation (one shared context) would defeat the polling-artifact guard and AL could turn a light back on right after it was turned off. Restore main's check order and instead add a time-bounded own-turn_on check inside the matching-context branch, symmetric with the group-member check. This also avoids emitting the 'should not happen' warning for self-context polling artifacts. - Add a regression test for the stale same-context turn_on case. - Add an end-to-end test driving the event-bus listeners for the #1378 scenario (group kept in manager.lights, as in the reported setups). - Docs: drop the inaccurate 'expands only one level deep' claim; explain that integration-level groups cannot be expanded and nested groups make tracking unpredictable.
This commit is contained in:
Bas Nijholt
GitHub
parent
d4d3d50ada
commit
3638fb3013
4 changed files with 282 additions and 1 deletions
|
|
@ -409,6 +409,11 @@ Expose only the group (not individual bulbs) in Home Assistant Dashboards and ex
|
|||
|
||||
> :warning: **If you control lights individually, `manual_control` cannot behave correctly! If you need to control lights individually as well, use a [Home Assistant Light Group](https://www.home-assistant.io/integrations/group/).**
|
||||
|
||||
When mixing group types, avoid nesting: do not add integration-level groups (e.g., Zigbee2MQTT groups) to a [Home Assistant Light Group](https://www.home-assistant.io/integrations/group/) that is managed by Adaptive Lighting, and do not nest Home Assistant Light Groups inside each other.
|
||||
Adaptive Lighting cannot expand an integration-level group into its member lights, and nested groups make it unpredictable which entity Adaptive Lighting tracks and adapts, which can prevent lights from being adapted at all (see [#1378](https://github.com/basnijholt/adaptive-lighting/issues/1378)).
|
||||
Instead, add the individual light entities or a single Zigbee group directly to the Adaptive Lighting configuration.
|
||||
Also note that bulbs turned on via a Zigbee group broadcast may briefly flash their last (cached) brightness and color before the adapted values arrive; this happens inside the bulbs and cannot be prevented by Home Assistant or Adaptive Lighting.
|
||||
|
||||
#### :rainbow: Light Colors Not Matching
|
||||
|
||||
Bulbs from different manufacturers or models may have varying color temperature specifications. For instance, if you have two Adaptive Lighting configurations—one with only Philips Hue White Ambiance bulbs and another with a mix of Philips Hue White Ambiance and Sengled bulbs—the Philips Hue bulbs may appear to have different color temperatures despite having identical settings.
|
||||
|
|
|
|||
|
|
@ -2746,7 +2746,45 @@ class AdaptiveLightingManager:
|
|||
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
|
||||
def _member_turn_on_explains_group_turn_on(
|
||||
self,
|
||||
entity_id: str,
|
||||
on_to_off_event: Event[EventStateChangedData],
|
||||
off_to_on_event: Event[EventStateChangedData],
|
||||
) -> bool:
|
||||
"""Check if a light group's 'off' → 'on' is caused by a member's 'light.turn_on'.
|
||||
|
||||
When a member of a light group is turned on while the group is off, the
|
||||
group turns on as a side effect. Home Assistant may reuse the context of
|
||||
an earlier 'light.turn_off' call for the group's state change (entities
|
||||
keep their context for a few seconds), which makes the group's turn-on
|
||||
look like a polling artifact of the turn-off.
|
||||
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
|
||||
members: list[str] = state.attributes[ATTR_ENTITY_ID]
|
||||
for member in members:
|
||||
member_turn_on = self.turn_on_event.get(member)
|
||||
if (
|
||||
member_turn_on is not None
|
||||
and on_to_off_event.time_fired
|
||||
< member_turn_on.time_fired
|
||||
<= off_to_on_event.time_fired
|
||||
):
|
||||
_LOGGER.debug(
|
||||
"just_turned_off: Light group '%s' turned on because its member"
|
||||
" '%s' was turned on (context.id='%s'), so this is a legitimate"
|
||||
" turn-on, not a polling artifact.",
|
||||
entity_id,
|
||||
member,
|
||||
member_turn_on.context.id,
|
||||
)
|
||||
return True
|
||||
return False
|
||||
|
||||
async def just_turned_off( # noqa: PLR0911, PLR0912
|
||||
self,
|
||||
entity_id: str,
|
||||
) -> bool:
|
||||
|
|
@ -2774,6 +2812,34 @@ class AdaptiveLightingManager:
|
|||
return False
|
||||
|
||||
if off_to_on_event.context.id == on_to_off_event.context.id:
|
||||
# Matching context IDs usually mean a polling artifact (HA briefly
|
||||
# reports 'on' while the light is still turning off). However, the
|
||||
# context is also reused when e.g. one automation turns the light
|
||||
# off and later back on, or when an integration writes the state
|
||||
# with the entity's cached context. Only treat the state change as
|
||||
# a legitimate turn-on if a 'light.turn_on' call for this light (or
|
||||
# for a member of this light group) fired between the two state
|
||||
# changes.
|
||||
turn_on_event = self.turn_on_event.get(entity_id)
|
||||
if (
|
||||
turn_on_event is not None
|
||||
and on_to_off_event.time_fired
|
||||
< turn_on_event.time_fired
|
||||
<= off_to_on_event.time_fired
|
||||
):
|
||||
_LOGGER.debug(
|
||||
"just_turned_off: 'light.turn_on' was called for '%s' between its"
|
||||
" 'on' → 'off' and 'off' → 'on' state changes, so this is a"
|
||||
" legitimate turn-on, not a polling artifact.",
|
||||
entity_id,
|
||||
)
|
||||
return False
|
||||
if self._member_turn_on_explains_group_turn_on(
|
||||
entity_id,
|
||||
on_to_off_event,
|
||||
off_to_on_event,
|
||||
):
|
||||
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.",
|
||||
|
|
|
|||
|
|
@ -71,6 +71,11 @@ Expose only the group (not individual bulbs) in Home Assistant Dashboards and ex
|
|||
|
||||
> :warning: **If you control lights individually, `manual_control` cannot behave correctly! If you need to control lights individually as well, use a [Home Assistant Light Group](https://www.home-assistant.io/integrations/group/).**
|
||||
|
||||
When mixing group types, avoid nesting: do not add integration-level groups (e.g., Zigbee2MQTT groups) to a [Home Assistant Light Group](https://www.home-assistant.io/integrations/group/) that is managed by Adaptive Lighting, and do not nest Home Assistant Light Groups inside each other.
|
||||
Adaptive Lighting cannot expand an integration-level group into its member lights, and nested groups make it unpredictable which entity Adaptive Lighting tracks and adapts, which can prevent lights from being adapted at all (see [#1378](https://github.com/basnijholt/adaptive-lighting/issues/1378)).
|
||||
Instead, add the individual light entities or a single Zigbee group directly to the Adaptive Lighting configuration.
|
||||
Also note that bulbs turned on via a Zigbee group broadcast may briefly flash their last (cached) brightness and color before the adapted values arrive; this happens inside the bulbs and cannot be prevented by Home Assistant or Adaptive Lighting.
|
||||
|
||||
#### :rainbow: Light Colors Not Matching
|
||||
|
||||
Bulbs from different manufacturers or models may have varying color temperature specifications. For instance, if you have two Adaptive Lighting configurations—one with only Philips Hue White Ambiance bulbs and another with a mix of Philips Hue White Ambiance and Sengled bulbs—the Philips Hue bulbs may appear to have different color temperatures despite having identical settings.
|
||||
|
|
|
|||
|
|
@ -2429,6 +2429,211 @@ async def test_light_group(
|
|||
assert len(events) == 3
|
||||
|
||||
|
||||
def _state_changed_event(entity_id: str, ts: float, context: Context) -> Event:
|
||||
return Event(
|
||||
EVENT_STATE_CHANGED,
|
||||
{"entity_id": entity_id},
|
||||
time_fired_timestamp=ts,
|
||||
context=context,
|
||||
)
|
||||
|
||||
|
||||
def _turn_on_service_event(entity_ids: list[str], ts: float, context: Context) -> Event:
|
||||
return Event(
|
||||
EVENT_CALL_SERVICE,
|
||||
{
|
||||
"domain": LIGHT_DOMAIN,
|
||||
"service": SERVICE_TURN_ON,
|
||||
"service_data": {ATTR_ENTITY_ID: entity_ids},
|
||||
},
|
||||
time_fired_timestamp=ts,
|
||||
context=context,
|
||||
)
|
||||
|
||||
|
||||
async def test_just_turned_off_group_context_reuse(hass, cleanup):
|
||||
"""Group 'off' → 'on' with a reused 'turn_off' context must still adapt.
|
||||
|
||||
When a member of a light group is turned on (e.g., by a motion sensor
|
||||
automation) while the group is off, the group turns on as a side effect,
|
||||
but Home Assistant may reuse the context of the earlier 'turn_off' call
|
||||
for the group's state change. `just_turned_off` used to treat this as a
|
||||
polling artifact and cancel adaptation.
|
||||
|
||||
Regression test for https://github.com/basnijholt/adaptive-lighting/issues/1378
|
||||
"""
|
||||
await setup_lights(hass, with_group=True)
|
||||
_, switch = await setup_switch(hass, {CONF_LIGHTS: ["light.light_group"]})
|
||||
await hass.async_block_till_done()
|
||||
manager = switch.manager
|
||||
|
||||
group = "light.light_group"
|
||||
member = "light.light_4"
|
||||
now = dt_util.utcnow().timestamp()
|
||||
turn_off_context = Context()
|
||||
|
||||
# The group was turned off 2 seconds ago...
|
||||
manager.on_to_off_event[group] = _state_changed_event(
|
||||
group,
|
||||
now - 2,
|
||||
turn_off_context,
|
||||
)
|
||||
# ...then an automation turned on a member light with a fresh context...
|
||||
manager.turn_on_event[member] = _turn_on_service_event(
|
||||
[member],
|
||||
now - 0.5,
|
||||
Context(),
|
||||
)
|
||||
# ...which turned the group back on, but HA reused the old turn_off context.
|
||||
manager.off_to_on_event[group] = _state_changed_event(
|
||||
group,
|
||||
now,
|
||||
turn_off_context,
|
||||
)
|
||||
|
||||
# The member's turn_on explains the group's turn-on: adaptation must proceed.
|
||||
assert not await manager.just_turned_off(group)
|
||||
|
||||
# A member turn_on from *before* the group was turned off does not explain
|
||||
# the group's turn-on: this must still be treated as a polling artifact.
|
||||
manager.turn_on_event[member] = _turn_on_service_event(
|
||||
[member],
|
||||
now - 10,
|
||||
Context(),
|
||||
)
|
||||
assert await manager.just_turned_off(group)
|
||||
|
||||
# Without any member turn_on event, the matching context IDs must still be
|
||||
# treated as a polling artifact.
|
||||
del manager.turn_on_event[member]
|
||||
assert await manager.just_turned_off(group)
|
||||
|
||||
|
||||
async def test_just_turned_off_same_automation_context(hass, cleanup):
|
||||
"""'turn_off' and 'turn_on' from one automation share a context.
|
||||
|
||||
An automation calling 'light.turn_off' and later 'light.turn_on' reuses
|
||||
its own context for both service calls, so the 'on' → 'off' and
|
||||
'off' → 'on' state changes have matching context IDs. The turn_on service
|
||||
call must take precedence over the matching-context polling-artifact check.
|
||||
"""
|
||||
await setup_lights(hass)
|
||||
_, switch = await setup_switch(hass, {CONF_LIGHTS: [ENTITY_LIGHT_1]})
|
||||
await hass.async_block_till_done()
|
||||
manager = switch.manager
|
||||
|
||||
now = dt_util.utcnow().timestamp()
|
||||
automation_context = Context()
|
||||
|
||||
manager.on_to_off_event[ENTITY_LIGHT_1] = _state_changed_event(
|
||||
ENTITY_LIGHT_1,
|
||||
now - 2,
|
||||
automation_context,
|
||||
)
|
||||
manager.turn_on_event[ENTITY_LIGHT_1] = _turn_on_service_event(
|
||||
[ENTITY_LIGHT_1],
|
||||
now - 0.5,
|
||||
automation_context,
|
||||
)
|
||||
manager.off_to_on_event[ENTITY_LIGHT_1] = _state_changed_event(
|
||||
ENTITY_LIGHT_1,
|
||||
now,
|
||||
automation_context,
|
||||
)
|
||||
assert not await manager.just_turned_off(ENTITY_LIGHT_1)
|
||||
|
||||
# A stale turn_on with an unrelated context does not explain the
|
||||
# 'off' → 'on' state change: still a polling artifact.
|
||||
manager.turn_on_event[ENTITY_LIGHT_1] = _turn_on_service_event(
|
||||
[ENTITY_LIGHT_1],
|
||||
now - 10,
|
||||
Context(),
|
||||
)
|
||||
assert await manager.just_turned_off(ENTITY_LIGHT_1)
|
||||
|
||||
# A stale turn_on *sharing the automation's context* but fired before the
|
||||
# 'on' → 'off' state change (i.e., 'turn_on' → delay → 'turn_off' in one
|
||||
# automation run) does not explain the 'off' → 'on' state change either:
|
||||
# `turn_on_event` entries are never cleaned up, so without the time bounds
|
||||
# this would defeat the polling-artifact detection.
|
||||
manager.turn_on_event[ENTITY_LIGHT_1] = _turn_on_service_event(
|
||||
[ENTITY_LIGHT_1],
|
||||
now - 10,
|
||||
automation_context,
|
||||
)
|
||||
assert await manager.just_turned_off(ENTITY_LIGHT_1)
|
||||
|
||||
|
||||
async def test_just_turned_off_group_context_reuse_end_to_end(hass, cleanup):
|
||||
"""Drive the issue #1378 scenario through the real event bus listeners.
|
||||
|
||||
Unlike `test_just_turned_off_group_context_reuse`, which calls
|
||||
`just_turned_off` directly, this test fires the service and state-changed
|
||||
events on the bus. Light groups are normally expanded out of
|
||||
`manager.lights`, but they can remain tracked in real setups (e.g., when a
|
||||
group is nested inside another configured group or is unavailable during
|
||||
setup), which is the configuration under which issue #1378 was reported.
|
||||
"""
|
||||
await setup_lights(hass, with_group=True)
|
||||
_, switch = await setup_switch(hass, {CONF_LIGHTS: ["light.light_group"]})
|
||||
await hass.async_block_till_done()
|
||||
manager = switch.manager
|
||||
|
||||
group = "light.light_group"
|
||||
member = "light.light_4"
|
||||
assert member in manager.lights
|
||||
# Simulate a setup in which the group entity itself remains tracked.
|
||||
manager.lights.add(group)
|
||||
|
||||
turn_off_context = Context()
|
||||
# The group was turned off...
|
||||
hass.bus.async_fire(
|
||||
EVENT_STATE_CHANGED,
|
||||
{
|
||||
"entity_id": group,
|
||||
"old_state": State(group, STATE_ON),
|
||||
"new_state": State(group, STATE_OFF),
|
||||
},
|
||||
context=turn_off_context,
|
||||
)
|
||||
await hass.async_block_till_done()
|
||||
assert group in manager.on_to_off_event
|
||||
|
||||
# ...then an automation turned on a member light with a fresh context...
|
||||
hass.bus.async_fire(
|
||||
EVENT_CALL_SERVICE,
|
||||
{
|
||||
"domain": LIGHT_DOMAIN,
|
||||
"service": SERVICE_TURN_ON,
|
||||
"service_data": {ATTR_ENTITY_ID: [member]},
|
||||
},
|
||||
context=Context(),
|
||||
)
|
||||
await hass.async_block_till_done()
|
||||
assert member in manager.turn_on_event
|
||||
|
||||
# ...which turned the group back on, but HA reused the old turn_off context.
|
||||
with patch.object(
|
||||
AdaptiveSwitch,
|
||||
"_respond_to_off_to_on_event",
|
||||
AsyncMock(),
|
||||
) as respond:
|
||||
hass.bus.async_fire(
|
||||
EVENT_STATE_CHANGED,
|
||||
{
|
||||
"entity_id": group,
|
||||
"old_state": State(group, STATE_OFF),
|
||||
"new_state": State(group, STATE_ON),
|
||||
},
|
||||
context=turn_off_context,
|
||||
)
|
||||
await hass.async_block_till_done()
|
||||
|
||||
# Adaptation must not have been cancelled as a polling artifact.
|
||||
respond.assert_called_once()
|
||||
assert respond.call_args[0][0] == group
|
||||
|
||||
|
||||
@pytest.mark.parametrize("brightness_mode", ["linear", "tanh"])
|
||||
@pytest.mark.parametrize(("dark", "light"), ([900, 1800], [1800, 900], [1800, 1800]))
|
||||
async def test_brightness_mode(hass, brightness_mode, dark, light):
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue