Improve roborock update handling (#1685)

Not all devices support all features, but we have currently no way of
knowing what is supported. In order to allow the embedding of 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.

This also embeds some more information (last clean details, mop dryer
settings).

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: