Update light transition module to work with child devices (#1017)

Fixes module to work with child devices, i.e. ks240
Interrogates the data to see whether maximums are available.
Fixes a bug whereby setting a duration while the feature is not
enabled does not actually enable it.

kasa/tests/device_fixtures.py

@@ -15,7 +15,13 @@ from kasa.smart import SmartDevice
 
 from .fakeprotocol_iot import FakeIotProtocol
 from .fakeprotocol_smart import FakeSmartProtocol
-from .fixtureinfo import FIXTURE_DATA, FixtureInfo, filter_fixtures, idgenerator
+from .fixtureinfo import (
+    FIXTURE_DATA,
+    ComponentFilter,
+    FixtureInfo,
+    filter_fixtures,
+    idgenerator,
+)
 
 # Tapo bulbs
 BULBS_SMART_VARIABLE_TEMP = {"L530E", "L930-5"}
@@ -175,7 +181,7 @@ def parametrize(
     *,
     model_filter=None,
     protocol_filter=None,
-    component_filter=None,
+    component_filter: str | ComponentFilter | None = None,
     data_root_filter=None,
     device_type_filter=None,
     ids=None,

kasa/tests/discovery_fixtures.py

@@ -12,6 +12,8 @@ from .fakeprotocol_iot import FakeIotProtocol
 from .fakeprotocol_smart import FakeSmartProtocol, FakeSmartTransport
 from .fixtureinfo import FixtureInfo, filter_fixtures, idgenerator
 
+DISCOVERY_MOCK_IP = "127.0.0.123"
+
 
 def _make_unsupported(device_family, encrypt_type):
     return {
@@ -73,7 +75,7 @@ new_discovery = parametrize_discovery(
 async def discovery_mock(request, mocker):
     """Mock discovery and patch protocol queries to use Fake protocols."""
     fixture_info: FixtureInfo = request.param
-    yield patch_discovery({"127.0.0.123": fixture_info}, mocker)
+    yield patch_discovery({DISCOVERY_MOCK_IP: fixture_info}, mocker)
 
 
 def create_discovery_mock(ip: str, fixture_data: dict):

kasa/tests/fakeprotocol_smart.py

@@ -78,7 +78,6 @@ class FakeSmartTransport(BaseTransport):
                 },
             },
         ),
-        "get_on_off_gradually_info": ("on_off_gradually", {"enable": True}),
         "get_latest_fw": (
             "firmware",
             {
@@ -164,6 +163,8 @@ class FakeSmartTransport(BaseTransport):
             return {"error_code": 0}
         elif child_method == "set_preset_rules":
             return self._set_child_preset_rules(info, child_params)
+        elif child_method == "set_on_off_gradually_info":
+            return self._set_on_off_gradually_info(info, child_params)
         elif child_method in child_device_calls:
             result = copy.deepcopy(child_device_calls[child_method])
             return {"result": result, "error_code": 0}
@@ -200,6 +201,49 @@ class FakeSmartTransport(BaseTransport):
             "Method %s not implemented for children" % child_method
         )
 
+    def _get_on_off_gradually_info(self, info, params):
+        if self.components["on_off_gradually"] == 1:
+            info["get_on_off_gradually_info"] = {"enable": True}
+        else:
+            info["get_on_off_gradually_info"] = {
+                "off_state": {"duration": 5, "enable": False, "max_duration": 60},
+                "on_state": {"duration": 5, "enable": False, "max_duration": 60},
+            }
+        return copy.deepcopy(info["get_on_off_gradually_info"])
+
+    def _set_on_off_gradually_info(self, info, params):
+        # Child devices can have the required properties directly in info
+
+        if self.components["on_off_gradually"] == 1:
+            info["get_on_off_gradually_info"] = {"enable": params["enable"]}
+        elif on_state := params.get("on_state"):
+            if "fade_on_time" in info and "gradually_on_mode" in info:
+                info["gradually_on_mode"] = 1 if on_state["enable"] else 0
+                if "duration" in on_state:
+                    info["fade_on_time"] = on_state["duration"]
+            else:
+                info["get_on_off_gradually_info"]["on_state"]["enable"] = on_state[
+                    "enable"
+                ]
+                if "duration" in on_state:
+                    info["get_on_off_gradually_info"]["on_state"]["duration"] = (
+                        on_state["duration"]
+                    )
+        elif off_state := params.get("off_state"):
+            if "fade_off_time" in info and "gradually_off_mode" in info:
+                info["gradually_off_mode"] = 1 if off_state["enable"] else 0
+                if "duration" in off_state:
+                    info["fade_off_time"] = off_state["duration"]
+            else:
+                info["get_on_off_gradually_info"]["off_state"]["enable"] = off_state[
+                    "enable"
+                ]
+                if "duration" in off_state:
+                    info["get_on_off_gradually_info"]["off_state"]["duration"] = (
+                        off_state["duration"]
+                    )
+        return {"error_code": 0}
+
     def _set_dynamic_light_effect(self, info, params):
         """Set or remove values as per the device behaviour."""
         info["get_device_info"]["dynamic_light_effect_enable"] = params["enable"]
@@ -294,6 +338,13 @@ class FakeSmartTransport(BaseTransport):
                 info[method] = copy.deepcopy(missing_result[1])
                 result = copy.deepcopy(info[method])
                 retval = {"result": result, "error_code": 0}
+            elif (
+                method == "get_on_off_gradually_info"
+                and "on_off_gradually" in self.components
+            ):
+                # Need to call a method here to determine which version schema to return
+                result = self._get_on_off_gradually_info(info, params)
+                return {"result": result, "error_code": 0}
             else:
                 # PARAMS error returned for KS240 when get_device_usage called
                 # on parent device.  Could be any error code though.
@@ -324,6 +375,8 @@ class FakeSmartTransport(BaseTransport):
             return self._set_preset_rules(info, params)
         elif method == "edit_preset_rules":
             return self._edit_preset_rules(info, params)
+        elif method == "set_on_off_gradually_info":
+            return self._set_on_off_gradually_info(info, params)
         elif method[:4] == "set_":
             target_method = f"get_{method[4:]}"
             info[target_method].update(params)

kasa/tests/fixtureinfo.py

@@ -17,6 +17,12 @@ class FixtureInfo(NamedTuple):
     data: dict
 
 
+class ComponentFilter(NamedTuple):
+    component_name: str
+    minimum_version: int = 0
+    maximum_version: int | None = None
+
+
 FixtureInfo.__hash__ = lambda self: hash((self.name, self.protocol))  # type: ignore[attr-defined, method-assign]
 FixtureInfo.__eq__ = lambda x, y: hash(x) == hash(y)  # type: ignore[method-assign]
 
@@ -88,7 +94,7 @@ def filter_fixtures(
     data_root_filter: str | None = None,
     protocol_filter: set[str] | None = None,
     model_filter: set[str] | None = None,
-    component_filter: str | None = None,
+    component_filter: str | ComponentFilter | None = None,
     device_type_filter: list[DeviceType] | None = None,
 ):
     """Filter the fixtures based on supplied parameters.
@@ -106,14 +112,26 @@ def filter_fixtures(
         file_model = file_model_region.split("(")[0]
         return file_model in model_filter
 
-    def _component_match(fixture_data: FixtureInfo, component_filter):
+    def _component_match(
+        fixture_data: FixtureInfo, component_filter: str | ComponentFilter
+    ):
         if (component_nego := fixture_data.data.get("component_nego")) is None:
             return False
         components = {
             component["id"]: component["ver_code"]
             for component in component_nego["component_list"]
         }
-        return component_filter in components
+        if isinstance(component_filter, str):
+            return component_filter in components
+        else:
+            return (
+                (ver_code := components.get(component_filter.component_name))
+                and ver_code >= component_filter.minimum_version
+                and (
+                    component_filter.maximum_version is None
+                    or ver_code <= component_filter.maximum_version
+                )
+            )
 
     def _device_type_match(fixture_data: FixtureInfo, device_type):
         if (component_nego := fixture_data.data.get("component_nego")) is None:

kasa/tests/smart/modules/test_lighttransition.py

@@ -0,0 +1,80 @@
+from pytest_mock import MockerFixture
+
+from kasa import Feature, Module
+from kasa.smart import SmartDevice
+from kasa.tests.device_fixtures import get_parent_and_child_modules, parametrize
+from kasa.tests.fixtureinfo import ComponentFilter
+
+light_transition_v1 = parametrize(
+    "has light transition",
+    component_filter=ComponentFilter(
+        component_name="on_off_gradually", maximum_version=1
+    ),
+    protocol_filter={"SMART"},
+)
+light_transition_gt_v1 = parametrize(
+    "has light transition",
+    component_filter=ComponentFilter(
+        component_name="on_off_gradually", minimum_version=2
+    ),
+    protocol_filter={"SMART"},
+)
+
+
+@light_transition_v1
+async def test_module_v1(dev: SmartDevice, mocker: MockerFixture):
+    """Test light transition module."""
+    assert isinstance(dev, SmartDevice)
+    light_transition = next(get_parent_and_child_modules(dev, Module.LightTransition))
+    assert light_transition
+    assert "smooth_transitions" in light_transition._module_features
+    assert "smooth_transition_on" not in light_transition._module_features
+    assert "smooth_transition_off" not in light_transition._module_features
+
+    await light_transition.set_enabled(True)
+    await dev.update()
+    assert light_transition.enabled is True
+
+    await light_transition.set_enabled(False)
+    await dev.update()
+    assert light_transition.enabled is False
+
+
+@light_transition_gt_v1
+async def test_module_gt_v1(dev: SmartDevice, mocker: MockerFixture):
+    """Test light transition module."""
+    assert isinstance(dev, SmartDevice)
+    light_transition = next(get_parent_and_child_modules(dev, Module.LightTransition))
+    assert light_transition
+    assert "smooth_transitions" not in light_transition._module_features
+    assert "smooth_transition_on" in light_transition._module_features
+    assert "smooth_transition_off" in light_transition._module_features
+
+    await light_transition.set_enabled(True)
+    await dev.update()
+    assert light_transition.enabled is True
+
+    await light_transition.set_enabled(False)
+    await dev.update()
+    assert light_transition.enabled is False
+
+    await light_transition.set_turn_on_transition(5)
+    await dev.update()
+    assert light_transition.turn_on_transition == 5
+    # enabled is true if either on or off is enabled
+    assert light_transition.enabled is True
+
+    await light_transition.set_turn_off_transition(10)
+    await dev.update()
+    assert light_transition.turn_off_transition == 10
+    assert light_transition.enabled is True
+
+    max_on = light_transition._module_features["smooth_transition_on"].maximum_value
+    assert max_on < Feature.DEFAULT_MAX
+    max_off = light_transition._module_features["smooth_transition_off"].maximum_value
+    assert max_off < Feature.DEFAULT_MAX
+
+    await light_transition.set_turn_on_transition(0)
+    await light_transition.set_turn_off_transition(0)
+    await dev.update()
+    assert light_transition.enabled is False

kasa/tests/test_device_factory.py

@@ -1,16 +1,25 @@
-# type: ignore
+"""Module for testing device factory.
+
+As this module tests the factory with discovery data and expects update to be
+called on devices it uses the discovery_mock handles all the patching of the
+query methods without actually replacing the device protocol class with one of
+the testing fake protocols.
+"""
+
 import logging
+from typing import cast
 
 import aiohttp
 import pytest  # type: ignore # https://github.com/pytest-dev/pytest/issues/3342
 
 from kasa import (
     Credentials,
-    Device,
     Discover,
     KasaException,
 )
 from kasa.device_factory import (
+    Device,
+    SmartDevice,
     _get_device_type_from_sys_info,
     connect,
     get_device_class_from_family,
@@ -23,7 +32,8 @@ from kasa.deviceconfig import (
     DeviceFamily,
 )
 from kasa.discover import DiscoveryResult
-from kasa.smart.smartdevice import SmartDevice
+
+from .conftest import DISCOVERY_MOCK_IP
 
 
 def _get_connection_type_device_class(discovery_info):
@@ -44,18 +54,22 @@ def _get_connection_type_device_class(discovery_info):
 
 
 async def test_connect(
-    discovery_data,
+    discovery_mock,
     mocker,
 ):
     """Test that if the protocol is passed in it gets set correctly."""
-    host = "127.0.0.1"
-    ctype, device_class = _get_connection_type_device_class(discovery_data)
+    host = DISCOVERY_MOCK_IP
+    ctype, device_class = _get_connection_type_device_class(
+        discovery_mock.discovery_data
+    )
 
     config = DeviceConfig(
         host=host, credentials=Credentials("foor", "bar"), connection_type=ctype
     )
     protocol_class = get_protocol(config).__class__
     close_mock = mocker.patch.object(protocol_class, "close")
+    # mocker.patch.object(SmartDevice, "update")
+    # mocker.patch.object(Device, "update")
     dev = await connect(
         config=config,
     )
@@ -69,10 +83,11 @@ async def test_connect(
 
 
 @pytest.mark.parametrize("custom_port", [123, None])
-async def test_connect_custom_port(discovery_data: dict, mocker, custom_port):
+async def test_connect_custom_port(discovery_mock, mocker, custom_port):
     """Make sure that connect returns an initialized SmartDevice instance."""
-    host = "127.0.0.1"
+    host = DISCOVERY_MOCK_IP
 
+    discovery_data = discovery_mock.discovery_data
     ctype, _ = _get_connection_type_device_class(discovery_data)
     config = DeviceConfig(
         host=host,
@@ -90,13 +105,14 @@ async def test_connect_custom_port(discovery_data: dict, mocker, custom_port):
 
 
 async def test_connect_logs_connect_time(
-    discovery_data: dict,
+    discovery_mock,
     caplog: pytest.LogCaptureFixture,
 ):
     """Test that the connect time is logged when debug logging is enabled."""
+    discovery_data = discovery_mock.discovery_data
     ctype, _ = _get_connection_type_device_class(discovery_data)
 
-    host = "127.0.0.1"
+    host = DISCOVERY_MOCK_IP
     config = DeviceConfig(
         host=host, credentials=Credentials("foor", "bar"), connection_type=ctype
     )
@@ -107,9 +123,10 @@ async def test_connect_logs_connect_time(
     assert "seconds to update" in caplog.text
 
 
-async def test_connect_query_fails(discovery_data, mocker):
+async def test_connect_query_fails(discovery_mock, mocker):
     """Make sure that connect fails when query fails."""
-    host = "127.0.0.1"
+    host = DISCOVERY_MOCK_IP
+    discovery_data = discovery_mock.discovery_data
     mocker.patch("kasa.IotProtocol.query", side_effect=KasaException)
     mocker.patch("kasa.SmartProtocol.query", side_effect=KasaException)
 
@@ -125,10 +142,10 @@ async def test_connect_query_fails(discovery_data, mocker):
     assert close_mock.call_count == 1
 
 
-async def test_connect_http_client(discovery_data, mocker):
+async def test_connect_http_client(discovery_mock, mocker):
     """Make sure that discover_single returns an initialized SmartDevice instance."""
-    host = "127.0.0.1"
-
+    host = DISCOVERY_MOCK_IP
+    discovery_data = discovery_mock.discovery_data
     ctype, _ = _get_connection_type_device_class(discovery_data)
 
     http_client = aiohttp.ClientSession()
@@ -157,9 +174,10 @@ async def test_connect_http_client(discovery_data, mocker):
 async def test_device_types(dev: Device):
     await dev.update()
     if isinstance(dev, SmartDevice):
-        device_type = dev._discovery_info["result"]["device_type"]
+        assert dev._discovery_info
+        device_type = cast(str, dev._discovery_info["result"]["device_type"])
         res = SmartDevice._get_device_type_from_components(
-            dev._components.keys(), device_type
+            list(dev._components.keys()), device_type
         )
     else:
         res = _get_device_type_from_sys_info(dev._last_update)