Fix iot light effect brightness (#1092)

Fixes issue where the brightness of the `iot` light effect is set properly
on the light effect but read back incorrectly from the light.

kasa/iot/iotbulb.py

@@ -365,7 +365,7 @@ class IotBulb(IotDevice):
 
         hue = light_state["hue"]
         saturation = light_state["saturation"]
-        value = light_state["brightness"]
+        value = self._brightness
 
         return HSV(hue, saturation, value)
 
@@ -455,6 +455,13 @@ class IotBulb(IotDevice):
         if not self._is_dimmable:  # pragma: no cover
             raise KasaException("Bulb is not dimmable.")
 
+        # If the device supports effects and one is active, we get the brightness
+        # from the effect. This is not required when setting the brightness as
+        # the device handles it via set_light_state
+        if (
+            light_effect := self.modules.get(Module.IotLightEffect)
+        ) is not None and light_effect.effect != light_effect.LIGHT_EFFECTS_OFF:
+            return light_effect.brightness
         light_state = self.light_state
         return int(light_state["brightness"])
 

kasa/iot/modules/lighteffect.py

@@ -3,7 +3,6 @@
 from __future__ import annotations
 
 from ...interfaces.lighteffect import LightEffect as LightEffectInterface
-from ...module import Module
 from ..effects import EFFECT_MAPPING_V1, EFFECT_NAMES_V1
 from ..iotmodule import IotModule
 
@@ -29,6 +28,11 @@ class LightEffect(IotModule, LightEffectInterface):
 
         return self.LIGHT_EFFECTS_OFF
 
+    @property
+    def brightness(self) -> int:
+        """Return light effect brightness."""
+        return self.data["lighting_effect_state"]["brightness"]
+
     @property
     def effect_list(self) -> list[str]:
         """Return built-in effects list.
@@ -60,18 +64,21 @@ class LightEffect(IotModule, LightEffectInterface):
         :param int transition: The wanted transition time
         """
         if effect == self.LIGHT_EFFECTS_OFF:
-            light_module = self._device.modules[Module.Light]
+            if self.effect in EFFECT_MAPPING_V1:
-            effect_off_state = light_module.state
+                # TODO: We could query get_lighting_effect here to
-            if brightness is not None:
+                # get the custom effect although not sure how to find
-                effect_off_state.brightness = brightness
+                # custom effects
-            if transition is not None:
+                effect_dict = EFFECT_MAPPING_V1[self.effect]
-                effect_off_state.transition = transition
+            else:
-            await light_module.set_state(effect_off_state)
+                effect_dict = EFFECT_MAPPING_V1["Aurora"]
+            effect_dict = {**effect_dict}
+            effect_dict["enable"] = 0
+            await self.set_custom_effect(effect_dict)
         elif effect not in EFFECT_MAPPING_V1:
             raise ValueError(f"The effect {effect} is not a built in effect.")
         else:
             effect_dict = EFFECT_MAPPING_V1[effect]
-
+            effect_dict = {**effect_dict}
             if brightness is not None:
                 effect_dict["brightness"] = brightness
             if transition is not None:

kasa/module.py

@@ -116,6 +116,7 @@ class Module(ABC):
     SmartLightEffect: Final[ModuleName[smart.SmartLightEffect]] = ModuleName(
         "LightEffect"
     )
+    IotLightEffect: Final[ModuleName[iot.LightEffect]] = ModuleName("LightEffect")
     TemperatureSensor: Final[ModuleName[smart.TemperatureSensor]] = ModuleName(
         "TemperatureSensor"
     )

kasa/smart/modules/lightstripeffect.py

@@ -106,14 +106,23 @@ class LightStripEffect(SmartModule, SmartLightEffect):
         """
         brightness_module = self._device.modules[Module.Brightness]
         if effect == self.LIGHT_EFFECTS_OFF:
-            state = self._device.modules[Module.Light].state
+            if self.effect in self._effect_mapping:
-            await self._device.modules[Module.Light].set_state(state)
+                # TODO: We could query get_lighting_effect here to
+                # get the custom effect although not sure how to find
+                # custom effects
+                effect_dict = self._effect_mapping[self.effect]
+            else:
+                effect_dict = self._effect_mapping["Aurora"]
+            effect_dict = {**effect_dict}
+            effect_dict["enable"] = 0
+            await self.set_custom_effect(effect_dict)
             return
 
         if effect not in self._effect_mapping:
             raise ValueError(f"The effect {effect} is not a built in effect.")
         else:
             effect_dict = self._effect_mapping[effect]
+            effect_dict = {**effect_dict}
 
         # Use explicitly given brightness
         if brightness is not None:

kasa/tests/fakeprotocol_iot.py

@@ -292,6 +292,26 @@ class FakeIotTransport(BaseTransport):
         self.proto["system"]["get_sysinfo"]["lighting_effect_state"] = dict(effect)
 
     def transition_light_state(self, state_changes, *args):
+        # Setting the light state on a device will turn off any active lighting effects.
+        # Unless it's just the brightness in which case it will update the brightness for
+        # the lighting effect
+        if lighting_effect_state := self.proto["system"]["get_sysinfo"].get(
+            "lighting_effect_state"
+        ):
+            if (
+                "hue" in state_changes
+                or "saturation" in state_changes
+                or "color_temp" in state_changes
+            ):
+                lighting_effect_state["enable"] = 0
+            elif (
+                lighting_effect_state["enable"] == 1
+                and state_changes.get("on_off") != 0
+                and (brightness := state_changes.get("brightness"))
+            ):
+                lighting_effect_state["brightness"] = brightness
+                return
+
         _LOGGER.debug("Setting light state to %s", state_changes)
         light_state = self.proto["system"]["get_sysinfo"]["light_state"]
 
@@ -317,12 +337,6 @@ class FakeIotTransport(BaseTransport):
         _LOGGER.debug("New light state: %s", new_state)
         self.proto["system"]["get_sysinfo"]["light_state"] = new_state
 
-        # Setting the light state on a device will turn off any active lighting effects.
-        if lighting_effect_state := self.proto["system"]["get_sysinfo"].get(
-            "lighting_effect_state"
-        ):
-            lighting_effect_state["enable"] = 0
-
     def set_preferred_state(self, new_state, *args):
         """Implement set_preferred_state."""
         self.proto["system"]["get_sysinfo"]["preferred_state"][new_state["index"]] = (

kasa/tests/fakeprotocol_smart.py

@@ -271,13 +271,14 @@ class FakeSmartTransport(BaseTransport):
 
     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)
+        if "enable" in params:
+            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"]
+            info["get_lighting_effect"] = copy.deepcopy(params)
 
     def _set_led_info(self, info, params):
         """Set or remove values as per the device behaviour."""

kasa/tests/smart/modules/test_light_strip_effect.py

@@ -30,26 +30,23 @@ async def test_light_strip_effect(dev: Device, mocker: MockerFixture):
 
     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):
+        if effect == LightEffect.LIGHT_EFFECTS_OFF:
+            off_effect = (
+                light_effect.effect
+                if light_effect.effect in light_effect._effect_mapping
+                else "Aurora"
+            )
         await light_effect.set_effect(effect)
 
-        if effect == LightEffect.LIGHT_EFFECTS_OFF:
+        if effect != LightEffect.LIGHT_EFFECTS_OFF:
-            light_call.assert_called()
+            params = {**light_effect._effect_mapping[effect]}
-            continue
+        else:
-
+            params = {**light_effect._effect_mapping[off_effect]}
-        # Start with the current effect data
+            params["enable"] = 0
-        params = light_effect.data["lighting_effect"]
+        params["brightness"] = brightness.brightness  # use the existing brightness
-        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)
 

kasa/tests/test_common_modules.py

@@ -133,6 +133,31 @@ async def test_light_effect_module(dev: Device, mocker: MockerFixture):
         call.assert_not_called()
 
 
+@light_effect
+async def test_light_effect_brightness(dev: Device, mocker: MockerFixture):
+    """Test that light module uses light_effect for brightness when active."""
+    light_module = dev.modules[Module.Light]
+
+    light_effect = dev.modules[Module.LightEffect]
+
+    await light_effect.set_effect(light_effect.LIGHT_EFFECTS_OFF)
+    await light_module.set_brightness(50)
+    await dev.update()
+    assert light_effect.effect == light_effect.LIGHT_EFFECTS_OFF
+    assert light_module.brightness == 50
+    await light_effect.set_effect(light_effect.effect_list[1])
+    await dev.update()
+    # assert light_module.brightness == 100
+
+    await light_module.set_brightness(75)
+    await dev.update()
+    assert light_module.brightness == 75
+
+    await light_effect.set_effect(light_effect.LIGHT_EFFECTS_OFF)
+    await dev.update()
+    assert light_module.brightness == 50
+
+
 @dimmable
 async def test_light_brightness(dev: Device):
     """Test brightness setter and getter."""
@@ -201,6 +226,9 @@ async def test_light_set_state(dev: Device):
     assert isinstance(dev, Device)
     light = next(get_parent_and_child_modules(dev, Module.Light))
     assert light
+    # For fixtures that have a light effect active switch off
+    if light_effect := light._device.modules.get(Module.LightEffect):
+        await light_effect.set_effect(light_effect.LIGHT_EFFECTS_OFF)
 
     await light.set_state(LightState(light_on=False))
     await dev.update()