Add opt-in option to adjust only lit members of a light group

tests/test_light.py

@@ -2324,3 +2324,138 @@ async def blocking_request(*args, **kwargs):
         await task
 
     assert entity.is_transitioning is False
+
+
+async def test_group_adjust_only_lit_members(zha_gateway: Gateway) -> None:
+    """Test the opt-in group_adjust_only_lit_members option."""
+
+    # Read once by recompute_capabilities() when the group entity is created, so
+    # it must be set before the group exists; toggling it afterwards has no
+    # effect without an explicit recompute_capabilities() call.
+    zha_gateway.config.config.light_options.group_adjust_only_lit_members = True
+
+    await coordinator_mock(zha_gateway)
+    device_light_1 = await device_light_1_mock(zha_gateway)
+    device_light_2 = await device_light_2_mock(zha_gateway)
+
+    members = [
+        GroupMemberReference(ieee=device_light_1.ieee, endpoint_id=1),
+        GroupMemberReference(ieee=device_light_2.ieee, endpoint_id=1),
+    ]
+    zha_group: Group = await zha_gateway.async_create_zigpy_group("Test Group", members)
+    await zha_gateway.async_block_till_done()
+
+    entity: GroupEntity = get_group_entity(zha_group, platform=Platform.LIGHT)
+
+    dev1_cluster_on_off = device_light_1.device.endpoints[1].on_off
+    group_cluster_on_off = zha_group.zigpy_group.endpoint[general.OnOff.cluster_id]
+    group_cluster_level = zha_group.zigpy_group.endpoint[
+        general.LevelControl.cluster_id
+    ]
+    group_cluster_color = zha_group.zigpy_group.endpoint[lighting.Color.cluster_id]
+
+    # Group cluster proxies are not built via create_mock_zigpy_device, so
+    # patch_cluster_for_testing never runs on them; wrap .request directly so
+    # calls can be inspected the same way as for a device's own clusters.
+    group_cluster_on_off.request = AsyncMock(wraps=group_cluster_on_off.request)
+    group_cluster_level.request = AsyncMock(wraps=group_cluster_level.request)
+    group_cluster_color.request = AsyncMock(wraps=group_cluster_color.request)
+
+    # with the whole group off, the option changes nothing: brightness still
+    # turns the group, and every member, on
+    await entity.async_turn_on(brightness=50)
+    await zha_gateway.async_block_till_done()
+    assert group_cluster_on_off.request.call_count == 0
+    assert group_cluster_level.request.call_count == 1
+    assert (
+        group_cluster_level.request.call_args.args[1]
+        == group_cluster_level.commands_by_name["move_to_level_with_on_off"].id
+    )
+
+    # one member turns on outside of the group entity
+    await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 1})
+    await _async_shift_time(zha_gateway)
+    assert bool(entity.state.on) is True
+
+    group_cluster_level.request.reset_mock()
+    group_cluster_on_off.request.reset_mock()
+
+    # a lit member exists, so brightness only adjusts it: no On command, and
+    # move_to_level rather than move_to_level_with_on_off
+    await entity.async_turn_on(brightness=30)
+    await zha_gateway.async_block_till_done()
+    assert group_cluster_on_off.request.call_count == 0
+    assert group_cluster_level.request.call_count == 1
+    assert (
+        group_cluster_level.request.call_args.args[1]
+        == group_cluster_level.commands_by_name["move_to_level"].id
+    )
+    assert group_cluster_level.request.call_args.kwargs["level"] == 30
+    assert entity.state.brightness == 30
+
+    group_cluster_level.request.reset_mock()
+
+    # brightness=0 is the exception: move_to_level_with_on_off is sent even
+    # under only_if_on, since it is a no-op for members that are already off
+    # and correctly extinguishes the lit one, rather than leaving it "on" at
+    # 0% brightness
+    await entity.async_turn_on(brightness=0)
+    await zha_gateway.async_block_till_done()
+    assert (
+        group_cluster_level.request.call_args.args[1]
+        == group_cluster_level.commands_by_name["move_to_level_with_on_off"].id
+    )
+    assert group_cluster_level.request.call_args.kwargs["level"] == 0
+    assert bool(entity.state.on) is False
+
+    # The group cast above never reaches dev1's own tracked state in this test
+    # harness (no simulated per-device response), so it is still reporting
+    # on_off=1 from earlier; report it off first, matching what the command
+    # actually did, then back on so the remaining only_if_on scenarios below
+    # have a lit member to adjust again.
+    await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 0})
+    await send_attributes_report(zha_gateway, dev1_cluster_on_off, {0: 1})
+    await zha_gateway.async_block_till_done()
+    entity.update()
+    assert bool(entity.state.on) is True
+
+    group_cluster_color.request.reset_mock()
+    group_cluster_on_off.request.reset_mock()
+
+    # same for color temperature
+    await entity.async_turn_on(color_temp=300)
+    await zha_gateway.async_block_till_done()
+    assert group_cluster_on_off.request.call_count == 0
+    assert group_cluster_color.request.call_count == 1
+    assert (
+        group_cluster_color.request.call_args.args[1]
+        == group_cluster_color.commands_by_name["move_to_color_temp"].id
+    )
+    assert group_cluster_color.request.call_args.kwargs["color_temp_mireds"] == 300
+    assert entity.state.color_temp == 300
+
+    group_cluster_on_off.request.reset_mock()
+    group_cluster_level.request.reset_mock()
+
+    # disabling the option restores the default behavior even with a lit member
+    zha_gateway.config.config.light_options.group_adjust_only_lit_members = False
+    entity.recompute_capabilities()
+    await entity.async_turn_on(brightness=80)
+    await zha_gateway.async_block_till_done()
+    assert (
+        group_cluster_level.request.call_args.args[1]
+        == group_cluster_level.commands_by_name["move_to_level_with_on_off"].id
+    )
+
+    group_cluster_level.request.reset_mock()
+
+    # `only_if_on` isn't part of the public API; a caller passing it anyway
+    # must not collide with the entity's own computed value (which is
+    # `False` here, since the option was just disabled above) or blow up
+    # `_make_members_assume_group_state`, which has no `**kwargs` to absorb it.
+    await entity.async_turn_on(brightness=90, only_if_on=True)
+    await zha_gateway.async_block_till_done()
+    assert (
+        group_cluster_level.request.call_args.args[1]
+        == group_cluster_level.commands_by_name["move_to_level_with_on_off"].id
+    )

