From b31a2ede7ff38b0612fd4883a573a45310341ce4 Mon Sep 17 00:00:00 2001 From: Teemu R Date: Mon, 1 Jul 2024 13:59:24 +0200 Subject: [PATCH] Fix changing brightness when effect is active (#1019) This PR changes the behavior of `brightness` module if an effect is active. Currently, changing the brightness disables the effect when the brightness is changed, this fixes that. This will also improve the `set_effect` interface to use the current brightness when an effect is activated. * light_strip_effect: passing `bAdjusted` with the changed properties changes the brightness. * light_effect: the brightness is stored only in the rule, so we modify it when adjusting the brightness. This is also done during the initial effect activation. --------- Co-authored-by: Steven B <51370195+sdb9696@users.noreply.github.com> --- kasa/module.py | 3 + kasa/smart/effects.py | 25 +++++ kasa/smart/modules/__init__.py | 2 + kasa/smart/modules/brightness.py | 15 ++- kasa/smart/modules/lighteffect.py | 76 +++++++++++-- kasa/smart/modules/lightstripeffect.py | 46 ++++++-- kasa/tests/fakeprotocol_smart.py | 18 +++- kasa/tests/smart/modules/test_light_effect.py | 42 ++++++++ .../smart/modules/test_light_strip_effect.py | 101 ++++++++++++++++++ kasa/tests/test_common_modules.py | 14 ++- 10 files changed, 321 insertions(+), 21 deletions(-) create mode 100644 kasa/tests/smart/modules/test_light_strip_effect.py diff --git a/kasa/module.py b/kasa/module.py index 3a090782..69c4e9e2 100644 --- a/kasa/module.py +++ b/kasa/module.py @@ -112,6 +112,9 @@ class Module(ABC): "LightTransition" ) ReportMode: Final[ModuleName[smart.ReportMode]] = ModuleName("ReportMode") + SmartLightEffect: Final[ModuleName[smart.SmartLightEffect]] = ModuleName( + "LightEffect" + ) TemperatureSensor: Final[ModuleName[smart.TemperatureSensor]] = ModuleName( "TemperatureSensor" ) diff --git a/kasa/smart/effects.py b/kasa/smart/effects.py index 28e27d3f..e0ed615c 100644 --- a/kasa/smart/effects.py +++ b/kasa/smart/effects.py @@ -2,8 +2,33 @@ from __future__ import annotations +from abc import ABC, abstractmethod from typing import cast +from ..interfaces.lighteffect import LightEffect as LightEffectInterface + + +class SmartLightEffect(LightEffectInterface, ABC): + """Abstract interface for smart light effects. + + This interface extends lighteffect interface to add brightness controls. + """ + + @abstractmethod + async def set_brightness(self, brightness: int, *, transition: int | None = None): + """Set effect brightness.""" + + @property + @abstractmethod + def brightness(self) -> int: + """Return effect brightness.""" + + @property + @abstractmethod + def is_active(self) -> bool: + """Return True if effect is active.""" + + EFFECT_AURORA = { "custom": 0, "id": "TapoStrip_1MClvV18i15Jq3bvJVf0eP", diff --git a/kasa/smart/modules/__init__.py b/kasa/smart/modules/__init__.py index ada52f91..fd987751 100644 --- a/kasa/smart/modules/__init__.py +++ b/kasa/smart/modules/__init__.py @@ -1,5 +1,6 @@ """Modules for SMART devices.""" +from ..effects import SmartLightEffect from .alarm import Alarm from .autooff import AutoOff from .batterysensor import BatterySensor @@ -54,4 +55,5 @@ __all__ = [ "WaterleakSensor", "ContactSensor", "FrostProtection", + "SmartLightEffect", ] diff --git a/kasa/smart/modules/brightness.py b/kasa/smart/modules/brightness.py index fbd90808..f5e6d6d6 100644 --- a/kasa/smart/modules/brightness.py +++ b/kasa/smart/modules/brightness.py @@ -3,7 +3,7 @@ from __future__ import annotations from ...feature import Feature -from ..smartmodule import SmartModule +from ..smartmodule import Module, SmartModule BRIGHTNESS_MIN = 0 BRIGHTNESS_MAX = 100 @@ -42,6 +42,12 @@ class Brightness(SmartModule): @property def brightness(self): """Return current brightness.""" + # If the device supports effects and one is active, use its brightness + if ( + light_effect := self._device.modules.get(Module.SmartLightEffect) + ) is not None and light_effect.is_active: + return light_effect.brightness + return self.data["brightness"] async def set_brightness(self, brightness: int, *, transition: int | None = None): @@ -59,6 +65,13 @@ class Brightness(SmartModule): if brightness == 0: return await self._device.turn_off() + + # If the device supports effects and one is active, we adjust its brightness + if ( + light_effect := self._device.modules.get(Module.SmartLightEffect) + ) is not None and light_effect.is_active: + return await light_effect.set_brightness(brightness) + return await self.call("set_device_info", {"brightness": brightness}) async def _check_supported(self): diff --git a/kasa/smart/modules/lighteffect.py b/kasa/smart/modules/lighteffect.py index 170cfbb3..07f6aece 100644 --- a/kasa/smart/modules/lighteffect.py +++ b/kasa/smart/modules/lighteffect.py @@ -3,14 +3,16 @@ from __future__ import annotations import base64 +import binascii +import contextlib import copy from typing import Any -from ...interfaces.lighteffect import LightEffect as LightEffectInterface -from ..smartmodule import SmartModule +from ..effects import SmartLightEffect +from ..smartmodule import Module, SmartModule -class LightEffect(SmartModule, LightEffectInterface): +class LightEffect(SmartModule, SmartLightEffect): """Implementation of dynamic light effects.""" REQUIRED_COMPONENT = "light_effect" @@ -36,8 +38,11 @@ class LightEffect(SmartModule, LightEffectInterface): # If the name has not been edited scene_name will be an empty string effect["scene_name"] = self.AVAILABLE_BULB_EFFECTS[effect["id"]] else: - # Otherwise it will be b64 encoded - effect["scene_name"] = base64.b64decode(effect["scene_name"]).decode() + # Otherwise it might be b64 encoded or raw string + with contextlib.suppress(binascii.Error): + effect["scene_name"] = base64.b64decode( + effect["scene_name"] + ).decode() self._effect_state_list = effects self._effect_list = [self.LIGHT_EFFECTS_OFF] @@ -77,6 +82,8 @@ class LightEffect(SmartModule, LightEffectInterface): ) -> None: """Set an effect for the device. + Calling this will modify the brightness of the effect on the device. + The device doesn't store an active effect while not enabled so store locally. """ if effect != self.LIGHT_EFFECTS_OFF and effect not in self._scenes_names_to_id: @@ -90,7 +97,64 @@ class LightEffect(SmartModule, LightEffectInterface): if enable: effect_id = self._scenes_names_to_id[effect] params["id"] = effect_id - return await self.call("set_dynamic_light_effect_rule_enable", params) + + # We set the wanted brightness before activating the effect + brightness_module = self._device.modules[Module.Brightness] + brightness = ( + brightness if brightness is not None else brightness_module.brightness + ) + await self.set_brightness(brightness, effect_id=effect_id) + + await self.call("set_dynamic_light_effect_rule_enable", params) + + @property + def is_active(self) -> bool: + """Return True if effect is active.""" + return bool(self._device._info["dynamic_light_effect_enable"]) + + def _get_effect_data(self, effect_id: str | None = None) -> dict[str, Any]: + """Return effect data for the *effect_id*. + + If *effect_id* is None, return the data for active effect. + """ + if effect_id is None: + effect_id = self.data["current_rule_id"] + + return self._effect_state_list[effect_id] + + @property + def brightness(self) -> int: + """Return effect brightness.""" + first_color_status = self._get_effect_data()["color_status_list"][0] + brightness = first_color_status[0] + + return brightness + + async def set_brightness( + self, + brightness: int, + *, + transition: int | None = None, + effect_id: str | None = None, + ): + """Set effect brightness.""" + new_effect = self._get_effect_data(effect_id=effect_id).copy() + + def _replace_brightness(data, new_brightness): + """Replace brightness. + + The first element is the brightness, the rest are unknown. + [[33, 0, 0, 2700], [33, 321, 99, 0], [33, 196, 99, 0], .. ] + """ + return [new_brightness, data[1], data[2], data[3]] + + new_color_status_list = [ + _replace_brightness(state, brightness) + for state in new_effect["color_status_list"] + ] + new_effect["color_status_list"] = new_color_status_list + + return await self.call("edit_dynamic_light_effect_rule", new_effect) async def set_custom_effect( self, diff --git a/kasa/smart/modules/lightstripeffect.py b/kasa/smart/modules/lightstripeffect.py index c2f35188..a80c20f3 100644 --- a/kasa/smart/modules/lightstripeffect.py +++ b/kasa/smart/modules/lightstripeffect.py @@ -4,15 +4,14 @@ from __future__ import annotations from typing import TYPE_CHECKING -from ...interfaces.lighteffect import LightEffect as LightEffectInterface -from ..effects import EFFECT_MAPPING, EFFECT_NAMES -from ..smartmodule import SmartModule +from ..effects import EFFECT_MAPPING, EFFECT_NAMES, SmartLightEffect +from ..smartmodule import Module, SmartModule if TYPE_CHECKING: from ..smartdevice import SmartDevice -class LightStripEffect(SmartModule, LightEffectInterface): +class LightStripEffect(SmartModule, SmartLightEffect): """Implementation of dynamic light effects.""" REQUIRED_COMPONENT = "light_strip_lighting_effect" @@ -22,6 +21,7 @@ class LightStripEffect(SmartModule, LightEffectInterface): effect_list = [self.LIGHT_EFFECTS_OFF] effect_list.extend(EFFECT_NAMES) self._effect_list = effect_list + self._effect_mapping = EFFECT_MAPPING @property def name(self) -> str: @@ -53,6 +53,28 @@ class LightStripEffect(SmartModule, LightEffectInterface): return name return self.LIGHT_EFFECTS_OFF + @property + def is_active(self) -> bool: + """Return if effect is active.""" + eff = self.data["lighting_effect"] + # softAP has enable=1, but brightness 0 which fails on tests + return bool(eff["enable"]) and eff["name"] in self._effect_list + + @property + def brightness(self) -> int: + """Return effect brightness.""" + eff = self.data["lighting_effect"] + return eff["brightness"] + + async def set_brightness(self, brightness: int, *, transition: int | None = None): + """Set effect brightness.""" + if brightness <= 0: + return await self.set_effect(self.LIGHT_EFFECTS_OFF) + + # Need to pass bAdjusted to keep the existing effect running + eff = {"brightness": brightness, "bAdjusted": True} + return await self.set_custom_effect(eff) + @property def effect_list(self) -> list[str]: """Return built-in effects list. @@ -81,16 +103,24 @@ class LightStripEffect(SmartModule, LightEffectInterface): :param int brightness: The wanted brightness :param int transition: The wanted transition time """ + brightness_module = self._device.modules[Module.Brightness] if effect == self.LIGHT_EFFECTS_OFF: - effect_dict = dict(self.data["lighting_effect"]) - effect_dict["enable"] = 0 - elif effect not in EFFECT_MAPPING: + state = self._device.modules[Module.Light].state + await self._device.modules[Module.Light].set_state(state) + return + + if effect not in self._effect_mapping: raise ValueError(f"The effect {effect} is not a built in effect.") else: - effect_dict = EFFECT_MAPPING[effect] + effect_dict = self._effect_mapping[effect] + # Use explicitly given brightness if brightness is not None: effect_dict["brightness"] = brightness + # Fall back to brightness reported by the brightness module + elif brightness_module.brightness: + effect_dict["brightness"] = brightness_module.brightness + if transition is not None: effect_dict["transition"] = transition diff --git a/kasa/tests/fakeprotocol_smart.py b/kasa/tests/fakeprotocol_smart.py index 94c75104..600cd75d 100644 --- a/kasa/tests/fakeprotocol_smart.py +++ b/kasa/tests/fakeprotocol_smart.py @@ -250,18 +250,31 @@ class FakeSmartTransport(BaseTransport): info["get_dynamic_light_effect_rules"]["enable"] = params["enable"] if params["enable"]: info["get_device_info"]["dynamic_light_effect_id"] = params["id"] - info["get_dynamic_light_effect_rules"]["current_rule_id"] = params["enable"] + info["get_dynamic_light_effect_rules"]["current_rule_id"] = params["id"] else: if "dynamic_light_effect_id" in info["get_device_info"]: del info["get_device_info"]["dynamic_light_effect_id"] if "current_rule_id" in info["get_dynamic_light_effect_rules"]: del info["get_dynamic_light_effect_rules"]["current_rule_id"] + def _set_edit_dynamic_light_effect_rule(self, info, params): + """Edit dynamic light effect rule.""" + rules = info["get_dynamic_light_effect_rules"]["rule_list"] + for rule in rules: + if rule["id"] == params["id"]: + rule.update(params) + return + + raise Exception("Unable to find rule with id") + def _set_light_strip_effect(self, info, params): """Set or remove values as per the device behaviour.""" info["get_device_info"]["lighting_effect"]["enable"] = params["enable"] info["get_device_info"]["lighting_effect"]["name"] = params["name"] info["get_device_info"]["lighting_effect"]["id"] = params["id"] + # Brightness is not always available + if (brightness := params.get("brightness")) is not None: + info["get_device_info"]["lighting_effect"]["brightness"] = brightness info["get_lighting_effect"] = copy.deepcopy(params) def _set_led_info(self, info, params): @@ -365,6 +378,9 @@ class FakeSmartTransport(BaseTransport): elif method == "set_dynamic_light_effect_rule_enable": self._set_dynamic_light_effect(info, params) return {"error_code": 0} + elif method == "edit_dynamic_light_effect_rule": + self._set_edit_dynamic_light_effect_rule(info, params) + return {"error_code": 0} elif method == "set_lighting_effect": self._set_light_strip_effect(info, params) return {"error_code": 0} diff --git a/kasa/tests/smart/modules/test_light_effect.py b/kasa/tests/smart/modules/test_light_effect.py index ed691e66..20435dde 100644 --- a/kasa/tests/smart/modules/test_light_effect.py +++ b/kasa/tests/smart/modules/test_light_effect.py @@ -39,3 +39,45 @@ async def test_light_effect(dev: Device, mocker: MockerFixture): with pytest.raises(ValueError): await light_effect.set_effect("foobar") + + +@light_effect +@pytest.mark.parametrize("effect_active", [True, False]) +async def test_light_effect_brightness( + dev: Device, effect_active: bool, mocker: MockerFixture +): + """Test that light module uses light_effect for brightness when active.""" + light_module = dev.modules[Module.Light] + + light_effect = dev.modules[Module.SmartLightEffect] + light_effect_set_brightness = mocker.spy(light_effect, "set_brightness") + mock_light_effect_call = mocker.patch.object(light_effect, "call") + + brightness = dev.modules[Module.Brightness] + brightness_set_brightness = mocker.spy(brightness, "set_brightness") + mock_brightness_call = mocker.patch.object(brightness, "call") + + mocker.patch.object( + type(light_effect), + "is_active", + new_callable=mocker.PropertyMock, + return_value=effect_active, + ) + if effect_active: # Set the rule L1 active for testing + light_effect.data["current_rule_id"] = "L1" + + await light_module.set_brightness(10) + + if effect_active: + assert light_effect.is_active + assert light_effect.brightness == dev.brightness + + light_effect_set_brightness.assert_called_with(10) + mock_light_effect_call.assert_called_with( + "edit_dynamic_light_effect_rule", mocker.ANY + ) + else: + assert not light_effect.is_active + + brightness_set_brightness.assert_called_with(10) + mock_brightness_call.assert_called_with("set_device_info", {"brightness": 10}) diff --git a/kasa/tests/smart/modules/test_light_strip_effect.py b/kasa/tests/smart/modules/test_light_strip_effect.py new file mode 100644 index 00000000..92ef2202 --- /dev/null +++ b/kasa/tests/smart/modules/test_light_strip_effect.py @@ -0,0 +1,101 @@ +from __future__ import annotations + +from itertools import chain + +import pytest +from pytest_mock import MockerFixture + +from kasa import Device, Feature, Module +from kasa.smart.modules import LightEffect, LightStripEffect +from kasa.tests.device_fixtures import parametrize + +light_strip_effect = parametrize( + "has light strip effect", + component_filter="light_strip_lighting_effect", + protocol_filter={"SMART"}, +) + + +@light_strip_effect +async def test_light_strip_effect(dev: Device, mocker: MockerFixture): + """Test light strip effect.""" + light_effect = dev.modules.get(Module.LightEffect) + + assert isinstance(light_effect, LightStripEffect) + + brightness = dev.modules[Module.Brightness] + + feature = dev.features["light_effect"] + assert feature.type == Feature.Type.Choice + + call = mocker.spy(light_effect, "call") + + light = dev.modules[Module.Light] + light_call = mocker.spy(light, "call") + + assert feature.choices == light_effect.effect_list + assert feature.choices + for effect in chain(reversed(feature.choices), feature.choices): + await light_effect.set_effect(effect) + + if effect == LightEffect.LIGHT_EFFECTS_OFF: + light_call.assert_called() + continue + + # Start with the current effect data + params = light_effect.data["lighting_effect"] + enable = effect != LightEffect.LIGHT_EFFECTS_OFF + params["enable"] = enable + if enable: + params = light_effect._effect_mapping[effect] + params["enable"] = enable + params["brightness"] = brightness.brightness # use the existing brightness + + call.assert_called_with("set_lighting_effect", params) + + await dev.update() + assert light_effect.effect == effect + assert feature.value == effect + + with pytest.raises(ValueError): + await light_effect.set_effect("foobar") + + +@light_strip_effect +@pytest.mark.parametrize("effect_active", [True, False]) +async def test_light_effect_brightness( + dev: Device, effect_active: bool, mocker: MockerFixture +): + """Test that light module uses light_effect for brightness when active.""" + light_module = dev.modules[Module.Light] + + light_effect = dev.modules[Module.SmartLightEffect] + light_effect_set_brightness = mocker.spy(light_effect, "set_brightness") + mock_light_effect_call = mocker.patch.object(light_effect, "call") + + brightness = dev.modules[Module.Brightness] + brightness_set_brightness = mocker.spy(brightness, "set_brightness") + mock_brightness_call = mocker.patch.object(brightness, "call") + + mocker.patch.object( + type(light_effect), + "is_active", + new_callable=mocker.PropertyMock, + return_value=effect_active, + ) + + await light_module.set_brightness(10) + + if effect_active: + assert light_effect.is_active + assert light_effect.brightness == dev.brightness + + light_effect_set_brightness.assert_called_with(10) + mock_light_effect_call.assert_called_with( + "set_lighting_effect", {"brightness": 10, "bAdjusted": True} + ) + else: + assert not light_effect.is_active + + brightness_set_brightness.assert_called_with(10) + mock_brightness_call.assert_called_with("set_device_info", {"brightness": 10}) diff --git a/kasa/tests/test_common_modules.py b/kasa/tests/test_common_modules.py index c0d90578..beed8e8b 100644 --- a/kasa/tests/test_common_modules.py +++ b/kasa/tests/test_common_modules.py @@ -89,35 +89,39 @@ async def test_light_effect_module(dev: Device, mocker: MockerFixture): assert light_effect_module.has_custom_effects is not None await light_effect_module.set_effect("Off") - assert call.call_count == 1 + call.assert_called() await dev.update() assert light_effect_module.effect == "Off" assert feat.value == "Off" + call.reset_mock() second_effect = effect_list[1] await light_effect_module.set_effect(second_effect) - assert call.call_count == 2 + call.assert_called() await dev.update() assert light_effect_module.effect == second_effect assert feat.value == second_effect + call.reset_mock() last_effect = effect_list[len(effect_list) - 1] await light_effect_module.set_effect(last_effect) - assert call.call_count == 3 + call.assert_called() await dev.update() assert light_effect_module.effect == last_effect assert feat.value == last_effect + call.reset_mock() # Test feature set await feat.set_value(second_effect) - assert call.call_count == 4 + call.assert_called() await dev.update() assert light_effect_module.effect == second_effect assert feat.value == second_effect + call.reset_mock() with pytest.raises(ValueError): await light_effect_module.set_effect("foobar") - assert call.call_count == 4 + call.assert_not_called() @dimmable