feat: add expand_light_groups option#1462
Conversation
7709e79 to
35405cb
Compare
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
…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>
|
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 The interceptor path was adjusted to respect the new flag, but switches = _switches_with_lights(self.hass, [entity_id])That helper uses the default group-expansion behavior. When Could you make this lookup respect the switch's |
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
5db2e6c to
fcc4d1c
Compare
|
Thank you for the careful review — and I'm sorry for taking even longer to reply. I fixed the bug in I did an amend for the fix and a separate commit for the test — hope that's fine. |
Some light group entities act as a proxy that must receive a single combined
light.turn_oncall 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 exposeColorMode.BRIGHTNESS, so sending separate per-member commands bypasses the mixing logic.Setting
expand_light_groups: falsekeeps the group entity inself.lightsinstead 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.