Skip to content

feat: add expand_light_groups option#1462

Open
hesseleo wants to merge 2 commits into
basnijholt:mainfrom
hesseleo:feat/expand-group-option
Open

feat: add expand_light_groups option#1462
hesseleo wants to merge 2 commits into
basnijholt:mainfrom
hesseleo:feat/expand-group-option

Conversation

@hesseleo
Copy link
Copy Markdown

Some light group entities act as a proxy that must receive a single combined light.turn_on call to function correctly — virtual mixers like https://github.com/mion00/color-temperature-light-mixer for instance, that blend a warm and a cold white channel into one entity. In such setups the individual member entities only expose ColorMode.BRIGHTNESS, so sending separate per-member commands bypasses the mixing logic.

Setting expand_light_groups: false keeps the group entity in self.lights instead of expanding it to its members. Adaptation commands go to the group, and the interceptor no longer skips group entities for that switch.

Default is true — no behaviour change for existing configurations.

@hesseleo hesseleo requested a review from basnijholt as a code owner April 16, 2026 21:32
@hesseleo hesseleo force-pushed the feat/expand-group-option branch from 7709e79 to 35405cb Compare April 16, 2026 21:41
florianhorner pushed a commit to florianhorner/adaptive-lighting that referenced this pull request Apr 19, 2026
Conflicts resolved:
- const.py: keep both CONF_ENABLE_DIAGNOSTIC_SENSORS (fork) and
  new CONF_EXPAND_LIGHT_GROUPS validator entries
- strings.json / en.json / de.json: keep fork's 5-step layout;
  add expand_light_groups strings alongside existing entries
florianhorner added a commit to florianhorner/adaptive-lighting that referenced this pull request Apr 19, 2026
…groups + race-condition regression test) (#12)

* Add regression test for separate_turn_on_commands race condition

Regression test for basnijholt#1373

This test verifies that when `separate_turn_on_commands: true` is enabled
and a light is turned off between split commands, the second command
should be skipped.

The bug occurs because the proactive adaptation context is never cleared
when `light.turn_off` is called, causing the off-check to be bypassed
in `_execute_adaptation_calls`.

This test currently FAILS (demonstrating the bug) and will PASS after
the fix is implemented.

* feat: add expand_light_groups option

Some light group entities act as a proxy that must receive a single combined
`light.turn_on` call to function correctly — for example virtual mixers like
<https://github.com/mion00/color-temperature-light-mixer> for instance,
that blend a warm and a cold white channel into one entity. In such setups the
individual member entities only expose `ColorMode.BRIGHTNESS`, so sending
separate per-member commands bypasses the mixing logic.

Setting `expand_light_groups: false` keeps the group entity in `self.lights`
instead of expanding it to its members. Adaptation commands go to the group,
and the interceptor no longer skips group entities for that switch.

Default is `true` — no behaviour change for existing configurations.

* test: mark PR basnijholt#1379 regression test as xfail

The test documents an unfixed bug (proactive adaptation context not
cleared on light.turn_off, causing second split command to be sent
to an off light). Upstream basnijholt#1379 added the test without the fix, so
we xfail it until the bug is addressed.

* fix: wire expand_light_groups into STEP_OPTIONS workarounds bucket

Without this, the option is defined in VALIDATION_TUPLES but invisible
in the 5-step options flow UI — users can't toggle it and it stays at
the default True forever. Caught by /plan-eng-review on PR #12.

---------

Co-authored-by: Bas Nijholt <bas@nijho.lt>
Co-authored-by: Leonhard Hesse <leonhard.hesse@outlook.com>
Co-authored-by: Florian Horner <florianhorner@macbook-pro-von-florian.tail6f28f8.ts.net>
@basnijholt
Copy link
Copy Markdown
Owner

Sorry it took me so long to get to this. I've been a bit overwhelmed by the number of AI-assisted PRs opened here recently, and I've also been spending nearly every spare hour on my biggest project so far, MindRoom.

I'm very supportive of using AI for coding, but many of these PRs still need careful human review because even plausible-looking changes can introduce subtle breakage. That backlog made me postpone reviewing them for a while.

I've now done a batch review with Codex / GPT-5.5 (xhigh). This comment is AI-assisted, but I've reviewed the concern before posting it.

I found a likely bug in the reactive/off-to-on path when expand_light_groups is disabled.

The interceptor path was adjusted to respect the new flag, but state_changed_event_listener still does:

switches = _switches_with_lights(self.hass, [entity_id])

That helper uses the default group-expansion behavior. When expand_light_groups: false, switch.lights contains the group entity, but this lookup expands the state-change entity to the child lights before intersecting with the switch's lights. The result is that the switch may not be found, so the off-to-on reactive adaptation path does not adapt the group.

Could you make this lookup respect the switch's expand_light_groups setting, or otherwise avoid expanding the event entity for this case? A regression test with a light group and expand_light_groups: false would be very useful here.

hesseleo added 2 commits May 11, 2026 23:54
Some light group entities act as a proxy that must receive a single combined
`light.turn_on` call to function correctly — virtual mixers like
<https://github.com/mion00/color-temperature-light-mixer> for instance,
that blend a warm and a cold white channel into one entity. In such setups the
individual member entities only expose `ColorMode.BRIGHTNESS`, so sending
separate per-member commands bypasses the mixing logic.

Setting `expand_light_groups: false` keeps the group entity in `self.lights`
instead of expanding it to its members. Adaptation commands go to the group,
and the interceptor no longer skips group entities for that switch.

Default is `true` — no behaviour change for existing configurations.
_switches_with_lights was expanding the incoming entity_id globally,
causing the switch to never be found when expand_light_groups=False
@hesseleo hesseleo force-pushed the feat/expand-group-option branch from 5db2e6c to fcc4d1c Compare May 11, 2026 22:15
@hesseleo
Copy link
Copy Markdown
Author

Thank you for the careful review — and I'm sorry for taking even longer to reply.

I fixed the bug in _switches_with_lights by checking each switch's own expand_light_groups flag before expanding the incoming entity ID, so a switch storing the group entity directly now correctly matches and I added a regression test as recommended.

I did an amend for the fix and a separate commit for the test — hope that's fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants