From a078538d3abcff4b0b20e78e410536917cf18108 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Sun, 29 Jan 2023 19:16:06 +0100 Subject: [PATCH 01/10] Cach the descriptors --- miio/device.py | 101 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/miio/device.py b/miio/device.py index 8fc3457e5..e25c879fe 100644 --- a/miio/device.py +++ b/miio/device.py @@ -87,6 +87,8 @@ def __init__( self._model: Optional[str] = model self._info: Optional[DeviceInfo] = None self._actions: Optional[Dict[str, ActionDescriptor]] = None + self._settings: Optional[Dict[str, SettingDescriptor]] = None + self._sensors: Optional[Dict[str, SensorDescriptor]] = None timeout = timeout if timeout is not None else self.timeout self._debug = debug self._protocol = MiIOProtocol( @@ -177,6 +179,63 @@ def _fetch_info(self) -> DeviceInfo: "Unable to request miIO.info from the device" ) from ex + def _setting_descriptors_from_status( + self, status: DeviceStatus + ) -> Dict[str, SettingDescriptor]: + """Get the setting descriptors from a DeviceStatus.""" + settings = status.settings() + for setting in settings.values(): + if setting.setter_name is not None: + setting.setter = getattr(self, setting.setter_name) + if setting.setter is None: + raise Exception( + f"Neither setter or setter_name was defined for {setting}" + ) + setting = cast(EnumSettingDescriptor, setting) + if ( + setting.type == SettingType.Enum + and setting.choices_attribute is not None + ): + retrieve_choices_function = getattr(self, setting.choices_attribute) + setting.choices = retrieve_choices_function() + if setting.type == SettingType.Number: + setting = cast(NumberSettingDescriptor, setting) + if setting.range_attribute is not None: + range_def = getattr(self, setting.range_attribute) + setting.min_value = range_def.min_value + setting.max_value = range_def.max_value + setting.step = range_def.step + + return settings + + def _sensor_descriptors_from_status( + self, status: DeviceStatus + ) -> Dict[str, SensorDescriptor]: + """Get the sensor descriptors from a DeviceStatus.""" + return status.sensors() + + def _action_descriptors(self) -> Dict[str, ActionDescriptor]: + """Get the action descriptors from a DeviceStatus.""" + actions = {} + for action_tuple in getmembers(self, lambda o: hasattr(o, "_action")): + method_name, method = action_tuple + action = method._action + action.method = method # bind the method + actions[method_name] = action + + return actions + + def _initialize_descriptors(self) -> None: + """Cache all the descriptors once on the first call.""" + if self._sensors is not None: + return + + status = self.status() + + self._sensors = self._sensor_descriptors_from_status(status) + self._settings = self._setting_descriptors_from_status(status) + self._actions = self._action_descriptors() + @property def device_id(self) -> int: """Return device id (did), if available.""" @@ -271,49 +330,23 @@ def status(self) -> DeviceStatus: def actions(self) -> Dict[str, ActionDescriptor]: """Return device actions.""" if self._actions is None: - self._actions = {} - for action_tuple in getmembers(self, lambda o: hasattr(o, "_action")): - method_name, method = action_tuple - action = method._action - action.method = method # bind the method - self._actions[method_name] = action + self._initialize_descriptors() return self._actions def settings(self) -> Dict[str, SettingDescriptor]: """Return device settings.""" - settings = self.status().settings() - for setting in settings.values(): - # TODO: Bind setter methods, this should probably done only once during init. - if setting.setter is None: - # TODO: this is ugly, how to fix the issue where setter_name is optional and thus not acceptable for getattr? - if setting.setter_name is None: - raise Exception( - f"Neither setter or setter_name was defined for {setting}" - ) - - setting.setter = getattr(self, setting.setter_name) - if ( - isinstance(setting, EnumSettingDescriptor) - and setting.choices_attribute is not None - ): - retrieve_choices_function = getattr(self, setting.choices_attribute) - setting.choices = retrieve_choices_function() # This can do IO - if setting.type == SettingType.Number: - setting = cast(NumberSettingDescriptor, setting) - if setting.range_attribute is not None: - range_def = getattr(self, setting.range_attribute) - setting.min_value = range_def.min_value - setting.max_value = range_def.max_value - setting.step = range_def.step + if self._settings is None: + self._initialize_descriptors() - return settings + return self._settings def sensors(self) -> Dict[str, SensorDescriptor]: """Return device sensors.""" - # TODO: the latest status should be cached and re-used by all meta information getters - sensors = self.status().sensors() - return sensors + if self._sensors is None: + self._initialize_descriptors() + + return self._sensors def supports_miot(self) -> bool: """Return True if the device supports miot commands. From db92cb1d1d5c109f91e8cd6dc34f1c48a83cfb86 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 13:16:48 +0100 Subject: [PATCH 02/10] fix mypy --- miio/device.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/miio/device.py b/miio/device.py index e25c879fe..36bbfbe06 100644 --- a/miio/device.py +++ b/miio/device.py @@ -331,6 +331,7 @@ def actions(self) -> Dict[str, ActionDescriptor]: """Return device actions.""" if self._actions is None: self._initialize_descriptors() + self._actions = cast(Dict[str, ActionDescriptor], self._actions) return self._actions @@ -338,6 +339,7 @@ def settings(self) -> Dict[str, SettingDescriptor]: """Return device settings.""" if self._settings is None: self._initialize_descriptors() + self._settings = cast(Dict[str, SettingDescriptor], self._settings) return self._settings @@ -345,6 +347,7 @@ def sensors(self) -> Dict[str, SensorDescriptor]: """Return device sensors.""" if self._sensors is None: self._initialize_descriptors() + self._sensors = cast(Dict[str, SensorDescriptor], self._sensors) return self._sensors From 0e90e708dff4b4f6b685fbd5259ce7fc3abf5ec9 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 13:29:25 +0100 Subject: [PATCH 03/10] fix tests --- miio/tests/dummies.py | 3 +++ miio/tests/test_devicestatus.py | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/miio/tests/dummies.py b/miio/tests/dummies.py index 730a9a882..eb99c243e 100644 --- a/miio/tests/dummies.py +++ b/miio/tests/dummies.py @@ -37,6 +37,9 @@ def __init__(self, *args, **kwargs): self.start_state = self.state.copy() self._protocol = DummyMiIOProtocol(self) self._info = None + self._settings = {} + self._sensors = {} + self._actions = {} # TODO: ugly hack to check for pre-existing _model if getattr(self, "_model", None) is None: self._model = "dummy.model" diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index 9368795b2..c838180b3 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -140,6 +140,7 @@ def level(self) -> int: return 1 mocker.patch("miio.Device.send") + mocker.patch("miio.Device.send_handshake") d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") # Patch status to return our class @@ -185,7 +186,9 @@ def level(self) -> int: return 1 mocker.patch("miio.Device.send") + mocker.patch("miio.Device.send_handshake") d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") + d._protocol._device_id = b"12345678" # Patch status to return our class mocker.patch.object(d, "status", return_value=Settings()) @@ -226,7 +229,9 @@ def level(self) -> TestEnum: return TestEnum.First mocker.patch("miio.Device.send") + mocker.patch("miio.Device.send_handshake") d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") + d._protocol._device_id = b"12345678" # Patch status to return our class mocker.patch.object(d, "status", return_value=Settings()) From bcdb56e5a18e817854962149ed733f924a4e56f3 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 16:10:08 +0100 Subject: [PATCH 04/10] Update miio/device.py Co-authored-by: Teemu R. --- miio/device.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/miio/device.py b/miio/device.py index 36bbfbe06..8f2734fb7 100644 --- a/miio/device.py +++ b/miio/device.py @@ -227,8 +227,6 @@ def _action_descriptors(self) -> Dict[str, ActionDescriptor]: def _initialize_descriptors(self) -> None: """Cache all the descriptors once on the first call.""" - if self._sensors is not None: - return status = self.status() From af606ddd06e3a4defc222ab118343d6ec1850c5a Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 20:00:24 +0100 Subject: [PATCH 05/10] Update test_devicestatus.py --- miio/tests/test_devicestatus.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index c838180b3..edd3d177a 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -140,8 +140,8 @@ def level(self) -> int: return 1 mocker.patch("miio.Device.send") - mocker.patch("miio.Device.send_handshake") d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") + d._protocol._device_id = b"12345678" # Patch status to return our class mocker.patch.object(d, "status", return_value=Settings()) @@ -186,7 +186,6 @@ def level(self) -> int: return 1 mocker.patch("miio.Device.send") - mocker.patch("miio.Device.send_handshake") d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") d._protocol._device_id = b"12345678" @@ -229,7 +228,6 @@ def level(self) -> TestEnum: return TestEnum.First mocker.patch("miio.Device.send") - mocker.patch("miio.Device.send_handshake") d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") d._protocol._device_id = b"12345678" From 1640670094c54608772a92817945b96d91ff13f1 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 20:18:40 +0100 Subject: [PATCH 06/10] Add test_cached_descriptors --- miio/tests/test_devicestatus.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index edd3d177a..46d565958 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -250,6 +250,20 @@ def level(self) -> TestEnum: setter.assert_called_with(TestEnum.Second) +@pytest.mark.parametrize("getter_name", ["actions", "settings", "sensors"]) +def test_cached_descriptors(getter_name, mocker): + d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") + getter = getattr(d, getter_name) + initialize_descriptors = mocker.patch("miio.Device._initialize_descriptors", side_effect=Device._initialize_descriptors, autospec=True) + mocker.patch("miio.Device.status") + mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) + mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) + mocker.patch("miio.Device._action_descriptors", return_value={}) + for i in range(5): + getter() + initialize_descriptors.assert_called_once() + + def test_embed(): class MainStatus(DeviceStatus): @property From 49a6ef4d492a86a072ccac8c75bec92ddec31896 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 20:21:10 +0100 Subject: [PATCH 07/10] fix black --- miio/tests/test_devicestatus.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index 46d565958..6e49c17d7 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -254,7 +254,11 @@ def level(self) -> TestEnum: def test_cached_descriptors(getter_name, mocker): d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") getter = getattr(d, getter_name) - initialize_descriptors = mocker.patch("miio.Device._initialize_descriptors", side_effect=Device._initialize_descriptors, autospec=True) + initialize_descriptors = mocker.patch( + "miio.Device._initialize_descriptors", + side_effect=Device._initialize_descriptors, + autospec=True, + ) mocker.patch("miio.Device.status") mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) From 3d4cfecc8ea122965cbcf43f9833fb1613a13d62 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 20:24:10 +0100 Subject: [PATCH 08/10] fix flake8 --- miio/tests/test_devicestatus.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index 6e49c17d7..f473582bf 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -263,7 +263,7 @@ def test_cached_descriptors(getter_name, mocker): mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) mocker.patch("miio.Device._action_descriptors", return_value={}) - for i in range(5): + for _i in range(5): getter() initialize_descriptors.assert_called_once() From 9fe8ca9482ee3de2555506b437f3f7f5a6299d12 Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 20:41:51 +0100 Subject: [PATCH 09/10] Use spy instead --- miio/tests/test_devicestatus.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index f473582bf..5f68ccab5 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -254,11 +254,7 @@ def level(self) -> TestEnum: def test_cached_descriptors(getter_name, mocker): d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") getter = getattr(d, getter_name) - initialize_descriptors = mocker.patch( - "miio.Device._initialize_descriptors", - side_effect=Device._initialize_descriptors, - autospec=True, - ) + initialize_descriptors = mocker.spy(d, "_initialize_descriptors") mocker.patch("miio.Device.status") mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) From 2ad5f53c1cf916686db6ac9a6be71a5f86773acf Mon Sep 17 00:00:00 2001 From: starkillerOG Date: Mon, 30 Jan 2023 20:43:38 +0100 Subject: [PATCH 10/10] Move test_cached_descriptors to test_device.py --- miio/tests/test_device.py | 14 ++++++++++++++ miio/tests/test_devicestatus.py | 14 -------------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/miio/tests/test_device.py b/miio/tests/test_device.py index a693728d1..cf8a99efa 100644 --- a/miio/tests/test_device.py +++ b/miio/tests/test_device.py @@ -182,3 +182,17 @@ def test_supports_miot(mocker): send.side_effect = None assert d.supports_miot() is True + + +@pytest.mark.parametrize("getter_name", ["actions", "settings", "sensors"]) +def test_cached_descriptors(getter_name, mocker): + d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") + getter = getattr(d, getter_name) + initialize_descriptors = mocker.spy(d, "_initialize_descriptors") + mocker.patch("miio.Device.status") + mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) + mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) + mocker.patch("miio.Device._action_descriptors", return_value={}) + for _i in range(5): + getter() + initialize_descriptors.assert_called_once() diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index 5f68ccab5..edd3d177a 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -250,20 +250,6 @@ def level(self) -> TestEnum: setter.assert_called_with(TestEnum.Second) -@pytest.mark.parametrize("getter_name", ["actions", "settings", "sensors"]) -def test_cached_descriptors(getter_name, mocker): - d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") - getter = getattr(d, getter_name) - initialize_descriptors = mocker.spy(d, "_initialize_descriptors") - mocker.patch("miio.Device.status") - mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) - mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) - mocker.patch("miio.Device._action_descriptors", return_value={}) - for _i in range(5): - getter() - initialize_descriptors.assert_called_once() - - def test_embed(): class MainStatus(DeviceStatus): @property