Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions roborock/devices/traits/v1/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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.
Expand Down
10 changes: 9 additions & 1 deletion roborock/devices/traits/v1/home.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,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 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)

map_content = await self._refresh_map_content()
Expand Down
34 changes: 23 additions & 11 deletions roborock/protocols/v1_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,26 +154,38 @@ 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:
Comment thread
Lash-L marked this conversation as resolved.
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}"
)
result = {}
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
Expand Down
44 changes: 44 additions & 0 deletions tests/devices/traits/v1/test_home.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling of 'apropriate' to 'appropriate' (note: this occurs in the PR description, not in code)

Copilot uses AI. Check for mistakes.
# 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,
Expand Down
22 changes: 22 additions & 0 deletions tests/protocols/test_v1_protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading