Add dynamic update interval to Roborock (#140563)

* Add dynamic update interval to Roborock

* mr comments

* update time intervals

* Set A01 to 1 minute

* set interval to 30

homeassistant/components/roborock/const.py

@@ -1,5 +1,7 @@
 """Constants for Roborock."""
 
+from datetime import timedelta
+
 from vacuum_map_parser_base.config.drawable import Drawable
 
 from homeassistant.const import Platform
@@ -43,8 +45,8 @@ PLATFORMS = [
     Platform.VACUUM,
 ]
 
-
-IMAGE_CACHE_INTERVAL = 90
+# This can be lowered in the future if we do not receive rate limiting issues.
+IMAGE_CACHE_INTERVAL = 30
 
 MAP_SLEEP = 3
 
@@ -54,3 +56,10 @@ MAP_FILE_FORMAT = "PNG"
 MAP_FILENAME_SUFFIX = ".png"
 SET_VACUUM_GOTO_POSITION_SERVICE_NAME = "set_vacuum_goto_position"
 GET_VACUUM_CURRENT_POSITION_SERVICE_NAME = "get_vacuum_current_position"
+
+
+A01_UPDATE_INTERVAL = timedelta(minutes=1)
+V1_CLOUD_IN_CLEANING_INTERVAL = timedelta(seconds=30)
+V1_CLOUD_NOT_CLEANING_INTERVAL = timedelta(minutes=1)
+V1_LOCAL_IN_CLEANING_INTERVAL = timedelta(seconds=15)
+V1_LOCAL_NOT_CLEANING_INTERVAL = timedelta(seconds=30)

homeassistant/components/roborock/coordinator.py

@@ -36,7 +36,14 @@ from homeassistant.helpers.typing import StateType
 from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 from homeassistant.util import slugify
 
-from .const import DOMAIN
+from .const import (
+    A01_UPDATE_INTERVAL,
+    DOMAIN,
+    V1_CLOUD_IN_CLEANING_INTERVAL,
+    V1_CLOUD_NOT_CLEANING_INTERVAL,
+    V1_LOCAL_IN_CLEANING_INTERVAL,
+    V1_LOCAL_NOT_CLEANING_INTERVAL,
+)
 from .models import RoborockA01HassDeviceInfo, RoborockHassDeviceInfo, RoborockMapInfo
 from .roborock_storage import RoborockMapStorage
 
@@ -85,7 +92,8 @@ class RoborockDataUpdateCoordinator(DataUpdateCoordinator[DeviceProp]):
             _LOGGER,
             config_entry=config_entry,
             name=DOMAIN,
-            update_interval=SCAN_INTERVAL,
+            # Assume we can use the local api.
+            update_interval=V1_LOCAL_NOT_CLEANING_INTERVAL,
         )
         self.roborock_device_info = RoborockHassDeviceInfo(
             device,
@@ -118,6 +126,7 @@ class RoborockDataUpdateCoordinator(DataUpdateCoordinator[DeviceProp]):
         )
         self._user_data = user_data
         self._api_client = api_client
+        self._is_cloud_api = False
 
     async def _async_setup(self) -> None:
         """Set up the coordinator."""
@@ -152,6 +161,8 @@ class RoborockDataUpdateCoordinator(DataUpdateCoordinator[DeviceProp]):
                 await self.api.async_disconnect()
                 # We use the cloud api if the local api fails to connect.
                 self.api = self.cloud_api
+                self.update_interval = V1_CLOUD_NOT_CLEANING_INTERVAL
+                self._is_cloud_api = True
                 # Right now this should never be called if the cloud api is the primary api,
                 # but in the future if it is, a new else should be added.
 
@@ -181,6 +192,15 @@ class RoborockDataUpdateCoordinator(DataUpdateCoordinator[DeviceProp]):
         except RoborockException as ex:
             _LOGGER.debug("Failed to update data: %s", ex)
             raise UpdateFailed(ex) from ex
+        if self.roborock_device_info.props.status.in_cleaning:
+            if self._is_cloud_api:
+                self.update_interval = V1_CLOUD_IN_CLEANING_INTERVAL
+            else:
+                self.update_interval = V1_LOCAL_IN_CLEANING_INTERVAL
+        elif self._is_cloud_api:
+            self.update_interval = V1_CLOUD_NOT_CLEANING_INTERVAL
+        else:
+            self.update_interval = V1_LOCAL_NOT_CLEANING_INTERVAL
         return self.roborock_device_info.props
 
     def _set_current_map(self) -> None:
