diff --git a/tests/test_light.py b/tests/test_light.py index 4ffa44c0a..06a841559 100644 --- a/tests/test_light.py +++ b/tests/test_light.py @@ -2324,3 +2324,154 @@ 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_color.request.reset_mock() + + # same for XY color + await entity.async_turn_on(xy_color=(0.1, 0.2)) + 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"].id + ) + assert group_cluster_color.request.call_args.kwargs["color_x"] == int(0.1 * 65535) + assert group_cluster_color.request.call_args.kwargs["color_y"] == int(0.2 * 65535) + assert entity.state.xy_color == (0.1, 0.2) + + 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 + ) diff --git a/zha/application/helpers.py b/zha/application/helpers.py index df65e5b55..0d03fddda 100644 --- a/zha/application/helpers.py +++ b/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) diff --git a/zha/application/platforms/light/__init__.py b/zha/application/platforms/light/__init__.py index ea1282609..5badcbb1b 100644 --- a/zha/application/platforms/light/__init__.py +++ b/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,11 +567,22 @@ 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 @@ -573,11 +590,16 @@ async def _async_turn_on_impl( # noqa: C901 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,43 @@ 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 + # 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 )