From fe0bbf1b98621f72229277c0ca0c64047a128c92 Mon Sep 17 00:00:00 2001 From: Steven B <51370195+sdb9696@users.noreply.github.com> Date: Mon, 10 Jun 2024 05:59:37 +0100 Subject: [PATCH] Do not expose child modules on parent devices (#964) Removes the logic to expose child modules on parent devices, which could cause complications with downstream consumers unknowingly duplicating things. --- kasa/smart/smartdevice.py | 17 +---------------- kasa/tests/device_fixtures.py | 13 +++++++++++++ kasa/tests/smart/features/test_brightness.py | 6 +++--- kasa/tests/smart/modules/test_fan.py | 13 ++++++------- kasa/tests/test_common_modules.py | 15 ++++++++------- kasa/tests/test_smartdevice.py | 8 +++----- 6 files changed, 34 insertions(+), 38 deletions(-) diff --git a/kasa/smart/smartdevice.py b/kasa/smart/smartdevice.py index 3250c98e..0c56dba8 100644 --- a/kasa/smart/smartdevice.py +++ b/kasa/smart/smartdevice.py @@ -57,7 +57,6 @@ class SmartDevice(Device): self._components: dict[str, int] = {} self._state_information: dict[str, Any] = {} self._modules: dict[str | ModuleName[Module], SmartModule] = {} - self._exposes_child_modules = False self._parent: SmartDevice | None = None self._children: Mapping[str, SmartDevice] = {} self._last_update = {} @@ -99,16 +98,6 @@ class SmartDevice(Device): @property def modules(self) -> ModuleMapping[SmartModule]: """Return the device modules.""" - if self._exposes_child_modules: - modules = {k: v for k, v in self._modules.items()} - for child in self._children.values(): - for k, v in child._modules.items(): - if k not in modules: - modules[k] = v - if TYPE_CHECKING: - return cast(ModuleMapping[SmartModule], modules) - return modules - if TYPE_CHECKING: # Needed for python 3.8 return cast(ModuleMapping[SmartModule], self._modules) return self._modules @@ -213,7 +202,6 @@ class SmartDevice(Device): skip_parent_only_modules = True elif self._children and self.device_type == DeviceType.WallSwitch: # _initialize_modules is called on the parent after the children - self._exposes_child_modules = True for child in self._children.values(): child_modules_to_skip.update(**child.modules) @@ -332,10 +320,7 @@ class SmartDevice(Device): ) for module in self.modules.values(): - # Check if module features have already been initialized. - # i.e. when _exposes_child_modules is true - if not module._module_features: - module._initialize_features() + module._initialize_features() for feat in module._module_features.values(): self._add_feature(feat) for child in self._children.values(): diff --git a/kasa/tests/device_fixtures.py b/kasa/tests/device_fixtures.py index 04b6d391..184eedaa 100644 --- a/kasa/tests/device_fixtures.py +++ b/kasa/tests/device_fixtures.py @@ -434,3 +434,16 @@ async def dev(request) -> AsyncGenerator[Device, None]: yield dev await dev.disconnect() + + +def get_parent_and_child_modules(device: Device, module_name): + """Return iterator of module if exists on parent and children. + + Useful for testing devices that have components listed on the parent that are only + supported on the children, i.e. ks240. + """ + if module_name in device.modules: + yield device.modules[module_name] + for child in device.children: + if module_name in child.modules: + yield child.modules[module_name] diff --git a/kasa/tests/smart/features/test_brightness.py b/kasa/tests/smart/features/test_brightness.py index e3c3c530..bbf4d6df 100644 --- a/kasa/tests/smart/features/test_brightness.py +++ b/kasa/tests/smart/features/test_brightness.py @@ -2,7 +2,7 @@ import pytest from kasa.iot import IotDevice from kasa.smart import SmartDevice -from kasa.tests.conftest import dimmable_iot, parametrize +from kasa.tests.conftest import dimmable_iot, get_parent_and_child_modules, parametrize brightness = parametrize("brightness smart", component_filter="brightness") @@ -10,13 +10,13 @@ brightness = parametrize("brightness smart", component_filter="brightness") @brightness async def test_brightness_component(dev: SmartDevice): """Test brightness feature.""" - brightness = dev.modules.get("Brightness") + brightness = next(get_parent_and_child_modules(dev, "Brightness")) assert brightness assert isinstance(dev, SmartDevice) assert "brightness" in dev._components # Test getting the value - feature = dev.features["brightness"] + feature = brightness._device.features["brightness"] assert isinstance(feature.value, int) assert feature.value > 1 and feature.value <= 100 diff --git a/kasa/tests/smart/modules/test_fan.py b/kasa/tests/smart/modules/test_fan.py index 6d5a0dd1..ee04015f 100644 --- a/kasa/tests/smart/modules/test_fan.py +++ b/kasa/tests/smart/modules/test_fan.py @@ -3,7 +3,7 @@ from pytest_mock import MockerFixture from kasa import Module from kasa.smart import SmartDevice -from kasa.tests.device_fixtures import parametrize +from kasa.tests.device_fixtures import get_parent_and_child_modules, parametrize fan = parametrize("has fan", component_filter="fan_control", protocol_filter={"SMART"}) @@ -11,10 +11,9 @@ fan = parametrize("has fan", component_filter="fan_control", protocol_filter={"S @fan async def test_fan_speed(dev: SmartDevice, mocker: MockerFixture): """Test fan speed feature.""" - fan = dev.modules.get(Module.Fan) + fan = next(get_parent_and_child_modules(dev, Module.Fan)) assert fan - - level_feature = dev.features["fan_speed_level"] + level_feature = fan._module_features["fan_speed_level"] assert ( level_feature.minimum_value <= level_feature.value @@ -36,9 +35,9 @@ async def test_fan_speed(dev: SmartDevice, mocker: MockerFixture): @fan async def test_sleep_mode(dev: SmartDevice, mocker: MockerFixture): """Test sleep mode feature.""" - fan = dev.modules.get(Module.Fan) + fan = next(get_parent_and_child_modules(dev, Module.Fan)) assert fan - sleep_feature = dev.features["fan_sleep_mode"] + sleep_feature = fan._module_features["fan_sleep_mode"] assert isinstance(sleep_feature.value, bool) call = mocker.spy(fan, "call") @@ -55,7 +54,7 @@ async def test_sleep_mode(dev: SmartDevice, mocker: MockerFixture): async def test_fan_module(dev: SmartDevice, mocker: MockerFixture): """Test fan speed on device interface.""" assert isinstance(dev, SmartDevice) - fan = dev.modules.get(Module.Fan) + fan = next(get_parent_and_child_modules(dev, Module.Fan)) assert fan device = fan._device diff --git a/kasa/tests/test_common_modules.py b/kasa/tests/test_common_modules.py index eaff5c07..c0d90578 100644 --- a/kasa/tests/test_common_modules.py +++ b/kasa/tests/test_common_modules.py @@ -7,6 +7,7 @@ from kasa.tests.device_fixtures import ( bulb_smart, dimmable_iot, dimmer_iot, + get_parent_and_child_modules, lightstrip_iot, parametrize, parametrize_combine, @@ -123,11 +124,11 @@ async def test_light_effect_module(dev: Device, mocker: MockerFixture): async def test_light_brightness(dev: Device): """Test brightness setter and getter.""" assert isinstance(dev, Device) - light = dev.modules.get(Module.Light) + light = next(get_parent_and_child_modules(dev, Module.Light)) assert light # Test getting the value - feature = dev.features["brightness"] + feature = light._device.features["brightness"] assert feature.minimum_value == 0 assert feature.maximum_value == 100 @@ -146,7 +147,7 @@ async def test_light_brightness(dev: Device): async def test_light_set_state(dev: Device): """Test brightness setter and getter.""" assert isinstance(dev, Device) - light = dev.modules.get(Module.Light) + light = next(get_parent_and_child_modules(dev, Module.Light)) assert light await light.set_state(LightState(light_on=False)) @@ -169,11 +170,11 @@ async def test_light_set_state(dev: Device): @light_preset async def test_light_preset_module(dev: Device, mocker: MockerFixture): """Test light preset module.""" - preset_mod = dev.modules[Module.LightPreset] + preset_mod = next(get_parent_and_child_modules(dev, Module.LightPreset)) assert preset_mod - light_mod = dev.modules[Module.Light] + light_mod = next(get_parent_and_child_modules(dev, Module.Light)) assert light_mod - feat = dev.features["light_preset"] + feat = preset_mod._device.features["light_preset"] preset_list = preset_mod.preset_list assert "Not set" in preset_list @@ -220,7 +221,7 @@ async def test_light_preset_module(dev: Device, mocker: MockerFixture): @light_preset async def test_light_preset_save(dev: Device, mocker: MockerFixture): """Test saving a new preset value.""" - preset_mod = dev.modules[Module.LightPreset] + preset_mod = next(get_parent_and_child_modules(dev, Module.LightPreset)) assert preset_mod preset_list = preset_mod.preset_list if len(preset_list) == 1: diff --git a/kasa/tests/test_smartdevice.py b/kasa/tests/test_smartdevice.py index 88880e10..2ffc40ba 100644 --- a/kasa/tests/test_smartdevice.py +++ b/kasa/tests/test_smartdevice.py @@ -16,6 +16,7 @@ from kasa.smart import SmartDevice from .conftest import ( device_smart, get_device_for_fixture_protocol, + get_parent_and_child_modules, ) @@ -144,11 +145,8 @@ async def test_get_modules(): # Modules on child module = dummy_device.modules.get("Fan") - assert module - assert module._device != dummy_device - assert module._device._parent == dummy_device - - module = dummy_device.modules.get(Module.Fan) + assert module is None + module = next(get_parent_and_child_modules(dummy_device, "Fan")) assert module assert module._device != dummy_device assert module._device._parent == dummy_device