-
-
Notifications
You must be signed in to change notification settings - Fork 197
Fix light groups not adapting when child lights turned on #1393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
## 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
…_turn_on The original test failed because HA light groups are expanded - the group itself is removed from self.lights and replaced with member lights. This means on_to_off_event is never stored for the group. Rewrote tests to directly test _off_to_on_state_event_is_from_turn_on(): - test_off_to_on_state_event_is_from_turn_on_detects_group_members - test_off_to_on_state_event_is_from_turn_on_respects_timing - test_just_turned_off_polling_artifact_still_detected (negative case)
The previous commit incorrectly moved the _off_to_on_state_event_is_from_turn_on check to before the context ID match check, which broke test_separate_turn_on_commands. The original order must be: 1. Check on_to_off_event is None → return False 2. Check context IDs match → return True (polling artifact) 3. Get transition info 4. Check _off_to_on_state_event_is_from_turn_on → return False This commit restores that order while keeping the light group detection logic in _off_to_on_state_event_is_from_turn_on.
Extract the light group turn-on detection logic into _member_turn_on_explains_group_turn_on() for better clarity: - Each method now has a single responsibility - The helper method has a descriptive name and docstring - The code flow is easier to follow This is a refactoring of the fix for issue #1378 with no behavior changes.
Add test_just_turned_off_context_reuse_with_light_groups which tests the actual scenario described in issue #1378: 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) 3. The group turns back on as a side effect 4. HA reuses the turn_off context (off_to_on_event has context A!) 5. just_turned_off() is called - should return False (allow adaptation) This test currently FAILS because the context ID match check at line 2804 returns True early, blocking adaptation before the member turn_on check at line 2824 is reached. The existing tests only test _off_to_on_state_event_is_from_turn_on() directly, bypassing just_turned_off(), which is why they pass despite the ordering issue. Refs: #1378
The context ID match check at line 2804 was returning True (blocking adaptation) before checking if a member light's turn_on event explained the group's turn-on. This caused the issue where light groups wouldn't adapt when a member was turned on by automation. The fix adds a check for _member_turn_on_explains_group_turn_on() inside the context ID match block. If a member's turn_on event happened after the group's on→off event, the group's turn-on is legitimate and not a polling artifact, so we return False (allow adaptation). This is a more targeted fix than reordering the checks, which broke test_separate_turn_on_commands. By keeping the context ID check first but adding the member check inside it, we: 1. Preserve the original polling artifact detection for non-groups 2. Properly handle light groups where HA reuses context IDs Fixes: #1378
Update the comment at line 2832 to accurately reflect that this check handles light groups when context IDs don't match. The primary fix for #1378 (matching context IDs) is handled earlier in the context ID match block.
Deep Dive Analysis: Light Group Context BehaviorAfter extensive investigation of the HA core source code (including spawning a separate agent to independently verify claims), I've identified the root cause of this issue. Executive SummaryThis is NOT a bug in Home Assistant Core. The observed behavior is expected when lights are controlled via integrations (like Zigbee2MQTT) that update state directly rather than going through HA's service handler. The context "reuse" happens because:
The Observed BehaviorFrom the debug logs: The question: Why does Verified Code ClaimsAll claims verified against HA core source with exact line numbers:
Detailed Code Flow AnalysisPhase 1: Group Turn Off (Context A Propagates to Members)Step 1: Service handler sets group context # service.py:888
entity.async_set_context(context) # bedroom_lights._context = AStep 2: Group forwards turn_off to members with its context # light.py:190-203
async def async_turn_off(self, **kwargs: Any) -> None:
await self.hass.services.async_call(
light.DOMAIN,
SERVICE_TURN_OFF,
data,
blocking=True,
context=self._context, # Forwards context A to members!
)Step 3: Each member receives and caches context A # service.py:888 (called for each member)
entity.async_set_context(context) # night_lights._context = A
# ceiling_lights._context = AResult: All member entities now have Phase 2: Motion Sensor Triggers Member (Context A Persists)Critical insight: When a motion sensor triggers via Zigbee2MQTT (or similar integration), it typically:
Step 1: Integration updates member state directly # Inside the integration (e.g., Zigbee2MQTT)
self._attr_is_on = True
self.async_write_ha_state() # Uses cached self._context!Step 2: Member writes state with cached context A # entity.py:1220-1236
if (
self._context_set is not None
and time_now - self._context_set > CONTEXT_RECENT_TIME_SECONDS # 5 seconds
):
self._context = None # Would clear, but only ~1.4 seconds passed!
# Context A is still valid:
self.hass.states.async_set_internal(..., self._context, ...) # Still context A!Step 3: State change event fires with context A Phase 3: Group Reacts to Member State ChangeStep 1: Group listener receives member's state change event # entity.py:80-89
@callback
def async_state_changed_listener(event):
self.async_set_context(event.context) # event.context = A (from member!)
self.async_defer_or_update_ha_state()Step 2: Group writes state with context A (inherited from member) Result: Group's OFF→ON state change has context A, matching the earlier ON→OFF. The Complete ScenarioWhy This Is NOT a Bug in HAThe behavior is by design:
Why the AL Fix is CorrectThe fix uses causality-based detection instead of context ID matching: def _member_turn_on_explains_group_turn_on(self, entity_id: str) -> bool:
for member in member_lights:
member_turn_on = self.turn_on_event.get(member)
if member_turn_on is not None:
if member_turn_on.time_fired > on_to_off_event.time_fired:
return True # Member turn_on explains group turn_on
return FalseThis works because:
Clarification Needed@GHM3434 - Before merging, I'd like to confirm a few things:
Summary
References
|
|
Clarification Needed Are your entities HA Light Groups? (created in Helpers → Create Helper → Group)
Or are they Zigbee2MQTT groups / integration-level groups?
Does your automation use light.turn_on service? This is an example of my kitchen automation yaml: alias: FL1 Kitchen light automation
action:
What is the relationship between your groups? Is bedroom_lights a "group of groups" containing night_lights and ceiling_lights?
|
|
@basnijholt omg i think this was an issue on my part (copied from comment above that I made): What is the relationship between your groups? Is bedroom_lights a "group of groups" containing night_lights and ceiling_lights?
I am still testing but this time, when the automation triggered, the lights turned red but then immediately turned to the correct color and brightness. |
|
Hello @basnijholt This is good news thank you for confirming and clarifying! For some reason I received an e-mail with your response but I can't find the comment anywhere on github... After you gave the solution to stop the light from flashing the old state right when turning on, I went to check my AL settings. Weirdly, the intercept settings were already enabled and it still appears to do the short flash at the beginning.
Question: Are You Testing the PR Branch? Understanding Your Group Structure: To make sure the fix covers your setup, can you confirm what's in your Adaptive Lighting configuration? Here is example of my ALL kitchen HA light group's member (currently):
Thank you |
|
@basnijholt Hi could you confirm you got my last response? I think I may be having issues with comments...and worried you didnt get my response. Thank you |
|
@GHM3434 Thanks, I see your responses now! The Core IssueYou have intercept enabled but still see the flash - this is expected with your current setup. Here's why: Your AL config has "All" HA Light Groups containing Z2M groups: Intercept can't help here because:
Your Question Answered
Neither. For AL to work properly (including intercept), your HA Light Groups should contain individual lights directly: This way:
The Problem with Z2M Groups + ALWhen Z2M groups are in the chain:
RecommendationRemove Z2M groups from your HA Light Groups and add individual lights directly. You can keep Z2M groups for other purposes, just don't put them in the HA groups that AL tracks. |
Thank goodness comments are working again! Okay, I will add individual Zigbee2MQTT "device" lights to the AL Home assistant light groups instead of the Zigbee2MQTT groups. I always like improving things! I was under the impression that Zigbee2MQTT groups were the optimal way to setup lights. I forget where I read it or what the reason was. I think it had something to do with network traffic causing issues or something. Questions:
Thank you |




