From 3474f46b09da9537aad5c5b663c6aeac13aecd4d Mon Sep 17 00:00:00 2001 From: Luke Date: Thu, 29 Jun 2023 13:13:37 -0400 Subject: [PATCH] Bump Roborock to 0.29.2 (#95549) * init work * fix tests --- homeassistant/components/roborock/device.py | 11 + .../components/roborock/manifest.json | 2 +- homeassistant/components/roborock/switch.py | 191 ++++++------------ requirements_all.txt | 2 +- requirements_test_all.txt | 2 +- tests/components/roborock/conftest.py | 10 +- tests/components/roborock/mock_data.py | 7 +- .../roborock/snapshots/test_diagnostics.ambr | 15 -- tests/components/roborock/test_select.py | 2 + tests/components/roborock/test_switch.py | 23 +-- 10 files changed, 95 insertions(+), 170 deletions(-) diff --git a/homeassistant/components/roborock/device.py b/homeassistant/components/roborock/device.py index 3801ccbecc9..90ca13c5146 100644 --- a/homeassistant/components/roborock/device.py +++ b/homeassistant/components/roborock/device.py @@ -2,6 +2,8 @@ from typing import Any +from roborock.api import AttributeCache +from roborock.command_cache import CacheableAttribute from roborock.containers import Status from roborock.exceptions import RoborockException from roborock.local_api import RoborockLocalClient @@ -27,6 +29,15 @@ class RoborockEntity(Entity): self._attr_device_info = device_info self._api = api + @property + def api(self) -> RoborockLocalClient: + """Returns the api.""" + return self._api + + def get_cache(self, attribute: CacheableAttribute) -> AttributeCache: + """Get an item from the api cache.""" + return self._api.cache.get(attribute) + async def send( self, command: RoborockCommand, params: dict[str, Any] | list[Any] | None = None ) -> dict: diff --git a/homeassistant/components/roborock/manifest.json b/homeassistant/components/roborock/manifest.json index 39e13412e90..baab687e64a 100644 --- a/homeassistant/components/roborock/manifest.json +++ b/homeassistant/components/roborock/manifest.json @@ -6,5 +6,5 @@ "documentation": "https://www.home-assistant.io/integrations/roborock", "iot_class": "local_polling", "loggers": ["roborock"], - "requirements": ["python-roborock==0.23.6"] + "requirements": ["python-roborock==0.29.2"] } diff --git a/homeassistant/components/roborock/switch.py b/homeassistant/components/roborock/switch.py index d16d7437202..a0b3d5be295 100644 --- a/homeassistant/components/roborock/switch.py +++ b/homeassistant/components/roborock/switch.py @@ -1,23 +1,27 @@ """Support for Roborock switch.""" +from __future__ import annotations + import asyncio from collections.abc import Callable, Coroutine from dataclasses import dataclass import logging from typing import Any -from roborock.exceptions import RoborockException -from roborock.roborock_typing import RoborockCommand +from roborock.api import AttributeCache +from roborock.command_cache import CacheableAttribute +from roborock.local_api import RoborockLocalClient from homeassistant.components.switch import SwitchEntity, SwitchEntityDescription from homeassistant.config_entries import ConfigEntry from homeassistant.const import EntityCategory from homeassistant.core import HomeAssistant +from homeassistant.helpers.entity import DeviceInfo from homeassistant.helpers.entity_platform import AddEntitiesCallback from homeassistant.util import slugify from .const import DOMAIN from .coordinator import RoborockDataUpdateCoordinator -from .device import RoborockCoordinatedEntity, RoborockEntity +from .device import RoborockEntity _LOGGER = logging.getLogger(__name__) @@ -27,23 +31,11 @@ class RoborockSwitchDescriptionMixin: """Define an entity description mixin for switch entities.""" # Gets the status of the switch - get_value: Callable[[RoborockEntity], Coroutine[Any, Any, dict]] - # Evaluate the result of get_value to determine a bool - evaluate_value: Callable[[dict], bool] + cache_key: CacheableAttribute # Sets the status of the switch - set_command: Callable[[RoborockEntity, bool], Coroutine[Any, Any, dict]] - # Check support of this feature - check_support: Callable[[RoborockDataUpdateCoordinator], Coroutine[Any, Any, dict]] - - -@dataclass -class RoborockCoordinatedSwitchDescriptionMixIn: - """Define an entity description mixin for switch entities.""" - - get_value: Callable[[RoborockCoordinatedEntity], bool] - set_command: Callable[[RoborockCoordinatedEntity, bool], Coroutine[Any, Any, dict]] - # Check support of this feature - check_support: Callable[[RoborockDataUpdateCoordinator], dict] + update_value: Callable[[AttributeCache, bool], Coroutine[Any, Any, dict]] + # Attribute from cache + attribute: str @dataclass @@ -53,59 +45,42 @@ class RoborockSwitchDescription( """Class to describe an Roborock switch entity.""" -@dataclass -class RoborockCoordinatedSwitchDescription( - SwitchEntityDescription, RoborockCoordinatedSwitchDescriptionMixIn -): - """Class to describe an Roborock switch entity that needs a coordinator.""" - - SWITCH_DESCRIPTIONS: list[RoborockSwitchDescription] = [ RoborockSwitchDescription( - set_command=lambda entity, value: entity.send( - RoborockCommand.SET_CHILD_LOCK_STATUS, {"lock_status": 1 if value else 0} + cache_key=CacheableAttribute.child_lock_status, + update_value=lambda cache, value: cache.update_value( + {"lock_status": 1 if value else 0} ), - get_value=lambda data: data.send(RoborockCommand.GET_CHILD_LOCK_STATUS), - check_support=lambda data: data.api.send_command( - RoborockCommand.GET_CHILD_LOCK_STATUS - ), - evaluate_value=lambda data: data["lock_status"] == 1, + attribute="lock_status", key="child_lock", translation_key="child_lock", icon="mdi:account-lock", entity_category=EntityCategory.CONFIG, ), RoborockSwitchDescription( - set_command=lambda entity, value: entity.send( - RoborockCommand.SET_FLOW_LED_STATUS, {"status": 1 if value else 0} + cache_key=CacheableAttribute.flow_led_status, + update_value=lambda cache, value: cache.update_value( + {"status": 1 if value else 0} ), - get_value=lambda data: data.send(RoborockCommand.GET_FLOW_LED_STATUS), - check_support=lambda data: data.api.send_command( - RoborockCommand.GET_FLOW_LED_STATUS - ), - evaluate_value=lambda data: data["status"] == 1, + attribute="status", key="status_indicator", translation_key="status_indicator", icon="mdi:alarm-light-outline", entity_category=EntityCategory.CONFIG, ), -] - -COORDINATED_SWITCH_DESCRIPTION = [ - RoborockCoordinatedSwitchDescription( - set_command=lambda entity, value: entity.send( - RoborockCommand.SET_DND_TIMER, + RoborockSwitchDescription( + cache_key=CacheableAttribute.dnd_timer, + update_value=lambda cache, value: cache.update_value( [ - entity.coordinator.roborock_device_info.props.dnd_timer.start_hour, - entity.coordinator.roborock_device_info.props.dnd_timer.start_minute, - entity.coordinator.roborock_device_info.props.dnd_timer.end_hour, - entity.coordinator.roborock_device_info.props.dnd_timer.end_minute, - ], + cache.value.get("start_hour"), + cache.value.get("start_minute"), + cache.value.get("end_hour"), + cache.value.get("end_minute"), + ] ) if value - else entity.send(RoborockCommand.CLOSE_DND_TIMER), - check_support=lambda data: data.roborock_device_info.props.dnd_timer, - get_value=lambda data: data.coordinator.roborock_device_info.props.dnd_timer.enabled, + else cache.close_value(), + attribute="enabled", key="dnd_switch", translation_key="dnd_switch", icon="mdi:bell-cancel", @@ -120,114 +95,74 @@ async def async_setup_entry( async_add_entities: AddEntitiesCallback, ) -> None: """Set up Roborock switch platform.""" - coordinators: dict[str, RoborockDataUpdateCoordinator] = hass.data[DOMAIN][ config_entry.entry_id ] possible_entities: list[ - tuple[str, RoborockDataUpdateCoordinator, RoborockSwitchDescription] + tuple[RoborockDataUpdateCoordinator, RoborockSwitchDescription] ] = [ - (device_id, coordinator, description) - for device_id, coordinator in coordinators.items() + (coordinator, description) + for coordinator in coordinators.values() for description in SWITCH_DESCRIPTIONS ] # We need to check if this function is supported by the device. results = await asyncio.gather( *( - description.check_support(coordinator) - for _, coordinator, description in possible_entities + coordinator.api.cache.get(description.cache_key).async_value() + for coordinator, description in possible_entities ), return_exceptions=True, ) - valid_entities: list[RoborockNonCoordinatedSwitchEntity] = [] - for posible_entity, result in zip(possible_entities, results): - if isinstance(result, Exception): - if not isinstance(result, RoborockException): - raise result + valid_entities: list[RoborockSwitch] = [] + for (coordinator, description), result in zip(possible_entities, results): + if result is None or isinstance(result, Exception): _LOGGER.debug("Not adding entity because of %s", result) else: valid_entities.append( - RoborockNonCoordinatedSwitchEntity( - f"{posible_entity[2].key}_{slugify(posible_entity[0])}", - posible_entity[1], - posible_entity[2], - result, + RoborockSwitch( + f"{description.key}_{slugify(coordinator.roborock_device_info.device.duid)}", + coordinator.device_info, + description, + coordinator.api, ) ) - async_add_entities( - valid_entities, - True, - ) - async_add_entities( - ( - RoborockCoordinatedSwitchEntity( - f"{description.key}_{slugify(device_id)}", - coordinator, - description, - ) - for device_id, coordinator in coordinators.items() - for description in COORDINATED_SWITCH_DESCRIPTION - if description.check_support(coordinator) is not None - ) - ) + async_add_entities(valid_entities) -class RoborockNonCoordinatedSwitchEntity(RoborockEntity, SwitchEntity): - """A class to let you turn functionality on Roborock devices on and off that does not need a coordinator.""" +class RoborockSwitch(RoborockEntity, SwitchEntity): + """A class to let you turn functionality on Roborock devices on and off that does need a coordinator.""" entity_description: RoborockSwitchDescription def __init__( self, unique_id: str, - coordinator: RoborockDataUpdateCoordinator, - entity_description: RoborockSwitchDescription, - initial_value: bool, + device_info: DeviceInfo, + description: RoborockSwitchDescription, + api: RoborockLocalClient, ) -> None: - """Create a switch entity.""" - self.entity_description = entity_description - super().__init__(unique_id, coordinator.device_info, coordinator.api) - self._attr_is_on = initial_value + """Initialize the entity.""" + super().__init__(unique_id, device_info, api) + self.entity_description = description async def async_turn_off(self, **kwargs: Any) -> None: """Turn off the switch.""" - await self.entity_description.set_command(self, False) - - async def async_turn_on(self, **kwargs: Any) -> None: - """Turn on the switch.""" - await self.entity_description.set_command(self, True) - - async def async_update(self) -> None: - """Update switch.""" - self._attr_is_on = self.entity_description.evaluate_value( - await self.entity_description.get_value(self) + await self.entity_description.update_value( + self.get_cache(self.entity_description.cache_key), False ) - -class RoborockCoordinatedSwitchEntity(RoborockCoordinatedEntity, SwitchEntity): - """A class to let you turn functionality on Roborock devices on and off that does need a coordinator.""" - - entity_description: RoborockCoordinatedSwitchDescription - - def __init__( - self, - unique_id: str, - coordinator: RoborockDataUpdateCoordinator, - entity_description: RoborockCoordinatedSwitchDescription, - ) -> None: - """Create a switch entity.""" - self.entity_description = entity_description - super().__init__(unique_id, coordinator) - - async def async_turn_off(self, **kwargs: Any) -> None: - """Turn off the switch.""" - await self.entity_description.set_command(self, False) - async def async_turn_on(self, **kwargs: Any) -> None: """Turn on the switch.""" - await self.entity_description.set_command(self, True) + await self.entity_description.update_value( + self.get_cache(self.entity_description.cache_key), True + ) @property def is_on(self) -> bool | None: - """Use the coordinator to determine if the switch is on.""" - return self.entity_description.get_value(self) + """Return True if entity is on.""" + return ( + self.get_cache(self.entity_description.cache_key).value.get( + self.entity_description.attribute + ) + == 1 + ) diff --git a/requirements_all.txt b/requirements_all.txt index aab14224dc9..851359306ad 100644 --- a/requirements_all.txt +++ b/requirements_all.txt @@ -2139,7 +2139,7 @@ python-qbittorrent==0.4.3 python-ripple-api==0.0.3 # homeassistant.components.roborock -python-roborock==0.23.6 +python-roborock==0.29.2 # homeassistant.components.smarttub python-smarttub==0.0.33 diff --git a/requirements_test_all.txt b/requirements_test_all.txt index 2863b4c90f7..74612b98385 100644 --- a/requirements_test_all.txt +++ b/requirements_test_all.txt @@ -1565,7 +1565,7 @@ python-picnic-api==1.1.0 python-qbittorrent==0.4.3 # homeassistant.components.roborock -python-roborock==0.23.6 +python-roborock==0.29.2 # homeassistant.components.smarttub python-smarttub==0.0.33 diff --git a/tests/components/roborock/conftest.py b/tests/components/roborock/conftest.py index d9c11bead74..eb281076825 100644 --- a/tests/components/roborock/conftest.py +++ b/tests/components/roborock/conftest.py @@ -20,11 +20,17 @@ from tests.common import MockConfigEntry @pytest.fixture(name="bypass_api_fixture") def bypass_api_fixture() -> None: """Skip calls to the API.""" - with patch("homeassistant.components.roborock.RoborockMqttClient.connect"), patch( - "homeassistant.components.roborock.RoborockMqttClient.send_command" + with patch( + "homeassistant.components.roborock.RoborockMqttClient.async_connect" + ), patch( + "homeassistant.components.roborock.RoborockMqttClient._send_command" ), patch( "homeassistant.components.roborock.coordinator.RoborockLocalClient.get_prop", return_value=PROP, + ), patch( + "roborock.api.AttributeCache.async_value" + ), patch( + "roborock.api.AttributeCache.value" ): yield diff --git a/tests/components/roborock/mock_data.py b/tests/components/roborock/mock_data.py index 15e69cee9d9..6a2e1f4b5f1 100644 --- a/tests/components/roborock/mock_data.py +++ b/tests/components/roborock/mock_data.py @@ -367,7 +367,12 @@ STATUS = S7Status.from_dict( "unsave_map_flag": 0, } ) -PROP = DeviceProp(STATUS, DND_TIMER, CLEAN_SUMMARY, CONSUMABLE, CLEAN_RECORD) +PROP = DeviceProp( + status=STATUS, + clean_summary=CLEAN_SUMMARY, + consumable=CONSUMABLE, + last_clean_record=CLEAN_RECORD, +) NETWORK_INFO = NetworkInfo( ip="123.232.12.1", ssid="wifi", mac="ac:cc:cc:cc:cc", bssid="bssid", rssi=90 diff --git a/tests/components/roborock/snapshots/test_diagnostics.ambr b/tests/components/roborock/snapshots/test_diagnostics.ambr index 432bad167cd..eb70e04110f 100644 --- a/tests/components/roborock/snapshots/test_diagnostics.ambr +++ b/tests/components/roborock/snapshots/test_diagnostics.ambr @@ -221,21 +221,6 @@ 'sideBrushWorkTime': 74382, 'strainerWorkTimes': 65, }), - 'dndTimer': dict({ - 'enabled': 1, - 'endHour': 7, - 'endMinute': 0, - 'endTime': dict({ - '__type': "", - 'isoformat': '07:00:00', - }), - 'startHour': 22, - 'startMinute': 0, - 'startTime': dict({ - '__type': "", - 'isoformat': '22:00:00', - }), - }), 'lastCleanRecord': dict({ 'area': 20965000, 'avoidCount': 19, diff --git a/tests/components/roborock/test_select.py b/tests/components/roborock/test_select.py index 3b0ba8183b3..bcea4e6246b 100644 --- a/tests/components/roborock/test_select.py +++ b/tests/components/roborock/test_select.py @@ -26,6 +26,8 @@ async def test_update_success( value: str, ) -> None: """Test allowed changing values for select entities.""" + # Ensure that the entity exist, as these test can pass even if there is no entity. + assert hass.states.get(entity_id) is not None with patch( "homeassistant.components.roborock.coordinator.RoborockLocalClient.send_message" ) as mock_send_message: diff --git a/tests/components/roborock/test_switch.py b/tests/components/roborock/test_switch.py index 9c079ef85b6..40ecdc267ed 100644 --- a/tests/components/roborock/test_switch.py +++ b/tests/components/roborock/test_switch.py @@ -2,11 +2,9 @@ from unittest.mock import patch import pytest -from roborock.exceptions import RoborockException from homeassistant.components.switch import SERVICE_TURN_OFF, SERVICE_TURN_ON from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError from tests.common import MockConfigEntry @@ -26,6 +24,8 @@ async def test_update_success( entity_id: str, ) -> None: """Test turning switch entities on and off.""" + # Ensure that the entity exist, as these test can pass even if there is no entity. + assert hass.states.get(entity_id) is not None with patch( "homeassistant.components.roborock.coordinator.RoborockLocalClient.send_message" ) as mock_send_message: @@ -48,22 +48,3 @@ async def test_update_success( target={"entity_id": entity_id}, ) assert mock_send_message.assert_called_once - - -async def test_update_failure( - hass: HomeAssistant, - bypass_api_fixture, - setup_entry: MockConfigEntry, -) -> None: - """Test that changing a value will raise a homeassistanterror when it fails.""" - with patch( - "homeassistant.components.roborock.coordinator.RoborockLocalClient.send_message", - side_effect=RoborockException(), - ), pytest.raises(HomeAssistantError): - await hass.services.async_call( - "switch", - SERVICE_TURN_ON, - service_data=None, - blocking=True, - target={"entity_id": "switch.roborock_s7_maxv_child_lock"}, - )