-
Notifications
You must be signed in to change notification settings - Fork 66
Add Q7 map_content support and room segment cleaning helper #774
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
base: main
Are you sure you want to change the base?
Changes from all commits
ac0c93b
0eb5720
acca342
b4fb0d4
aea98ed
3a0b944
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 |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| import asyncio | ||
| import json | ||
| import logging | ||
| import weakref | ||
| from typing import Any | ||
|
|
||
| from roborock.devices.transport.mqtt_channel import MqttChannel | ||
|
|
@@ -14,10 +15,19 @@ | |
| decode_rpc_response, | ||
| encode_mqtt_payload, | ||
| ) | ||
| from roborock.roborock_message import RoborockMessage | ||
| from roborock.roborock_message import RoborockMessage, RoborockMessageProtocol | ||
|
|
||
| _LOGGER = logging.getLogger(__name__) | ||
| _TIMEOUT = 10.0 | ||
| _map_command_locks: weakref.WeakKeyDictionary[MqttChannel, asyncio.Lock] = weakref.WeakKeyDictionary() | ||
|
|
||
|
|
||
| def _get_map_command_lock(mqtt_channel: MqttChannel) -> asyncio.Lock: | ||
| lock = _map_command_locks.get(mqtt_channel) | ||
| if lock is None: | ||
| lock = asyncio.Lock() | ||
| _map_command_locks[mqtt_channel] = lock | ||
| return lock | ||
|
|
||
|
|
||
| async def send_decoded_command( | ||
|
|
@@ -99,3 +109,62 @@ def find_response(response_message: RoborockMessage) -> None: | |
| raise | ||
| finally: | ||
| unsub() | ||
|
|
||
|
|
||
| async def send_map_command(mqtt_channel: MqttChannel, request_message: Q7RequestMessage) -> bytes: | ||
| """Send map upload command and wait for MAP_RESPONSE payload bytes. | ||
|
|
||
| Map requests are serialized per channel so concurrent map calls cannot | ||
| cross-wire responses between callers. | ||
| """ | ||
|
|
||
| async with _get_map_command_lock(mqtt_channel): | ||
| roborock_message = encode_mqtt_payload(request_message) | ||
| future: asyncio.Future[bytes] = asyncio.get_running_loop().create_future() | ||
|
|
||
| def find_response(response_message: RoborockMessage) -> None: | ||
| if future.done(): | ||
| return | ||
|
|
||
| if response_message.protocol == RoborockMessageProtocol.MAP_RESPONSE and response_message.payload: | ||
| if not future.done(): | ||
| future.set_result(response_message.payload) | ||
| return | ||
|
|
||
| try: | ||
|
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. Does this happen in practice that we get a normal protocol response back or is this speculative? I'm wondering if this could have a high level comment describing the scenarios in which we observe this happening. Second, i think this is complex enough that likely we'd want to reuse the common parts across both protocols, in a way that is easy to read. |
||
| decoded_dps = decode_rpc_response(response_message) | ||
| except RoborockException: | ||
| return | ||
|
|
||
| for dps_value in decoded_dps.values(): | ||
| if not isinstance(dps_value, str): | ||
| continue | ||
| try: | ||
| inner = json.loads(dps_value) | ||
| except (json.JSONDecodeError, TypeError): | ||
| continue | ||
| if not isinstance(inner, dict) or inner.get("msgId") != str(request_message.msg_id): | ||
| continue | ||
| code = inner.get("code", 0) | ||
| if code != 0: | ||
| if not future.done(): | ||
| future.set_exception( | ||
| RoborockException(f"B01 command failed with code {code} ({request_message})") | ||
| ) | ||
| return | ||
| data = inner.get("data") | ||
| if isinstance(data, dict) and isinstance(data.get("payload"), str): | ||
| try: | ||
| if not future.done(): | ||
| future.set_result(bytes.fromhex(data["payload"])) | ||
| except ValueError: | ||
| pass | ||
|
|
||
| unsub = await mqtt_channel.subscribe(find_response) | ||
| try: | ||
| await mqtt_channel.publish(roborock_message) | ||
| return await asyncio.wait_for(future, timeout=_TIMEOUT) | ||
| except TimeoutError as ex: | ||
| raise RoborockException(f"B01 map command timed out after {_TIMEOUT}s ({request_message})") from ex | ||
| finally: | ||
| unsub() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| """Map content trait for B01/Q7 devices.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass | ||
| from typing import Any | ||
|
|
||
| from roborock.devices.rpc.b01_q7_channel import send_decoded_command, send_map_command | ||
| from roborock.devices.traits import Trait | ||
| from roborock.devices.traits.v1.map_content import MapContent | ||
|
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. Let's not directly depend on the traits of another device type. For now, create a new object with the fields we need here rather than reusing this and we can decide to reuse in the future. The protocols are different enough that I don't think we'll have calling code or anything that will treat these map objects the same. |
||
| from roborock.devices.transport.mqtt_channel import MqttChannel | ||
| from roborock.exceptions import RoborockException | ||
| from roborock.map.b01_map_parser import decode_b01_map_payload, parse_scmap_payload, render_map_png | ||
| from roborock.protocols.b01_q7_protocol import Q7RequestMessage | ||
| from roborock.roborock_typing import RoborockB01Q7Methods | ||
|
|
||
|
|
||
| @dataclass | ||
| class B01MapContent(MapContent): | ||
| """B01 map content wrapper.""" | ||
|
|
||
| rooms: dict[int, str] | None = None | ||
|
|
||
|
|
||
| def _extract_current_map_id(map_list_response: dict[str, Any] | None) -> int | None: | ||
| if not isinstance(map_list_response, dict): | ||
| return None | ||
| map_list = map_list_response.get("map_list") | ||
| if not isinstance(map_list, list) or not map_list: | ||
| return None | ||
|
|
||
| for entry in map_list: | ||
| if isinstance(entry, dict) and entry.get("cur") and isinstance(entry.get("id"), int): | ||
| return entry["id"] | ||
|
|
||
| first = map_list[0] | ||
| if isinstance(first, dict) and isinstance(first.get("id"), int): | ||
| return first["id"] | ||
| return None | ||
|
|
||
|
|
||
| class Q7MapContentTrait(B01MapContent, Trait): | ||
| """Fetch and parse map content from B01/Q7 devices.""" | ||
|
|
||
| def __init__(self, channel: MqttChannel, *, local_key: str, serial: str, model: str) -> None: | ||
| super().__init__() | ||
| self._channel = channel | ||
| self._local_key = local_key | ||
| self._serial = serial | ||
| self._model = model | ||
|
|
||
| async def refresh(self) -> B01MapContent: | ||
| map_list_response = await send_decoded_command( | ||
|
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. In v1 we have a separate trait for holding on to the map list and current map We decided to separate them in v1 just because there was a lot of existing code to unwind and introduced the Can you review the existing MapsTrait / HomeTrait / MapContentTrait for v1 and consider what would be a reasonable first step to take with that in mind? I do worry that getting the |
||
| self._channel, | ||
| Q7RequestMessage(dps=10000, command=RoborockB01Q7Methods.GET_MAP_LIST, params={}), | ||
|
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. Can we extract a named constant for |
||
| ) | ||
| map_id = _extract_current_map_id(map_list_response) | ||
| if map_id is None: | ||
| raise RoborockException(f"Unable to determine map_id from map list response: {map_list_response!r}") | ||
|
|
||
| raw_payload = await send_map_command( | ||
| self._channel, | ||
| Q7RequestMessage( | ||
| dps=10000, | ||
| command=RoborockB01Q7Methods.UPLOAD_BY_MAPID, | ||
| params={"map_id": map_id}, | ||
| ), | ||
| ) | ||
| inflated = decode_b01_map_payload( | ||
| raw_payload, | ||
| local_key=self._local_key, | ||
| serial=self._serial, | ||
| model=self._model, | ||
| ) | ||
| parsed = parse_scmap_payload(inflated) | ||
| self.raw_api_response = raw_payload | ||
| self.map_data = None | ||
| self.rooms = parsed.rooms | ||
| self.image_content = render_map_png(parsed) | ||
| return self | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,13 @@ | ||
| """Module for Roborock map related data classes.""" | ||
| """Utilities and data classes for working with Roborock maps.""" | ||
|
|
||
| from .b01_map_parser import B01MapData, decode_b01_map_payload, parse_scmap_payload, render_map_png | ||
| from .map_parser import MapParserConfig, ParsedMapData | ||
|
|
||
| __all__ = [ | ||
| "B01MapData", | ||
| "MapParserConfig", | ||
| "ParsedMapData", | ||
| "decode_b01_map_payload", | ||
| "parse_scmap_payload", | ||
| "render_map_png", | ||
| ] |
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.
Please move the
asyncio.Lockto the trait, which is one per device. We don't need to do this indirectly via a map to the channel as we can just do it there before calling into this map function.