Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion miio/device.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import warnings
from enum import Enum
from pprint import pformat as pf
from typing import _ProtocolMeta # type: ignore[attr-defined]
from typing import Any, Dict, List, Optional # noqa: F401

import click
Expand Down Expand Up @@ -47,7 +48,16 @@ def __repr__(self):
return s


class Device(metaclass=DeviceGroupMeta):
class _CombinedMeta(DeviceGroupMeta, _ProtocolMeta):
"""Combine typing._ProtocolMeta and our own meta class to allow inheriting from
interfaces.

This is necessary as typing.Protocol defines its own metaclass causing a metaclass
conflict with the DeviceGroupMeta due to differing base classes.
"""


class Device(metaclass=_CombinedMeta):
"""Base class for all device implementations.

This is the main class providing the basic protocol handling for devices using the
Expand Down
3 changes: 2 additions & 1 deletion miio/integrations/vacuum/roborock/vacuum.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
)
from miio.device import Device, DeviceInfo
from miio.exceptions import DeviceException, DeviceInfoUnavailableException
from miio.interfaces import Vacuum

from .vacuumcontainers import (
CarpetModeStatus,
Expand Down Expand Up @@ -169,7 +170,7 @@ class CarpetCleaningMode(enum.Enum):
]


class RoborockVacuum(Device):
class RoborockVacuum(Device, Vacuum):
"""Main class for roborock vacuums (roborock.vacuum.*)."""

_supported_models = SUPPORTED_MODELS
Expand Down
2 changes: 2 additions & 0 deletions miio/interfaces/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# flake8: noqa
from .vacuum import Vacuum
32 changes: 32 additions & 0 deletions miio/interfaces/vacuum.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from typing import Dict, Protocol, runtime_checkable

from miio import DeviceStatus

FanspeedPresets = Dict[str, int]


@runtime_checkable
class Vacuum(Protocol):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider rename interface to IVacuum. It is useful to understand from the name, whether it is a class or interface.

def start(self):
"""Start vacuuming."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally it is good to specify more details about behavior of the interface methods, format of the arguments and returning value. This helps to keep implementations more consistent.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I just made this PR to show the idea, but your earlier proposal feels like a better solution than this.


def pause(self):
"""Pause vacuuming."""
Copy link
Contributor

@2pirko 2pirko Mar 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some implementation does not contain pause method. It would be good to describe, what should happen, if the method is not supported. Raise an exception?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On roborock, calling pause differs from stop in sense that resuming from a pause will continue using the existing internal map if the cleaning is resumed, in contrast to starting a completely new cleaning round.


def stop(self):
"""Stop vacuuming."""

def home(self):
"""Return back to base."""

def find(self):
"""Request vacuum to play a sound to reveal its location."""

def status(self) -> DeviceStatus:
"""Return device status."""

def set_fan_speed(self, int):
"""Set fan speed."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to describe argument. Is argument in range 0-100 (percent)? Or can the argument be only one of the "presets"? What the method do if the argument does not match?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this was just based on the current roborock implementation, but neither viomi nor g1vacuum seem to support percentage-based fan speed setting.

The solution would be to have a set_fan_speed_preset(x) that accepts "something" that is returned by the implementations' fan_speed_presets(). A feature flag & a separate set_fan_speed_percentage(int) could be introduced for devices that support arbitrary fan speeds.

I think this is more clearly visible on some fan/air(de)humidifer implementations where the "levels" or "presets" are more common than continuous values between 0 and 100.


def fan_speed_presets(self) -> FanspeedPresets:
"""Return available fan speed presets."""