From d01acfaba73cb5b88ed5c65724d4bdf3183e571a Mon Sep 17 00:00:00 2001 From: Teemu Rytilahti Date: Sun, 5 Mar 2023 01:12:54 +0100 Subject: [PATCH] Generalize settings and sensors into properties This removes the unnecessary division between sensors and settings in favor of just having "properties" that can have different access flags. This will greatly simplify the API as all properties are alike, the difference is just in the access flags. The earlier API to get settable properties (settings) and readable properties (sensors) is kept intact for the time being. * SettingDescriptor is no more, all readable and writable properties are described using PropertyDescriptors. * SettingType is replaced with PropertyConstraint to allow signaling allowed ranges or choices. * EnumSettingDescriptor is now EnumDescriptor * NumberSettingDescriptor is now RangeDescriptor * Add 'access' to Descriptor * This will also allow for generic `descriptors` interface in the future, if needed. * Add 'property' to Descriptor * None for actions, allwos more generic interface for properties. * Add 'properties' method to 'Device' to get all property descriptors. 'settings' and 'sensors' return a filtered dict based on the properties for backwards compat. --- miio/descriptors.py | 96 +++++++------- miio/device.py | 125 +++++++++--------- miio/devicestatus.py | 111 +++++++--------- miio/integrations/genericmiot/cli_helpers.py | 4 +- miio/integrations/genericmiot/genericmiot.py | 55 ++++---- .../viomidishwasher/test_viomidishwasher.py | 3 +- miio/integrations/yeelight/light/yeelight.py | 20 ++- miio/miot_models.py | 114 ++++++++-------- miio/tests/test_device.py | 4 +- miio/tests/test_devicestatus.py | 36 ++--- miio/tests/test_miot_models.py | 34 ++--- 11 files changed, 293 insertions(+), 309 deletions(-) diff --git a/miio/descriptors.py b/miio/descriptors.py index 751a5257f..df5c5733b 100644 --- a/miio/descriptors.py +++ b/miio/descriptors.py @@ -2,15 +2,13 @@ The descriptors contain information that can be used to provide generic, dynamic user-interfaces. -If you are a downstream developer, use :func:`~miio.device.Device.sensors()`, -:func:`~miio.device.Device.settings()`, and +If you are a downstream developer, use :func:`~miio.device.Device.properties()`, :func:`~miio.device.Device.actions()` to access the functionality exposed by the integration developer. If you are developing an integration, prefer :func:`~miio.devicestatus.sensor`, :func:`~miio.devicestatus.setting`, and :func:`~miio.devicestatus.action` decorators over creating the descriptors manually. -If needed, you can override the methods listed to add more descriptors to your integration. """ -from enum import Enum, auto +from enum import Enum, Flag, auto from typing import Any, Callable, Dict, List, Optional, Type import attr @@ -18,13 +16,30 @@ @attr.s(auto_attribs=True) class ValidSettingRange: - """Describes a valid input range for a setting.""" + """Describes a valid input range for a property.""" min_value: int max_value: int step: int = 1 +class AccessFlags(Flag): + """Defines the access rights for the property behind the descriptor.""" + + Read = auto() + Write = auto() + Execute = auto() + + def __str__(self): + """Return pretty printable string representation.""" + s = "" + s += "r" if self & AccessFlags.Read else "-" + s += "w" if self & AccessFlags.Write else "-" + s += "x" if self & AccessFlags.Execute else "-" + s += "" + return s + + @attr.s(auto_attribs=True) class Descriptor: """Base class for all descriptors.""" @@ -32,7 +47,9 @@ class Descriptor: id: str name: str type: Optional[type] = None + property: Optional[str] = None extras: Dict = attr.ib(factory=dict, repr=False) + access: AccessFlags = attr.ib(default=AccessFlags.Read | AccessFlags.Write) @attr.s(auto_attribs=True) @@ -43,68 +60,55 @@ class ActionDescriptor(Descriptor): method: Optional[Callable] = attr.ib(default=None, repr=False) inputs: Optional[List[Any]] = attr.ib(default=None, repr=True) + access: AccessFlags = attr.ib(default=AccessFlags.Execute) -@attr.s(auto_attribs=True, kw_only=True) -class SensorDescriptor(Descriptor): - """Describes a sensor exposed by the device. - - This information can be used by library users to programatically - access information what types of data is available to display to users. - Prefer :meth:`@sensor ` for constructing these. - """ +class PropertyConstraint(Enum): + """Defines constraints for integer based properties.""" - property: str - unit: Optional[str] = None + Unset = auto() + Range = auto() + Choice = auto() -class SettingType(Enum): - Undefined = auto() - Number = auto() - Boolean = auto() - Enum = auto() +@attr.s(auto_attribs=True, kw_only=True) +class PropertyDescriptor(Descriptor): + """Describes a property exposed by the device. + This information can be used by library users to programmatically + access information what types of data is available to display to users. -@attr.s(auto_attribs=True, kw_only=True) -class SettingDescriptor(Descriptor): - """Presents a settable value.""" + Prefer :meth:`@sensor ` or + :meth:`@setting `for constructing these. + """ + #: The name of the property to use to access the value from a status container. property: str + #: Sensors are read-only and settings are (usually) read-write. + access: AccessFlags = attr.ib(default=AccessFlags.Read) unit: Optional[str] = None - setting_type = SettingType.Undefined + + #: Constraint type defining the allowed values for an integer property. + constraint: PropertyConstraint = attr.ib(default=PropertyConstraint.Unset) + #: Callable to set the value of the property. setter: Optional[Callable] = attr.ib(default=None, repr=False) + #: Name of the method in the device class that can be used to set the value. + #: This will be used to bind the setter callable. setter_name: Optional[str] = attr.ib(default=None, repr=False) - def cast_value(self, value: int): - """Casts value to the expected type.""" - cast_map = { - SettingType.Boolean: bool, - SettingType.Enum: int, - SettingType.Number: int, - } - return cast_map[self.setting_type](int(value)) - - -@attr.s(auto_attribs=True, kw_only=True) -class BooleanSettingDescriptor(SettingDescriptor): - """Presents a settable boolean value.""" - - type: type = bool - setting_type: SettingType = SettingType.Boolean - @attr.s(auto_attribs=True, kw_only=True) -class EnumSettingDescriptor(SettingDescriptor): +class EnumDescriptor(PropertyDescriptor): """Presents a settable, enum-based value.""" - setting_type: SettingType = SettingType.Enum + constraint: PropertyConstraint = PropertyConstraint.Choice choices_attribute: Optional[str] = attr.ib(default=None, repr=False) choices: Optional[Type[Enum]] = attr.ib(default=None, repr=False) @attr.s(auto_attribs=True, kw_only=True) -class NumberSettingDescriptor(SettingDescriptor): - """Presents a settable, numerical value. +class RangeDescriptor(PropertyDescriptor): + """Presents a settable, numerical value constrained by min, max, and step. If `range_attribute` is set, the named property that should return :class:ValidSettingRange will be used to obtain {min,max}_value and step. @@ -115,4 +119,4 @@ class NumberSettingDescriptor(SettingDescriptor): step: int range_attribute: Optional[str] = attr.ib(default=None) type: type = int - setting_type: SettingType = SettingType.Number + constraint: PropertyConstraint = PropertyConstraint.Range diff --git a/miio/device.py b/miio/device.py index 020967a2d..5a53e4167 100644 --- a/miio/device.py +++ b/miio/device.py @@ -1,18 +1,18 @@ import logging from enum import Enum from inspect import getmembers -from typing import Any, Dict, List, Optional, Union, cast # noqa: F401 +from typing import Any, Dict, List, Optional, Union, cast, final # noqa: F401 import click from .click_common import DeviceGroupMeta, LiteralParamType, command, format_output from .descriptors import ( + AccessFlags, ActionDescriptor, - EnumSettingDescriptor, - NumberSettingDescriptor, - SensorDescriptor, - SettingDescriptor, - SettingType, + EnumDescriptor, + PropertyConstraint, + PropertyDescriptor, + RangeDescriptor, ) from .deviceinfo import DeviceInfo from .devicestatus import DeviceStatus @@ -88,8 +88,7 @@ def __init__( self._model: Optional[str] = model self._info: Optional[DeviceInfo] = None self._actions: Optional[Dict[str, ActionDescriptor]] = None - self._settings: Optional[Dict[str, SettingDescriptor]] = None - self._sensors: Optional[Dict[str, SensorDescriptor]] = None + self._properties: Optional[Dict[str, PropertyDescriptor]] = None timeout = timeout if timeout is not None else self.timeout self._debug = debug self._protocol = MiIOProtocol( @@ -180,56 +179,44 @@ def _fetch_info(self) -> DeviceInfo: "Unable to request miIO.info from the device" ) from ex - def _setting_descriptors_from_status( + def _set_constraints_from_attributes( self, status: DeviceStatus - ) -> Dict[str, SettingDescriptor]: + ) -> Dict[str, PropertyDescriptor]: """Get the setting descriptors from a DeviceStatus.""" - settings = status.settings() + properties = status.properties() unsupported_settings = [] - for key, setting in settings.items(): - if setting.setter_name is not None: - setting.setter = getattr(self, setting.setter_name) - if setting.setter is None: - raise Exception( - f"Neither setter or setter_name was defined for {setting}" - ) - - if setting.setting_type == SettingType.Enum: - setting = cast(EnumSettingDescriptor, setting) - if setting.choices_attribute is not None: - retrieve_choices_function = getattr(self, setting.choices_attribute) + for key, prop in properties.items(): + if prop.setter_name is not None: + prop.setter = getattr(self, prop.setter_name) + if prop.setter is None: + raise Exception(f"Neither setter or setter_name was defined for {prop}") + + if prop.constraint == PropertyConstraint.Choice: + prop = cast(EnumDescriptor, prop) + if prop.choices_attribute is not None: + retrieve_choices_function = getattr(self, prop.choices_attribute) try: - setting.choices = retrieve_choices_function() + prop.choices = retrieve_choices_function() except UnsupportedFeatureException: + # TODO: this should not be done here unsupported_settings.append(key) continue - elif setting.setting_type == SettingType.Number: - setting = cast(NumberSettingDescriptor, setting) - if setting.range_attribute is not None: - range_def = getattr(self, setting.range_attribute) - setting.min_value = range_def.min_value - setting.max_value = range_def.max_value - setting.step = range_def.step - - elif setting.setting_type == SettingType.Boolean: - pass # just to exhaust known types + elif prop.constraint == PropertyConstraint.Range: + prop = cast(RangeDescriptor, prop) + if prop.range_attribute is not None: + range_def = getattr(self, prop.range_attribute) + prop.min_value = range_def.min_value + prop.max_value = range_def.max_value + prop.step = range_def.step else: - raise NotImplementedError( - "Unknown setting type: %s" % setting.setting_type - ) + _LOGGER.debug("Got a regular setting without constraints: %s", prop) for unsupp_key in unsupported_settings: - settings.pop(unsupp_key) - - return settings + properties.pop(unsupp_key) - def _sensor_descriptors_from_status( - self, status: DeviceStatus - ) -> Dict[str, SensorDescriptor]: - """Get the sensor descriptors from a DeviceStatus.""" - return status.sensors() + return properties def _action_descriptors(self) -> Dict[str, ActionDescriptor]: """Get the action descriptors from a DeviceStatus.""" @@ -238,22 +225,20 @@ def _action_descriptors(self) -> Dict[str, ActionDescriptor]: method_name, method = action_tuple action = method._action action.method = method # bind the method - actions[method_name] = action + actions[action.id] = action return actions def _initialize_descriptors(self) -> None: """Cache all the descriptors once on the first call.""" - status = self.status() - self._sensors = self._sensor_descriptors_from_status(status) - self._settings = self._setting_descriptors_from_status(status) + self._properties = self._set_constraints_from_attributes(status) self._actions = self._action_descriptors() @property def device_id(self) -> int: - """Return device id (did), if available.""" + """Return the device id (did).""" if not self._protocol._device_id: self.send_handshake() return int.from_bytes(self._protocol._device_id, byteorder="big") @@ -346,25 +331,41 @@ def actions(self) -> Dict[str, ActionDescriptor]: """Return device actions.""" if self._actions is None: self._initialize_descriptors() - self._actions = cast(Dict[str, ActionDescriptor], self._actions) - return self._actions + # TODO: we ignore the return value for now as these should always be initialized + return self._actions # type: ignore[return-value] + + def properties(self) -> Dict[str, PropertyDescriptor]: + """Return all device properties.""" + if self._properties is None: + self._initialize_descriptors() + + # TODO: we ignore the return value for now as these should always be initialized + return self._properties # type: ignore[return-value] - def settings(self) -> Dict[str, SettingDescriptor]: - """Return device settings.""" - if self._settings is None: + @final + def settings(self) -> Dict[str, PropertyDescriptor]: + """Return settable properties.""" + if self._properties is None: self._initialize_descriptors() - self._settings = cast(Dict[str, SettingDescriptor], self._settings) - return self._settings + return { + prop.id: prop + for prop in self.properties().values() + if prop.access & AccessFlags.Write + } - def sensors(self) -> Dict[str, SensorDescriptor]: - """Return device sensors.""" - if self._sensors is None: + @final + def sensors(self) -> Dict[str, PropertyDescriptor]: + """Return read-only properties.""" + if self._properties is None: self._initialize_descriptors() - self._sensors = cast(Dict[str, SensorDescriptor], self._sensors) - return self._sensors + return { + prop.id: prop + for prop in self.properties().values() + if prop.access ^ AccessFlags.Write + } def supports_miot(self) -> bool: """Return True if the device supports miot commands. diff --git a/miio/devicestatus.py b/miio/devicestatus.py index c4218627e..ca7cbe79f 100644 --- a/miio/devicestatus.py +++ b/miio/devicestatus.py @@ -17,13 +17,11 @@ import attr from .descriptors import ( + AccessFlags, ActionDescriptor, - BooleanSettingDescriptor, - EnumSettingDescriptor, - NumberSettingDescriptor, - SensorDescriptor, - SettingDescriptor, - SettingType, + EnumDescriptor, + PropertyDescriptor, + RangeDescriptor, ) from .identifiers import StandardIdentifier @@ -36,25 +34,20 @@ class _StatusMeta(type): def __new__(metacls, name, bases, namespace, **kwargs): cls = super().__new__(metacls, name, bases, namespace) - # TODO: clean up to contain all of these in a single container - cls._sensors: Dict[str, SensorDescriptor] = {} - cls._settings: Dict[str, SettingDescriptor] = {} - + cls._properties: Dict[str, PropertyDescriptor] = {} cls._parent: Optional["DeviceStatus"] = None cls._embedded: Dict[str, "DeviceStatus"] = {} - descriptor_map = { - "sensor": cls._sensors, - "setting": cls._settings, - } for n in namespace: prop = getattr(namespace[n], "fget", None) if prop: - for type_, container in descriptor_map.items(): - item = getattr(prop, f"_{type_}", None) - if item: - _LOGGER.debug(f"Found {type_} for {name} {item}") - container[n] = item + descriptor = getattr(prop, "_descriptor", None) + if descriptor: + _LOGGER.debug(f"Found descriptor for {name} {descriptor}") + if n in cls._properties: + raise ValueError(f"Duplicate {n} for {name} {descriptor}") + cls._properties[n] = descriptor + _LOGGER.debug("Created %s.%s: %s", name, n, descriptor) return cls @@ -93,19 +86,24 @@ def __repr__(self): s += ">" return s - def sensors(self) -> Dict[str, SensorDescriptor]: + def properties(self) -> Dict[str, PropertyDescriptor]: """Return the dict of sensors exposed by the status container. - You can use @sensor decorator to define sensors inside your status class. + Use @sensor and @setting decorators to define properties. """ - return self._sensors # type: ignore[attr-defined] + return self._properties # type: ignore[attr-defined] - def settings(self) -> Dict[str, SettingDescriptor]: + def settings(self) -> Dict[str, PropertyDescriptor]: """Return the dict of settings exposed by the status container. - You can use @setting decorator to define settings inside your status class. + This is just a dict of writable properties, see :meth:`properties`. """ - return self._settings # type: ignore[attr-defined] + # TODO: this is not probably worth having, remove? + return { + prop.id: prop + for prop in self.properties().values() + if prop.access & AccessFlags.Write + } def embed(self, name: str, other: "DeviceStatus"): """Embed another status container to current one. @@ -119,45 +117,33 @@ def embed(self, name: str, other: "DeviceStatus"): self._embedded[name] = other other._parent = self # type: ignore[attr-defined] - for sensor_name, sensor in other.sensors().items(): - final_name = f"{name}__{sensor_name}" + for property_name, prop in other.properties().items(): + final_name = f"{name}__{property_name}" - self._sensors[final_name] = attr.evolve(sensor, property=final_name) - - for setting_name, setting in other.settings().items(): - final_name = f"{name}__{setting_name}" - self._settings[final_name] = attr.evolve(setting, property=final_name) + self._properties[final_name] = attr.evolve(prop, property=final_name) def __dir__(self) -> Iterable[str]: """Overridden to include properties from embedded containers.""" - return ( - list(super().__dir__()) - + list(self._embedded) - + list(self._sensors) - + list(self._settings) - ) + return list(super().__dir__()) + list(self._embedded) + list(self._properties) @property def __cli_output__(self) -> str: """Return a CLI formatted output of the status.""" out = "" - for entry in list(self.sensors().values()) + list(self.settings().values()): + for descriptor in self.properties().values(): try: - value = getattr(self, entry.property) + value = getattr(self, descriptor.property) except KeyError: continue # skip missing properties if value is None: # skip none values - _LOGGER.debug("Skipping %s because it's None", entry.name) + _LOGGER.debug("Skipping %s because it's None", descriptor.name) continue - if isinstance(entry, SettingDescriptor): - out += "[RW] " - - out += f"{entry.name} ({entry.id}): {value}" + out += f"{descriptor.access} {descriptor.name} ({descriptor.id}): {value}" - if entry.unit is not None: - out += f" {entry.unit}" + if descriptor.unit is not None: + out += f" {descriptor.unit}" out += "\n" @@ -188,6 +174,15 @@ def _get_qualified_name(func, id_: Optional[Union[str, StandardIdentifier]]): return id_ or str(func.__qualname__) +def _sensor_type_for_return_type(func): + """Return the return type for a method from its type hint.""" + rtype = get_type_hints(func).get("return") + if get_origin(rtype) is Union: # Unwrap Optional[] + rtype, _ = get_args(rtype) + + return rtype + + def sensor( name: str, *, @@ -209,15 +204,8 @@ def decorator_sensor(func): property_name = str(func.__name__) qualified_name = _get_qualified_name(func, id) - def _sensor_type_for_return_type(func): - rtype = get_type_hints(func).get("return") - if get_origin(rtype) is Union: # Unwrap Optional[] - rtype, _ = get_args(rtype) - - return rtype - sensor_type = _sensor_type_for_return_type(func) - descriptor = SensorDescriptor( + descriptor = PropertyDescriptor( id=qualified_name, property=property_name, name=name, @@ -225,7 +213,7 @@ def _sensor_type_for_return_type(func): type=sensor_type, extras=kwargs, ) - func._sensor = descriptor + func._descriptor = descriptor return func @@ -245,7 +233,6 @@ def setting( range_attribute: Optional[str] = None, choices: Optional[Type[Enum]] = None, choices_attribute: Optional[str] = None, - type: Optional[SettingType] = None, **kwargs, ): """Syntactic sugar to create SettingDescriptor objects. @@ -279,10 +266,12 @@ def decorator_setting(func): "setter": setter, "setter_name": setter_name, "extras": kwargs, + "type": _sensor_type_for_return_type(func), + "access": AccessFlags.Read | AccessFlags.Write, } if min_value or max_value or range_attribute: - descriptor = NumberSettingDescriptor( + descriptor = RangeDescriptor( **common_values, min_value=min_value or 0, max_value=max_value, @@ -290,15 +279,15 @@ def decorator_setting(func): range_attribute=range_attribute, ) elif choices or choices_attribute: - descriptor = EnumSettingDescriptor( + descriptor = EnumDescriptor( **common_values, choices=choices, choices_attribute=choices_attribute, ) else: - descriptor = BooleanSettingDescriptor(**common_values) + descriptor = PropertyDescriptor(**common_values) - func._setting = descriptor + func._descriptor = descriptor return func diff --git a/miio/integrations/genericmiot/cli_helpers.py b/miio/integrations/genericmiot/cli_helpers.py index cab7187b5..5b36d32ec 100644 --- a/miio/integrations/genericmiot/cli_helpers.py +++ b/miio/integrations/genericmiot/cli_helpers.py @@ -1,6 +1,6 @@ from typing import Dict, cast -from miio.descriptors import ActionDescriptor, SettingDescriptor +from miio.descriptors import ActionDescriptor, PropertyDescriptor from miio.miot_models import MiotProperty, MiotService # TODO: these should be moved to a generic implementation covering all actions and settings @@ -32,7 +32,7 @@ def pretty_actions(result: Dict[str, ActionDescriptor]): return out -def pretty_settings(result: Dict[str, SettingDescriptor]): +def pretty_properties(result: Dict[str, PropertyDescriptor]): """Pretty print settings.""" out = "" verbose = False diff --git a/miio/integrations/genericmiot/genericmiot.py b/miio/integrations/genericmiot/genericmiot.py index 392f4dbf1..8a642cd0d 100644 --- a/miio/integrations/genericmiot/genericmiot.py +++ b/miio/integrations/genericmiot/genericmiot.py @@ -6,7 +6,7 @@ from miio import DeviceInfo, MiotDevice from miio.click_common import LiteralParamType, command, format_output -from miio.descriptors import ActionDescriptor, SensorDescriptor, SettingDescriptor +from miio.descriptors import AccessFlags, ActionDescriptor, PropertyDescriptor from miio.miot_cloud import MiotCloud from miio.miot_device import MiotMapping from miio.miot_models import ( @@ -17,7 +17,7 @@ MiotService, ) -from .cli_helpers import pretty_actions, pretty_settings +from .cli_helpers import pretty_actions, pretty_properties from .status import GenericMiotStatus _LOGGER = logging.getLogger(__name__) @@ -53,9 +53,8 @@ def __init__( self._miot_model: Optional[DeviceModel] = None self._actions: Dict[str, ActionDescriptor] = {} - self._sensors: Dict[str, SensorDescriptor] = {} - self._settings: Dict[str, SettingDescriptor] = {} - self._properties: List[MiotProperty] = [] + self._properties: Dict[str, PropertyDescriptor] = {} + self._all_properties: List[MiotProperty] = [] def initialize_model(self): """Initialize the miot model and create descriptions.""" @@ -71,7 +70,7 @@ def initialize_model(self): def status(self) -> GenericMiotStatus: """Return status based on the miot model.""" properties = [] - for prop in self._properties: + for prop in self._all_properties: if MiotAccess.Read not in prop.access: continue @@ -113,14 +112,14 @@ def _create_actions(self, serv: MiotService): self._actions[act_desc.name] = act_desc - def _create_sensors_and_settings(self, serv: MiotService): + def _create_properties(self, serv: MiotService): """Create sensor and setting descriptors for a service.""" for prop in serv.properties: if prop.access == [MiotAccess.Notify]: _LOGGER.debug("Skipping notify-only property: %s", prop) continue if not prop.access: - # some properties are defined only to be used as inputs for actions + # some properties are defined only to be used as inputs or outputs for actions _LOGGER.debug( "%s (%s) reported no access information", prop.name, @@ -130,17 +129,14 @@ def _create_sensors_and_settings(self, serv: MiotService): desc = prop.get_descriptor() - if isinstance(desc, SensorDescriptor): - self._sensors[prop.name] = desc - elif isinstance(desc, SettingDescriptor): + if desc.access & AccessFlags.Write: desc.setter = partial( self.set_property_by, prop.siid, prop.piid, name=prop.name ) - self._settings[prop.name] = desc - else: - raise Exception("unknown descriptor type") - self._properties.append(prop) + self._properties[prop.name] = desc + # TODO: all properties is only used as the descriptors (stored in _properties) do not have siid/piid + self._all_properties.append(prop) def _create_descriptors(self): """Create descriptors based on the miot model.""" @@ -149,17 +145,14 @@ def _create_descriptors(self): continue # Skip device details self._create_actions(serv) - self._create_sensors_and_settings(serv) + self._create_properties(serv) _LOGGER.debug("Created %s actions", len(self._actions)) for act in self._actions.values(): _LOGGER.debug(f"\t{act}") - _LOGGER.debug("Created %s sensors", len(self._sensors)) - for sensor in self._sensors.values(): + _LOGGER.debug("Created %s properties", len(self._properties)) + for sensor in self._properties.values(): _LOGGER.debug(f"\t{sensor}") - _LOGGER.debug("Created %s settings", len(self._settings)) - for setting in self._settings.values(): - _LOGGER.debug(f"\t{setting}") def _get_action_by_name(self, name: str): """Return action by name.""" @@ -192,11 +185,13 @@ def call_action(self, name: str, params=None): def change_setting(self, name: str, params=None): """Change setting value.""" params = params if params is not None else [] - setting = self._settings.get(name, None) + setting = self._properties.get(name, None) if setting is None: - raise ValueError("No setting found for name %s" % name) + raise ValueError("No property found for name %s" % name) + if setting.access ^ AccessFlags.Write: + raise ValueError("Property %s is not writable" % name) - return setting.setter(value=setting.cast_value(params)) + return setting.setter(value=params) def _fetch_info(self) -> DeviceInfo: """Hook to perform the model initialization.""" @@ -210,15 +205,11 @@ def actions(self) -> Dict[str, ActionDescriptor]: """Return available actions.""" return self._actions - @command() - def sensors(self) -> Dict[str, SensorDescriptor]: + @command(default_output=format_output(result_msg_fmt=pretty_properties)) + def properties(self) -> Dict[str, PropertyDescriptor]: """Return available sensors.""" - return self._sensors - - @command(default_output=format_output(result_msg_fmt=pretty_settings)) - def settings(self) -> Dict[str, SettingDescriptor]: - """Return available settings.""" - return self._settings + # TODO: move pretty-properties to be generic for all devices + return self._properties @property def device_type(self) -> Optional[str]: diff --git a/miio/integrations/viomi/viomidishwasher/test_viomidishwasher.py b/miio/integrations/viomi/viomidishwasher/test_viomidishwasher.py index dc9858957..4e118cba5 100644 --- a/miio/integrations/viomi/viomidishwasher/test_viomidishwasher.py +++ b/miio/integrations/viomi/viomidishwasher/test_viomidishwasher.py @@ -2,7 +2,6 @@ from unittest import TestCase import pytest -from freezegun import freeze_time from miio import ViomiDishwasher from miio.tests.dummies import DummyDevice @@ -146,7 +145,7 @@ def test_program(self): self.device.start(Program.Intensive) assert self.state().program == Program.Intensive - @freeze_time() + @pytest.mark.skip(reason="this breaks between 12am and 1am") def test_schedule(self): self.device.on() # ensure on assert self.is_on() is True diff --git a/miio/integrations/yeelight/light/yeelight.py b/miio/integrations/yeelight/light/yeelight.py index bc2f4d7ea..fa447e21b 100644 --- a/miio/integrations/yeelight/light/yeelight.py +++ b/miio/integrations/yeelight/light/yeelight.py @@ -5,11 +5,7 @@ import click from miio.click_common import command, format_output -from miio.descriptors import ( - NumberSettingDescriptor, - SettingDescriptor, - ValidSettingRange, -) +from miio.descriptors import PropertyDescriptor, RangeDescriptor, ValidSettingRange from miio.device import Device, DeviceStatus from miio.devicestatus import sensor, setting from miio.identifiers import LightId @@ -354,18 +350,18 @@ def status(self) -> YeelightStatus: return YeelightStatus(dict(zip(properties, values))) - def settings(self) -> Dict[str, SettingDescriptor]: - """Return settings based on supported features. + def properties(self) -> Dict[str, PropertyDescriptor]: + """Return properties. - This extends the decorated settings with color temperature and color, if - supported by the device. + This is overridden to inject the color temperature and color settings, if they + are supported by the device. """ # TODO: unclear semantics on settings, as making changes here will affect other instances of the class... - settings = super().settings().copy() + settings = super().properties().copy() ct = self._light_info.color_temp if ct.min_value != ct.max_value: _LOGGER.debug("Got ct for %s: %s", self.model, ct) - settings[LightId.ColorTemperature.value] = NumberSettingDescriptor( + settings[LightId.ColorTemperature.value] = RangeDescriptor( name="Color temperature", id=LightId.ColorTemperature.value, property="color_temp", @@ -377,7 +373,7 @@ def settings(self) -> Dict[str, SettingDescriptor]: ) if self._light_info.supports_color: _LOGGER.debug("Got color for %s", self.model) - settings[LightId.Color.value] = NumberSettingDescriptor( + settings[LightId.Color.value] = RangeDescriptor( name="Color", id=LightId.Color.value, property="rgb_int", diff --git a/miio/miot_models.py b/miio/miot_models.py index df6949804..b40df4785 100644 --- a/miio/miot_models.py +++ b/miio/miot_models.py @@ -1,17 +1,16 @@ import logging from datetime import timedelta from enum import Enum -from typing import Any, Dict, List, Optional, Union +from typing import Any, Dict, List, Optional from pydantic import BaseModel, Field, PrivateAttr, root_validator from .descriptors import ( + AccessFlags, ActionDescriptor, - BooleanSettingDescriptor, - EnumSettingDescriptor, - NumberSettingDescriptor, - SensorDescriptor, - SettingDescriptor, + EnumDescriptor, + PropertyDescriptor, + RangeDescriptor, ) _LOGGER = logging.getLogger(__name__) @@ -270,7 +269,7 @@ def pretty_input_constraints(self) -> str: return out - def get_descriptor(self) -> Union[SensorDescriptor, SettingDescriptor]: + def get_descriptor(self) -> PropertyDescriptor: """Create a descriptor based on the property information.""" # TODO: initialize inside __init__? extras = self.extras @@ -279,24 +278,33 @@ def get_descriptor(self) -> Union[SensorDescriptor, SettingDescriptor]: extras["piid"] = self.piid extras["miot_property"] = self - # Handle settable ranged properties + desc: PropertyDescriptor + + # Handle ranged properties if self.range is not None: - return self._descriptor_for_ranged() + desc = self._create_range_descriptor() - # Handle settable enums + # Handle enums elif self.choices is not None: - # TODO: handle two-value enums as booleans? - return self._descriptor_for_choices() + desc = self._create_enum_descriptor() + + else: + desc = self._create_regular_descriptor() - # Handle settable booleans - elif MiotAccess.Write in self.access and self.format == bool: - return self._create_boolean_setting() + return desc - # Fallback to sensors - return self._create_sensor() + def _miot_access_list_to_access(self, access_list: List[MiotAccess]) -> AccessFlags: + """Convert miot access list to property access list.""" + access = AccessFlags(0) + if MiotAccess.Read in access_list: + access |= AccessFlags.Read + if MiotAccess.Write in access_list: + access |= AccessFlags.Write - def _descriptor_for_choices(self) -> Union[SensorDescriptor, EnumSettingDescriptor]: - """Create a descriptor for enum-based setting.""" + return access + + def _create_enum_descriptor(self) -> EnumDescriptor: + """Create a descriptor for enum-based property.""" try: choices = Enum( self.description, {c.description: c.value for c in self.choices} @@ -306,59 +314,49 @@ def _descriptor_for_choices(self) -> Union[SensorDescriptor, EnumSettingDescript _LOGGER.error("Unable to create enum for %s: %s", self, ex) raise - if MiotAccess.Write in self.access: - desc = EnumSettingDescriptor( - id=self.name, - name=self.description, - property=self.normalized_name, - unit=self.unit, - choices=choices, - extras=self.extras, - type=self.format, - ) - return desc - else: - return self._create_sensor() + desc = EnumDescriptor( + id=self.name, + name=self.description, + property=self.normalized_name, + unit=self.unit, + choices=choices, + extras=self.extras, + type=self.format, + access=self._miot_access_list_to_access(self.access), + ) - def _descriptor_for_ranged( - self, - ) -> Union[NumberSettingDescriptor, SensorDescriptor]: - """Create a descriptor for range-based setting.""" - if MiotAccess.Write in self.access and self.range: - desc = NumberSettingDescriptor( - id=self.name, - name=self.description, - property=self.normalized_name, - min_value=self.range[0], - max_value=self.range[1], - step=self.range[2], - unit=self.unit, - extras=self.extras, - type=self.format, - ) - return desc - else: - return self._create_sensor() + return desc - def _create_boolean_setting(self) -> BooleanSettingDescriptor: - """Create boolean setting descriptor.""" - return BooleanSettingDescriptor( + def _create_range_descriptor( + self, + ) -> RangeDescriptor: + """Create a descriptor for range-based property.""" + if self.range is None: + raise ValueError("Range is None") + desc = RangeDescriptor( id=self.name, name=self.description, property=self.normalized_name, + min_value=self.range[0], + max_value=self.range[1], + step=self.range[2], unit=self.unit, extras=self.extras, - type=bool, + type=self.format, + access=self._miot_access_list_to_access(self.access), ) - def _create_sensor(self) -> SensorDescriptor: - """Create sensor descriptor for a property.""" - return SensorDescriptor( + return desc + + def _create_regular_descriptor(self) -> PropertyDescriptor: + """Create boolean setting descriptor.""" + return PropertyDescriptor( id=self.name, name=self.description, property=self.normalized_name, type=self.format, extras=self.extras, + access=self._miot_access_list_to_access(self.access), ) class Config: diff --git a/miio/tests/test_device.py b/miio/tests/test_device.py index cf8a99efa..7face6221 100644 --- a/miio/tests/test_device.py +++ b/miio/tests/test_device.py @@ -189,9 +189,9 @@ def test_cached_descriptors(getter_name, mocker): d = Device("127.0.0.1", "68ffffffffffffffffffffffffffffff") getter = getattr(d, getter_name) initialize_descriptors = mocker.spy(d, "_initialize_descriptors") + mocker.patch("miio.Device.send") mocker.patch("miio.Device.status") - mocker.patch("miio.Device._sensor_descriptors_from_status", return_value={}) - mocker.patch("miio.Device._setting_descriptors_from_status", return_value={}) + mocker.patch("miio.Device._set_constraints_from_attributes", return_value={}) mocker.patch("miio.Device._action_descriptors", return_value={}) for _i in range(5): getter() diff --git a/miio/tests/test_devicestatus.py b/miio/tests/test_devicestatus.py index 41376f27e..3718fbf18 100644 --- a/miio/tests/test_devicestatus.py +++ b/miio/tests/test_devicestatus.py @@ -4,11 +4,7 @@ import pytest from miio import Device, DeviceStatus -from miio.descriptors import ( - EnumSettingDescriptor, - NumberSettingDescriptor, - ValidSettingRange, -) +from miio.descriptors import EnumDescriptor, RangeDescriptor, ValidSettingRange from miio.devicestatus import sensor, setting @@ -113,7 +109,7 @@ def unknown(self): pass status = DecoratedProps() - sensors = status.sensors() + sensors = status.properties() assert len(sensors) == 3 all_kwargs = sensors["all_kwargs"] @@ -131,6 +127,7 @@ def test_setting_decorator_number(mocker): class Settings(DeviceStatus): @property @setting( + id="level", name="Level", unit="something", setter_name="set_level", @@ -153,7 +150,7 @@ def level(self) -> int: assert len(settings) == 1 desc = settings["level"] - assert isinstance(desc, NumberSettingDescriptor) + assert isinstance(desc, RangeDescriptor) assert getattr(d.status(), desc.property) == 1 @@ -175,6 +172,7 @@ def test_setting_decorator_number_range_attribute(mocker): class Settings(DeviceStatus): @property @setting( + id="level", name="Level", unit="something", setter_name="set_level", @@ -200,7 +198,7 @@ def level(self) -> int: assert len(settings) == 1 desc = settings["level"] - assert isinstance(desc, NumberSettingDescriptor) + assert isinstance(desc, RangeDescriptor) assert getattr(d.status(), desc.property) == 1 @@ -223,7 +221,11 @@ class TestEnum(Enum): class Settings(DeviceStatus): @property @setting( - name="Level", unit="something", setter_name="set_level", choices=TestEnum + id="level", + name="Level", + unit="something", + setter_name="set_level", + choices=TestEnum, ) def level(self) -> TestEnum: return TestEnum.First @@ -241,7 +243,7 @@ def level(self) -> TestEnum: assert len(settings) == 1 desc = settings["level"] - assert isinstance(desc, EnumSettingDescriptor) + assert isinstance(desc, EnumDescriptor) assert getattr(d.status(), desc.property) == TestEnum.First assert desc.name == "Level" @@ -265,11 +267,11 @@ def sub_sensor(self): return "sub" main = MainStatus() - assert len(main.sensors()) == 1 + assert len(main.properties()) == 1 sub = SubStatus() main.embed("SubStatus", sub) - sensors = main.sensors() + sensors = main.properties() assert len(sensors) == 2 assert sub._parent == main @@ -277,7 +279,7 @@ def sub_sensor(self): assert getattr(main, sensors["SubStatus__sub_sensor"].property) == "sub" with pytest.raises(KeyError): - main.sensors()["nonexisting_sensor"] + main.properties()["nonexisting_sensor"] assert ( repr(main) @@ -323,10 +325,10 @@ def sensor_returning_none(self): status = Status() expected_regex = [ - "sensor_without_unit (.+?): 1", - "sensor_with_unit (.+?): 2 V", - r"\[RW\] setting_without_unit (.+?): 3", - r"\[RW\] setting_with_unit (.+?): 4 V", + "r-- sensor_without_unit (.+?): 1", + "r-- sensor_with_unit (.+?): 2 V", + r"rw- setting_without_unit (.+?): 3", + r"rw- setting_with_unit (.+?): 4 V", ] for idx, line in enumerate(status.__cli_output__.splitlines()): diff --git a/miio/tests/test_miot_models.py b/miio/tests/test_miot_models.py index e117818c6..bf540039a 100644 --- a/miio/tests/test_miot_models.py +++ b/miio/tests/test_miot_models.py @@ -7,11 +7,11 @@ from pydantic import BaseModel from miio.descriptors import ( - BooleanSettingDescriptor, - EnumSettingDescriptor, - NumberSettingDescriptor, - SensorDescriptor, - SettingType, + AccessFlags, + EnumDescriptor, + PropertyConstraint, + PropertyDescriptor, + RangeDescriptor, ) from miio.miot_models import ( URN, @@ -261,10 +261,13 @@ def test_property(): @pytest.mark.parametrize( - ("read_only", "expected"), - [(True, SensorDescriptor), (False, BooleanSettingDescriptor)], + ("read_only", "access"), + [ + (True, AccessFlags.Read), + (False, AccessFlags.Read | AccessFlags.Write), + ], ) -def test_get_descriptor_bool_property(read_only, expected): +def test_get_descriptor_bool_property(read_only, access): """Test that boolean property creates a sensor.""" boolean_prop = load_fixture("boolean_property.json") if read_only: @@ -273,15 +276,16 @@ def test_get_descriptor_bool_property(read_only, expected): prop = MiotProperty.parse_obj(boolean_prop) desc = prop.get_descriptor() - assert isinstance(desc, expected) assert desc.type == bool - if not read_only: - assert desc.setting_type == SettingType.Boolean + assert desc.access == access + + if read_only: + assert desc.access ^ AccessFlags.Write @pytest.mark.parametrize( ("read_only", "expected"), - [(True, SensorDescriptor), (False, NumberSettingDescriptor)], + [(True, PropertyDescriptor), (False, RangeDescriptor)], ) def test_get_descriptor_ranged_property(read_only, expected): """Test value-range descriptors.""" @@ -295,12 +299,12 @@ def test_get_descriptor_ranged_property(read_only, expected): assert isinstance(desc, expected) assert desc.type == int if not read_only: - assert desc.setting_type == SettingType.Number + assert desc.constraint == PropertyConstraint.Range @pytest.mark.parametrize( ("read_only", "expected"), - [(True, SensorDescriptor), (False, EnumSettingDescriptor)], + [(True, PropertyDescriptor), (False, EnumDescriptor)], ) def test_get_descriptor_enum_property(read_only, expected): """Test enum descriptors.""" @@ -314,7 +318,7 @@ def test_get_descriptor_enum_property(read_only, expected): assert isinstance(desc, expected) assert desc.type == int if not read_only: - assert desc.setting_type == SettingType.Enum + assert desc.constraint == PropertyConstraint.Choice @pytest.mark.xfail(reason="not implemented")