From f6f5d62a13801755201ac0bf829d1a74b000cf1b Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Thu, 26 Jan 2023 14:42:57 +0100 Subject: [PATCH 1/5] Improve roborock update handling Not all devices support all features, but we have currently no way of knowing what is supported. In order to allow embedding all supported information in the status container while avoiding making unnecessary I/O on subsequent queries, this introduces a small helper to do just that. The initial status() call will call all defined devicestatus-returning methods to find out which information is acquired correctly, and skip the unsupported queries in the following update cycles. --- .../roborock/vacuum/updatehelper.py | 41 ++++++++++++ miio/integrations/roborock/vacuum/vacuum.py | 62 +++++++++++-------- 2 files changed, 78 insertions(+), 25 deletions(-) create mode 100644 miio/integrations/roborock/vacuum/updatehelper.py diff --git a/miio/integrations/roborock/vacuum/updatehelper.py b/miio/integrations/roborock/vacuum/updatehelper.py new file mode 100644 index 000000000..ed6a3e50a --- /dev/null +++ b/miio/integrations/roborock/vacuum/updatehelper.py @@ -0,0 +1,41 @@ +import logging +from typing import Callable, Dict + +from miio import DeviceException, DeviceStatus + +_LOGGER = logging.getLogger(__name__) + + +class UpdateHelper: + """Helper class to construct status containers using multiple status methods. + + This is used to perform status fetching on integrations that require calling + multiple methods, some of which may not be supported by the target device. + + This class automatically removes the methods that failed from future updates, + to avoid unnecessary device I/O. + """ + + def __init__(self, main_update_method: Callable): + self._update_methods: Dict[str, Callable] = {} + self._main_update_method = main_update_method + + def add_update_method(self, name: str, update_method: Callable): + """Add status method to be called.""" + _LOGGER.debug(f"Adding {name} to update cycle: {update_method}") + self._update_methods[name] = update_method + + def status(self) -> DeviceStatus: + statuses = self._update_methods.copy() + main_status = self._main_update_method() + for name, method in statuses.items(): + try: + main_status.embed(method()) + _LOGGER.debug(f"Success for {name}") + except DeviceException as ex: + _LOGGER.debug( + "Unable to query %s, removing from next query: %s", name, ex + ) + self._update_methods.pop(name) + + return main_status diff --git a/miio/integrations/roborock/vacuum/vacuum.py b/miio/integrations/roborock/vacuum/vacuum.py index 449a9a4e8..2fa6b9edf 100644 --- a/miio/integrations/roborock/vacuum/vacuum.py +++ b/miio/integrations/roborock/vacuum/vacuum.py @@ -7,7 +7,8 @@ import os import pathlib import time -from typing import List, Optional, Type, Union +from enum import Enum +from typing import Any, List, Optional, Type import click import pytz @@ -21,10 +22,11 @@ command, ) from miio.device import Device, DeviceInfo -from miio.devicestatus import action +from miio.devicestatus import DeviceStatus, action from miio.exceptions import DeviceInfoUnavailableException, UnsupportedFeatureException from miio.interfaces import FanspeedPresets, VacuumInterface +from .updatehelper import UpdateHelper from .vacuum_enums import ( CarpetCleaningMode, Consumable, @@ -143,6 +145,33 @@ def __init__( self.manual_seqnum = -1 self._maps: Optional[MapList] = None self._map_enum_cache = None + self._status_helper = UpdateHelper(self.vacuum_status) + self._status_helper.add_update_method("consumables", self.consumable_status) + self._status_helper.add_update_method("dnd_status", self.dnd_status) + self._status_helper.add_update_method("clean_history", self.clean_history) + self._status_helper.add_update_method("last_clean", self.last_clean_details) + self._status_helper.add_update_method("mop_dryer", self.mop_dryer_settings) + + def send( + self, + command: str, + parameters: Optional[Any] = None, + retry_count: Optional[int] = None, + *, + extra_parameters=None, + ) -> Any: + """Send command to the device. + + This is overridden to raise an exception on unknown methods. + """ + res = super().send( + command, parameters, retry_count, extra_parameters=extra_parameters + ) + if res == "unknown_method": + raise UnsupportedFeatureException( + f"Command {command} is not supported by the device" + ) + return res @command() def start(self): @@ -335,13 +364,9 @@ def manual_control( self.send("app_rc_move", [params]) @command() - def status(self) -> VacuumStatus: + def status(self) -> DeviceStatus: """Return status of the vacuum.""" - status = self.vacuum_status() - status.embed("consumables", self.consumable_status()) - status.embed("cleaning_history", self.clean_history()) - status.embed("dnd", self.dnd_status()) - return status + return self._status_helper.status() @command() def vacuum_status(self) -> VacuumStatus: @@ -382,7 +407,7 @@ def get_maps(self) -> MapList: self._maps = MapList(self.send("get_multi_maps_list")[0]) return self._maps - def _map_enum(self) -> Optional[enum.Enum]: + def _map_enum(self) -> Optional[Type[Enum]]: """Enum of the available map names.""" if self._map_enum_cache is not None: return self._map_enum_cache @@ -508,9 +533,7 @@ def last_clean_details(self) -> Optional[CleaningDetails]: @command( click.argument("id_", type=int, metavar="ID"), ) - def clean_details( - self, id_: int - ) -> Union[List[CleaningDetails], Optional[CleaningDetails]]: + def clean_details(self, id_: int) -> Optional[CleaningDetails]: """Return details about specific cleaning.""" details = self.send("get_clean_record", [id_]) @@ -583,7 +606,7 @@ def update_timer(self, timer_id: str, mode: TimerState): return self.send("upd_timer", [timer_id, mode.value]) @command() - def dnd_status(self): + def dnd_status(self) -> DNDStatus: """Returns do-not-disturb status.""" # {'result': [{'enabled': 1, 'start_minute': 0, 'end_minute': 0, # 'start_hour': 22, 'end_hour': 8}], 'id': 1} @@ -760,7 +783,7 @@ def configure_wifi(self, ssid, password, uid=0, timezone=None): return super().configure_wifi(ssid, password, uid, extra_params) @command() - def carpet_mode(self): + def carpet_mode(self) -> CarpetModeStatus: """Get carpet mode settings.""" return CarpetModeStatus(self.send("get_carpet_mode")[0]) @@ -975,28 +998,19 @@ def set_child_lock(self, lock: bool) -> bool: """Set child lock setting.""" return self.send("set_child_lock_status", {"lock_status": int(lock)})[0] == "ok" - def _verify_mop_dryer_supported(self) -> None: - """Checks if model supports mop dryer add-on.""" - # dryer add-on is only supported by following models - if self.model not in [ROCKROBO_S7, ROCKROBO_S7_MAXV]: - raise UnsupportedFeatureException("Dryer not supported by %s", self.model) - @command() def mop_dryer_settings(self) -> MopDryerSettings: """Get mop dryer settings.""" - self._verify_mop_dryer_supported() return MopDryerSettings(self.send("app_get_dryer_setting")) @command(click.argument("enabled", type=bool)) def set_mop_dryer_enabled(self, enabled: bool) -> bool: """Set mop dryer add-on enabled.""" - self._verify_mop_dryer_supported() return self.send("app_set_dryer_setting", {"status": int(enabled)})[0] == "ok" @command(click.argument("dry_time", type=int)) def set_mop_dryer_dry_time(self, dry_time_seconds: int) -> bool: """Set mop dryer add-on dry time.""" - self._verify_mop_dryer_supported() return ( self.send("app_set_dryer_setting", {"on": {"dry_time": dry_time_seconds}})[ 0 @@ -1008,14 +1022,12 @@ def set_mop_dryer_dry_time(self, dry_time_seconds: int) -> bool: @action(name="Start mop drying", icon="mdi:tumble-dryer") def start_mop_drying(self) -> bool: """Start mop drying.""" - self._verify_mop_dryer_supported() return self.send("app_set_dryer_status", {"status": 1})[0] == "ok" @command() @action(name="Stop mop drying", icon="mdi:tumble-dryer") def stop_mop_drying(self) -> bool: """Stop mop drying.""" - self._verify_mop_dryer_supported() return self.send("app_set_dryer_status", {"status": 0})[0] == "ok" @command() From 58450f1349c6f2e6bbccb50438c3dce664610b6a Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Fri, 3 Feb 2023 17:58:58 +0100 Subject: [PATCH 2/5] Raise DeviceError on unsupported methods for tests --- miio/tests/dummies.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/miio/tests/dummies.py b/miio/tests/dummies.py index eb99c243e..9278657fc 100644 --- a/miio/tests/dummies.py +++ b/miio/tests/dummies.py @@ -1,3 +1,6 @@ +from miio import DeviceError + + class DummyMiIOProtocol: """DummyProtocol allows you mock MiIOProtocol.""" @@ -8,7 +11,10 @@ def __init__(self, dummy_device): def send(self, command: str, parameters=None, retry_count=3, extra_parameters=None): """Overridden send() to return values from `self.return_values`.""" - return self.dummy_device.return_values[command](parameters) + try: + return self.dummy_device.return_values[command](parameters) + except KeyError: + raise DeviceError({"code": -32601, "message": "Method not found."}) class DummyDevice: From d70dfeff0517665fe49295d02ad029d55c5a987b Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Fri, 3 Feb 2023 18:00:00 +0100 Subject: [PATCH 3/5] Fix tests --- .../roborock/vacuum/tests/test_vacuum.py | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/miio/integrations/roborock/vacuum/tests/test_vacuum.py b/miio/integrations/roborock/vacuum/tests/test_vacuum.py index 9f5e2f392..bb057e004 100644 --- a/miio/integrations/roborock/vacuum/tests/test_vacuum.py +++ b/miio/integrations/roborock/vacuum/tests/test_vacuum.py @@ -4,9 +4,10 @@ import pytest -from miio import RoborockVacuum, UnsupportedFeatureException -from miio.tests.dummies import DummyDevice +from miio import DeviceError, RoborockVacuum, UnsupportedFeatureException +from miio.tests.dummies import DummyDevice, DummyMiIOProtocol +from ..updatehelper import UpdateHelper from ..vacuum import ( ROCKROBO_Q7_MAX, ROCKROBO_S7, @@ -18,6 +19,20 @@ from ..vacuumcontainers import VacuumStatus +class DummyRoborockProtocol(DummyMiIOProtocol): + """Roborock-specific dummy protocol handler. + + The vacuum reports 'unknown_method' instead of device error for unknown commands. + """ + + def send(self, command: str, parameters=None, retry_count=3, extra_parameters=None): + """Overridden send() to return values from `self.return_values`.""" + try: + return super().send(command, parameters, retry_count, extra_parameters) + except DeviceError: + return "unknown_method" + + class DummyVacuum(DummyDevice, RoborockVacuum): STATE_CHARGING = 8 STATE_CLEANING = 5 @@ -48,7 +63,7 @@ def __init__(self, *args, **kwargs): } self._maps = None self._map_enum_cache = None - + self._status_helper = UpdateHelper(self.vacuum_status) self.dummies = { "consumables": [ { @@ -138,6 +153,7 @@ def __init__(self, *args, **kwargs): } super().__init__(args, kwargs) + self._protocol = DummyRoborockProtocol(self) def set_water_box_custom_mode_callback(self, parameters): assert parameters == self.dummies["water_box_custom_mode"] From 72fe1adbd3e351f10c391f4d748b986c63b4ffc8 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Fri, 3 Feb 2023 18:11:32 +0100 Subject: [PATCH 4/5] Add tests to updatehelper --- .../vacuum/tests/test_updatehelper.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 miio/integrations/roborock/vacuum/tests/test_updatehelper.py diff --git a/miio/integrations/roborock/vacuum/tests/test_updatehelper.py b/miio/integrations/roborock/vacuum/tests/test_updatehelper.py new file mode 100644 index 000000000..2c1a33167 --- /dev/null +++ b/miio/integrations/roborock/vacuum/tests/test_updatehelper.py @@ -0,0 +1,28 @@ +from unittest.mock import MagicMock + +from miio import DeviceException + +from ..updatehelper import UpdateHelper + + +def test_updatehelper(): + """Test that update helper removes erroring methods from future updates.""" + main_status = MagicMock() + second_status = MagicMock() + unsupported = MagicMock(side_effect=DeviceException("Broken")) + helper = UpdateHelper(main_status) + helper.add_update_method("working", second_status) + helper.add_update_method("broken", unsupported) + + helper.status() + + main_status.assert_called_once() + second_status.assert_called_once() + unsupported.assert_called_once() + + # perform second update + helper.status() + + assert main_status.call_count == 2 + assert second_status.call_count == 2 + assert unsupported.call_count == 1 From 114f3e21150b083e989ed82bacc476020a036082 Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Fri, 3 Feb 2023 18:32:53 +0100 Subject: [PATCH 5/5] Name sub status containers --- miio/integrations/roborock/vacuum/updatehelper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miio/integrations/roborock/vacuum/updatehelper.py b/miio/integrations/roborock/vacuum/updatehelper.py index ed6a3e50a..e2737fb21 100644 --- a/miio/integrations/roborock/vacuum/updatehelper.py +++ b/miio/integrations/roborock/vacuum/updatehelper.py @@ -30,7 +30,7 @@ def status(self) -> DeviceStatus: main_status = self._main_update_method() for name, method in statuses.items(): try: - main_status.embed(method()) + main_status.embed(name, method()) _LOGGER.debug(f"Success for {name}") except DeviceException as ex: _LOGGER.debug(