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

Add XML command "SetFanSpeed" #560

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open
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
7 changes: 5 additions & 2 deletions deebot_client/commands/xml/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,27 @@

from .charge_state import GetChargeState
from .error import GetError
from .fan_speed import GetFanSpeed
from .fan_speed import GetCleanSpeed, SetCleanSpeed
from .pos import GetPos
from .stats import GetCleanSum

if TYPE_CHECKING:
from .common import XmlCommand

__all__ = [
"GetCleanSpeed",
"SetCleanSpeed",
"GetChargeState",
"GetCleanSum",
"GetError",
"GetFanSpeed",
"GetPos",
]

# fmt: off
# ordered by file asc
_COMMANDS: list[type[XmlCommand]] = [
GetCleanSpeed,
SetCleanSpeed,
GetError,
]
# fmt: on
Expand Down
29 changes: 26 additions & 3 deletions deebot_client/commands/xml/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@

from defusedxml import ElementTree # type: ignore[import-untyped]

from deebot_client.command import Command, CommandWithMessageHandling
from deebot_client.command import Command, CommandWithMessageHandling, SetCommand
from deebot_client.const import DataType
from deebot_client.logging_filter import get_logger
from deebot_client.message import HandlingResult, MessageStr
from deebot_client.message import HandlingResult, MessageStr, HandlingState

if TYPE_CHECKING:
from deebot_client.event_bus import EventBus
Expand All @@ -25,7 +25,6 @@ class XmlCommand(Command):
data_type: DataType = DataType.XML

@property # type: ignore[misc]
@classmethod
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you remove it?

Copy link
Contributor Author

@flubshi flubshi Sep 4, 2024

Choose a reason for hiding this comment

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

Otherwise it throws an error TypeError: 'classmethod' object is not callable.
Maybe you can reproduce it locally when reverting and running the tests.

@edenhaus Do you have a better solution?

def has_sub_element(cls) -> bool:
"""Return True if command has inner element."""
return False
Expand Down Expand Up @@ -65,3 +64,27 @@ def _handle_str(cls, event_bus: EventBus, message: str) -> HandlingResult:
"""
xml = ElementTree.fromstring(message)
return cls._handle_xml(event_bus, xml)


class ExecuteCommand(XmlCommandWithMessageHandling, ABC):
"""Command, which is executing something (ex. Charge)."""

@classmethod
def _handle_xml(cls, event_bus: EventBus, xml: Element) -> HandlingResult:
"""Handle message->xml and notify the correct event subscribers.

:return: A message response
"""
# Success event looks like <ctl ret='ok'/>
if xml.attrib.get("ret") == "ok":
return HandlingResult.success()

_LOGGER.warning('Command "%s" was not successful. XML response: %s', cls.name, xml)
return HandlingResult(HandlingState.FAILED)
flubshi marked this conversation as resolved.
Show resolved Hide resolved


class XmlSetCommand(ExecuteCommand, SetCommand, ABC):
"""Xml base set command.

