Various fixes to roborockvacuum (#1916)

* Include descriptors from `status()` response. This performs I/O during
the initialization to find out which information is available.
Alternative would be changing `updatehelper.py` to collect the
descriptors from all embedded updates, unsure what's the best approach.
* Fix exposing vacuum state.
* Expose fan speed presets.

Author: rytilahti

miio/descriptorcollection.py

@@ -1,5 +1,6 @@
 import logging
 from collections import UserDict
+from enum import Enum
 from inspect import getmembers
 from typing import TYPE_CHECKING, Generic, TypeVar, cast
 
@@ -12,6 +13,7 @@
     PropertyDescriptor,
     RangeDescriptor,
 )
+from .exceptions import DeviceException
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -42,10 +44,11 @@ def descriptors_from_object(self, obj):
         2. Going through all members and looking if they have a '_descriptor' attribute set by a decorator
         """
         _LOGGER.debug("Adding descriptors from %s", obj)
+        descriptors_to_add = []
         # 1. Check for existence of _descriptors as DeviceStatus' metaclass collects them already
         if descriptors := getattr(obj, "_descriptors"):  # noqa: B009
             for _name, desc in descriptors.items():
-                self.add_descriptor(desc)
+                descriptors_to_add.append(desc)
 
         # 2. Check if object members have descriptors
         for _name, method in getmembers(obj, lambda o: hasattr(o, "_descriptor")):
@@ -55,7 +58,10 @@ def descriptors_from_object(self, obj):
                 continue
 
             prop_desc.method = method
-            self.add_descriptor(prop_desc)
+            descriptors_to_add.append(prop_desc)
+
+        for desc in descriptors_to_add:
+            self.add_descriptor(desc)
 
     def add_descriptor(self, descriptor: Descriptor):
         """Add a descriptor to the collection.
@@ -102,7 +108,11 @@ def _handle_property_descriptor(self, prop: PropertyDescriptor) -> None:
         if prop.access & AccessFlags.Write and prop.setter is None:
             raise ValueError(f"Neither setter or setter_name was defined for {prop}")
 
-        self._handle_constraints(prop)
+        # TODO: temporary hack as this should not cause I/O nor fail
+        try:
+            self._handle_constraints(prop)
+        except DeviceException as ex:
+            _LOGGER.error("Adding constraints failed: %s", ex)
 
     def _handle_constraints(self, prop: PropertyDescriptor) -> None:
         """Set attribute-based constraints for the descriptor."""
@@ -112,7 +122,11 @@ def _handle_constraints(self, prop: PropertyDescriptor) -> None:
                 retrieve_choices_function = getattr(
                     self._device, prop.choices_attribute
                 )
-                prop.choices = retrieve_choices_function()
+                choices = retrieve_choices_function()
+                if isinstance(choices, dict):
+                    prop.choices = Enum(f"GENERATED_ENUM_{prop.name}", choices)
+                else:
+                    prop.choices = choices
 
             if prop.choices is None:
                 raise ValueError(

miio/device.py

@@ -153,6 +153,9 @@ def _initialize_descriptors(self) -> None:
         This can be overridden to add additional descriptors to the device.
         If you do so, do not forget to call this method.
         """
+        if self._initialized:
+            return
+
         self._descriptors.descriptors_from_object(self)
 
         # Read descriptors from the status class

miio/integrations/roborock/vacuum/vacuum.py

@@ -1061,6 +1061,18 @@ def firmware_features(self) -> List[int]:
         """
         return self.send("get_fw_features")
 
+    def _initialize_descriptors(self) -> None:
+        """Initialize device descriptors.
+
+        Overridden to collect descriptors also from the update helper.
+        """
+        if self._initialized:
+            return
+
+        super()._initialize_descriptors()
+        res = self.status()
+        self._descriptors.descriptors_from_object(res)
+
     @classmethod
     def get_device_group(cls):
         @click.pass_context

miio/integrations/roborock/vacuum/vacuumcontainers.py

@@ -192,6 +192,7 @@ def state(self) -> str:
             self.state_code, f"Unknown state (code: {self.state_code})"
         )
 
+    @property
     @sensor("Vacuum state", id=VacuumId.State)
     def vacuum_state(self) -> VacuumState:
         """Return vacuum state."""
@@ -276,6 +277,17 @@ def fanspeed(self) -> Optional[int]:
             return None
         return fan_power
 
+    @property
+    @setting(
+        "Fanspeed preset",
+        choices_attribute="fan_speed_presets",
+        setter_name="set_fan_speed_preset",
+        icon="mdi:fan",
+        id=VacuumId.FanSpeedPreset,
+    )
+    def fan_speed_preset(self):
+        return self.data["fan_power"]
+
     @property
     @setting(
         "Mop scrub intensity",

miio/tests/dummies.py

@@ -1,4 +1,4 @@
-from miio import DeviceError
+from miio import DescriptorCollection, DeviceError
 
 
 class DummyMiIOProtocol:
@@ -46,6 +46,8 @@ def __init__(self, *args, **kwargs):
         self._settings = {}
         self._sensors = {}
         self._actions = {}
+        self._initialized = False
+        self._descriptors = DescriptorCollection(device=self)
         # TODO: ugly hack to check for pre-existing _model
         if getattr(self, "_model", None) is None:
             self._model = "dummy.model"

miio/tests/test_descriptorcollection.py

@@ -1,3 +1,5 @@
+from enum import Enum
+
 import pytest
 
 from miio import (
@@ -119,22 +121,39 @@ def test_handle_enum_constraints(dummy_device, mocker):
         "status_attribute": "attr",
     }
 
-    mocker.patch.object(dummy_device, "choices_attr", create=True)
-
     # Check that error is raised if choices are missing
     invalid = EnumDescriptor(id="missing", **data)
     with pytest.raises(
         ValueError, match="Neither choices nor choices_attribute was defined"
     ):
         coll.add_descriptor(invalid)
 
-    # Check that binding works
+    # Check that enum binding works
+    mocker.patch.object(
+        dummy_device,
+        "choices_attr",
+        create=True,
+        return_value=Enum("test enum", {"foo": 1}),
+    )
     choices_attribute = EnumDescriptor(
         id="with_choices_attr", choices_attribute="choices_attr", **data
     )
     coll.add_descriptor(choices_attribute)
     assert len(coll) == 1
-    assert coll["with_choices_attr"].choices is not None
+
+    assert issubclass(coll["with_choices_attr"].choices, Enum)
+
+    # Check that dict binding works
+    mocker.patch.object(
+        dummy_device, "choices_attr_dict", create=True, return_value={"test": "dict"}
+    )
+    choices_attribute_dict = EnumDescriptor(
+        id="with_choices_attr_dict", choices_attribute="choices_attr_dict", **data
+    )
+    coll.add_descriptor(choices_attribute_dict)
+    assert len(coll) == 2
+
+    assert issubclass(coll["with_choices_attr_dict"].choices, Enum)
 
 
 def test_handle_range_constraints(dummy_device, mocker):

miio/tests/test_device.py

@@ -143,9 +143,10 @@ def test_init_signature(cls, mocker):
     """Make sure that __init__ of every device-inheriting class accepts the expected
     parameters."""
     mocker.patch("miio.Device.send")
+    mocker.patch("miio.Device.send_handshake")
     parent_init = mocker.spy(Device, "__init__")
     kwargs = {
-        "ip": "IP",
+        "ip": "127.123.123.123",
         "token": None,
         "start_id": 0,
         "debug": False,
@@ -181,7 +182,9 @@ def test_supports_miot(mocker):
     assert d.supports_miot() is True
 
 
-@pytest.mark.parametrize("getter_name", ["actions", "settings", "sensors"])
+@pytest.mark.parametrize(
+    "getter_name", ["actions", "settings", "sensors", "descriptors"]
+)
 def test_cached_descriptors(getter_name, mocker, caplog):
     d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff")
     getter = getattr(d, getter_name)