diff --git a/README.md b/README.md index 3a26df24..a819f788 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/custom_components/adaptive_lighting/switch.py b/custom_components/adaptive_lighting/switch.py index e80e6089..3b042702 100644 --- a/custom_components/adaptive_lighting/switch.py +++ b/custom_components/adaptive_lighting/switch.py @@ -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.", diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 22374949..cdff4487 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -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. diff --git a/tests/test_switch.py b/tests/test_switch.py index 46957466..eabe226a 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -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):