-
Notifications
You must be signed in to change notification settings - Fork 75
q7/b01: command-layer segment clean + map payload retrieval helpers (split 1/3) #778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 6 commits
2d03d6a
861b338
632b160
733159c
a20ae01
0b8f6fe
211f486
76531ad
4887362
8303a8c
71b785f
88bdb42
3ca8043
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,23 +21,33 @@ | |
| from roborock.roborock_typing import RoborockB01Q7Methods | ||
|
|
||
| from .clean_summary import CleanSummaryTrait | ||
| from .map import MapTrait, Q7MapList, Q7MapListEntry | ||
|
|
||
| __all__ = [ | ||
| "Q7PropertiesApi", | ||
| "CleanSummaryTrait", | ||
| "MapTrait", | ||
| "Q7MapList", | ||
| "Q7MapListEntry", | ||
| ] | ||
|
|
||
| _Q7_DPS = 10000 | ||
|
|
||
|
|
||
| class Q7PropertiesApi(Trait): | ||
| """API for interacting with B01 devices.""" | ||
|
|
||
| clean_summary: CleanSummaryTrait | ||
| """Trait for clean records / clean summary (Q7 `service.get_record_list`).""" | ||
|
|
||
| map: MapTrait | ||
| """Trait for map list metadata + raw map payload retrieval.""" | ||
|
|
||
| def __init__(self, channel: MqttChannel) -> None: | ||
| """Initialize the B01Props API.""" | ||
| self._channel = channel | ||
| self.clean_summary = CleanSummaryTrait(channel) | ||
| self.map = MapTrait(channel) | ||
|
|
||
| async def query_values(self, props: list[RoborockB01Props]) -> B01Props | None: | ||
| """Query the device for the values of the given Q7 properties.""" | ||
|
|
@@ -87,6 +97,17 @@ async def start_clean(self) -> None: | |
| }, | ||
| ) | ||
|
|
||
| async def clean_segments(self, segment_ids: list[int]) -> None: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can live on the map trait. that way the interaction is .map.clean_segments(), but i'm not sold. Thoughts @allenporter ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like a "vacuum" command since its asking it to clean? But i get the point that segments are part of a map. |
||
| """Start segment cleaning for the given ids (Q7 uses room ids).""" | ||
| await self.send( | ||
| command=RoborockB01Q7Methods.SET_ROOM_CLEAN, | ||
| params={ | ||
| "clean_type": CleanTaskTypeMapping.ROOM.code, | ||
| "ctrl_value": SCDeviceCleanParam.START.code, | ||
| "room_ids": segment_ids, | ||
| }, | ||
| ) | ||
|
|
||
| async def pause_clean(self) -> None: | ||
| """Pause cleaning.""" | ||
| await self.send( | ||
|
|
@@ -127,7 +148,7 @@ async def send(self, command: CommandType, params: ParamsType) -> Any: | |
| """Send a command to the device.""" | ||
| return await send_decoded_command( | ||
| self._channel, | ||
| Q7RequestMessage(dps=10000, command=command, params=params), | ||
| Q7RequestMessage(dps=_Q7_DPS, command=command, params=params), | ||
| ) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,101 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| """Map trait for B01 Q7 devices.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||||||||||||||||||||||||
| from dataclasses import dataclass, field | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.data import RoborockBase | ||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.devices.rpc.b01_q7_channel import send_decoded_command, send_map_command | ||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.devices.traits import Trait | ||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.devices.transport.mqtt_channel import MqttChannel | ||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.exceptions import RoborockException | ||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.protocols.b01_q7_protocol import Q7RequestMessage | ||||||||||||||||||||||||||||||||||||||||||||||
| from roborock.roborock_typing import RoborockB01Q7Methods | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| _Q7_DPS = 10000 | ||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we don't need to define this constant multiple times if it is always the same |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @dataclass | ||||||||||||||||||||||||||||||||||||||||||||||
| class Q7MapListEntry(RoborockBase): | ||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, data package for b01 q7 |
||||||||||||||||||||||||||||||||||||||||||||||
| """Single map list entry returned by `service.get_map_list`.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| id: int | None = None | ||||||||||||||||||||||||||||||||||||||||||||||
| cur: bool | None = None | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @dataclass | ||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's move this into the data folder for b01 q7 |
||||||||||||||||||||||||||||||||||||||||||||||
| class Q7MapList(RoborockBase): | ||||||||||||||||||||||||||||||||||||||||||||||
| """Map list response returned by `service.get_map_list`.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| map_list: list[Q7MapListEntry] = field(default_factory=list) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class MapTrait(Trait): | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider making this subclass |
||||||||||||||||||||||||||||||||||||||||||||||
| """Map retrieval + map metadata helpers for Q7 devices.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, channel: MqttChannel) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||
| self._channel = channel | ||||||||||||||||||||||||||||||||||||||||||||||
| # Map uploads are serialized per-device to avoid response cross-wiring. | ||||||||||||||||||||||||||||||||||||||||||||||
| self._map_command_lock = asyncio.Lock() | ||||||||||||||||||||||||||||||||||||||||||||||
| self._map_list: Q7MapList | None = None | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||||||
| def map_list(self) -> Q7MapList | None: | ||||||||||||||||||||||||||||||||||||||||||||||
| """Latest cached map list metadata, populated by ``refresh()``.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| return self._map_list | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||||||||||||||||
| def current_map_id(self) -> int | None: | ||||||||||||||||||||||||||||||||||||||||||||||
| """Current map id derived from cached map list metadata.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| if self._map_list is None: | ||||||||||||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||||||||||||
| return self._extract_current_map_id(self._map_list) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| async def refresh(self) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||
| """Refresh cached map list metadata from the device.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| response = await send_decoded_command( | ||||||||||||||||||||||||||||||||||||||||||||||
| self._channel, | ||||||||||||||||||||||||||||||||||||||||||||||
| Q7RequestMessage(dps=_Q7_DPS, command=RoborockB01Q7Methods.GET_MAP_LIST, params={}), | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| if not isinstance(response, dict): | ||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(f"Unexpected response type for GET_MAP_LIST: {type(response).__name__}: {response!r}") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| parsed = Q7MapList.from_dict(response) | ||||||||||||||||||||||||||||||||||||||||||||||
| if parsed is None: | ||||||||||||||||||||||||||||||||||||||||||||||
| raise TypeError(f"Failed to decode map list response: {response!r}") | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| self._map_list = parsed | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think callers won't necessarily except a TypeError here so we could rewrite as this:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| async def _get_map_payload(self, *, map_id: int) -> bytes: | ||||||||||||||||||||||||||||||||||||||||||||||
| """Fetch raw map payload bytes for the given map id.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| request = Q7RequestMessage( | ||||||||||||||||||||||||||||||||||||||||||||||
| dps=_Q7_DPS, | ||||||||||||||||||||||||||||||||||||||||||||||
| command=RoborockB01Q7Methods.UPLOAD_BY_MAPID, | ||||||||||||||||||||||||||||||||||||||||||||||
| params={"map_id": map_id}, | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| async with self._map_command_lock: | ||||||||||||||||||||||||||||||||||||||||||||||
| return await send_map_command(self._channel, request) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| async def get_current_map_payload(self) -> bytes: | ||||||||||||||||||||||||||||||||||||||||||||||
| """Fetch raw map payload bytes for the currently selected map.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| if self._map_list is None: | ||||||||||||||||||||||||||||||||||||||||||||||
| await self.refresh() | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| map_id = self.current_map_id | ||||||||||||||||||||||||||||||||||||||||||||||
| if map_id is None: | ||||||||||||||||||||||||||||||||||||||||||||||
| raise RoborockException(f"Unable to determine map_id from map list response: {self._map_list!r}") | ||||||||||||||||||||||||||||||||||||||||||||||
| return await self._get_map_payload(map_id=map_id) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| @staticmethod | ||||||||||||||||||||||||||||||||||||||||||||||
| def _extract_current_map_id(map_list_response: Q7MapList) -> int | None: | ||||||||||||||||||||||||||||||||||||||||||||||
| map_list = map_list_response.map_list | ||||||||||||||||||||||||||||||||||||||||||||||
| if not map_list: | ||||||||||||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| for entry in map_list: | ||||||||||||||||||||||||||||||||||||||||||||||
| if entry.cur and isinstance(entry.id, int): | ||||||||||||||||||||||||||||||||||||||||||||||
| return entry.id | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| first = map_list[0] | ||||||||||||||||||||||||||||||||||||||||||||||
| if isinstance(first.id, int): | ||||||||||||||||||||||||||||||||||||||||||||||
| return first.id | ||||||||||||||||||||||||||||||||||||||||||||||
| return None | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can simplify this to work like this:
Suggested change
I would also say add |
||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ | |
| from roborock.devices.traits.b01.q7 import Q7PropertiesApi | ||
| from roborock.exceptions import RoborockException | ||
| from roborock.protocols.b01_q7_protocol import B01_VERSION, Q7RequestMessage | ||
| from roborock.roborock_message import RoborockB01Props, RoborockMessageProtocol | ||
| from roborock.roborock_message import RoborockB01Props, RoborockMessage, RoborockMessageProtocol | ||
| from tests.fixtures.channel_fixtures import FakeChannel | ||
|
|
||
| from . import B01MessageBuilder | ||
|
|
@@ -257,3 +257,108 @@ async def test_q7_api_find_me(q7_api: Q7PropertiesApi, fake_channel: FakeChannel | |
| payload_data = json.loads(unpad(message.payload, AES.block_size)) | ||
| assert payload_data["dps"]["10000"]["method"] == "service.find_device" | ||
| assert payload_data["dps"]["10000"]["params"] == {} | ||
|
|
||
|
|
||
| async def test_q7_api_clean_segments( | ||
| q7_api: Q7PropertiesApi, fake_channel: FakeChannel, message_builder: B01MessageBuilder | ||
| ): | ||
| """Test room/segment cleaning helper for Q7.""" | ||
| fake_channel.response_queue.append(message_builder.build({"result": "ok"})) | ||
| await q7_api.clean_segments([10, 11]) | ||
|
|
||
| assert len(fake_channel.published_messages) == 1 | ||
| message = fake_channel.published_messages[0] | ||
| payload_data = json.loads(unpad(message.payload, AES.block_size)) | ||
| assert payload_data["dps"]["10000"]["method"] == "service.set_room_clean" | ||
| assert payload_data["dps"]["10000"]["params"] == { | ||
| "clean_type": CleanTaskTypeMapping.ROOM.code, | ||
| "ctrl_value": SCDeviceCleanParam.START.code, | ||
| "room_ids": [10, 11], | ||
| } | ||
|
|
||
|
|
||
| async def test_q7_api_get_current_map_payload( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These can live in |
||
| q7_api: Q7PropertiesApi, | ||
| fake_channel: FakeChannel, | ||
| message_builder: B01MessageBuilder, | ||
| ): | ||
| """Fetch current map by map-list lookup, then upload_by_mapid.""" | ||
| fake_channel.response_queue.append(message_builder.build({"map_list": [{"id": 1772093512, "cur": True}]})) | ||
| fake_channel.response_queue.append( | ||
| RoborockMessage( | ||
| protocol=RoborockMessageProtocol.MAP_RESPONSE, | ||
| payload=b"raw-map-payload", | ||
| version=b"B01", | ||
| seq=message_builder.seq + 1, | ||
| ) | ||
| ) | ||
|
|
||
| raw_payload = await q7_api.map.get_current_map_payload() | ||
| assert raw_payload == b"raw-map-payload" | ||
|
|
||
| assert len(fake_channel.published_messages) == 2 | ||
|
|
||
| first = fake_channel.published_messages[0] | ||
| first_payload = json.loads(unpad(first.payload, AES.block_size)) | ||
| assert first_payload["dps"]["10000"]["method"] == "service.get_map_list" | ||
| assert first_payload["dps"]["10000"]["params"] == {} | ||
|
|
||
| second = fake_channel.published_messages[1] | ||
| second_payload = json.loads(unpad(second.payload, AES.block_size)) | ||
| assert second_payload["dps"]["10000"]["method"] == "service.upload_by_mapid" | ||
| assert second_payload["dps"]["10000"]["params"] == {"map_id": 1772093512} | ||
|
|
||
|
|
||
| async def test_q7_api_map_trait_refresh_populates_cached_values( | ||
| q7_api: Q7PropertiesApi, | ||
| fake_channel: FakeChannel, | ||
| message_builder: B01MessageBuilder, | ||
| ): | ||
| """Map trait follows refresh + cached-value access pattern.""" | ||
| fake_channel.response_queue.append(message_builder.build({"map_list": [{"id": 101, "cur": True}]})) | ||
|
|
||
| assert q7_api.map.map_list is None | ||
| assert q7_api.map.current_map_id is None | ||
|
|
||
| await q7_api.map.refresh() | ||
|
|
||
| assert len(fake_channel.published_messages) == 1 | ||
| assert q7_api.map.map_list is not None | ||
| assert q7_api.map.map_list.map_list[0].id == 101 | ||
| assert q7_api.map.map_list.map_list[0].cur is True | ||
| assert q7_api.map.current_map_id == 101 | ||
|
|
||
|
|
||
| async def test_q7_api_get_current_map_payload_falls_back_to_first_map( | ||
| q7_api: Q7PropertiesApi, | ||
| fake_channel: FakeChannel, | ||
| message_builder: B01MessageBuilder, | ||
| ): | ||
| """If no current map marker exists, first map in list is used.""" | ||
| fake_channel.response_queue.append(message_builder.build({"map_list": [{"id": 111}, {"id": 222, "cur": False}]})) | ||
| fake_channel.response_queue.append( | ||
| RoborockMessage( | ||
| protocol=RoborockMessageProtocol.MAP_RESPONSE, | ||
| payload=b"raw-map-payload", | ||
| version=b"B01", | ||
| seq=message_builder.seq + 1, | ||
| ) | ||
| ) | ||
|
|
||
| await q7_api.map.get_current_map_payload() | ||
|
|
||
| second = fake_channel.published_messages[1] | ||
| second_payload = json.loads(unpad(second.payload, AES.block_size)) | ||
| assert second_payload["dps"]["10000"]["params"] == {"map_id": 111} | ||
|
|
||
|
|
||
| async def test_q7_api_get_current_map_payload_errors_without_map_list( | ||
| q7_api: Q7PropertiesApi, | ||
| fake_channel: FakeChannel, | ||
| message_builder: B01MessageBuilder, | ||
| ): | ||
| """Current-map payload fetch should fail clearly when map list is unusable.""" | ||
| fake_channel.response_queue.append(message_builder.build({"map_list": []})) | ||
|
|
||
| with pytest.raises(RoborockException, match="Unable to determine map_id"): | ||
| await q7_api.map.get_current_map_payload() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't this use the send_decoded_command() in b01_q7_channel?