zha/application/helpers.py

@@ -333,6 +333,7 @@ class LightOptions:
     enable_light_transitioning_flag: bool = dataclasses.field(default=True)
     always_prefer_xy_color_mode: bool = dataclasses.field(default=True)
     group_members_assume_state: bool = dataclasses.field(default=True)
+    group_adjust_only_lit_members: bool = dataclasses.field(default=False)
 
 
 @dataclass(kw_only=True, slots=True)

zha/application/platforms/light/__init__.py

@@ -381,6 +381,10 @@ async def async_turn_on(
         flash: FlashMode | None = None,
         color_temp: int | None = None,
         xy_color: tuple[float, float] | None = None,
+        # `LightGroup` only: adjust members already on rather than also turning
+        # on ones that are off. See `_async_turn_on_impl` for where this changes
+        # command selection.
+        only_if_on: bool = False,
     ) -> None:
         """Turn the entity on."""
         duration = (
@@ -445,6 +449,7 @@ async def async_turn_on(
                 brightness_supported=brightness_supported,
                 set_transition_flag=set_transition_flag,
                 transition_time=transition_time,
+                only_if_on=only_if_on,
             )
         finally:
             # If the task was cancelled (e.g. by a mode: restart automation) before
@@ -466,6 +471,7 @@ async def _async_turn_on_impl(  # noqa: C901
         brightness_supported: bool,
         set_transition_flag: bool,
         transition_time: float,
+        only_if_on: bool,
     ) -> None:
         """Implement the turn on logic."""
         # If the light is currently off but a turn_on call with a color/temperature is
@@ -561,23 +567,39 @@ async def _async_turn_on_impl(  # noqa: C901
         ):
             assert self._level_cluster is not None
 
-            result = await self._level_cluster.move_to_level_with_on_off(
+            # `only_if_on` (LightGroup only) sends the plain command instead: it
+            # is what a group-bound wall dimmer sends, and it leaves members
+            # that are off alone rather than turning them on. `level == 0` is
+            # the exception: `move_to_level_with_on_off` is safe there too,
+            # since it executes regardless of a member's current on/off state
+            # and, at level 0, drives an "off" transition. That is a no-op for
+            # a member that is already off, and the correct way to actually
+            # extinguish one that is lit.
+            level_command_name = (
+                "move_to_level" if only_if_on and level else "move_to_level_with_on_off"
+            )
+            result = await getattr(self._level_cluster, level_command_name)(
                 level=level,
                 transition_time=int(10 * duration),
             )
-            t_log["move_to_level_with_on_off"] = result
+            t_log[level_command_name] = result
             if result[1] is not Status.SUCCESS:
                 # First 'move to level' call failed, so if the transitioning delay
                 # isn't running from a previous call, the flag can be unset immediately
                 if set_transition_flag and not self._transition_listener:
                     self.async_transition_complete()
                 self.debug("turned on: %s", t_log)
                 return
+            # `move_to_level` (only_if_on, level > 0) never changes on/off,
+            # unlike `move_to_level_with_on_off`, but this stays correct either
+            # way: that command is only used when `level` is nonzero, and
+            # `only_if_on` guarantees `_state` was already `True`, so this
+            # assigns `True` to something already `True` in that case.
             self._state = bool(level)
             if level:
                 self._brightness = level
 
-        if (
+        if not only_if_on and (
             (brightness is None and transition is None)
             and not new_color_provided_while_off
             or (self._FORCE_ON and brightness != 0)
@@ -1387,6 +1409,9 @@ def recompute_capabilities(self) -> None:
         self._zha_config_group_members_assume_state = (
             light_options.group_members_assume_state
         )
+        self._zha_config_group_adjust_only_lit_members = (
+            light_options.group_adjust_only_lit_members
+        )
 
         self._zha_config_enhanced_light_transition = False
         self._GROUP_SUPPORTS_EXECUTE_IF_OFF: bool = True
@@ -1437,12 +1462,46 @@ async def on_remove(self) -> None:
 
     async def async_turn_on(self, **kwargs: Any) -> None:
         """Turn the entity on."""
+        # `only_if_on` isn't part of the public `light.turn_on` API; this
+        # method's own computed value below is authoritative. Without this,
+        # a caller-supplied `only_if_on` in `kwargs` would collide with the
+        # explicit one passed to `super().async_turn_on()` (`TypeError: got
+        # multiple values`) and, separately, `_make_members_assume_group_state`
+        # has no `**kwargs` to absorb it at all.
+        kwargs.pop("only_if_on", None)
+
+        only_if_on = (
+            self._zha_config_group_adjust_only_lit_members
+            # "At least one member on": with the group fully off there is
+            # nothing to adjust, so the call keeps its normal turn-on semantics.
+            and self._state
+            # `effect`/`flash` have no group-cast equivalent, so a call carrying
+            # either keeps its normal turn-on semantics too.
+            and kwargs.get("effect") is None
+            and kwargs.get("flash") is None
+            and (
+                (
+                    kwargs.get("brightness") is not None
+                    or kwargs.get("transition") is not None
+                )
+                # Mirrors the level gating `_async_turn_on_impl` applies on its
+                # own, minus that method's `self._level_cluster is not None`
+                # check: `LightGroup.__init__` always sets it from the group's
+                # own synthetic endpoint, so for a group it is never `None`.
+                and is_brightness_supported(self._internal_supported_color_modes)
+                or kwargs.get("color_temp") is not None
+                or kwargs.get("xy_color") is not None
+            )
+        )
+
         # "off with transition" and "off brightness" will get overridden when
         # turning on the group, but they are needed for setting the assumed
         # member state correctly, so save them here
         off_brightness = self._off_brightness if self._off_with_transition else None
-        await super().async_turn_on(**kwargs)
-        if self._zha_config_group_members_assume_state:
+        await super().async_turn_on(**kwargs, only_if_on=only_if_on)
+        # `_make_members_assume_group_state(state=True, ...)` marks every member
+        # on; that is exactly what `only_if_on` exists to avoid, so skip it there.
+        if self._zha_config_group_members_assume_state and not only_if_on:
             self._make_members_assume_group_state(
                 state=True, off_brightness=off_brightness, **kwargs
             )