@@ -269,7 +289,7 @@ class RoborockDataUpdateCoordinatorA01(
             _LOGGER,
             config_entry=config_entry,
             name=DOMAIN,
-            update_interval=SCAN_INTERVAL,
+            update_interval=A01_UPDATE_INTERVAL,
         )
         self.api = api
         self.device_info = DeviceInfo(

homeassistant/components/roborock/quality_scale.yaml

@@ -1,12 +1,7 @@
 rules:
   # Bronze
   action-setup: done
-  appropriate-polling:
-    status: todo
-    comment: |
-      The device currently polls every 30 seconds, which is a bit high when idle.
-      We should consider dynamic polling intervals (e.g. when cleaning) and
-      separate cloud vs local intervals.
+  appropriate-polling: done
   brands: done
   common-modules: done
   config-flow: done

tests/components/roborock/test_coordinator.py

@@ -0,0 +1,107 @@
+"""Test Roborock Coordinator specific logic."""
+
+import copy
+from datetime import timedelta
+from unittest.mock import patch
+
+import pytest
+from roborock.exceptions import RoborockException
+
+from homeassistant.components.roborock.const import (
+    V1_CLOUD_IN_CLEANING_INTERVAL,
+    V1_CLOUD_NOT_CLEANING_INTERVAL,
+    V1_LOCAL_IN_CLEANING_INTERVAL,
+    V1_LOCAL_NOT_CLEANING_INTERVAL,
+)
+from homeassistant.core import HomeAssistant
+from homeassistant.util import dt as dt_util
+
+from .mock_data import PROP
+
+from tests.common import MockConfigEntry, async_fire_time_changed
+
+
+@pytest.mark.parametrize(
+    ("interval", "in_cleaning"),
+    [
+        (V1_CLOUD_IN_CLEANING_INTERVAL, 1),
+        (V1_CLOUD_NOT_CLEANING_INTERVAL, 0),
+    ],
+)
+async def test_dynamic_cloud_scan_interval(
+    hass: HomeAssistant,
+    mock_roborock_entry: MockConfigEntry,
+    bypass_api_fixture_v1_only,
+    interval: timedelta,
+    in_cleaning: int,
+) -> None:
+    """Test dynamic scan interval."""
+    prop = copy.deepcopy(PROP)
+    prop.status.in_cleaning = in_cleaning
+    with (
+        # Force the system to use the cloud api.
+        patch(
+            "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.ping",
+            side_effect=RoborockException(),
+        ),
+        patch(
+            "homeassistant.components.roborock.RoborockMqttClientV1.get_prop",
+            return_value=prop,
+        ),
+    ):
+        await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
+    assert hass.states.get("sensor.roborock_s7_maxv_battery").state == "100"
+    prop = copy.deepcopy(prop)
+    prop.status.battery = 20
+    with patch(
+        "homeassistant.components.roborock.RoborockMqttClientV1.get_prop",
+        return_value=prop,
+    ):
+        async_fire_time_changed(
+            hass, dt_util.utcnow() + interval - timedelta(seconds=5)
+        )
+        assert hass.states.get("sensor.roborock_s7_maxv_battery").state == "100"
+        async_fire_time_changed(hass, dt_util.utcnow() + interval)
+
+    assert hass.states.get("sensor.roborock_s7_maxv_battery").state == "20"
+
+
+@pytest.mark.parametrize(
+    ("interval", "in_cleaning"),
+    [
+        (V1_LOCAL_IN_CLEANING_INTERVAL, 1),
+        (V1_LOCAL_NOT_CLEANING_INTERVAL, 0),
+    ],
+)
+async def test_dynamic_local_scan_interval(
+    hass: HomeAssistant,
+    mock_roborock_entry: MockConfigEntry,
+    bypass_api_fixture_v1_only,
+    interval: timedelta,
+    in_cleaning: int,
+) -> None:
+    """Test dynamic scan interval."""
+    prop = copy.deepcopy(PROP)
+    prop.status.in_cleaning = in_cleaning
+    with (
+        patch(
+            "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.get_prop",
+            return_value=prop,
+        ),
+    ):
+        await hass.config_entries.async_setup(mock_roborock_entry.entry_id)
+    assert hass.states.get("sensor.roborock_s7_maxv_battery").state == "100"
+    prop = copy.deepcopy(prop)
+    prop.status.battery = 20
+    with patch(
+        "homeassistant.components.roborock.coordinator.RoborockLocalClientV1.get_prop",
+        return_value=prop,
+    ):
+        async_fire_time_changed(
+            hass, dt_util.utcnow() + interval - timedelta(seconds=5)
+        )
+        assert hass.states.get("sensor.roborock_s7_maxv_battery").state == "100"
+
+        async_fire_time_changed(hass, dt_util.utcnow() + interval)
+
+    assert hass.states.get("sensor.roborock_s7_maxv_battery").state == "20"