Command needs to be linked to the "get" command, for handling (updating) the sensors.
"""
26 changes: 21 additions & 5 deletions deebot_client/commands/xml/fan_speed.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,23 @@

from __future__ import annotations

from types import MappingProxyType
from typing import TYPE_CHECKING

from deebot_client.command import InitParam
from deebot_client.events import FanSpeedEvent, FanSpeedLevel
from deebot_client.message import HandlingResult

from .common import XmlCommandWithMessageHandling
from .common import XmlCommandWithMessageHandling, XmlSetCommand

if TYPE_CHECKING:
from xml.etree.ElementTree import Element

from deebot_client.event_bus import EventBus


class GetFanSpeed(XmlCommandWithMessageHandling):
"""GetFanSpeed command."""
class GetCleanSpeed(XmlCommandWithMessageHandling):
"""GetCleanSpeed command."""

name = "GetCleanSpeed"

Expand All @@ -33,12 +35,26 @@ def _handle_xml(cls, event_bus: EventBus, xml: Element) -> HandlingResult:

match speed.lower():
case "standard":
event = FanSpeedEvent(FanSpeedLevel.NORMAL)
event = FanSpeedEvent(FanSpeedLevel.STANDARD)
case "strong":
event = FanSpeedEvent(FanSpeedLevel.MAX)
event = FanSpeedEvent(FanSpeedLevel.STRONG)

if event:
event_bus.notify(event)
return HandlingResult.success()

return HandlingResult.analyse()


class SetCleanSpeed(XmlSetCommand):
"""Set clean speed command."""

name = "SetCleanSpeed"
get_command = GetCleanSpeed
_mqtt_params = MappingProxyType({"speed": InitParam(FanSpeedLevel)})

def __init__(self, speed: FanSpeedLevel | str) -> None:
if isinstance(speed, FanSpeedLevel):
speed = speed.name.lower()
super().__init__({"speed": speed})

2 changes: 2 additions & 0 deletions deebot_client/events/fan_speed.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@ class FanSpeedLevel(IntEnum):

# Values should be sort from low to high on their meanings
QUIET = 1000
STANDARD = -1
Copy link
Contributor

Choose a reason for hiding this comment

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

Inconsistent enumeration value.

The STANDARD value is assigned a negative value (-1), which is inconsistent with the existing positive values in the FanSpeedLevel enumeration. This breaks the ordering of the values from low to high based on their meanings, as stated in the comment above the enumeration.

Consider assigning a positive value to STANDARD that maintains the correct ordering of the values based on their meanings. For example:

QUIET = 1000
STANDARD = 1500
NORMAL = 2000
MAX = 2500
STRONG = 3000
MAX_PLUS = 3500

NORMAL = 0
MAX = 1
STRONG = -2
Copy link
Contributor

Choose a reason for hiding this comment

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

Inconsistent enumeration value.

The STRONG value is assigned a negative value (-2), which is inconsistent with the existing positive values in the FanSpeedLevel enumeration. This breaks the ordering of the values from low to high based on their meanings, as stated in the comment above the enumeration.

Consider assigning a positive value to STRONG that maintains the correct ordering of the values based on their meanings. For example:

QUIET = 1000
STANDARD = 1500
NORMAL = 2000
MAX = 2500
STRONG = 3000
MAX_PLUS = 3500

MAX_PLUS = 2


Expand Down
22 changes: 17 additions & 5 deletions tests/commands/xml/test_fan_speed.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import pytest

from deebot_client.command import CommandResult
from deebot_client.commands.xml import GetFanSpeed
from deebot_client.commands.xml import GetCleanSpeed, SetCleanSpeed
from deebot_client.events import FanSpeedEvent, FanSpeedLevel
from deebot_client.message import HandlingState
from tests.commands import assert_command
Expand All @@ -19,14 +19,14 @@
@pytest.mark.parametrize(
("speed", "expected_event"),
[
("standard", FanSpeedEvent(FanSpeedLevel.NORMAL)),
("strong", FanSpeedEvent(FanSpeedLevel.MAX)),
("standard", FanSpeedEvent(FanSpeedLevel.STANDARD)),
("strong", FanSpeedEvent(FanSpeedLevel.STRONG)),
],
ids=["standard", "strong"],
)
async def test_get_fan_speed(speed: str, expected_event: Event) -> None:
json = get_request_xml(f"<ctl ret='ok' speed='{speed}'/>")
await assert_command(GetFanSpeed(), json, expected_event)
await assert_command(GetCleanSpeed(), json, expected_event)


@pytest.mark.parametrize(
Expand All @@ -37,8 +37,20 @@ async def test_get_fan_speed(speed: str, expected_event: Event) -> None:
async def test_get_fan_speed_error(xml: str) -> None:
json = get_request_xml(xml)
await assert_command(
GetFanSpeed(),
GetCleanSpeed(),
json,
None,
command_result=CommandResult(HandlingState.ANALYSE_LOGGED),
)


async def test_set_fan_speed() -> None:
command = SetCleanSpeed(FanSpeedLevel.STRONG)
json = get_request_xml("<ctl ret='ok' />")
await assert_command(command, json, None, command_result=CommandResult(HandlingState.SUCCESS))


async def test_set_fan_speed_error() -> None:
command = SetCleanSpeed("invalid")
json = get_request_xml("<ctl ret='error' />")
await assert_command(command, json, None, command_result=CommandResult(HandlingState.FAILED))
Loading