Skip to content

Commit c576d5f

Browse files
allenporterCopilot
andauthored
feat: Allow device manager to perform rediscovery of devices (#674)
* feat: Allow device manager to perform rediscovery of devices This adds an opton to skip the cache, but also supports fallback to the old device list. Adds tests exercising redisocvery and various cache falblack cases on failure. * chore: Fix pydoc string Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * chore: style cleanup re-raising a bare exception Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * chore: Remove unnecessary assert in test Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 5520a56 commit c576d5f

3 files changed

Lines changed: 108 additions & 8 deletions

File tree

roborock/devices/device_manager.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
UserData,
1616
)
1717
from roborock.devices.device import DeviceReadyCallback, RoborockDevice
18+
from roborock.exceptions import RoborockException
1819
from roborock.map.map_parser import MapParserConfig
1920
from roborock.mqtt.roborock_session import create_lazy_mqtt_session
2021
from roborock.mqtt.session import MqttSession
@@ -68,12 +69,17 @@ def __init__(
6869
self._devices: dict[str, RoborockDevice] = {}
6970
self._mqtt_session = mqtt_session
7071

71-
async def discover_devices(self) -> list[RoborockDevice]:
72+
async def discover_devices(self, prefer_cache: bool = True) -> list[RoborockDevice]:
7273
"""Discover all devices for the logged-in user."""
7374
cache_data = await self._cache.get()
74-
if not cache_data.home_data:
75-
_LOGGER.debug("No cached home data found, fetching from API")
76-
cache_data.home_data = await self._web_api.get_home_data()
75+
if not cache_data.home_data or not prefer_cache:
76+
_LOGGER.debug("Fetching home data (prefer_cache=%s)", prefer_cache)
77+
try:
78+
cache_data.home_data = await self._web_api.get_home_data()
79+
except RoborockException as ex:
80+
if not cache_data.home_data:
81+
raise
82+
_LOGGER.debug("Failed to fetch home data, using cached data: %s", ex)
7783
await self._cache.set(cache_data)
7884
home_data = cache_data.home_data
7985

tests/devices/test_device_manager.py

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import asyncio
44
import datetime
55
from collections.abc import Generator, Iterator
6+
from typing import Any
67
from unittest.mock import AsyncMock, Mock, patch
78

89
import pytest
@@ -149,7 +150,8 @@ async def test_create_home_data_api_exception() -> None:
149150
await api.get_home_data()
150151

151152

152-
async def test_cache_logic() -> None:
153+
@pytest.mark.parametrize(("prefer_cache", "expected_call_count"), [(True, 1), (False, 2)])
154+
async def test_cache_logic(prefer_cache: bool, expected_call_count: int) -> None:
153155
"""Test that the cache logic works correctly."""
154156
call_count = 0
155157

@@ -167,8 +169,8 @@ async def mock_home_data_with_counter(*args, **kwargs) -> HomeData:
167169
assert call_count == 1
168170

169171
# Second call should use cache, not increment call_count
170-
devices2 = await device_manager.discover_devices()
171-
assert call_count == 1 # Should still be 1, not 2
172+
devices2 = await device_manager.discover_devices(prefer_cache=prefer_cache)
173+
assert call_count == expected_call_count
172174
assert len(devices2) == 1
173175

174176
await device_manager.close()
@@ -178,6 +180,29 @@ async def mock_home_data_with_counter(*args, **kwargs) -> HomeData:
178180
await device_manager.close()
179181

180182

183+
async def test_home_data_api_fails_with_cache_fallback() -> None:
184+
"""Test that home data exceptions may still fall back to use the cache when available."""
185+
186+
cache = InMemoryCache()
187+
cache_data = await cache.get()
188+
cache_data.home_data = HomeData.from_dict(mock_data.HOME_DATA_RAW)
189+
await cache.set(cache_data)
190+
191+
with patch(
192+
"roborock.devices.device_manager.RoborockApiClient.get_home_data_v3",
193+
side_effect=RoborockException("Test exception"),
194+
):
195+
# This call will skip the API and use the cache
196+
device_manager = await create_device_manager(USER_PARAMS, cache=cache)
197+
198+
# This call will hit the API since we're not preferring the cache
199+
# but will fallback to the cache data on exception
200+
devices2 = await device_manager.discover_devices(prefer_cache=False)
201+
assert len(devices2) == 1
202+
203+
await device_manager.close()
204+
205+
181206
async def test_ready_callback(home_data: HomeData) -> None:
182207
"""Test that the ready callback is invoked when a device connects."""
183208
ready_devices: list[RoborockDevice] = []
@@ -245,6 +270,74 @@ async def test_start_connect_failure(home_data: HomeData, channel_failure: Mock,
245270
assert mock_unsub.call_count == 1
246271

247272

273+
async def test_rediscover_devices(mock_rpc_channel: AsyncMock) -> None:
274+
"""Test that we can discover devices multiple times and discover new devices."""
275+
raw_devices: list[dict[str, Any]] = mock_data.HOME_DATA_RAW["devices"]
276+
assert len(raw_devices) > 0
277+
raw_device_1 = raw_devices[0]
278+
279+
home_data_responses = [
280+
HomeData.from_dict(mock_data.HOME_DATA_RAW),
281+
# New device added on second call. We make a copy and updated fields to simulate
282+
# a new device.
283+
HomeData.from_dict(
284+
{
285+
**mock_data.HOME_DATA_RAW,
286+
"devices": [
287+
raw_device_1,
288+
{
289+
**raw_device_1,
290+
"duid": "new_device_duid",
291+
"name": "New Device",
292+
"model": "roborock.newmodel.v1",
293+
"mac": "00:11:22:33:44:55",
294+
},
295+
],
296+
}
297+
),
298+
]
299+
300+
mock_rpc_channel.send_command.side_effect = [
301+
[mock_data.APP_GET_INIT_STATUS],
302+
mock_data.STATUS,
303+
# Device #2
304+
[mock_data.APP_GET_INIT_STATUS],
305+
mock_data.STATUS,
306+
]
307+
308+
async def mock_home_data_with_counter(*args, **kwargs) -> HomeData:
309+
nonlocal home_data_responses
310+
return home_data_responses.pop(0)
311+
312+
# First call happens during create_device_manager initialization
313+
with patch(
314+
"roborock.devices.device_manager.RoborockApiClient.get_home_data_v3",
315+
side_effect=mock_home_data_with_counter,
316+
):
317+
device_manager = await create_device_manager(USER_PARAMS, cache=InMemoryCache())
318+
assert len(await device_manager.get_devices()) == 1
319+
320+
# Second call should use cache and does not add new device
321+
await device_manager.discover_devices(prefer_cache=True)
322+
assert len(await device_manager.get_devices()) == 1
323+
324+
# Third call should fetch new home data and add the new device
325+
await device_manager.discover_devices(prefer_cache=False)
326+
assert len(await device_manager.get_devices()) == 2
327+
328+
# Verify the two devices exist with correct data
329+
device_1 = await device_manager.get_device("abc123")
330+
assert device_1 is not None
331+
assert device_1.name == "Roborock S7 MaxV"
332+
333+
new_device = await device_manager.get_device("new_device_duid")
334+
assert new_device
335+
assert new_device.name == "New Device"
336+
337+
# Ensure closing again works without error
338+
await device_manager.close()
339+
340+
248341
@pytest.mark.parametrize(
249342
("channel_exception"),
250343
[

tests/mock_data.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import hashlib
44
import json
5+
from typing import Any
56

67
# All data is based on a U.S. customer with a Roborock S7 MaxV Ultra
78
USER_EMAIL = "user@domain.com"
@@ -119,7 +120,7 @@
119120
"type": "WORKFLOW",
120121
}
121122
]
122-
HOME_DATA_RAW = {
123+
HOME_DATA_RAW: dict[str, Any] = {
123124
"id": 123456,
124125
"name": "My Home",
125126
"lon": None,

0 commit comments

Comments
 (0)