Skip to content

Commit 9897379

Browse files
authored
fix: handle different error format for map status (#744)
1 parent 85fc958 commit 9897379

File tree

5 files changed

+120
-15
lines changed

5 files changed

+120
-15
lines changed

roborock/devices/traits/v1/home.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
from roborock.data.v1.v1_code_mappings import RoborockStateCode
2525
from roborock.devices.cache import DeviceCache
2626
from roborock.devices.traits.v1 import common
27-
from roborock.exceptions import RoborockDeviceBusy, RoborockException
27+
from roborock.exceptions import RoborockDeviceBusy, RoborockException, RoborockInvalidStatus
2828
from roborock.roborock_typing import RoborockCommand
2929

3030
from .map_content import MapContent, MapContentTrait
@@ -171,7 +171,12 @@ async def _build_home_map_info(self) -> tuple[dict[int, CombinedMapInfo], dict[i
171171
# We need to load each map to get its room data
172172
if len(sorted_map_infos) > 1:
173173
_LOGGER.debug("Loading map %s", map_info.map_flag)
174-
await self._maps_trait.set_current_map(map_info.map_flag)
174+
try:
175+
await self._maps_trait.set_current_map(map_info.map_flag)
176+
except RoborockInvalidStatus as ex:
177+
# Device is in a state that forbids map switching. Translate to
178+
# "busy" so callers can fall back to refreshing the current map only.
179+
raise RoborockDeviceBusy("Cannot switch maps right now (device action locked)") from ex
175180
await asyncio.sleep(MAP_SLEEP)
176181

177182
map_content = await self._refresh_map_content()

roborock/exceptions.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,5 +87,9 @@ class RoborockDeviceBusy(RoborockException):
8787
"""Class for Roborock device busy exceptions."""
8888

8989

90+
class RoborockInvalidStatus(RoborockException):
91+
"""Class for Roborock invalid status exceptions (device action locked)."""
92+
93+
9094
class RoborockUnsupportedFeature(RoborockException):
9195
"""Class for Roborock unsupported feature exceptions."""

roborock/protocols/v1_protocol.py

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from typing import Any, Protocol, TypeVar, overload
1414

1515
from roborock.data import RoborockBase, RRiot
16-
from roborock.exceptions import RoborockException, RoborockUnsupportedFeature
16+
from roborock.exceptions import RoborockException, RoborockInvalidStatus, RoborockUnsupportedFeature
1717
from roborock.protocol import Utils
1818
from roborock.roborock_message import RoborockMessage, RoborockMessageProtocol
1919
from roborock.roborock_typing import RoborockCommand
@@ -106,6 +106,24 @@ def _as_payload(self, security_data: SecurityData | None) -> bytes:
106106

107107
ResponseData = dict[str, Any] | list | int
108108

109+
# V1 RPC error code mappings to specific exception types
110+
_V1_ERROR_CODE_EXCEPTIONS: dict[int, type[RoborockException]] = {
111+
-10007: RoborockInvalidStatus, # "invalid status" - device action locked
112+
}
113+
114+
115+
def _create_api_error(error: Any) -> RoborockException:
116+
"""Create an appropriate exception for a V1 RPC error response.
117+
118+
Maps known error codes to specific exception types for easier handling
119+
at higher levels.
120+
"""
121+
if isinstance(error, dict):
122+
code = error.get("code")
123+
if isinstance(code, int) and (exc_type := _V1_ERROR_CODE_EXCEPTIONS.get(code)):
124+
return exc_type(error)
125+
return RoborockException(error)
126+
109127

110128
@dataclass(kw_only=True, frozen=True)
111129
class ResponseMessage:
@@ -154,26 +172,38 @@ def decode_rpc_response(message: RoborockMessage) -> ResponseMessage:
154172
) from e
155173

156174
request_id: int | None = data_point_response.get("id")
157-
exc: RoborockException | None = None
175+
api_error: RoborockException | None = None
158176
if error := data_point_response.get("error"):
159-
exc = RoborockException(error)
177+
api_error = _create_api_error(error)
178+
160179
if (result := data_point_response.get("result")) is None:
161-
exc = RoborockException(f"Invalid V1 message format: missing 'result' in data point for {message.payload!r}")
180+
# Some firmware versions return an error-only response (no "result" key).
181+
# Preserve that error instead of overwriting it with a parsing exception.
182+
if api_error is None:
183+
api_error = RoborockException(
184+
f"Invalid V1 message format: missing 'result' in data point for {message.payload!r}"
185+
)
186+
result = {}
162187
else:
163188
_LOGGER.debug("Decoded V1 message result: %s", result)
164189
if isinstance(result, str):
165190
if result == "unknown_method":
166-
exc = RoborockUnsupportedFeature("The method called is not recognized by the device.")
191+
api_error = RoborockUnsupportedFeature("The method called is not recognized by the device.")
167192
elif result != "ok":
168-
exc = RoborockException(f"Unexpected API Result: {result}")
193+
api_error = RoborockException(f"Unexpected API Result: {result}")
169194
result = {}
170195
if not isinstance(result, dict | list | int):
171-
raise RoborockException(
172-
f"Invalid V1 message format: 'result' was unexpected type {type(result)}. {message.payload!r}"
173-
)
174-
if not request_id and exc:
175-
raise exc
176-
return ResponseMessage(request_id=request_id, data=result, api_error=exc)
196+
# If we already have an API error, prefer returning a response object
197+
# rather than failing to decode the message entirely.
198+
if api_error is None:
199+
raise RoborockException(
200+
f"Invalid V1 message format: 'result' was unexpected type {type(result)}. {message.payload!r}"
201+
)
202+
result = {}
203+
204+
if not request_id and api_error:
205+
raise api_error
206+
return ResponseMessage(request_id=request_id, data=result, api_error=api_error)
177207

178208

179209
@dataclass

tests/devices/traits/v1/test_home.py

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
from roborock.devices.traits.v1.maps import MapsTrait
1717
from roborock.devices.traits.v1.rooms import RoomsTrait
1818
from roborock.devices.traits.v1.status import StatusTrait
19-
from roborock.exceptions import RoborockDeviceBusy, RoborockException
19+
from roborock.exceptions import RoborockDeviceBusy, RoborockException, RoborockInvalidStatus
2020
from roborock.map.map_parser import ParsedMapData
2121
from roborock.roborock_typing import RoborockCommand
2222
from tests import mock_data
@@ -539,6 +539,50 @@ async def test_discover_home_device_busy_cleaning(
539539
assert len(device_cache_data.home_map_content_base64) == 2
540540

541541

542+
async def test_refresh_falls_back_when_map_switch_action_locked(
543+
status_trait: StatusTrait,
544+
home_trait: HomeTrait,
545+
mock_rpc_channel: AsyncMock,
546+
mock_mqtt_rpc_channel: AsyncMock,
547+
mock_map_rpc_channel: AsyncMock,
548+
device_cache: DeviceCache,
549+
) -> None:
550+
"""Test that refresh falls back to current map when map switching is locked."""
551+
# Discovery attempt: we can list maps, but switching maps fails with -10007.
552+
mock_mqtt_rpc_channel.send_command.side_effect = [
553+
MULTI_MAP_LIST_DATA, # Maps refresh during discover_home()
554+
RoborockInvalidStatus({"code": -10007, "message": "invalid status"}), # LOAD_MULTI_MAP action locked
555+
MULTI_MAP_LIST_DATA, # Maps refresh during refresh() fallback
556+
]
557+
558+
# Fallback refresh should still be able to refresh the current map.
559+
mock_rpc_channel.send_command.side_effect = [
560+
ROOM_MAPPING_DATA_MAP_0, # Rooms for current map
561+
]
562+
mock_map_rpc_channel.send_command.side_effect = [
563+
MAP_BYTES_RESPONSE_1, # Map bytes for current map
564+
]
565+
566+
await home_trait.refresh()
567+
568+
current_data = home_trait.current_map_data
569+
assert current_data is not None
570+
assert current_data.map_flag == 0
571+
assert current_data.name == "Ground Floor"
572+
573+
assert home_trait.home_map_info is not None
574+
assert home_trait.home_map_info.keys() == {0}
575+
assert home_trait.home_map_content is not None
576+
assert home_trait.home_map_content.keys() == {0}
577+
map_0_content = home_trait.home_map_content[0]
578+
assert map_0_content.image_content == TEST_IMAGE_CONTENT_1
579+
580+
# Discovery did not complete, so the persistent cache should not be updated.
581+
cache_data = await device_cache.get()
582+
assert not cache_data.home_map_info
583+
assert not cache_data.home_map_content_base64
584+
585+
542586
async def test_single_map_no_switching(
543587
home_trait: HomeTrait,
544588
mock_rpc_channel: AsyncMock,

tests/protocols/test_v1_protocol.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,28 @@ def test_decode_no_request_id():
275275
decode_rpc_response(message)
276276

277277

278+
def test_decode_error_without_result() -> None:
279+
"""Test decoding an error-only V1 RPC response (no 'result' key)."""
280+
payload = (
281+
b'{"t":1768419740,"dps":{"102":"{\\"id\\":20062,\\"error\\":{\\"code\\":-10007,'
282+
b'\\"message\\":\\"invalid status\\"}}"}}'
283+
)
284+
message = RoborockMessage(
285+
protocol=RoborockMessageProtocol.GENERAL_RESPONSE,
286+
payload=payload,
287+
seq=12750,
288+
version=b"1.0",
289+
random=97431,
290+
timestamp=1768419740,
291+
)
292+
decoded_message = decode_rpc_response(message)
293+
assert decoded_message.request_id == 20062
294+
assert decoded_message.data == {}
295+
assert decoded_message.api_error
296+
assert isinstance(decoded_message.api_error.args[0], dict)
297+
assert decoded_message.api_error.args[0]["code"] == -10007
298+
299+
278300
def test_invalid_unicode() -> None:
279301
"""Test an error while decoding unicode bytes"""
280302
message = RoborockMessage(

0 commit comments

Comments
 (0)