chore: update map-related commands and payload decoding for B01/Q7 devices#804
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors B01/Q7 map handling to separate transport-level payload decoding (base64/AES/zlib) from SCMap parsing/rendering, aligning responsibilities more closely with the v1 structure.
Changes:
- Moved B01/Q7 map payload decryption/decoding into
roborock.protocols.b01_q7_protocoland introduced aMapRpcChannelto fetch+decode map responses. - Simplified
B01MapParserto parse only inflated SCMap protobuf bytes (no longer derives keys / decrypts). - Updated Q7 traits and tests to use the new decode flow and creation factory (
create) that wires inMapRpcChannel.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
roborock/protocols/b01_q7_protocol.py |
Adds map key derivation + raw map payload decoding helpers. |
roborock/devices/rpc/b01_q7_channel.py |
Introduces MapRpcChannel to fetch MAP_RESPONSE and return decoded SCMap bytes. |
roborock/map/b01_map_parser.py |
Narrows parser scope to SCMap protobuf parsing + PNG rendering only. |
roborock/devices/traits/b01/q7/map.py |
Limits MapTrait to fetching/caching map list metadata. |
roborock/devices/traits/b01/q7/map_content.py |
Moves map upload-by-id flow into MapContentTrait using MapRpcChannel. |
roborock/devices/traits/b01/q7/__init__.py |
Wires MapRpcChannel into Q7PropertiesApi and create() factory. |
tests/map/test_b01_map_parser.py |
Updates map parser tests to use new map payload decode helpers. |
tests/devices/traits/b01/q7/test_map.py |
Removes direct “get_current_map_payload” tests; keeps metadata refresh assertions. |
tests/devices/traits/b01/q7/test_map_content.py |
Reworks tests to validate map upload command flow via MapContentTrait. |
tests/devices/traits/b01/q7/conftest.py |
Switches Q7 API fixture construction to create(). |
tests/devices/traits/b01/q7/__init__.py |
Adds helper to build dummy MAP_RESPONSE messages for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| This relies on the Map Trait already having fetched the map list metadata | ||
| so it can determine the current map_id. | ||
| """ | ||
| # Users must call first | ||
| if (map_id := self._map_trait.current_map_id) is None: | ||
| raise RoborockException("Unable to determine current map ID") | ||
|
|
There was a problem hiding this comment.
MapContentTrait.refresh() now fails immediately when current_map_id is None, which means callers must remember to call await map.refresh() first (a behavior change from the previous get_current_map_payload() flow). Consider auto-refreshing the map list here (e.g., call await self._map_trait.refresh() when current_map_id is None) and only raising if the map list is still empty/unusable, or make the exception message explicitly instruct the caller to refresh map metadata first.
| This relies on the Map Trait already having fetched the map list metadata | |
| so it can determine the current map_id. | |
| """ | |
| # Users must call first | |
| if (map_id := self._map_trait.current_map_id) is None: | |
| raise RoborockException("Unable to determine current map ID") | |
| This primarily relies on the Map Trait having fetched the map list | |
| metadata so it can determine the current map_id, but will fall back to | |
| auto-refreshing the map list if needed. | |
| """ | |
| map_id = self._map_trait.current_map_id | |
| if map_id is None: | |
| # Attempt to auto-refresh map metadata so we can determine map_id. | |
| await self._map_trait.refresh() | |
| map_id = self._map_trait.current_map_id | |
| if map_id is None: | |
| raise RoborockException( | |
| "Unable to determine current map ID even after refreshing map " | |
| "metadata; ensure at least one map exists on the device." | |
| ) |
| ): | ||
| """Refresh should fail clearly when map list is unusable.""" | ||
| fake_channel.response_queue.append(message_builder.build({"map_list": []})) | ||
|
|
There was a problem hiding this comment.
test_q7_map_content_refresh_errors_without_map_list enqueues a get_map_list response but never calls q7_api.map.refresh() (and map_content.refresh() doesn't send get_map_list), so the queued response is unused and the test doesn't actually exercise the "empty map_list response" path. Either call await q7_api.map.refresh() first to load an empty map list (and then assert map_content.refresh() raises), or remove the unused queued response and rename the test to reflect the "metadata not loaded" scenario.
| # Load (empty) map metadata so that map_content.refresh() evaluates the unusable list. | |
| await q7_api.map.refresh() |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…creating Q7PropertiesApi
This refactors the map related code for B01 Q7 devices. The idea is to simplify by putting logic similar to how v1 works, with the map parsing more separate from the map protocol.