Summary
Fixes issue where light groups don't adapt when child lights are turned on by an automation (e.g., motion sensor).
What Happened (from user's debug logs)
The user reported that their bedroom lights turned on red (previous color from a scene) instead of being adapted by Adaptive Lighting when a motion sensor triggered them.
Timeline from debug logs:
The problem: When the motion sensor turned on the child lights (
night_lights,ceiling_lights), the parent group (bedroom_lights) also turned on as a side effect. But Home Assistant reused the old turn_off context ID for the parent's state change.The
just_turned_off()function saw matching context IDs and incorrectly assumed this was a "polling artifact" (HA briefly reporting ON during a turn_off transition), so it cancelled adaptation.Root Cause
This check was designed to catch polling artifacts, but it didn't account for light groups where HA reuses context IDs.
Fix
Use causality-based detection instead of just context ID matching:
_off_to_on_state_event_is_from_turn_on()to check if any member light of a group has aturn_on_eventthat happened after the group's on→off event: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
Expected Behavior After Fix
With the user's scenario:
night_lights,ceiling_lights)turn_on_eventis stored for these membersbedroom_lights) turns on as side effect_off_to_on_state_event_is_from_turn_on()checks group membersturn_on_eventfornight_lightsthat happened after group's turn_offTrue→just_turned_off()returnsFalse→ Adaptation proceeds!Test Plan
test_just_turned_off_context_reuse_with_light_groups- verifies light group members' turn_on events are detectedtest_just_turned_off_polling_artifact_still_detected- verifies polling artifacts are still correctly blockedFixes #1378