From c18baff1cec3607dfb7821424d93f811743cf662 Mon Sep 17 00:00:00 2001 From: Luke Date: Thu, 15 Jan 2026 20:54:06 -0500 Subject: [PATCH 1/3] fix: handle different error format for map status --- roborock/devices/traits/v1/home.py | 24 ++++++++++++++- roborock/protocols/v1_protocol.py | 33 ++++++++++++++------- tests/devices/traits/v1/test_home.py | 44 ++++++++++++++++++++++++++++ tests/protocols/test_v1_protocol.py | 22 ++++++++++++++ 4 files changed, 111 insertions(+), 12 deletions(-) diff --git a/roborock/devices/traits/v1/home.py b/roborock/devices/traits/v1/home.py index b1f94747..cca49f38 100644 --- a/roborock/devices/traits/v1/home.py +++ b/roborock/devices/traits/v1/home.py @@ -37,6 +37,20 @@ MAP_SLEEP = 3 +def _extract_api_error_code(err: RoborockException) -> int | None: + """Extract an API error code from a RoborockException, if present. + + V1 RPC error responses typically look like: {"code": -10007, "message": "..."}. + """ + if not err.args: + return None + payload = err.args[0] + if isinstance(payload, dict): + code = payload.get("code") + return code if isinstance(code, int) else None + return None + + class HomeTrait(RoborockBase, common.V1TraitMixin): """Trait that represents a full view of the home layout.""" @@ -150,7 +164,15 @@ async def _build_home_map_info(self) -> tuple[dict[int, CombinedMapInfo], dict[i # We need to load each map to get its room data if len(sorted_map_infos) > 1: _LOGGER.debug("Loading map %s", map_info.map_flag) - await self._maps_trait.set_current_map(map_info.map_flag) + try: + await self._maps_trait.set_current_map(map_info.map_flag) + except RoborockException as ex: + # Some firmware revisions return -10007 ("invalid status"/action locked) when attempting + # to switch maps while the device is in a state that forbids it. Treat this as a + # "busy" condition so callers can fall back to refreshing the current map only. + if _extract_api_error_code(ex) == -10007: + raise RoborockDeviceBusy("Cannot switch maps right now (device action locked)") from ex + raise await asyncio.sleep(MAP_SLEEP) map_content = await self._refresh_map_content() diff --git a/roborock/protocols/v1_protocol.py b/roborock/protocols/v1_protocol.py index 86f8b4e8..84b2da29 100644 --- a/roborock/protocols/v1_protocol.py +++ b/roborock/protocols/v1_protocol.py @@ -154,26 +154,37 @@ def decode_rpc_response(message: RoborockMessage) -> ResponseMessage: ) from e request_id: int | None = data_point_response.get("id") - exc: RoborockException | None = None + api_error: RoborockException | None = None if error := data_point_response.get("error"): - exc = RoborockException(error) + api_error = RoborockException(error) + if (result := data_point_response.get("result")) is None: - exc = RoborockException(f"Invalid V1 message format: missing 'result' in data point for {message.payload!r}") + # Some firmware versions return an error-only response (no "result" key). + # Preserve that error instead of overwriting it with a parsing exception. + if api_error is None: + api_error = RoborockException( + f"Invalid V1 message format: missing 'result' in data point for {message.payload!r}" + ) else: _LOGGER.debug("Decoded V1 message result: %s", result) if isinstance(result, str): if result == "unknown_method": - exc = RoborockUnsupportedFeature("The method called is not recognized by the device.") + api_error = RoborockUnsupportedFeature("The method called is not recognized by the device.") elif result != "ok": - exc = RoborockException(f"Unexpected API Result: {result}") + api_error = RoborockException(f"Unexpected API Result: {result}") result = {} if not isinstance(result, dict | list | int): - raise RoborockException( - f"Invalid V1 message format: 'result' was unexpected type {type(result)}. {message.payload!r}" - ) - if not request_id and exc: - raise exc - return ResponseMessage(request_id=request_id, data=result, api_error=exc) + # If we already have an API error, prefer returning a response object + # rather than failing to decode the message entirely. + if api_error is None: + raise RoborockException( + f"Invalid V1 message format: 'result' was unexpected type {type(result)}. {message.payload!r}" + ) + result = {} + + if not request_id and api_error: + raise api_error + return ResponseMessage(request_id=request_id, data=result, api_error=api_error) @dataclass diff --git a/tests/devices/traits/v1/test_home.py b/tests/devices/traits/v1/test_home.py index e42570ca..86ceccd7 100644 --- a/tests/devices/traits/v1/test_home.py +++ b/tests/devices/traits/v1/test_home.py @@ -538,6 +538,50 @@ async def test_discover_home_device_busy_cleaning( assert len(device_cache_data.home_map_content_base64) == 2 +async def test_refresh_falls_back_when_map_switch_action_locked( + status_trait: StatusTrait, + home_trait: HomeTrait, + mock_rpc_channel: AsyncMock, + mock_mqtt_rpc_channel: AsyncMock, + mock_map_rpc_channel: AsyncMock, + device_cache: DeviceCache, +) -> None: + """Test that refresh falls back to current map when map switching is locked.""" + # Discovery attempt: we can list maps, but switching maps fails with -10007. + mock_mqtt_rpc_channel.send_command.side_effect = [ + MULTI_MAP_LIST_DATA, # Maps refresh during discover_home() + RoborockException({"code": -10007, "message": "invalid status"}), # LOAD_MULTI_MAP action locked + MULTI_MAP_LIST_DATA, # Maps refresh during refresh() fallback + ] + + # Fallback refresh should still be able to refresh the current map. + mock_rpc_channel.send_command.side_effect = [ + ROOM_MAPPING_DATA_MAP_0, # Rooms for current map + ] + mock_map_rpc_channel.send_command.side_effect = [ + MAP_BYTES_RESPONSE_1, # Map bytes for current map + ] + + await home_trait.refresh() + + current_data = home_trait.current_map_data + assert current_data is not None + assert current_data.map_flag == 0 + assert current_data.name == "Ground Floor" + + assert home_trait.home_map_info is not None + assert home_trait.home_map_info.keys() == {0} + assert home_trait.home_map_content is not None + assert home_trait.home_map_content.keys() == {0} + map_0_content = home_trait.home_map_content[0] + assert map_0_content.image_content == TEST_IMAGE_CONTENT_1 + + # Discovery did not complete, so the persistent cache should not be updated. + cache_data = await device_cache.get() + assert not cache_data.home_map_info + assert not cache_data.home_map_content_base64 + + async def test_single_map_no_switching( home_trait: HomeTrait, mock_rpc_channel: AsyncMock, diff --git a/tests/protocols/test_v1_protocol.py b/tests/protocols/test_v1_protocol.py index 86195f96..1ec5026e 100644 --- a/tests/protocols/test_v1_protocol.py +++ b/tests/protocols/test_v1_protocol.py @@ -275,6 +275,28 @@ def test_decode_no_request_id(): decode_rpc_response(message) +def test_decode_error_without_result() -> None: + """Test decoding an error-only V1 RPC response (no 'result' key).""" + payload = ( + b'{"t":1768419740,"dps":{"102":"{\\"id\\":20062,\\"error\\":{\\"code\\":-10007,' + b'\\"message\\":\\"invalid status\\"}}"}}' + ) + message = RoborockMessage( + protocol=RoborockMessageProtocol.GENERAL_RESPONSE, + payload=payload, + seq=12750, + version=b"1.0", + random=97431, + timestamp=1768419740, + ) + decoded_message = decode_rpc_response(message) + assert decoded_message.request_id == 20062 + assert decoded_message.data == {} + assert decoded_message.api_error + assert isinstance(decoded_message.api_error.args[0], dict) + assert decoded_message.api_error.args[0]["code"] == -10007 + + def test_invalid_unicode() -> None: """Test an error while decoding unicode bytes""" message = RoborockMessage( From 3dc64de937df4e43b7f4d94a69eeb91111ed1d9a Mon Sep 17 00:00:00 2001 From: Luke Date: Thu, 15 Jan 2026 21:07:23 -0500 Subject: [PATCH 2/3] chore: move extract api --- roborock/devices/traits/v1/common.py | 15 +++++++++++++++ roborock/devices/traits/v1/home.py | 16 +--------------- roborock/protocols/v1_protocol.py | 1 + 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/roborock/devices/traits/v1/common.py b/roborock/devices/traits/v1/common.py index 63ae2e20..8cfa7205 100644 --- a/roborock/devices/traits/v1/common.py +++ b/roborock/devices/traits/v1/common.py @@ -9,6 +9,7 @@ from typing import ClassVar, Self from roborock.data import RoborockBase +from roborock.exceptions import RoborockException from roborock.protocols.v1_protocol import V1RpcChannel from roborock.roborock_typing import RoborockCommand @@ -17,6 +18,20 @@ V1ResponseData = dict | list | int | str +def extract_v1_api_error_code(err: RoborockException) -> int | None: + """Extract a V1 RPC API error code from a RoborockException, if present. + + V1 RPC error responses typically look like: {"code": -10007, "message": "..."}. + """ + if not err.args: + return None + payload = err.args[0] + if isinstance(payload, dict): + code = payload.get("code") + return code if isinstance(code, int) else None + return None + + @dataclass class V1TraitMixin(ABC): """Base model that supports v1 traits. diff --git a/roborock/devices/traits/v1/home.py b/roborock/devices/traits/v1/home.py index cca49f38..c3996e91 100644 --- a/roborock/devices/traits/v1/home.py +++ b/roborock/devices/traits/v1/home.py @@ -37,20 +37,6 @@ MAP_SLEEP = 3 -def _extract_api_error_code(err: RoborockException) -> int | None: - """Extract an API error code from a RoborockException, if present. - - V1 RPC error responses typically look like: {"code": -10007, "message": "..."}. - """ - if not err.args: - return None - payload = err.args[0] - if isinstance(payload, dict): - code = payload.get("code") - return code if isinstance(code, int) else None - return None - - class HomeTrait(RoborockBase, common.V1TraitMixin): """Trait that represents a full view of the home layout.""" @@ -170,7 +156,7 @@ async def _build_home_map_info(self) -> tuple[dict[int, CombinedMapInfo], dict[i # Some firmware revisions return -10007 ("invalid status"/action locked) when attempting # to switch maps while the device is in a state that forbids it. Treat this as a # "busy" condition so callers can fall back to refreshing the current map only. - if _extract_api_error_code(ex) == -10007: + if common.extract_v1_api_error_code(ex) == -10007: raise RoborockDeviceBusy("Cannot switch maps right now (device action locked)") from ex raise await asyncio.sleep(MAP_SLEEP) diff --git a/roborock/protocols/v1_protocol.py b/roborock/protocols/v1_protocol.py index 84b2da29..38f433a3 100644 --- a/roborock/protocols/v1_protocol.py +++ b/roborock/protocols/v1_protocol.py @@ -165,6 +165,7 @@ def decode_rpc_response(message: RoborockMessage) -> ResponseMessage: api_error = RoborockException( f"Invalid V1 message format: missing 'result' in data point for {message.payload!r}" ) + result = {} else: _LOGGER.debug("Decoded V1 message result: %s", result) if isinstance(result, str): From e6b4e2b8fe4943d8257617ed66fe850a9a08bdf0 Mon Sep 17 00:00:00 2001 From: Luke Date: Sat, 17 Jan 2026 16:38:05 -0500 Subject: [PATCH 3/3] chore: make v1_protocol responsible for parsing api error code --- roborock/devices/traits/v1/common.py | 15 --------------- roborock/devices/traits/v1/home.py | 13 +++++-------- roborock/exceptions.py | 4 ++++ roborock/protocols/v1_protocol.py | 22 ++++++++++++++++++++-- tests/devices/traits/v1/test_home.py | 4 ++-- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/roborock/devices/traits/v1/common.py b/roborock/devices/traits/v1/common.py index 8cfa7205..63ae2e20 100644 --- a/roborock/devices/traits/v1/common.py +++ b/roborock/devices/traits/v1/common.py @@ -9,7 +9,6 @@ from typing import ClassVar, Self from roborock.data import RoborockBase -from roborock.exceptions import RoborockException from roborock.protocols.v1_protocol import V1RpcChannel from roborock.roborock_typing import RoborockCommand @@ -18,20 +17,6 @@ V1ResponseData = dict | list | int | str -def extract_v1_api_error_code(err: RoborockException) -> int | None: - """Extract a V1 RPC API error code from a RoborockException, if present. - - V1 RPC error responses typically look like: {"code": -10007, "message": "..."}. - """ - if not err.args: - return None - payload = err.args[0] - if isinstance(payload, dict): - code = payload.get("code") - return code if isinstance(code, int) else None - return None - - @dataclass class V1TraitMixin(ABC): """Base model that supports v1 traits. diff --git a/roborock/devices/traits/v1/home.py b/roborock/devices/traits/v1/home.py index c3996e91..f1688457 100644 --- a/roborock/devices/traits/v1/home.py +++ b/roborock/devices/traits/v1/home.py @@ -24,7 +24,7 @@ from roborock.data.v1.v1_code_mappings import RoborockStateCode from roborock.devices.cache import DeviceCache from roborock.devices.traits.v1 import common -from roborock.exceptions import RoborockDeviceBusy, RoborockException +from roborock.exceptions import RoborockDeviceBusy, RoborockException, RoborockInvalidStatus from roborock.roborock_typing import RoborockCommand from .map_content import MapContent, MapContentTrait @@ -152,13 +152,10 @@ async def _build_home_map_info(self) -> tuple[dict[int, CombinedMapInfo], dict[i _LOGGER.debug("Loading map %s", map_info.map_flag) try: await self._maps_trait.set_current_map(map_info.map_flag) - except RoborockException as ex: - # Some firmware revisions return -10007 ("invalid status"/action locked) when attempting - # to switch maps while the device is in a state that forbids it. Treat this as a - # "busy" condition so callers can fall back to refreshing the current map only. - if common.extract_v1_api_error_code(ex) == -10007: - raise RoborockDeviceBusy("Cannot switch maps right now (device action locked)") from ex - raise + except RoborockInvalidStatus as ex: + # Device is in a state that forbids map switching. Translate to + # "busy" so callers can fall back to refreshing the current map only. + raise RoborockDeviceBusy("Cannot switch maps right now (device action locked)") from ex await asyncio.sleep(MAP_SLEEP) map_content = await self._refresh_map_content() diff --git a/roborock/exceptions.py b/roborock/exceptions.py index 48fcd7be..3c5b8295 100644 --- a/roborock/exceptions.py +++ b/roborock/exceptions.py @@ -87,5 +87,9 @@ class RoborockDeviceBusy(RoborockException): """Class for Roborock device busy exceptions.""" +class RoborockInvalidStatus(RoborockException): + """Class for Roborock invalid status exceptions (device action locked).""" + + class RoborockUnsupportedFeature(RoborockException): """Class for Roborock unsupported feature exceptions.""" diff --git a/roborock/protocols/v1_protocol.py b/roborock/protocols/v1_protocol.py index 38f433a3..355043c5 100644 --- a/roborock/protocols/v1_protocol.py +++ b/roborock/protocols/v1_protocol.py @@ -13,7 +13,7 @@ from typing import Any, Protocol, TypeVar, overload from roborock.data import RoborockBase, RRiot -from roborock.exceptions import RoborockException, RoborockUnsupportedFeature +from roborock.exceptions import RoborockException, RoborockInvalidStatus, RoborockUnsupportedFeature from roborock.protocol import Utils from roborock.roborock_message import RoborockMessage, RoborockMessageProtocol from roborock.roborock_typing import RoborockCommand @@ -106,6 +106,24 @@ def _as_payload(self, security_data: SecurityData | None) -> bytes: ResponseData = dict[str, Any] | list | int +# V1 RPC error code mappings to specific exception types +_V1_ERROR_CODE_EXCEPTIONS: dict[int, type[RoborockException]] = { + -10007: RoborockInvalidStatus, # "invalid status" - device action locked +} + + +def _create_api_error(error: Any) -> RoborockException: + """Create an appropriate exception for a V1 RPC error response. + + Maps known error codes to specific exception types for easier handling + at higher levels. + """ + if isinstance(error, dict): + code = error.get("code") + if isinstance(code, int) and (exc_type := _V1_ERROR_CODE_EXCEPTIONS.get(code)): + return exc_type(error) + return RoborockException(error) + @dataclass(kw_only=True, frozen=True) class ResponseMessage: @@ -156,7 +174,7 @@ def decode_rpc_response(message: RoborockMessage) -> ResponseMessage: request_id: int | None = data_point_response.get("id") api_error: RoborockException | None = None if error := data_point_response.get("error"): - api_error = RoborockException(error) + api_error = _create_api_error(error) if (result := data_point_response.get("result")) is None: # Some firmware versions return an error-only response (no "result" key). diff --git a/tests/devices/traits/v1/test_home.py b/tests/devices/traits/v1/test_home.py index 86ceccd7..6a63cde7 100644 --- a/tests/devices/traits/v1/test_home.py +++ b/tests/devices/traits/v1/test_home.py @@ -15,7 +15,7 @@ from roborock.devices.traits.v1.maps import MapsTrait from roborock.devices.traits.v1.rooms import RoomsTrait from roborock.devices.traits.v1.status import StatusTrait -from roborock.exceptions import RoborockDeviceBusy, RoborockException +from roborock.exceptions import RoborockDeviceBusy, RoborockException, RoborockInvalidStatus from roborock.map.map_parser import ParsedMapData from roborock.roborock_typing import RoborockCommand from tests import mock_data @@ -550,7 +550,7 @@ async def test_refresh_falls_back_when_map_switch_action_locked( # Discovery attempt: we can list maps, but switching maps fails with -10007. mock_mqtt_rpc_channel.send_command.side_effect = [ MULTI_MAP_LIST_DATA, # Maps refresh during discover_home() - RoborockException({"code": -10007, "message": "invalid status"}), # LOAD_MULTI_MAP action locked + RoborockInvalidStatus({"code": -10007, "message": "invalid status"}), # LOAD_MULTI_MAP action locked MULTI_MAP_LIST_DATA, # Maps refresh during refresh() fallback ]