Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various fixes to roborockvacuum #1916

Merged
merged 10 commits into from
Mar 18, 2024
22 changes: 18 additions & 4 deletions miio/descriptorcollection.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -12,6 +13,7 @@
PropertyDescriptor,
RangeDescriptor,
)
from .exceptions import DeviceException

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -42,10 +44,11 @@
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")):
Expand All @@ -55,7 +58,10 @@
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.
Expand Down Expand Up @@ -102,7 +108,11 @@
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)

Check warning on line 115 in miio/descriptorcollection.py

View check run for this annotation

Codecov / codecov/patch

miio/descriptorcollection.py#L115

Added line #L115 was not covered by tests

def _handle_constraints(self, prop: PropertyDescriptor) -> None:
"""Set attribute-based constraints for the descriptor."""
Expand All @@ -112,7 +122,11 @@
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(
Expand Down
3 changes: 3 additions & 0 deletions miio/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@
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

Check warning on line 157 in miio/device.py

View check run for this annotation

Codecov / codecov/patch

miio/device.py#L157

Added line #L157 was not covered by tests

self._descriptors.descriptors_from_object(self)

# Read descriptors from the status class
Expand Down
12 changes: 12 additions & 0 deletions miio/integrations/roborock/vacuum/vacuum.py
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,18 @@
"""
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

Check warning on line 1070 in miio/integrations/roborock/vacuum/vacuum.py

View check run for this annotation

Codecov / codecov/patch

miio/integrations/roborock/vacuum/vacuum.py#L1070

Added line #L1070 was not covered by tests

super()._initialize_descriptors()
res = self.status()
self._descriptors.descriptors_from_object(res)

Check warning on line 1074 in miio/integrations/roborock/vacuum/vacuum.py

View check run for this annotation

Codecov / codecov/patch

miio/integrations/roborock/vacuum/vacuum.py#L1072-L1074

Added lines #L1072 - L1074 were not covered by tests

@classmethod
def get_device_group(cls):
@click.pass_context
Expand Down
12 changes: 12 additions & 0 deletions miio/integrations/roborock/vacuum/vacuumcontainers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion miio/tests/dummies.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from miio import DeviceError
from miio import DescriptorCollection, DeviceError


class DummyMiIOProtocol:
Expand Down Expand Up @@ -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"
Expand Down
27 changes: 23 additions & 4 deletions miio/tests/test_descriptorcollection.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from enum import Enum

import pytest

from miio import (
Expand Down Expand Up @@ -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):
Expand Down
7 changes: 5 additions & 2 deletions miio/tests/test_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
Loading