From c0668db4a29bc87d5dcf4d0d84a0f3ebda658395 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Tue, 30 Apr 2024 12:05:23 +0100 Subject: [PATCH 1/9] Add LightEffectModule --- kasa/cli.py | 30 +++-- kasa/smart/modules/__init__.py | 2 + kasa/smart/modules/lighteffectmodule.py | 112 ++++++++++++++++++ kasa/smart/smartdevice.py | 58 +-------- kasa/tests/fakeprotocol_smart.py | 14 +++ kasa/tests/smart/modules/test_light_effect.py | 42 +++++++ 6 files changed, 197 insertions(+), 61 deletions(-) create mode 100644 kasa/smart/modules/lighteffectmodule.py create mode 100644 kasa/tests/smart/modules/test_light_effect.py diff --git a/kasa/cli.py b/kasa/cli.py index d8191a8f0..a9c575cfd 100755 --- a/kasa/cli.py +++ b/kasa/cli.py @@ -1219,6 +1219,22 @@ async def feature(dev: Device, child: str, name: str, value): If only *name* is given, the value of named feature is returned. If both *name* and *value* are set, the described setting is changed. """ + + def _print_feature(name, feat, indent=""): + try: + unit = f" {feat.unit}" if feat.unit else "" + if feat.type == Feature.Type.Choice: + vals = feat.choices + index = vals.index(feat.value) + vals.remove(feat.value) + vals.insert(index, f"*{feat.value}*") + val = " ".join(vals) + else: + val = feat.value + echo(f"{indent}{feat.name} ({name}): {val}{unit}") + except Exception as ex: + echo(f"{indent}{feat.name} ({name}): [red]{ex}[/red]") + if child is not None: echo(f"Targeting child device {child}") dev = dev.get_child_device(child) @@ -1226,11 +1242,7 @@ async def feature(dev: Device, child: str, name: str, value): def _print_features(dev): for name, feat in dev.features.items(): - try: - unit = f" {feat.unit}" if feat.unit else "" - echo(f"\t{feat.name} ({name}): {feat.value}{unit}") - except Exception as ex: - echo(f"\t{feat.name} ({name}): [red]{ex}[/red]") + _print_feature(name, feat, "\t") echo("[bold]== Features ==[/bold]") _print_features(dev) @@ -1249,13 +1261,15 @@ def _print_features(dev): feat = dev.features[name] if value is None: - unit = f" {feat.unit}" if feat.unit else "" - echo(f"{feat.name} ({name}): {feat.value}{unit}") + _print_feature(name, feat) return feat.value echo(f"Setting {name} to {value}") value = ast.literal_eval(value) - return await dev.features[name].set_value(value) + response = await dev.features[name].set_value(value) + await dev.update() + _print_feature(name, feat) + return response if __name__ == "__main__": diff --git a/kasa/smart/modules/__init__.py b/kasa/smart/modules/__init__.py index b3b1d9f47..6215d61ee 100644 --- a/kasa/smart/modules/__init__.py +++ b/kasa/smart/modules/__init__.py @@ -14,6 +14,7 @@ from .firmware import Firmware from .humidity import HumiditySensor from .ledmodule import LedModule +from .lighteffectmodule import LightEffectModule from .lighttransitionmodule import LightTransitionModule from .reportmodule import ReportModule from .temperature import TemperatureSensor @@ -37,6 +38,7 @@ "FanModule", "Firmware", "CloudModule", + "LightEffectModule", "LightTransitionModule", "ColorTemperatureModule", "ColorModule", diff --git a/kasa/smart/modules/lighteffectmodule.py b/kasa/smart/modules/lighteffectmodule.py new file mode 100644 index 000000000..80c0a83a5 --- /dev/null +++ b/kasa/smart/modules/lighteffectmodule.py @@ -0,0 +1,112 @@ +"""Module for light effects.""" + +from __future__ import annotations + +import base64 +import copy +from typing import TYPE_CHECKING, Any + +from ...exceptions import KasaException +from ...feature import Feature +from ..smartmodule import SmartModule + +if TYPE_CHECKING: + from ..smartdevice import SmartDevice + + +class LightEffectModule(SmartModule): + """Implementation of gradual on/off.""" + + REQUIRED_COMPONENT = "light_effect" + QUERY_GETTER_NAME = "get_dynamic_light_effect_rules" + AVAILABLE_BULB_EFFECTS = { + "L1": "Party", + "L2": "Relax", + } + LIGHT_EFFECTS_OFF = "Off" + + def __init__(self, device: SmartDevice, module: str): + super().__init__(device, module) + self._scenes: dict[str, str] = {} + + def _initialize_features(self): + """Initialize features.""" + device = self._device + self._add_feature( + Feature( + device, + "Light effect theme", + container=self, + attribute_getter="effect", + attribute_setter="set_effect", + category=Feature.Category.Config, + type=Feature.Type.Choice, + choices_getter="effect_list", + ) + ) + + def _get_effects(self) -> dict[str, dict[str, Any]]: + """Return built-in effects.""" + # Copy the effects so scene name updates do not update the underlying dict. + effects = copy.deepcopy( + {effect["id"]: effect for effect in self.data["rule_list"]} + ) + for effect in effects.values(): + if not effect["scene_name"]: + # 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() + self._scenes = { + effect["scene_name"]: effect["id"] for effect in effects.values() + } + return effects + + @property + def effect_list(self) -> list[str] | None: + """Return built-in effects list. + + Example: + ['Party', 'Relax', ...] + """ + effects = [self.LIGHT_EFFECTS_OFF] + effects.extend( + [effect["scene_name"] for effect in self._get_effects().values()] + ) + return effects + + @property + def effect(self) -> str: + """Return effect scene name.""" + # get_dynamic_light_effect_rules also has an enable property and current_rule_id + # property that could be used here as an alternative + if self._device._info["dynamic_light_effect_enable"]: + return self._get_effects()[self._device._info["dynamic_light_effect_id"]][ + "scene_name" + ] + return self.LIGHT_EFFECTS_OFF + + async def set_effect( + self, + effect: str, + ) -> None: + """Set an effect for 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: + raise KasaException( + f"Cannot set theme to {effect}, possible values " + f"are: {self.LIGHT_EFFECTS_OFF} {' '.join(self._scenes.keys())}" + ) + enable = effect != self.LIGHT_EFFECTS_OFF + params: dict[str, bool | str] = {"enable": enable} + if enable: + effect_id = self._scenes[effect] + params["id"] = effect_id + return await self.call("set_dynamic_light_effect_rule_enable", params) + + def query(self) -> dict: + """Query to execute during the update cycle.""" + return {self.QUERY_GETTER_NAME: {"start_index": 0}} diff --git a/kasa/smart/smartdevice.py b/kasa/smart/smartdevice.py index 577ae0908..04b717c46 100644 --- a/kasa/smart/smartdevice.py +++ b/kasa/smart/smartdevice.py @@ -38,11 +38,6 @@ # same issue, homekit perhaps? WALL_SWITCH_PARENT_ONLY_MODULES = [DeviceModule, TimeModule, Firmware, CloudModule] # noqa: F405 -AVAILABLE_BULB_EFFECTS = { - "L1": "Party", - "L2": "Relax", -} - class SmartDevice(Device, Bulb): """Base class to represent a SMART protocol based device.""" @@ -665,44 +660,6 @@ def valid_temperature_range(self) -> ColorTempRange: ColorTemperatureModule, self.modules["ColorTemperatureModule"] ).valid_temperature_range - @property - def has_effects(self) -> bool: - """Return True if the device supports effects.""" - return "dynamic_light_effect_enable" in self._info - - @property - def effect(self) -> dict: - """Return effect state. - - This follows the format used by SmartLightStrip. - - Example: - {'brightness': 50, - 'custom': 0, - 'enable': 0, - 'id': '', - 'name': ''} - """ - # If no effect is active, dynamic_light_effect_id does not appear in info - current_effect = self._info.get("dynamic_light_effect_id", "") - data = { - "brightness": self.brightness, - "enable": current_effect != "", - "id": current_effect, - "name": AVAILABLE_BULB_EFFECTS.get(current_effect, ""), - } - - return data - - @property - def effect_list(self) -> list[str] | None: - """Return built-in effects list. - - Example: - ['Party', 'Relax', ...] - """ - return list(AVAILABLE_BULB_EFFECTS.keys()) if self.has_effects else None - @property def hsv(self) -> HSV: """Return the current HSV state of the bulb. @@ -789,17 +746,12 @@ async def set_brightness( brightness ) - async def set_effect( - self, - effect: str, - *, - brightness: int | None = None, - transition: int | None = None, - ) -> None: - """Set an effect on the device.""" - raise NotImplementedError() - @property def presets(self) -> list[BulbPreset]: """Return a list of available bulb setting presets.""" return [] + + @property + def has_effects(self) -> bool: + """Return True if the device supports effects.""" + return "LightEffectModule" in self.modules diff --git a/kasa/tests/fakeprotocol_smart.py b/kasa/tests/fakeprotocol_smart.py index 7340b5b7d..8bb715aa5 100644 --- a/kasa/tests/fakeprotocol_smart.py +++ b/kasa/tests/fakeprotocol_smart.py @@ -223,6 +223,20 @@ def _send_request(self, request_dict: dict): return retval elif method == "set_qs_info": return {"error_code": 0} + elif method == "set_dynamic_light_effect_rule_enable": + info["get_device_info"]["dynamic_light_effect_enable"] = params["enable"] + 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" + ] + 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"] + return {"error_code": 0} elif method[:4] == "set_": target_method = f"get_{method[4:]}" info[target_method].update(params) diff --git a/kasa/tests/smart/modules/test_light_effect.py b/kasa/tests/smart/modules/test_light_effect.py new file mode 100644 index 000000000..dff32fe70 --- /dev/null +++ b/kasa/tests/smart/modules/test_light_effect.py @@ -0,0 +1,42 @@ +from __future__ import annotations + +from itertools import chain +from typing import cast + +import pytest +from pytest_mock import MockerFixture + +from kasa import Device, Feature, KasaException +from kasa.smart.modules import LightEffectModule +from kasa.tests.device_fixtures import parametrize + +light_effect = parametrize( + "has light effect", component_filter="light_effect", protocol_filter={"SMART"} +) + + +@light_effect +async def test_light_effect(dev: Device, mocker: MockerFixture): + """Test fan speed feature.""" + light_effect = cast(LightEffectModule, dev.modules.get("LightEffectModule")) + assert light_effect + + feature = light_effect._module_features["light_effect_theme"] + assert feature.type == Feature.Type.Choice + + call = mocker.spy(light_effect, "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) + enable = effect != LightEffectModule.LIGHT_EFFECTS_OFF + params: dict[str, bool | str] = {"enable": enable} + if enable: + params["id"] = light_effect._scenes[effect] + call.assert_called_with("set_dynamic_light_effect_rule_enable", params) + await dev.update() + assert light_effect.effect == effect + assert feature.value == effect + + with pytest.raises(KasaException): + await light_effect.set_effect("foobar") From fb90e02151d091d93afb46e8fe6e97ff12170087 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Tue, 30 Apr 2024 12:15:29 +0100 Subject: [PATCH 2/9] Add cli test for feature command on all fixtures --- kasa/tests/test_cli.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/kasa/tests/test_cli.py b/kasa/tests/test_cli.py index a803fdc26..930c00249 100644 --- a/kasa/tests/test_cli.py +++ b/kasa/tests/test_cli.py @@ -689,6 +689,17 @@ async def test_feature(mocker, runner): assert res.exit_code == 0 +async def test_features_all(discovery_mock, mocker, runner): + """Test feature command on all fixtures.""" + res = await runner.invoke( + cli, + ["--host", "127.0.0.123", "--debug", "feature"], + catch_exceptions=False, + ) + assert "== Features ==" in res.output + assert res.exit_code == 0 + + async def test_feature_single(mocker, runner): """Test feature command returning single value.""" dummy_device = await get_device_for_fixture_protocol( From 555f26d3a0dbeba5eebc0d195af54a1091560381 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Tue, 30 Apr 2024 17:25:03 +0100 Subject: [PATCH 3/9] Update post review --- kasa/cli.py | 50 +++++++++---------------- kasa/feature.py | 10 ++++- kasa/smart/modules/lighteffectmodule.py | 4 +- kasa/tests/fakeprotocol_smart.py | 26 +++++++------ 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/kasa/cli.py b/kasa/cli.py index a9c575cfd..1de6b8350 100755 --- a/kasa/cli.py +++ b/kasa/cli.py @@ -590,6 +590,7 @@ def _echo_features( title: str, category: Feature.Category | None = None, verbose: bool = False, + indent: str = "\t", ): """Print out a listing of features and their values.""" if category is not None: @@ -602,13 +603,13 @@ def _echo_features( echo(f"[bold]{title}[/bold]") for _, feat in features.items(): try: - echo(f"\t{feat}") + echo(f"{indent}{feat}") if verbose: - echo(f"\t\tType: {feat.type}") - echo(f"\t\tCategory: {feat.category}") - echo(f"\t\tIcon: {feat.icon}") + echo(f"{indent}\tType: {feat.type}") + echo(f"{indent}\tCategory: {feat.category}") + echo(f"{indent}\tIcon: {feat.icon}") except Exception as ex: - echo(f"\t{feat.name} ({feat.id}): got exception (%s)" % ex) + echo(f"{indent}{feat.name} ({feat.id}): [red]got exception ({ex})[/red]") def _echo_all_features(features, *, verbose=False, title_prefix=None): @@ -1219,38 +1220,19 @@ async def feature(dev: Device, child: str, name: str, value): If only *name* is given, the value of named feature is returned. If both *name* and *value* are set, the described setting is changed. """ - - def _print_feature(name, feat, indent=""): - try: - unit = f" {feat.unit}" if feat.unit else "" - if feat.type == Feature.Type.Choice: - vals = feat.choices - index = vals.index(feat.value) - vals.remove(feat.value) - vals.insert(index, f"*{feat.value}*") - val = " ".join(vals) - else: - val = feat.value - echo(f"{indent}{feat.name} ({name}): {val}{unit}") - except Exception as ex: - echo(f"{indent}{feat.name} ({name}): [red]{ex}[/red]") - if child is not None: echo(f"Targeting child device {child}") dev = dev.get_child_device(child) if not name: - - def _print_features(dev): - for name, feat in dev.features.items(): - _print_feature(name, feat, "\t") - - echo("[bold]== Features ==[/bold]") - _print_features(dev) + _echo_features(dev.features, "\n[bold]== Features ==[/bold]\n", indent="") if dev.children: for child_dev in dev.children: - echo(f"[bold]== Child {child_dev.alias} ==") - _print_features(child_dev) + _echo_features( + child_dev.features, + f"\n[bold]== Child {child_dev.alias} ==\n", + indent="", + ) return @@ -1260,15 +1242,17 @@ def _print_features(dev): feat = dev.features[name] + _echo_features( + {name: feat}, "\n[bold]== Feature Current Value ==[/bold]\n", indent="" + ) if value is None: - _print_feature(name, feat) return feat.value - echo(f"Setting {name} to {value}") + echo(f"\nSetting {name} to {value}") value = ast.literal_eval(value) response = await dev.features[name].set_value(value) await dev.update() - _print_feature(name, feat) + _echo_features({name: feat}, "\n[bold]== Feature Updated To ==[/bold]\n", indent="") return response diff --git a/kasa/feature.py b/kasa/feature.py index 30acf362e..efb9af5de 100644 --- a/kasa/feature.py +++ b/kasa/feature.py @@ -172,7 +172,15 @@ async def set_value(self, value): return await getattr(container, self.attribute_setter)(value) def __repr__(self): - value = self.value + if self.type == Feature.Type.Choice: + value = self.choices.copy() + val = self.value + index = value.index(val) + value.remove(val) + value.insert(index, f"*{val}*") + value = " ".join(value) + else: + value = self.value if self.precision_hint is not None and value is not None: value = round(self.value, self.precision_hint) s = f"{self.name} ({self.id}): {value}" diff --git a/kasa/smart/modules/lighteffectmodule.py b/kasa/smart/modules/lighteffectmodule.py index 80c0a83a5..3f01794c0 100644 --- a/kasa/smart/modules/lighteffectmodule.py +++ b/kasa/smart/modules/lighteffectmodule.py @@ -15,7 +15,7 @@ class LightEffectModule(SmartModule): - """Implementation of gradual on/off.""" + """Implementation of dynamic light effects.""" REQUIRED_COMPONENT = "light_effect" QUERY_GETTER_NAME = "get_dynamic_light_effect_rules" @@ -78,7 +78,7 @@ def effect_list(self) -> list[str] | None: @property def effect(self) -> str: - """Return effect scene name.""" + """Return effect name.""" # get_dynamic_light_effect_rules also has an enable property and current_rule_id # property that could be used here as an alternative if self._device._info["dynamic_light_effect_enable"]: diff --git a/kasa/tests/fakeprotocol_smart.py b/kasa/tests/fakeprotocol_smart.py index 8bb715aa5..731c8f8a9 100644 --- a/kasa/tests/fakeprotocol_smart.py +++ b/kasa/tests/fakeprotocol_smart.py @@ -176,6 +176,19 @@ def _handle_control_child(self, params: dict): "Method %s not implemented for children" % child_method ) + def _set_dynamic_light_effect_rule_enable(self, info, params): + """Set or remove values as per the device behaviour.""" + info["get_device_info"]["dynamic_light_effect_enable"] = params["enable"] + 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"] + 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 _send_request(self, request_dict: dict): method = request_dict["method"] params = request_dict["params"] @@ -224,18 +237,7 @@ def _send_request(self, request_dict: dict): elif method == "set_qs_info": return {"error_code": 0} elif method == "set_dynamic_light_effect_rule_enable": - info["get_device_info"]["dynamic_light_effect_enable"] = params["enable"] - 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" - ] - 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"] + self._set_dynamic_light_effect_rule_enable(info, params) return {"error_code": 0} elif method[:4] == "set_": target_method = f"get_{method[4:]}" From 9304eee6045f9725c5e5fe05ce5697ea9e957a17 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Wed, 1 May 2024 15:17:20 +0100 Subject: [PATCH 4/9] Update post review --- kasa/cli.py | 5 ++-- kasa/feature.py | 15 +++++------ kasa/smart/modules/lighteffectmodule.py | 26 +++++++++---------- kasa/tests/smart/modules/test_light_effect.py | 2 +- 4 files changed, 23 insertions(+), 25 deletions(-) diff --git a/kasa/cli.py b/kasa/cli.py index 9343d70c0..8eab0e4c9 100755 --- a/kasa/cli.py +++ b/kasa/cli.py @@ -1243,16 +1243,15 @@ async def feature(dev: Device, child: str, name: str, value): feat = dev.features[name] _echo_features( - {name: feat}, "\n[bold]== Feature Current Value ==[/bold]\n", indent="" + {name: feat}, "\n[bold]== Feature Current Value ==[/bold]", indent="" ) if value is None: return feat.value - echo(f"\nSetting {name} to {value}") value = ast.literal_eval(value) response = await dev.features[name].set_value(value) await dev.update() - _echo_features({name: feat}, "\n[bold]== Feature Updated To ==[/bold]\n", indent="") + _echo_features({name: feat}, "\n[bold]== Feature Updated To ==[/bold]", indent="") return response diff --git a/kasa/feature.py b/kasa/feature.py index efb9af5de..10d0d6bfe 100644 --- a/kasa/feature.py +++ b/kasa/feature.py @@ -172,15 +172,14 @@ async def set_value(self, value): return await getattr(container, self.attribute_setter)(value) def __repr__(self): + value = self.value if self.type == Feature.Type.Choice: - value = self.choices.copy() - val = self.value - index = value.index(val) - value.remove(val) - value.insert(index, f"*{val}*") - value = " ".join(value) - else: - value = self.value + value = " ".join( + [ + f"*{choice}*" if choice == value else choice + for choice in self.choices + ] + ) if self.precision_hint is not None and value is not None: value = round(self.value, self.precision_hint) s = f"{self.name} ({self.id}): {value}" diff --git a/kasa/smart/modules/lighteffectmodule.py b/kasa/smart/modules/lighteffectmodule.py index 3f01794c0..c62af5501 100644 --- a/kasa/smart/modules/lighteffectmodule.py +++ b/kasa/smart/modules/lighteffectmodule.py @@ -6,7 +6,6 @@ import copy from typing import TYPE_CHECKING, Any -from ...exceptions import KasaException from ...feature import Feature from ..smartmodule import SmartModule @@ -27,7 +26,7 @@ class LightEffectModule(SmartModule): def __init__(self, device: SmartDevice, module: str): super().__init__(device, module) - self._scenes: dict[str, str] = {} + self._scenes_names_to_id: dict[str, str] = {} def _initialize_features(self): """Initialize features.""" @@ -45,7 +44,7 @@ def _initialize_features(self): ) ) - def _get_effects(self) -> dict[str, dict[str, Any]]: + def _initialize_effects(self) -> dict[str, dict[str, Any]]: """Return built-in effects.""" # Copy the effects so scene name updates do not update the underlying dict. effects = copy.deepcopy( @@ -58,7 +57,7 @@ def _get_effects(self) -> dict[str, dict[str, Any]]: else: # Otherwise it will be b64 encoded effect["scene_name"] = base64.b64decode(effect["scene_name"]).decode() - self._scenes = { + self._scenes_names_to_id = { effect["scene_name"]: effect["id"] for effect in effects.values() } return effects @@ -72,7 +71,7 @@ def effect_list(self) -> list[str] | None: """ effects = [self.LIGHT_EFFECTS_OFF] effects.extend( - [effect["scene_name"] for effect in self._get_effects().values()] + [effect["scene_name"] for effect in self._initialize_effects().values()] ) return effects @@ -82,9 +81,9 @@ def effect(self) -> str: # get_dynamic_light_effect_rules also has an enable property and current_rule_id # property that could be used here as an alternative if self._device._info["dynamic_light_effect_enable"]: - return self._get_effects()[self._device._info["dynamic_light_effect_id"]][ - "scene_name" - ] + return self._initialize_effects()[ + self._device._info["dynamic_light_effect_id"] + ]["scene_name"] return self.LIGHT_EFFECTS_OFF async def set_effect( @@ -95,15 +94,16 @@ async def set_effect( 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: - raise KasaException( - f"Cannot set theme to {effect}, possible values " - f"are: {self.LIGHT_EFFECTS_OFF} {' '.join(self._scenes.keys())}" + if effect != self.LIGHT_EFFECTS_OFF and effect not in self._scenes_names_to_id: + raise ValueError( + f"Cannot set light effect theme to {effect}, possible values " + f"are: {self.LIGHT_EFFECTS_OFF} " + f"{' '.join(self._scenes_names_to_id.keys())}" ) enable = effect != self.LIGHT_EFFECTS_OFF params: dict[str, bool | str] = {"enable": enable} if enable: - effect_id = self._scenes[effect] + effect_id = self._scenes_names_to_id[effect] params["id"] = effect_id return await self.call("set_dynamic_light_effect_rule_enable", params) diff --git a/kasa/tests/smart/modules/test_light_effect.py b/kasa/tests/smart/modules/test_light_effect.py index dff32fe70..72ed3ec62 100644 --- a/kasa/tests/smart/modules/test_light_effect.py +++ b/kasa/tests/smart/modules/test_light_effect.py @@ -32,7 +32,7 @@ async def test_light_effect(dev: Device, mocker: MockerFixture): enable = effect != LightEffectModule.LIGHT_EFFECTS_OFF params: dict[str, bool | str] = {"enable": enable} if enable: - params["id"] = light_effect._scenes[effect] + params["id"] = light_effect._scenes_names_to_id[effect] call.assert_called_with("set_dynamic_light_effect_rule_enable", params) await dev.update() assert light_effect.effect == effect From 6c453c8c16e87caf59783497bb6f2c1f5f208992 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Wed, 1 May 2024 15:30:22 +0100 Subject: [PATCH 5/9] Fix tests --- kasa/tests/smart/modules/test_light_effect.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kasa/tests/smart/modules/test_light_effect.py b/kasa/tests/smart/modules/test_light_effect.py index 72ed3ec62..78d76406a 100644 --- a/kasa/tests/smart/modules/test_light_effect.py +++ b/kasa/tests/smart/modules/test_light_effect.py @@ -6,7 +6,7 @@ import pytest from pytest_mock import MockerFixture -from kasa import Device, Feature, KasaException +from kasa import Device, Feature from kasa.smart.modules import LightEffectModule from kasa.tests.device_fixtures import parametrize @@ -38,5 +38,5 @@ async def test_light_effect(dev: Device, mocker: MockerFixture): assert light_effect.effect == effect assert feature.value == effect - with pytest.raises(KasaException): + with pytest.raises(ValueError): await light_effect.set_effect("foobar") From 232dee008a55cd510f91ecd4f87526e2d8e63e99 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Wed, 1 May 2024 15:37:52 +0100 Subject: [PATCH 6/9] Fix tests again --- kasa/cli.py | 1 + 1 file changed, 1 insertion(+) diff --git a/kasa/cli.py b/kasa/cli.py index 8eab0e4c9..123fd4e2b 100755 --- a/kasa/cli.py +++ b/kasa/cli.py @@ -1248,6 +1248,7 @@ async def feature(dev: Device, child: str, name: str, value): if value is None: return feat.value + echo(f"Setting {name} to {value}") value = ast.literal_eval(value) response = await dev.features[name].set_value(value) await dev.update() From 12e6c76874be8eda6dfaa082d9f65ba5ba41ef52 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Thu, 2 May 2024 14:34:10 +0100 Subject: [PATCH 7/9] Drop theme from light effect --- kasa/smart/modules/lighteffectmodule.py | 4 ++-- kasa/tests/smart/modules/test_light_effect.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kasa/smart/modules/lighteffectmodule.py b/kasa/smart/modules/lighteffectmodule.py index c62af5501..7f03b8ff6 100644 --- a/kasa/smart/modules/lighteffectmodule.py +++ b/kasa/smart/modules/lighteffectmodule.py @@ -34,7 +34,7 @@ def _initialize_features(self): self._add_feature( Feature( device, - "Light effect theme", + "Light effect", container=self, attribute_getter="effect", attribute_setter="set_effect", @@ -96,7 +96,7 @@ async def set_effect( """ if effect != self.LIGHT_EFFECTS_OFF and effect not in self._scenes_names_to_id: raise ValueError( - f"Cannot set light effect theme to {effect}, possible values " + f"Cannot set light effect to {effect}, possible values " f"are: {self.LIGHT_EFFECTS_OFF} " f"{' '.join(self._scenes_names_to_id.keys())}" ) diff --git a/kasa/tests/smart/modules/test_light_effect.py b/kasa/tests/smart/modules/test_light_effect.py index 78d76406a..0a659569c 100644 --- a/kasa/tests/smart/modules/test_light_effect.py +++ b/kasa/tests/smart/modules/test_light_effect.py @@ -21,7 +21,7 @@ async def test_light_effect(dev: Device, mocker: MockerFixture): light_effect = cast(LightEffectModule, dev.modules.get("LightEffectModule")) assert light_effect - feature = light_effect._module_features["light_effect_theme"] + feature = light_effect._module_features["light_effect"] assert feature.type == Feature.Type.Choice call = mocker.spy(light_effect, "call") From ca30eba8c8e0961dbd7df1a772541ece4e092715 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Thu, 2 May 2024 15:06:21 +0100 Subject: [PATCH 8/9] Update post review --- kasa/cli.py | 9 ++++----- kasa/feature.py | 4 ++-- kasa/tests/fakeprotocol_smart.py | 4 ++-- kasa/tests/smart/modules/test_light_effect.py | 2 +- kasa/tests/test_cli.py | 8 ++++---- 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/kasa/cli.py b/kasa/cli.py index 8142ad2b9..bb704622e 100755 --- a/kasa/cli.py +++ b/kasa/cli.py @@ -1238,17 +1238,16 @@ async def feature(dev: Device, child: str, name: str, value): feat = dev.features[name] - _echo_features( - {name: feat}, "\n[bold]== Feature Current Value ==[/bold]", indent="" - ) if value is None: + echo(f"Feature {name} has value {feat.value}") return feat.value - echo(f"Setting {name} to {value}") value = ast.literal_eval(value) + echo(f"Changing {name} from {feat.value} to {value}") response = await dev.features[name].set_value(value) await dev.update() - _echo_features({name: feat}, "\n[bold]== Feature Updated To ==[/bold]", indent="") + echo(f"New state: {feat.value}") + return response diff --git a/kasa/feature.py b/kasa/feature.py index 12b75e3e3..2000b21af 100644 --- a/kasa/feature.py +++ b/kasa/feature.py @@ -175,12 +175,12 @@ def __repr__(self): try: value = self.value choices = self.choices - if choices and value not in choices: - return f"Value {value} is not a valid choice ({self.id}): {choices}" except Exception as ex: return f"Unable to read value ({self.id}): {ex}" if self.type == Feature.Type.Choice: + if not isinstance(choices, list) or value not in choices: + return f"Value {value} is not a valid choice ({self.id}): {choices}" value = " ".join( [f"*{choice}*" if choice == value else choice for choice in choices] ) diff --git a/kasa/tests/fakeprotocol_smart.py b/kasa/tests/fakeprotocol_smart.py index 731c8f8a9..ae1a7ad66 100644 --- a/kasa/tests/fakeprotocol_smart.py +++ b/kasa/tests/fakeprotocol_smart.py @@ -176,7 +176,7 @@ def _handle_control_child(self, params: dict): "Method %s not implemented for children" % child_method ) - def _set_dynamic_light_effect_rule_enable(self, info, params): + def _set_light_effect(self, info, params): """Set or remove values as per the device behaviour.""" info["get_device_info"]["dynamic_light_effect_enable"] = params["enable"] info["get_dynamic_light_effect_rules"]["enable"] = params["enable"] @@ -237,7 +237,7 @@ def _send_request(self, request_dict: dict): elif method == "set_qs_info": return {"error_code": 0} elif method == "set_dynamic_light_effect_rule_enable": - self._set_dynamic_light_effect_rule_enable(info, params) + self._set_light_effect(info, params) return {"error_code": 0} elif method[:4] == "set_": target_method = f"get_{method[4:]}" diff --git a/kasa/tests/smart/modules/test_light_effect.py b/kasa/tests/smart/modules/test_light_effect.py index 0a659569c..ba1b22934 100644 --- a/kasa/tests/smart/modules/test_light_effect.py +++ b/kasa/tests/smart/modules/test_light_effect.py @@ -17,7 +17,7 @@ @light_effect async def test_light_effect(dev: Device, mocker: MockerFixture): - """Test fan speed feature.""" + """Test light effect.""" light_effect = cast(LightEffectModule, dev.modules.get("LightEffectModule")) assert light_effect diff --git a/kasa/tests/test_cli.py b/kasa/tests/test_cli.py index 7111bc5c8..7addd4348 100644 --- a/kasa/tests/test_cli.py +++ b/kasa/tests/test_cli.py @@ -747,7 +747,7 @@ async def test_feature_set(mocker, runner): ) led_setter.assert_called_with(True) - assert "Setting led to True" in res.output + assert "Changing led from False to True" in res.output assert res.exit_code == 0 @@ -773,14 +773,14 @@ async def test_feature_set_child(mocker, runner): "--child", child_id, "state", - "False", + "True", ], catch_exceptions=False, ) get_child_device.assert_called() - setter.assert_called_with(False) + setter.assert_called_with(True) assert f"Targeting child device {child_id}" - assert "Setting state to False" in res.output + assert "Changing state from False to True" in res.output assert res.exit_code == 0 From 71325e372c3fa10c077202262fb95b6bf28b3016 Mon Sep 17 00:00:00 2001 From: sdb9696 Date: Thu, 2 May 2024 15:16:36 +0100 Subject: [PATCH 9/9] Tweak single feature output --- kasa/cli.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kasa/cli.py b/kasa/cli.py index bb704622e..696dee274 100755 --- a/kasa/cli.py +++ b/kasa/cli.py @@ -1239,7 +1239,8 @@ async def feature(dev: Device, child: str, name: str, value): feat = dev.features[name] if value is None: - echo(f"Feature {name} has value {feat.value}") + unit = f" {feat.unit}" if feat.unit else "" + echo(f"{feat.name} ({name}): {feat.value}{unit}") return feat.value value = ast.literal_eval(value)