Improve roborock update handling

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

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"]

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(name, 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

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()

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: