diff --git a/roborock/cli.py b/roborock/cli.py index 9def26af..89123186 100644 --- a/roborock/cli.py +++ b/roborock/cli.py @@ -594,32 +594,34 @@ async def maps(ctx, device_id: str): await _display_v1_trait(context, device_id, lambda v1: v1.maps) -# The Q10 pushes its map ~9s after a dpRequestDps; firmware throttles pushes to -# ~once per 60-70s, so a single request is answered quickly but rapid re-requests -# may not be. This bounds how long a one-shot CLI command waits for that push. +# The Q10 publishes its map asynchronously after a dpMultiMap list/get request. +# Firmware throttles pushes to ~once per 60-70s, so rapid re-requests may not be +# answered immediately. This bounds how long a one-shot CLI command waits. _Q10_MAP_PUSH_TIMEOUT = 30.0 async def _await_q10_map_push( properties: Q10PropertiesApi, predicate: Callable[[], bool], + generation: Callable[[], int], *, timeout: float = _Q10_MAP_PUSH_TIMEOUT, allow_cached_on_timeout: bool = False, ) -> bool: """Nudge a Q10 to push its map/trace and wait for a fresh update. - The Q10 map API is entirely push-driven: there is no synchronous get-map - request. A ``dpRequestDps`` causes the device to publish a ``MAP_RESPONSE``, - which the device's subscribe loop feeds into the map trait. Here we register - an update listener, send the request, and wait for a newly pushed update to - satisfy ``predicate``. Returns whether it did within ``timeout``. + The Q10 map response remains asynchronous: ``refresh`` starts a + ``dpMultiMap`` list/get exchange, after which the device publishes a + ``MAP_RESPONSE`` that its subscribe loop feeds into the map trait. Here we + register an update listener, send the request, and wait for a newly pushed + update to satisfy ``predicate``. Returns whether it did within ``timeout``. """ loop = asyncio.get_running_loop() updated: asyncio.Future[None] = loop.create_future() + initial_generation = generation() def on_update() -> None: - if predicate() and not updated.done(): + if generation() != initial_generation and predicate() and not updated.done(): updated.set_result(None) unsub = properties.map.add_update_listener(on_update) @@ -648,6 +650,7 @@ async def map_image(ctx, device_id: str, output_file: str): await _await_q10_map_push( properties, lambda: properties.map.image_content is not None, + lambda: properties.map.map_generation, allow_cached_on_timeout=True, ) image_content = properties.map.image_content @@ -706,7 +709,11 @@ async def q10_position(ctx, device_id: str, include_path: bool): click.echo("Feature not supported by device") return properties = device.b01_q10_properties - got_trace = await _await_q10_map_push(properties, lambda: bool(properties.map.path)) + got_trace = await _await_q10_map_push( + properties, + lambda: bool(properties.map.path), + lambda: properties.map.trace_generation, + ) if not got_trace: click.echo("No live trace available (the robot only reports position while cleaning).") return @@ -871,6 +878,7 @@ async def rooms(ctx, device_id: str): await _await_q10_map_push( properties, lambda: properties.map.image_content is not None, + lambda: properties.map.map_generation, allow_cached_on_timeout=True, ) click.echo(dump_json({room.id: room.name for room in properties.map.rooms})) diff --git a/roborock/devices/traits/b01/q10/__init__.py b/roborock/devices/traits/b01/q10/__init__.py index 3c8c73ff..4c774b0c 100644 --- a/roborock/devices/traits/b01/q10/__init__.py +++ b/roborock/devices/traits/b01/q10/__init__.py @@ -2,10 +2,12 @@ import asyncio import logging +from typing import Any from roborock.data.b01_q10.b01_q10_code_mappings import B01_Q10_DP from roborock.devices.rpc.b01_q10_channel import B01Q10Channel from roborock.devices.traits import Trait +from roborock.exceptions import RoborockException from roborock.map.b01_q10_map_parser import Q10MapPacket, Q10TracePacket from roborock.protocols.b01_q10_protocol import Q10DpsUpdate, Q10Message @@ -38,6 +40,24 @@ ] _LOGGER = logging.getLogger(__name__) +_MAP_LIST_REQUEST_TIMEOUT = 30.0 + + +def _map_id_from_list_response(response: Any) -> int | str | None: + """Return the first usable map ID from a ``dpMultiMap`` list response.""" + if not isinstance(response, dict) or response.get("op") != "list": + return None + data = response.get("data") + if not isinstance(data, list): + return None + for map_info in data: + if isinstance(map_info, dict): + map_id = map_info.get("id") + else: + map_id = map_info + if isinstance(map_id, (int, str)) and not isinstance(map_id, bool): + return map_id + return None class Q10PropertiesApi(Trait): @@ -100,7 +120,7 @@ def __init__(self, channel: B01Q10Channel) -> None: self.network_info = NetworkInfoTrait() self.consumable = ConsumableTrait() self._map_dps = MapDpsTrait() - self.map = MapContentTrait(self._map_dps) + self.map = MapContentTrait(self._map_dps, self.status) self.clean_history = CleanHistoryTrait(self.command) # Read-model traits updated from the device's DPS push stream. self._updatable_traits = [ @@ -115,6 +135,9 @@ def __init__(self, channel: B01Q10Channel) -> None: self._map_dps, ] self._subscribe_task: asyncio.Task[None] | None = None + self._map_request_lock = asyncio.Lock() + self._map_list_request_token: object | None = None + self._map_list_requested_at: float | None = None async def start(self) -> None: """Start any necessary subscriptions for the trait.""" @@ -132,22 +155,54 @@ async def close(self) -> None: async def refresh(self) -> None: """Refresh all traits.""" - # Sending the REQUEST_DPS will cause the device to send all DPS values - # to the device. Updates will be received by the subscribe loop below. + # Status and map retrieval use separate Q10 requests. A bare REQUEST_DPS + # reliably refreshes status but does not reliably make every firmware + # publish its map. await self.command.send(B01_Q10_DP.REQUEST_DPS, params={}) + await self.request_map() + + async def request_map(self) -> None: + """Request the current saved map through the Q10 multi-map protocol. + + The list response arrives asynchronously on the subscribe stream. + ``_handle_message`` extracts its first map ID and follows up with the + matching ``get`` command; the resulting protocol-301 map packet is + routed to :attr:`map`. + """ + async with self._map_request_lock: + now = asyncio.get_running_loop().time() + if ( + self._map_list_request_token is not None + and self._map_list_requested_at is not None + and now - self._map_list_requested_at < _MAP_LIST_REQUEST_TIMEOUT + ): + return + token = object() + self._map_list_request_token = token + self._map_list_requested_at = now + try: + await self.command.send( + B01_Q10_DP.COMMON, + {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}}, + ) + except BaseException: + if self._map_list_request_token is token: + self._map_list_request_token = None + self._map_list_requested_at = None + raise async def _subscribe_loop(self) -> None: """Persistent loop dispatching decoded messages to the read-model traits.""" async for message in self._channel.subscribe_stream(): - self._handle_message(message) + await self._handle_message(message) - def _handle_message(self, message: Q10Message) -> None: + async def _handle_message(self, message: Q10Message) -> None: """Route a single decoded message to the trait responsible for it. - Map and trace packets arrive as protocol-301 ``MAP_RESPONSE`` pushes (the - Q10 is entirely push-driven: there is no synchronous get-map request, a - ``dpRequestDps`` just nudges the device to publish its current map). DPS - updates feed the read-model traits. More traits can be dispatched here below. + Map and trace packets arrive as protocol-301 ``MAP_RESPONSE`` pushes. + A ``dpMultiMap`` list response completes the asynchronous request flow + started by :meth:`request_map`; other DPS updates feed the read-model + traits. """ if isinstance(message, Q10MapPacket): self.map.update_from_map_packet(message) @@ -157,8 +212,45 @@ def _handle_message(self, message: Q10Message) -> None: _LOGGER.debug("Received Q10 status update: %s", message.dps) # Notify all read-model traits about the new message; each trait # only updates the fields that it is responsible for. - for trait in self._updatable_traits: - trait.update_from_dps(message.dps) + self.map.begin_source_update() + try: + for trait in self._updatable_traits: + trait.update_from_dps(message.dps) + finally: + self.map.end_source_update() + await self._request_map_from_list_response(message.dps) + + async def _request_map_from_list_response(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None: + """Request map content after receiving our pending map-list response.""" + response = decoded_dps.get(B01_Q10_DP.MULTI_MAP) + if self._map_list_request_token is None or not isinstance(response, dict) or response.get("op") != "list": + return + + token = self._map_list_request_token + if (map_id := _map_id_from_list_response(response)) is None: + if self._map_list_request_token is token: + self._map_list_request_token = None + self._map_list_requested_at = None + _LOGGER.debug("Q10 map list response did not contain a usable map ID") + return + + try: + await self.command.send( + B01_Q10_DP.COMMON, + { + str(B01_Q10_DP.MULTI_MAP.code): { + "op": "get", + "id": map_id, + } + }, + ) + except RoborockException as ex: + # A failed follow-up must not kill the persistent subscribe loop. + _LOGGER.debug("Failed to request Q10 map content: %s", ex) + finally: + if self._map_list_request_token is token: + self._map_list_request_token = None + self._map_list_requested_at = None def create(channel: B01Q10Channel) -> Q10PropertiesApi: diff --git a/roborock/devices/traits/b01/q10/map.py b/roborock/devices/traits/b01/q10/map.py index ee51352a..97fd967b 100644 --- a/roborock/devices/traits/b01/q10/map.py +++ b/roborock/devices/traits/b01/q10/map.py @@ -1,15 +1,17 @@ """Push-driven map traits for B01 Q10 devices. -Map-related state arrives on three independent streams: +Map-related state arrives on four independent streams: * map packets are decoded from map-protocol responses; * trace packets are decoded from trace-protocol responses; * restricted zones and virtual walls arrive as ordinary DPS values. +* the device status indicates when an idle robot is charging at the saved dock. ``MapDpsTrait`` owns the low-level DPS read model. ``MapContentTrait`` depends -on it and combines that state with the latest map/trace packets through the pure -functions in :mod:`roborock.map.b01_q10_render`. The high-level trait keeps only -the latest value from each source and one replace-whole rendered image; +on it and the status trait, then combines that state with the latest map/trace +packets through the pure functions in :mod:`roborock.map.b01_q10_render`. The +high-level trait keeps only the latest value from each source and one +replace-whole rendered image; calibration, path placement and overlay placement remain inside the renderer. """ @@ -18,7 +20,7 @@ from typing import Any from roborock.data import RoborockBase -from roborock.data.b01_q10.b01_q10_code_mappings import B01_Q10_DP +from roborock.data.b01_q10.b01_q10_code_mappings import B01_Q10_DP, YXDeviceState from roborock.devices.traits.common import DpsDataConverter, TraitUpdateListener from roborock.exceptions import RoborockException from roborock.map.b01_q10_map_parser import ( @@ -32,8 +34,10 @@ from roborock.map.b01_q10_render import Q10MapOverlays, render_q10_map from .common import UpdatableTrait +from .status import StatusTrait _LOGGER = logging.getLogger(__name__) +_DOCKED_STATES = {YXDeviceState.CHARGING, YXDeviceState.EMPTYING_THE_BIN} @dataclass @@ -73,23 +77,31 @@ def update_from_dps(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None: class MapContentTrait(TraitUpdateListener): """High-level composed Q10 map view. - The latest map and trace packets are combined with the injected - :class:`MapDpsTrait` whenever any of those three sources changes. + The latest map and trace packets are combined with the injected map DPS and + status traits whenever any source changes. """ def __init__( self, map_dps: MapDpsTrait, + status: StatusTrait | None = None, *, map_parser_config: B01Q10MapParserConfig | None = None, ) -> None: TraitUpdateListener.__init__(self, logger=_LOGGER) self._config = map_parser_config or B01Q10MapParserConfig() self._map_dps = map_dps + self._status = status or StatusTrait() + self._robot_at_dock = self._status.status in _DOCKED_STATES self._map_packet: Q10MapPacket | None = None self._trace_packet: Q10TracePacket | None = None self._image_content: bytes | None = None + self._map_generation = 0 + self._trace_generation = 0 + self._source_update_depth = 0 + self._source_update_pending = False self._map_dps.add_update_listener(self._map_dps_updated) + self._status.add_update_listener(self._status_updated) @property def image_content(self) -> bytes | None: @@ -116,25 +128,72 @@ def robot_heading(self) -> int | None: """Current heading for orienting a robot marker on a caller-rendered map.""" return self._trace_packet.heading if self._trace_packet else None + @property + def map_generation(self) -> int: + """Number of map packets received by this trait.""" + return self._map_generation + + @property + def trace_generation(self) -> int: + """Number of trace packets received by this trait.""" + return self._trace_generation + def update_from_map_packet(self, packet: Q10MapPacket) -> None: """Store a map-protocol update and render the latest sources.""" self._map_packet = packet + self._map_generation += 1 self._render() self._notify_update() def update_from_trace_packet(self, packet: Q10TracePacket) -> None: """Store a trace-protocol update and render the latest sources.""" - self._trace_packet = packet + # A late packet from the completed clean cannot move a robot that the + # status stream already confirmed is docked. + self._trace_packet = None if self._robot_at_dock else packet + self._trace_generation += 1 self._render() self._notify_update() - def _map_dps_updated(self) -> None: - """Render after the low-level DPS source changes.""" - if self._map_packet is None: + def begin_source_update(self) -> None: + """Defer dependent rendering while one DPS message is applied.""" + self._source_update_depth += 1 + + def end_source_update(self) -> None: + """Render once after all traits consumed the same DPS message.""" + self._source_update_depth -= 1 + if self._source_update_depth == 0 and self._source_update_pending: + self._source_update_pending = False + self._render_and_notify() + + def _source_updated(self, *, notify_without_map: bool = False) -> None: + """Render now, or defer until the current DPS update is complete.""" + if self._map_packet is None and not notify_without_map: + return + if self._source_update_depth: + self._source_update_pending = True return + self._render_and_notify() + + def _render_and_notify(self) -> None: + """Recompose the current map and publish one update.""" self._render() self._notify_update() + def _map_dps_updated(self) -> None: + """Render after the low-level map DPS source changes.""" + self._source_updated() + + def _status_updated(self) -> None: + """Render only when the status changes whether the robot is docked.""" + robot_at_dock = self._status.status in _DOCKED_STATES + if robot_at_dock == self._robot_at_dock: + return + self._robot_at_dock = robot_at_dock + trace_cleared = robot_at_dock and self._trace_packet is not None + if robot_at_dock: + self._trace_packet = None + self._source_updated(notify_without_map=trace_cleared) + def _render(self) -> None: """Render the required map with the latest optional trace and overlays.""" if self._map_packet is None: @@ -145,6 +204,7 @@ def _render(self) -> None: self._trace_packet, self._map_dps.overlays, config=self._config, + robot_at_dock=self._robot_at_dock, ) except RoborockException as ex: _LOGGER.debug("Failed to render Q10 map packet: %s", ex) diff --git a/roborock/map/b01_q10_map_parser.py b/roborock/map/b01_q10_map_parser.py index 062f85f3..95e08cc3 100644 --- a/roborock/map/b01_q10_map_parser.py +++ b/roborock/map/b01_q10_map_parser.py @@ -1,7 +1,7 @@ """Parser for Roborock Q10 (B01/ss07) map packets. -Q10 devices deliver map data as a protocol-301 ``MAP_RESPONSE`` message (pushed a -few seconds after a ``dpRequestDps`` request). Unlike the Q7 ``SCMap`` protobuf +Q10 devices deliver map data as a protocol-301 ``MAP_RESPONSE`` message after a +``dpMultiMap`` list/get request. Unlike the Q7 ``SCMap`` protobuf format, the Q10 uses a custom, unencrypted binary packet: - ``01 01`` marker, then a ``u32be`` map id (bytes 2-5) and two consecutive @@ -19,15 +19,15 @@ https://github.com/v1b3c0d3x3r/roborock-qseries-map-bridge """ -import colorsys import io import math import statistics from dataclasses import dataclass, field, replace from PIL import Image +from vacuum_map_parser_base.config.color import ColorsPalette, SupportedColor from vacuum_map_parser_base.config.image_config import ImageConfig -from vacuum_map_parser_base.map_data import ImageData, MapData +from vacuum_map_parser_base.map_data import ImageData, MapData, Point from roborock.exceptions import RoborockException @@ -40,6 +40,7 @@ decompose_grid, ) from .map_parser import ParsedMapData +from .room_colors import adjacency_aware_room_colors _MAP_FILE_FORMAT = "PNG" @@ -93,15 +94,18 @@ def classify_q10_cell(value: int) -> str: # 50 mm/px, so dividing by 10 yields the origin in grid pixels -- the (ox, oy) # that solve_calibration otherwise recovers by sliding the path. # - 15-16 resolution (u16be): reads 5 (= 0.05 m/px = 50 mm/px) universally. -# - 17-18 charger x, 19-20 charger y (s16be, 5 mm units), 21-22 charger phi. +# - 17-18 charger x, 19-20 charger y (s16be, absolute map decipixels), +# 21-22 charger phi. The official app's device-point transform confirms these +# coordinates are already in the map-array frame and must not receive x_min / +# y_min a second time. _ORIGIN_X_OFFSET = 11 _ORIGIN_Y_OFFSET = 13 _HEADER_RESOLUTION_OFFSET = 15 _CHARGER_X_OFFSET = 17 _CHARGER_Y_OFFSET = 19 _CHARGER_PHI_OFFSET = 21 -# The header origin/charger are in 5 mm units and the grid is 50 mm/px, so a -# header coordinate maps to grid pixels by dividing by this. +# The header origin and charger fields both divide by 10 to reach grid pixels, +# although they use different coordinate frames as documented above. _HEADER_UNITS_PER_PIXEL = 10 # Grid cell values >= this are walls / borders rather than room segments. @@ -129,7 +133,8 @@ class Q10EraseZone: These are the app's *Erase* tool rectangles -- regions the user marked to be removed from the map (e.g. phantom floor the lidar mapped through windows). - Coordinates are world units (millimetres), same frame as the path/zones. + Coordinates are 5 mm world units, the same frame as restriction zones. + Trace points use a separate 2.5 mm scale. Confirmed by a controlled diff: removing the two erase zones on a live device dropped this section's count from 2 to 0 while the grid and the trailing @@ -148,9 +153,11 @@ class Q10HeaderCalibration: straight from the map packet -- no cleaning path / fit required, so it works docked or pre-clean. See :meth:`origin_pixels`. - ``origin_x`` / ``origin_y`` and the charger coordinates are in 5 mm units; - ``resolution`` is the raw header field (5 == 50 mm/px). ``charger_phi`` is the - raw dock heading field. Reported and verified by @andrewlyeats (ss07). + ``origin_x`` / ``origin_y`` are in 5 mm units and define the world-coordinate + origin. The charger coordinates are absolute decipixels in the map-array + frame, so they are divided by 10 without applying that origin again. + ``resolution`` is the raw header field (5 == 50 mm/px). ``charger_phi`` is + the raw dock heading field. Reported and verified by @andrewlyeats (ss07). """ origin_x: int @@ -176,6 +183,15 @@ def origin_pixels(self) -> tuple[float, float] | None: return None return (self.origin_x / _HEADER_UNITS_PER_PIXEL, self.origin_y / _HEADER_UNITS_PER_PIXEL) + def charger_pixels(self) -> tuple[float, float] | None: + """Return the saved dock in absolute map-array pixel coordinates.""" + if (self.charger_x == 0 and self.charger_y == 0) or self.charger_x == -1 or self.charger_y == -1: + return None + return ( + self.charger_x / _HEADER_UNITS_PER_PIXEL, + self.charger_y / _HEADER_UNITS_PER_PIXEL, + ) + @dataclass class Q10MapPacket: @@ -639,7 +655,12 @@ def parsed_from_packet(self, packet: Q10MapPacket) -> ParsedMapData: width=packet.width, image_config=ImageConfig(scale=self._config.map_scale), data=image, - img_transformation=lambda p: p, + # ImageDimensions uses V1's bottom-up map convention before + # projecting into the top-down PNG. Q10 points are already + # top-down grid pixels, so this adapter cancels that standard flip + # and lets Q10 use the shared V1 ImageGenerator without moving + # overlays vertically. + img_transformation=lambda p: Point(p.x, packet.height - p.y - 1, p.a), ) room_names = {room.id: room.name for room in packet.rooms} if room_names: @@ -655,12 +676,12 @@ def parsed_from_packet(self, packet: Q10MapPacket) -> ParsedMapData: return ParsedMapData(image_content=image_bytes.getvalue(), map_data=map_data) def _render(self, packet: Q10MapPacket) -> Image.Image: - """Render the Q10 grid: rooms get distinct colors, walls white, rest dark.""" - palette = _build_palette(packet.grid) - rgb = bytearray() + """Render the Q10 grid with the V1 map palette.""" + palette = _build_palette(packet.grid, packet.width) + rgba = bytearray() for value in packet.grid: - rgb.extend(palette[value]) - img = Image.frombytes("RGB", (packet.width, packet.height), bytes(rgb)) + rgba.extend(palette[value]) + img = Image.frombytes("RGBA", (packet.width, packet.height), bytes(rgba)) # The ss07 grid is stored top-down (row 0 = top of the home), so it is # rendered as-is -- unlike the V1/Q7 convention, no vertical flip. scale = self._config.map_scale @@ -669,15 +690,32 @@ def _render(self, packet: Q10MapPacket) -> Image.Image: return img -def _build_palette(grid: bytes) -> list[tuple[int, int, int]]: - """Map each grid value to an RGB color (rooms distinct, walls white).""" - palette: list[tuple[int, int, int]] = [(28, 30, 38)] * 256 # default: unknown/outside - room_values = sorted({v for v in set(grid) if 0 < v < _WALL_THRESHOLD}) - for index, value in enumerate(room_values): - hue = (index * 0.139) % 1.0 - r, g, b = colorsys.hsv_to_rgb(hue, 0.5, 0.95) - palette[value] = (int(r * 255), int(g * 255), int(b * 255)) +def _opaque(color: tuple[int, ...]) -> tuple[int, int, int, int]: + """Return a palette color as RGBA.""" + return (color[0], color[1], color[2], color[3] if len(color) == 4 else 255) + + +def _build_palette(grid: bytes, width: int) -> list[tuple[int, int, int, int]]: + """Map Q10 cells onto the same colors used by the V1 map renderer.""" + + def room_id(value: int) -> int | None: + return max(1, value // 4) if 0 < value < _WALL_THRESHOLD else None + + colors = ColorsPalette() + room_colors = adjacency_aware_room_colors( + grid, + width, + colors, + room_id, + ) + outside = (0, 0, 0, 0) + palette = [outside] * 256 + for value in {value for value in grid if 0 < value < _WALL_THRESHOLD}: + palette[value] = _opaque(room_colors[max(1, value // 4)]) + wall = _opaque(colors.get_color(SupportedColor.GREY_WALL)) for value in range(_WALL_THRESHOLD, 256): - palette[value] = (235, 235, 240) # walls / borders - palette[0] = (28, 30, 38) + palette[value] = wall + palette[_UNSEGMENTED_FLOOR_VALUE] = _opaque(colors.get_color(SupportedColor.MAP_INSIDE)) + palette[_BACKGROUND_VALUE] = outside + palette[0] = outside return palette diff --git a/roborock/map/b01_q10_overlays.py b/roborock/map/b01_q10_overlays.py index 45b9bd80..307ab210 100644 --- a/roborock/map/b01_q10_overlays.py +++ b/roborock/map/b01_q10_overlays.py @@ -19,11 +19,12 @@ zones (first wire word = x), confirmed against the app. Provenance and the byte-level breakdown are in PR #850's review thread. -Coordinates are in the device's world units (the same space as the cleaning -path), so a :class:`~roborock.map.b01_grid_layers.GridCalibration` maps them to -map pixels. ``type`` distinguishes the restriction kind (2 = no-mop, 3 = door -threshold, anything else -- incl. 0 -- a no-go zone); it is preserved verbatim -so callers can route polygons to the right ``MapData`` layer. +Coordinates are in 5 mm device world units, the same space as erase polygons. +Cleaning trace points use a separate 2.5 mm scale, so callers must not project +both through the same resolution. ``type`` distinguishes the restriction kind +(2 = no-mop, 3 = door threshold, anything else -- incl. 0 -- a no-go zone); it +is preserved verbatim so callers can route polygons to the right ``MapData`` +layer. """ import base64 diff --git a/roborock/map/b01_q10_render.py b/roborock/map/b01_q10_render.py index 8c241c98..4954d622 100644 --- a/roborock/map/b01_q10_render.py +++ b/roborock/map/b01_q10_render.py @@ -18,7 +18,8 @@ from collections.abc import Sequence from dataclasses import dataclass -from PIL import Image, ImageDraw +from vacuum_map_parser_base.config.drawable import Drawable +from vacuum_map_parser_base.config.size import Size, Sizes from vacuum_map_parser_base.map_data import Area, MapData, Path, Point, Wall from roborock.exceptions import RoborockException @@ -38,6 +39,7 @@ erased_packet, ) from .b01_q10_overlays import ZONE_TYPE_NO_GO, ZONE_TYPE_NO_MOP, Q10Zone +from .map_parser import DEFAULT_DRAWABLES, MapParserConfig, create_image_generator # Path-units-per-pixel candidates for calibration. A dense ss07 path lands a # best fit of 20.0 around the header origin -- ground-truthed June 2026 on the @@ -49,6 +51,10 @@ # reach 20 (it railed at the bound), biasing the fit. A dense cleaning path # selects the best fit within this bracket. _Q10_RESOLUTIONS = [step * 0.5 for step in range(24, 53)] # 12.0 .. 26.0 +# The header resolution is expressed in centimetres per grid pixel, while +# erase/restriction vectors use 5 mm units: one header unit therefore equals +# two vector units. Trace points use a separate 2.5 mm coordinate scale. +_Q10_VECTOR_UNITS_PER_HEADER_RESOLUTION_UNIT = 2.0 # A path needs enough shape to constrain a full (origin + resolution) fit; a few # points cannot. _MIN_CALIBRATION_POINTS = 20 @@ -57,6 +63,16 @@ # one). See :func:`solve_calibration_with_origin`. _MIN_HEADER_CALIBRATION_POINTS = 4 +_Q10_DRAWABLE_TYPES = { + Drawable.CHARGER, + Drawable.NO_GO_AREAS, + Drawable.NO_MOPPING_AREAS, + Drawable.PATH, + Drawable.VACUUM_POSITION, + Drawable.VIRTUAL_WALLS, +} +_Q10_DRAWABLES = [drawable for drawable in DEFAULT_DRAWABLES if drawable in _Q10_DRAWABLE_TYPES] + @dataclass(frozen=True) class Q10MapOverlays: @@ -72,21 +88,22 @@ def render_q10_map( overlays: Q10MapOverlays, *, config: B01Q10MapParserConfig, + robot_at_dock: bool = False, ) -> bytes: """Compose the latest map, trace and DPS inputs into one PNG image. - Calibration is derived from ``packet`` (layers + header calibration) and - ``trace`` (path points). Once calibrated, erase zones are blanked out of the - raster and trace/overlay data is projected and drawn in pixel space. - Without a usable trace only the base raster is rendered. Raises - :class:`RoborockException` if map rendering fails. + Separate transforms are derived for 2.5 mm trace points and 5 mm + erase/restriction vectors. Erase zones are blanked out of the raster, while + available trace and DPS overlays are projected and drawn in pixel space. + Raises :class:`RoborockException` if map rendering fails. """ parser = B01Q10MapParser(config) - calibration = solve_q10_calibration(packet, trace) + trace_calibration = solve_q10_calibration(packet, trace) + vector_calibration = _vector_calibration(packet, trace_calibration) render_packet = packet - if calibration is not None: - cells = _erased_cells(packet.layers, packet.erase_zones, calibration) + if vector_calibration is not None: + cells = _erased_cells(packet.layers, packet.erase_zones, vector_calibration) if cells: # Blank the erase-zone cells before parsing the raster so phantom # areas disappear (as the app shows). @@ -97,10 +114,19 @@ def render_q10_map( raise RoborockException("Failed to render Q10 map image") map_data = parsed.map_data - if calibration is not None and trace is not None: - _place_trace(map_data, calibration, trace) - _place_overlays(map_data, calibration, overlays) - return _draw_map_content(parsed.image_content, map_data, config=config) + has_drawables = False + if trace_calibration is not None and trace is not None: + charger_heading = packet.header_calibration.charger_phi if packet.header_calibration is not None else None + _place_trace(map_data, trace_calibration, trace, charger_heading=charger_heading) + has_drawables = True + has_drawables = _place_charger_from_header(map_data, packet) or has_drawables + if robot_at_dock: + has_drawables = _place_docked_robot(map_data) or has_drawables + if vector_calibration is not None: + _place_overlays(map_data, vector_calibration, overlays) + has_drawables = has_drawables or bool(map_data.no_go_areas or map_data.no_mopping_areas or map_data.walls) + if has_drawables: + return _draw_map_content(map_data, config=config) return parsed.image_content @@ -137,6 +163,41 @@ def _calibration_from_header( return solve_calibration_with_origin(packet.layers, points, origin, resolutions=_Q10_RESOLUTIONS) +def _calibration_from_header_metadata( + packet: Q10MapPacket, + *, + y_sign: int = 1, +) -> GridCalibration | None: + """Build the vector transform directly from a usable map header.""" + header = packet.header_calibration + if header is None or header.resolution <= 0 or (origin := header.origin_pixels()) is None: + return None + return GridCalibration( + resolution=header.resolution * _Q10_VECTOR_UNITS_PER_HEADER_RESOLUTION_UNIT, + origin_x=origin[0], + origin_y=origin[1], + y_sign=y_sign, + ) + + +def _vector_calibration( + packet: Q10MapPacket, + trace_calibration: GridCalibration | None, +) -> GridCalibration | None: + """Derive the 5 mm erase/restriction-vector transform.""" + y_sign = trace_calibration.y_sign if trace_calibration is not None else 1 + if calibration := _calibration_from_header_metadata(packet, y_sign=y_sign): + return calibration + if trace_calibration is None: + return None + return GridCalibration( + resolution=trace_calibration.resolution / 2, + origin_x=trace_calibration.origin_x, + origin_y=trace_calibration.origin_y, + y_sign=trace_calibration.y_sign, + ) + + def _calibration_from_fit(layers: GridLayers, points: list[tuple[float, float]]) -> GridCalibration | None: """Full origin + resolution fit; needs a reasonably dense path.""" if len(points) < _MIN_CALIBRATION_POINTS: @@ -170,6 +231,8 @@ def _place_trace( map_data: MapData, calibration: GridCalibration, trace: Q10TracePacket, + *, + charger_heading: int | None = None, ) -> None: """Project trace path, position, heading and charger into pixel space. @@ -181,11 +244,48 @@ def _place_trace( robot_position = trace.robot_position if robot_position is not None: px, py = calibration.world_to_pixel(robot_position.x, robot_position.y) - # Store the heading in projected image coordinates so drawing does not - # need to retain the world-to-pixel calibration. - map_data.vacuum_position = Point(px, py, -calibration.y_sign * trace.heading) + # The shared V1 marker expects a map-space heading (+Y is up), not the + # top-down PNG angle previously used by Q10's custom marker. + map_data.vacuum_position = Point(px, py, calibration.y_sign * trace.heading) if pixels: - map_data.charger = pixels[0] + map_data.charger = Point( + pixels[0].x, + pixels[0].y, + calibration.y_sign * charger_heading if charger_heading is not None else None, + ) + + +def _place_charger_from_header( + map_data: MapData, + packet: Q10MapPacket, +) -> bool: + """Place the saved dock using its absolute header pixel coordinates.""" + header = packet.header_calibration + if header is None or (position := header.charger_pixels()) is None: + return False + map_data.charger = Point(*position, header.charger_phi) + return True + + +def _place_docked_robot(map_data: MapData) -> bool: + """Place a charging robot immediately in front of the saved dock. + + A zero-point idle trace has no robot coordinates. The dock heading does, + however, identify its outward-facing side. Offset the robot by the shared + unscaled V1 charger radius so the two standard glyphs meet without one + covering the other, and preserve the saved dock heading. + """ + charger = map_data.charger + if charger is None or charger.a is None: + return False + angle = math.radians(charger.a) + offset = Sizes.SIZES[Size.CHARGER_RADIUS] + map_data.vacuum_position = Point( + charger.x + offset * math.cos(angle), + charger.y - offset * math.sin(angle), + charger.a, + ) + return True def _place_overlays( @@ -217,69 +317,18 @@ def to_area(zone: Q10Zone) -> Area | None: def _draw_map_content( - image_content: bytes, map_data: MapData, *, config: B01Q10MapParserConfig, - line_color: tuple[int, int, int, int] = (235, 64, 52, 255), - position_color: tuple[int, int, int, int] = (255, 211, 0, 255), ) -> bytes: - """Draw projected map content onto a base PNG and return a fresh PNG.""" - scale = config.map_scale - base = Image.open(io.BytesIO(image_content)).convert("RGBA") - - def to_image(point: Point) -> tuple[float, float]: - return (point.x * scale, point.y * scale) - - draw = ImageDraw.Draw(base, "RGBA") - - # Erase zones are applied to the raster itself (cells blanked), so they are - # not drawn here -- the base image already reflects them. - - # No-go (blue) and no-mop (magenta) zones beneath the path. - for areas, fill, outline in ( - (map_data.no_go_areas or [], (0, 120, 255, 70), (0, 80, 200, 255)), - (map_data.no_mopping_areas or [], (255, 0, 200, 70), (200, 0, 160, 255)), - ): - for area in areas: - polygon = [ - (area.x0 * scale, area.y0 * scale), - (area.x1 * scale, area.y1 * scale), - (area.x2 * scale, area.y2 * scale), - (area.x3 * scale, area.y3 * scale), - ] - draw.polygon(polygon, fill=fill, outline=outline) - - # Virtual walls (line segments, not polygons) drawn over the zones. - for wall in map_data.walls or []: - draw.line( - [(wall.x0 * scale, wall.y0 * scale), (wall.x1 * scale, wall.y1 * scale)], - fill=(255, 64, 64, 255), - width=max(2, scale), - ) - - for path in map_data.path.path if map_data.path else []: - if len(path) >= 2: - draw.line([to_image(point) for point in path], fill=line_color, width=max(1, scale // 2)) - if map_data.charger is not None: - dx, dy = to_image(map_data.charger) - draw.ellipse([dx - scale, dy - scale, dx + scale, dy + scale], outline=(40, 200, 40, 255), width=2) - robot_position = map_data.vacuum_position - if robot_position is not None: - cx, cy = to_image(robot_position) - radius = scale - draw.ellipse([cx - radius, cy - radius, cx + radius, cy + radius], fill=position_color) - robot_heading = robot_position.a - if robot_heading is not None: - # Heading was projected into image coordinates alongside the robot - # position, so no calibration state is needed during drawing. - angle = math.radians(robot_heading) - tick = 4 * radius - draw.line( - [cx, cy, cx + math.cos(angle) * tick, cy + math.sin(angle) * tick], - fill=position_color, - width=max(1, scale // 2), - ) + """Draw Q10 content with the shared V1 image generator.""" + if map_data.image is None: + raise RoborockException("Failed to render Q10 map image") + generator = create_image_generator( + MapParserConfig(map_scale=config.map_scale), + drawables=_Q10_DRAWABLES, + ) + generator.draw_map(map_data) buffer = io.BytesIO() - base.save(buffer, format="PNG") + map_data.image.data.save(buffer, format="PNG") return buffer.getvalue() diff --git a/roborock/map/map_parser.py b/roborock/map/map_parser.py index 44924a8a..597b879a 100644 --- a/roborock/map/map_parser.py +++ b/roborock/map/map_parser.py @@ -2,17 +2,22 @@ import io import logging +import threading from dataclasses import dataclass, field -from vacuum_map_parser_base.config.color import ColorsPalette, SupportedColor +from vacuum_map_parser_base.config.color import Color, ColorsPalette, SupportedColor from vacuum_map_parser_base.config.drawable import Drawable from vacuum_map_parser_base.config.image_config import ImageConfig from vacuum_map_parser_base.config.size import Size, Sizes +from vacuum_map_parser_base.image_generator import ImageGenerator from vacuum_map_parser_base.map_data import MapData +from vacuum_map_parser_roborock.image_parser import RoborockImageParser from vacuum_map_parser_roborock.map_data_parser import RoborockMapDataParser from roborock.exceptions import RoborockException +from .room_colors import adjacency_aware_room_colors + _LOGGER = logging.getLogger(__name__) DEFAULT_DRAWABLES = { @@ -98,10 +103,93 @@ def parse(self, map_bytes: bytes) -> ParsedMapData | None: return ParsedMapData(image_content=img_byte_arr.getvalue(), map_data=parsed_map) +class _AdjacencyAwareRoborockImageParser(RoborockImageParser): + """Apply the shared adjacency color policy to V1 room cells.""" + + def __init__( + self, + palette: ColorsPalette, + image_config: ImageConfig, + *, + recolor_rooms: bool = True, + ) -> None: + super().__init__(palette, image_config) + self._room_palette = palette + self._base_room_colors = palette.cached_room_colors.copy() + self._recolor_rooms = recolor_rooms + self._palette_lock = threading.Lock() + + def parse( + self, + raw_data: bytes, + width: int, + height: int, + carpet_map: set[int] | None, + removed_map: set[int] | None = None, + ): + """Assign non-conflicting room colors before the V1 image pass.""" + with self._palette_lock: + self._room_palette.cached_room_colors.clear() + self._room_palette.cached_room_colors.update(self._base_room_colors) + + if self._recolor_rooms: + + def room_id(value: int) -> int | None: + if value in (self.MAP_OUTSIDE, self.MAP_WALL, self.MAP_INSIDE, self.MAP_SCAN): + return None + return self._get_room_number(value) if value & 0x07 == 0x07 else None + + room_colors = adjacency_aware_room_colors(raw_data, width, self._room_palette, room_id) + for number, color in room_colors.items(): + self._room_palette.cached_room_colors[number] = color + self._room_palette.cached_room_colors[str(number)] = color + return super().parse(raw_data, width, height, carpet_map, removed_map) + + def _create_map_data_parser(config: MapParserConfig) -> RoborockMapDataParser: """Create a RoborockMapDataParser based on the config entry.""" - color_dicts = {} - room_colors = {} + palette, sizes, image_config = _create_rendering_components(config) + parser = RoborockMapDataParser( + palette, + sizes, + config.drawables, + image_config, + [], + ) + parser._image_parser = _AdjacencyAwareRoborockImageParser( + palette, + image_config, + recolor_rooms=config.show_rooms, + ) + return parser + + +def create_image_generator( + config: MapParserConfig, + *, + drawables: list[Drawable] | None = None, +) -> ImageGenerator: + """Create the image generator used by V1-compatible map renderers. + + Other protocols should use this factory for shared drawables instead of + copying V1 colors, sizes, glyphs, or transparency behavior. + """ + palette, sizes, image_config = _create_rendering_components(config) + return ImageGenerator( + palette, + sizes, + config.drawables if drawables is None else drawables, + image_config, + [], + ) + + +def _create_rendering_components( + config: MapParserConfig, +) -> tuple[ColorsPalette, Sizes, ImageConfig]: + """Build the shared V1 palette, scaled sizes, and image configuration.""" + color_dicts: dict[SupportedColor, Color] = {} + room_colors: dict[str, Color] = {} if not config.show_background: color_dicts[SupportedColor.MAP_OUTSIDE] = (0, 0, 0, 0) @@ -112,12 +200,10 @@ def _create_map_data_parser(config: MapParserConfig) -> RoborockMapDataParser: color_dicts[SupportedColor.MAP_WALL_V2] = (0, 0, 0, 0) if not config.show_rooms: - room_colors = {str(x): (0, 0, 0, 0) for x in range(1, 32)} + room_colors = {str(room_id): (0, 0, 0, 0) for room_id in map(int, ColorsPalette.ROOM_COLORS)} - return RoborockMapDataParser( + return ( ColorsPalette(color_dicts, room_colors), Sizes({k: v * config.map_scale for k, v in Sizes.SIZES.items() if k != Size.MOP_PATH_WIDTH}), - config.drawables, ImageConfig(scale=config.map_scale), - [], ) diff --git a/roborock/map/room_colors.py b/roborock/map/room_colors.py new file mode 100644 index 00000000..7fa9c401 --- /dev/null +++ b/roborock/map/room_colors.py @@ -0,0 +1,59 @@ +"""Deterministic room colors that keep adjacent segments distinguishable.""" + +from collections.abc import Callable, Sequence + +from vacuum_map_parser_base.config.color import Color, ColorsPalette + +RoomIdFromCell = Callable[[int], int | None] + + +def adjacency_aware_room_colors( + grid: Sequence[int], + width: int, + palette: ColorsPalette, + room_id_from_cell: RoomIdFromCell, +) -> dict[int, Color]: + """Return room colors, changing only adjacent same-color conflicts. + + Room IDs remain the stable preference, matching the existing V1 palette. + When two rooms sharing an edge resolve to the same RGB value, the + higher-numbered room receives the first palette color not already used by + one of its colored neighbors. + """ + if width <= 0: + return {} + + room_ids: set[int] = set() + neighbors: dict[int, set[int]] = {} + for index, value in enumerate(grid): + room_id = room_id_from_cell(value) + if room_id is None: + continue + room_ids.add(room_id) + neighbors.setdefault(room_id, set()) + + for neighbor_index in (index - 1 if index % width else -1, index - width): + if neighbor_index < 0: + continue + neighbor_id = room_id_from_cell(grid[neighbor_index]) + if neighbor_id is None or neighbor_id == room_id: + continue + neighbors[room_id].add(neighbor_id) + neighbors.setdefault(neighbor_id, set()).add(room_id) + + candidates: list[Color] = [] + for palette_id in map(int, ColorsPalette.ROOM_COLORS): + color = palette.get_room_color(palette_id) + if color not in candidates: + candidates.append(color) + + assigned: dict[int, Color] = {} + for room_id in sorted(room_ids): + preferred = palette.get_room_color(room_id) + neighbor_colors = {assigned[neighbor] for neighbor in neighbors[room_id] if neighbor in assigned} + assigned[room_id] = ( + preferred + if preferred not in neighbor_colors + else next((color for color in candidates if color not in neighbor_colors), preferred) + ) + return assigned diff --git a/tests/devices/traits/b01/q10/test_map.py b/tests/devices/traits/b01/q10/test_map.py index 61120a7e..ed1e3e06 100644 --- a/tests/devices/traits/b01/q10/test_map.py +++ b/tests/devices/traits/b01/q10/test_map.py @@ -1,10 +1,10 @@ """Tests for the Q10 B01 map content trait. -The Q10 map API is push-driven: the device publishes ``MAP_RESPONSE`` messages -which the protocol layer decodes into typed map/trace packets; the trait updates -its cached state from them via ``update_from_map_packet`` / -``update_from_trace_packet`` (there is no synchronous get-map request). These -tests cover that state management; the pixel/geometry work it drives is tested in +The Q10 map API uses an asynchronous ``dpMultiMap`` list/get exchange. The +device then publishes ``MAP_RESPONSE`` messages which the protocol layer +decodes into typed map/trace packets; the trait updates its cached state from +them via ``update_from_map_packet`` / ``update_from_trace_packet``. These tests +cover that state management; the pixel/geometry work it drives is tested in ``tests/map/test_b01_q10_render.py``. """ @@ -13,7 +13,7 @@ from collections.abc import AsyncGenerator from pathlib import Path from typing import cast -from unittest.mock import Mock, patch +from unittest.mock import AsyncMock, Mock, patch import pytest @@ -21,6 +21,7 @@ from roborock.data.b01_q10.b01_q10_code_mappings import B01_Q10_DP from roborock.devices.traits.b01.q10 import Q10PropertiesApi, create from roborock.devices.traits.b01.q10.map import MapContentTrait, MapDpsTrait +from roborock.devices.traits.b01.q10.status import StatusTrait from roborock.exceptions import RoborockException from roborock.map.b01_q10_map_parser import ( Q10Point, @@ -29,7 +30,7 @@ parse_trace_packet, ) from roborock.map.b01_q10_render import Q10MapOverlays -from roborock.protocols.b01_q10_protocol import Q10Message +from roborock.protocols.b01_q10_protocol import Q10DpsUpdate, Q10Message from .conftest import FakeB01Q10Channel @@ -111,7 +112,10 @@ async def test_await_q10_map_push_waits_for_fresh_update() -> None: properties.map.update_from_trace_packet(Q10TracePacket(points=[Q10Point(1, 2)])) got_trace = await _await_q10_map_push( - cast(Q10PropertiesApi, properties), lambda: bool(properties.map.path), timeout=0.01 + cast(Q10PropertiesApi, properties), + lambda: bool(properties.map.path), + lambda: properties.map.trace_generation, + timeout=0.01, ) assert got_trace is False @@ -122,7 +126,10 @@ async def test_await_q10_map_push_returns_true_after_update() -> None: properties = _FakeQ10PropertiesWithTrace() got_trace = await _await_q10_map_push( - cast(Q10PropertiesApi, properties), lambda: bool(properties.map.path), timeout=0.01 + cast(Q10PropertiesApi, properties), + lambda: bool(properties.map.path), + lambda: properties.map.trace_generation, + timeout=0.01, ) assert got_trace is True @@ -136,6 +143,7 @@ async def test_await_q10_map_push_can_fall_back_to_cached_map_on_timeout() -> No got_map = await _await_q10_map_push( cast(Q10PropertiesApi, properties), lambda: properties.map.image_content is not None, + lambda: properties.map.map_generation, timeout=0.01, allow_cached_on_timeout=True, ) @@ -144,6 +152,30 @@ async def test_await_q10_map_push_can_fall_back_to_cached_map_on_timeout() -> No assert properties.refresh_count == 1 +async def test_await_q10_map_push_ignores_status_only_render() -> None: + """A status recomposition cannot masquerade as a fresh map packet.""" + status = StatusTrait() + properties = _FakeQ10Properties() + properties.map = MapContentTrait(MapDpsTrait(), status) + properties.map.update_from_map_packet(parse_map_packet(FIXTURE.read_bytes())) + + async def refresh_status_only() -> None: + properties.refresh_count += 1 + status.update_from_dps({B01_Q10_DP.STATUS: 8}) + + properties.refresh = refresh_status_only # type: ignore[method-assign] + + got_map = await _await_q10_map_push( + cast(Q10PropertiesApi, properties), + lambda: properties.map.image_content is not None, + lambda: properties.map.map_generation, + timeout=0.01, + ) + + assert got_map is False + assert properties.map.map_generation == 1 + + # --- Integration through the Q10PropertiesApi subscribe loop ----------------- @@ -204,6 +236,143 @@ async def test_subscribe_loop_routes_trace_push( assert q10_api.map.robot_position is not None +async def test_map_list_response_requests_first_map( + q10_api: Q10PropertiesApi, + mock_channel: FakeB01Q10Channel, + message_queue: asyncio.Queue[Q10Message], +) -> None: + """A requested map list is followed by a get for its first map ID.""" + await q10_api.request_map() + assert mock_channel.published_commands == [ + ( + B01_Q10_DP.COMMON, + {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}}, + ) + ] + + message_queue.put_nowait( + Q10DpsUpdate( + dps={ + B01_Q10_DP.MULTI_MAP: { + "data": [ + {"id": "12345", "name": "Current", "timestamp": 1}, + {"id": "67890", "name": "Other", "timestamp": 2}, + ], + "op": "list", + "result": 1, + } + } + ) + ) + + await _wait_for(lambda: len(mock_channel.published_commands) == 2) + assert mock_channel.published_commands[1] == ( + B01_Q10_DP.COMMON, + { + str(B01_Q10_DP.MULTI_MAP.code): { + "op": "get", + "id": "12345", + } + }, + ) + + +async def test_unsolicited_map_list_does_not_request_map( + q10_api: Q10PropertiesApi, + mock_channel: FakeB01Q10Channel, + message_queue: asyncio.Queue[Q10Message], +) -> None: + """A list response not initiated by this API cannot trigger a map get.""" + message_queue.put_nowait( + Q10DpsUpdate( + dps={ + B01_Q10_DP.MULTI_MAP: { + "data": [{"id": "12345"}], + "op": "list", + "result": 1, + } + } + ) + ) + + await asyncio.sleep(0.01) + assert mock_channel.published_commands == [] + + +async def test_concurrent_map_requests_are_coalesced(q10_api: Q10PropertiesApi) -> None: + """Only one list request remains pending when callers refresh together.""" + started = asyncio.Event() + release = asyncio.Event() + + async def send_list(*_args, **_kwargs) -> None: + started.set() + await release.wait() + + with patch.object(q10_api.command, "send", side_effect=send_list) as send: + first = asyncio.create_task(q10_api.request_map()) + await started.wait() + second = asyncio.create_task(q10_api.request_map()) + await asyncio.sleep(0) + release.set() + await asyncio.gather(first, second) + + send.assert_awaited_once() + + +async def test_cancelled_map_request_can_be_retried(q10_api: Q10PropertiesApi) -> None: + """Cancellation clears only its own pending list request.""" + started = asyncio.Event() + blocked = asyncio.Event() + + async def send_list(*_args, **_kwargs) -> None: + started.set() + await blocked.wait() + + with patch.object(q10_api.command, "send", side_effect=send_list): + request = asyncio.create_task(q10_api.request_map()) + await started.wait() + request.cancel() + with pytest.raises(asyncio.CancelledError): + await request + + with patch.object(q10_api.command, "send", new=AsyncMock()) as retry: + await q10_api.request_map() + + retry.assert_awaited_once() + + +async def test_map_request_stays_coalesced_through_get(q10_api: Q10PropertiesApi) -> None: + """The pending flow includes the follow-up get publish.""" + get_started = asyncio.Event() + release_get = asyncio.Event() + + async def send_command(_dp, params) -> None: + operation = params[str(B01_Q10_DP.MULTI_MAP.code)]["op"] + if operation == "get": + get_started.set() + await release_get.wait() + + response = { + B01_Q10_DP.MULTI_MAP: { + "data": [{"id": "12345"}], + "op": "list", + "result": 1, + } + } + with patch.object(q10_api.command, "send", side_effect=send_command) as send: + await q10_api.request_map() + get_request = asyncio.create_task(q10_api._request_map_from_list_response(response)) + await get_started.wait() + + await q10_api.request_map() + assert send.await_count == 2 + + release_get.set() + await get_request + await q10_api.request_map() + assert send.await_count == 3 + + # --- Source composition + rendering ------------------------------------------ @@ -312,3 +481,97 @@ def test_map_dps_push_without_overlay_data_points_is_noop() -> None: assert map_dps.overlays == Q10MapOverlays() assert not notified + + +def test_charging_status_renders_robot_at_dock() -> None: + """Charging status adds the idle robot marker without inventing a path.""" + status = StatusTrait() + trait = MapContentTrait(MapDpsTrait(), status) + packet = parse_map_packet(FIXTURE.read_bytes()) + notified: list[None] = [] + trait.add_update_listener(lambda: notified.append(None)) + + with patch( + "roborock.devices.traits.b01.q10.map.render_q10_map", + side_effect=[b"map with dock", b"map with docked robot"], + ) as render: + trait.update_from_map_packet(packet) + notified.clear() + status.update_from_dps({B01_Q10_DP.STATUS: 8}) + status.update_from_dps({B01_Q10_DP.BATTERY: 50}) + + assert trait.image_content == b"map with docked robot" + assert trait.path == [] + assert notified == [None] + assert render.call_count == 2 + assert render.call_args.kwargs["robot_at_dock"] is True + + +def test_entering_docked_state_clears_stale_live_trace() -> None: + """A completed cleaning path cannot remain the caller-facing live position.""" + status = StatusTrait() + trait = MapContentTrait(MapDpsTrait(), status) + trait.update_from_trace_packet(Q10TracePacket(points=[Q10Point(1, 2), Q10Point(3, 4)])) + notified: list[None] = [] + trait.add_update_listener(lambda: notified.append(None)) + + status.update_from_dps({B01_Q10_DP.STATUS: 8}) + + assert trait.path == [] + assert trait.robot_position is None + assert trait.robot_heading is None + assert notified == [None] + + +def test_late_trace_does_not_move_docked_robot() -> None: + """A delayed trace packet cannot revive a completed cleaning path.""" + status = StatusTrait() + status.update_from_dps({B01_Q10_DP.STATUS: 8}) + trait = MapContentTrait(MapDpsTrait(), status) + + trait.update_from_trace_packet(Q10TracePacket(points=[Q10Point(1, 2)])) + + assert trait.path == [] + assert trait.robot_position is None + assert trait.trace_generation == 1 + + +def test_emptying_state_keeps_robot_at_dock() -> None: + """Dock emptying must not briefly remove the docked robot marker.""" + status = StatusTrait() + trait = MapContentTrait(MapDpsTrait(), status) + packet = parse_map_packet(FIXTURE.read_bytes()) + + with patch( + "roborock.devices.traits.b01.q10.map.render_q10_map", + side_effect=[b"map with dock", b"map while emptying"], + ) as render: + trait.update_from_map_packet(packet) + status.update_from_dps({B01_Q10_DP.STATUS: 22}) + + assert trait.image_content == b"map while emptying" + assert render.call_args.kwargs["robot_at_dock"] is True + + +async def test_combined_status_and_overlay_update_renders_once(q10_api: Q10PropertiesApi) -> None: + """One DPS message publishes one coherent composed-map update.""" + q10_api.map.update_from_map_packet(parse_map_packet(FIXTURE.read_bytes())) + notified: list[None] = [] + q10_api.map.add_update_listener(lambda: notified.append(None)) + + with patch( + "roborock.devices.traits.b01.q10.map.render_q10_map", + return_value=b"combined map", + ) as render: + await q10_api._handle_message( + Q10DpsUpdate( + dps={ + B01_Q10_DP.STATUS: 8, + B01_Q10_DP.RESTRICTED_ZONE_UP: _zone_blob(), + } + ) + ) + + assert render.call_count == 1 + assert notified == [None] + assert q10_api.map.image_content == b"combined map" diff --git a/tests/devices/traits/b01/q10/test_status.py b/tests/devices/traits/b01/q10/test_status.py index edd4bcb1..8f301483 100644 --- a/tests/devices/traits/b01/q10/test_status.py +++ b/tests/devices/traits/b01/q10/test_status.py @@ -128,10 +128,13 @@ async def test_status_trait_refresh( # Send a refresh command await q10_api.refresh() - assert len(mock_channel.published_commands) == 1 - command, params = mock_channel.published_commands[0] - assert command == B01_Q10_DP.REQUEST_DPS - assert params == {} + assert mock_channel.published_commands == [ + (B01_Q10_DP.REQUEST_DPS, {}), + ( + B01_Q10_DP.COMMON, + {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}}, + ), + ] # Push the response message into the queue message_queue.put_nowait(message) diff --git a/tests/map/test_b01_q10_map_parser.py b/tests/map/test_b01_q10_map_parser.py index 9e815e29..c0200ff5 100644 --- a/tests/map/test_b01_q10_map_parser.py +++ b/tests/map/test_b01_q10_map_parser.py @@ -159,6 +159,37 @@ def test_parse_map_packet_allows_zero_room_metadata() -> None: assert packet.rooms == [] +def test_parser_renders_distinct_background_floor_and_walls() -> None: + """Background must not hide the wall layer by sharing its color.""" + grid = bytes([243, 240, 249, 8]) + payload = _synthetic_map_payload(width=4, decoded_layout=grid + b"\x01\x00") + parser = B01Q10MapParser() + + parsed = parser.parse(payload) + + assert parsed.image_content is not None + image = Image.open(io.BytesIO(parsed.image_content)) + scale = parser.config.map_scale + assert image.getpixel((0, 0)) == (0, 0, 0, 0) + assert image.getpixel((scale, 0)) == (32, 115, 185, 255) + assert image.getpixel((scale * 2, 0)) == (93, 109, 126, 255) + assert image.getpixel((scale * 3, 0)) == (133, 193, 233, 255) + + +def test_parser_gives_adjacent_rooms_distinct_palette_colors() -> None: + """Repeated V1 palette entries do not merge neighboring Q10 rooms.""" + grid = bytes([2 * 4, 12 * 4]) + payload = _synthetic_map_payload(width=2, decoded_layout=grid + b"\x01\x00") + parser = B01Q10MapParser() + + parsed = parser.parse(payload) + + assert parsed.image_content is not None + image = Image.open(io.BytesIO(parsed.image_content)) + scale = parser.config.map_scale + assert image.getpixel((0, 0)) != image.getpixel((scale, 0)) + + def test_parse_map_packet_reads_header_height() -> None: """Width and height come straight from the u16be header fields.""" grid = bytes([8]) * 6 + bytes([12]) * 6 # two rooms, 4x3 grid @@ -425,6 +456,7 @@ def test_parse_header_calibration_fields() -> None: assert not cal.is_keepalive # 5 mm units / (50 mm/px) -> divide by 10 for grid pixels. assert cal.origin_pixels() == (-376.0, 192.0) + assert cal.charger_pixels() == (-5.0, 3.0) def test_parse_header_calibration_keepalive_has_no_origin() -> None: diff --git a/tests/map/test_b01_q10_render.py b/tests/map/test_b01_q10_render.py index ff18ed65..cfdaaf37 100644 --- a/tests/map/test_b01_q10_render.py +++ b/tests/map/test_b01_q10_render.py @@ -10,6 +10,8 @@ from pathlib import Path from PIL import Image +from vacuum_map_parser_base.config.size import Size, Sizes +from vacuum_map_parser_base.map_data import MapData, Point from roborock.map.b01_grid_layers import GridCalibration from roborock.map.b01_q10_map_parser import ( @@ -30,7 +32,11 @@ from roborock.map.b01_q10_render import ( _Q10_RESOLUTIONS, Q10MapOverlays, + _calibration_from_header_metadata, _erased_cells, + _place_charger_from_header, + _place_docked_robot, + _vector_calibration, render_q10_map, solve_q10_calibration, ) @@ -41,8 +47,9 @@ # identity-ish calibration used across the geometry tests: world (x, y) -> grid # pixel (x, 5 - y) over the fixture's 8x6 grid (top-down, no flip). IDENTITY = GridCalibration(resolution=1.0, origin_x=0.0, origin_y=5.0, y_sign=1) -HEADER = Q10HeaderCalibration(origin_x=0, origin_y=50, resolution=5, charger_x=0, charger_y=0, charger_phi=0) +HEADER = Q10HeaderCalibration(origin_x=0, origin_y=50, resolution=5, charger_x=30, charger_y=30, charger_phi=90) TRACE_CALIBRATION = GridCalibration(resolution=20.0, origin_x=0.0, origin_y=5.0, y_sign=1) +VECTOR_CALIBRATION = GridCalibration(resolution=10.0, origin_x=0.0, origin_y=5.0, y_sign=1) def _packet() -> Q10MapPacket: @@ -107,28 +114,103 @@ def test_render_draws_path_and_position() -> None: image_position = (round(px * CONFIG.map_scale), round(py * CONFIG.map_scale)) rendered = Image.open(io.BytesIO(image)).convert("RGBA") assert rendered.size == (8 * 4, 6 * 4) - assert rendered.getpixel(image_position) == (255, 211, 0, 255) + # The shared V1 robot glyph has a white body at its center. + assert rendered.getpixel(image_position) == (255, 255, 255, 255) def test_render_draws_zones_and_virtual_walls() -> None: """Decoded DPS overlays are included in the composed image.""" packet, trace = _calibrated_inputs() zones = [ - Q10Zone(type=ZONE_TYPE_NO_GO, vertices=[(0, 0), (4, 0), (4, 4), (0, 4)]), - Q10Zone(type=ZONE_TYPE_NO_MOP, vertices=[(1, 1), (2, 1), (2, 2), (1, 2)]), + Q10Zone(type=ZONE_TYPE_NO_GO, vertices=_world_vertices(VECTOR_CALIBRATION, [(1, 1), (4, 1), (4, 4), (1, 4)])), + Q10Zone(type=ZONE_TYPE_NO_MOP, vertices=_world_vertices(VECTOR_CALIBRATION, [(4, 1), (6, 1), (6, 3), (4, 3)])), ] - walls = [Q10Zone(type=ZONE_TYPE_VIRTUAL_WALL, vertices=[(0, 0), (4, 0)])] + walls = [Q10Zone(type=ZONE_TYPE_VIRTUAL_WALL, vertices=_world_vertices(VECTOR_CALIBRATION, [(1, 1), (6, 1)]))] base = _render(packet, trace=trace) rendered = _render(packet, trace=trace, overlays=Q10MapOverlays(zones=zones, virtual_walls=walls)) assert rendered != base +def test_render_draws_zones_and_virtual_walls_without_trace() -> None: + """Header calibration is sufficient to place restrictions while idle.""" + packet = replace(_packet(), header_calibration=HEADER) + zones = [ + Q10Zone( + type=ZONE_TYPE_NO_GO, + vertices=_world_vertices(VECTOR_CALIBRATION, [(1, 1), (4, 1), (4, 4), (1, 4)]), + ) + ] + walls = [ + Q10Zone( + type=ZONE_TYPE_VIRTUAL_WALL, + vertices=_world_vertices(VECTOR_CALIBRATION, [(1, 1), (6, 1)]), + ) + ] + + base = _render(packet) + rendered = _render(packet, overlays=Q10MapOverlays(zones=zones, virtual_walls=walls)) + + assert rendered != base + + +def test_render_draws_dock_from_header_without_trace() -> None: + """The saved dock remains visible while the robot has no cleaning trace.""" + packet = _packet() + + base = _render(packet) + rendered = _render(replace(packet, header_calibration=HEADER)) + + assert rendered != base + + +def test_place_charger_uses_absolute_header_pixels() -> None: + """The dock coordinates do not receive the world origin a second time.""" + packet = replace(_packet(), header_calibration=HEADER) + map_data = MapData() + + assert _place_charger_from_header(map_data, packet) + + assert map_data.charger == Point(3, 3, 90) + + +def test_place_docked_robot_uses_shared_v1_marker_geometry() -> None: + """The idle robot sits beside the dock, facing it, without a path.""" + map_data = MapData() + map_data.charger = Point(20, 30, 90) + + assert _place_docked_robot(map_data) + + assert map_data.vacuum_position == Point( + 20, + 30 - Sizes.SIZES[Size.CHARGER_RADIUS], + 90, + ) + assert map_data.path is None + + +def test_q10_zero_degree_dock_places_robot_to_its_right() -> None: + """The Q10 dock heading is already its outward-facing direction.""" + packet = replace(_packet(), header_calibration=replace(HEADER, charger_phi=0)) + map_data = MapData() + + assert _place_charger_from_header(map_data, packet) + assert _place_docked_robot(map_data) + + assert map_data.charger == Point(3, 3, 0) + assert map_data.vacuum_position == Point( + 3 + Sizes.SIZES[Size.CHARGER_RADIUS], + 3, + 0, + ) + + def test_render_applies_erase_zones() -> None: """With a calibration, erase-zone cells are blanked from the image.""" packet, trace = _calibrated_inputs() base = _render(packet, trace=trace) - calibration = solve_q10_calibration(packet, trace) - assert calibration is not None + trace_calibration = solve_q10_calibration(packet, trace) + calibration = _vector_calibration(packet, trace_calibration) + assert calibration == VECTOR_CALIBRATION # A rectangle covering the whole grid in world coords erases every cell. corners = [(-1, -1), (8, -1), (8, 6), (-1, 6)] @@ -140,12 +222,29 @@ def test_render_applies_erase_zones() -> None: assert render != base +def test_render_applies_erase_zones_from_header_without_trace() -> None: + """The map header is sufficient to erase zones while the robot is idle.""" + packet = replace(_packet(), header_calibration=HEADER) + calibration = _calibration_from_header_metadata(packet) + assert calibration == VECTOR_CALIBRATION + + corners = [(-1, -1), (8, -1), (8, 2), (-1, 2)] + erase_zone = Q10EraseZone(vertices=_world_vertices(calibration, corners)) + cells = _erased_cells(packet.layers, [erase_zone], calibration) + base = _render(packet) + render = _render(replace(packet, erase_zones=[erase_zone])) + + assert 0 < len(cells) < packet.layers.width * packet.layers.height + assert render != base + + def test_render_partial_erase() -> None: """An erase rectangle only blanks the cells it covers, leaving the rest.""" packet, trace = _calibrated_inputs() base = _render(packet, trace=trace) - calibration = solve_q10_calibration(packet, trace) - assert calibration is not None + trace_calibration = solve_q10_calibration(packet, trace) + calibration = _vector_calibration(packet, trace_calibration) + assert calibration == VECTOR_CALIBRATION # Cover only the top two grid rows. corners = [(-1, -1), (8, -1), (8, 2), (-1, 2)] @@ -157,26 +256,12 @@ def test_render_partial_erase() -> None: assert render != base -def test_render_draws_heading_indicator() -> None: - """A known heading draws a facing tick from the robot marker. +def test_render_robot_marker_reflects_heading() -> None: + """The shared V1 robot glyph rotates its details with the Q10 heading.""" + packet, trace_right = _calibrated_inputs(heading=0) + _, trace_up = _calibrated_inputs(heading=90) - With heading 0 (= +x world) and the identity-ish calibration, the tick - extends to the right of the robot pixel; with the marker at image (12, 12) - the tick covers pixels at x > 12 along y == 12. - """ - packet, trace = _calibrated_inputs(heading=0) - image = _render(packet, trace=trace) - calibration = solve_q10_calibration(packet, trace) - assert calibration is not None - assert trace.robot_position is not None - px, py = calibration.world_to_pixel(trace.robot_position.x, trace.robot_position.y) - cx = round(px * CONFIG.map_scale) - cy = round(py * CONFIG.map_scale) - rendered = Image.open(io.BytesIO(image)).convert("RGBA") - # Tick runs +x from the marker (4 * radius = 16 px at scale 4). - assert rendered.getpixel((cx + 8, cy)) == (255, 211, 0, 255) - # ...and not behind it (the marker is a small disc; sample well to the left) - assert rendered.getpixel((cx - 8, cy)) != (255, 211, 0, 255) + assert _render(packet, trace=trace_right) != _render(packet, trace=trace_up) def test_solve_q10_calibration_uses_header_origin_with_short_path() -> None: diff --git a/tests/map/test_map_parser.py b/tests/map/test_map_parser.py index 4c648972..604e4bed 100644 --- a/tests/map/test_map_parser.py +++ b/tests/map/test_map_parser.py @@ -3,9 +3,19 @@ from pathlib import Path import pytest +from vacuum_map_parser_base.config.color import ColorsPalette +from vacuum_map_parser_base.config.drawable import Drawable +from vacuum_map_parser_base.config.image_config import ImageConfig +from vacuum_map_parser_base.config.size import Size from roborock.exceptions import RoborockException -from roborock.map.map_parser import MapParser, MapParserConfig +from roborock.map.map_parser import ( + MapParser, + MapParserConfig, + _AdjacencyAwareRoborockImageParser, + _create_map_data_parser, + create_image_generator, +) MAP_DATA_FILE = Path(__file__).parent / "raw_map_data" DEFAULT_MAP_CONFIG = MapParserConfig() @@ -19,4 +29,53 @@ def test_invalid_map_content(map_content: bytes): parser.parse(map_content) +def test_shared_image_generator_matches_v1_rendering_components() -> None: + """Protocol renderers share V1 colors, scaled sizes, and image config.""" + config = MapParserConfig(map_scale=3) + drawables = [ + Drawable.CHARGER, + Drawable.PATH, + Drawable.VACUUM_POSITION, + Drawable.VIRTUAL_WALLS, + ] + shared = create_image_generator(config, drawables=drawables) + v1 = _create_map_data_parser(config)._image_generator + + assert shared._palette.cached_colors == v1._palette.cached_colors + assert shared._palette.cached_room_colors == v1._palette.cached_room_colors + assert shared._image_config == v1._image_config + assert shared._drawables == drawables + for size in Size: + assert shared._sizes.get_size(size) == v1._sizes.get_size(size) + + +def test_v1_parser_gives_adjacent_rooms_distinct_palette_colors() -> None: + """Repeated palette entries do not merge neighboring V1 rooms.""" + palette = ColorsPalette() + original_room_12 = palette.get_room_color(12) + image_parser = _AdjacencyAwareRoborockImageParser(palette, ImageConfig()) + raw_data = bytes([(2 << 3) | 7, (12 << 3) | 7]) + + image, _rooms = image_parser.parse(raw_data, 2, 1, None) + + assert image is not None + assert image.getpixel((0, 0)) != image.getpixel((1, 0)) + + isolated_image, _rooms = image_parser.parse(bytes([(12 << 3) | 7]), 1, 1, None) + + assert isolated_image is not None + assert isolated_image.getpixel((0, 0))[: len(original_room_12)] == original_room_12 + + +def test_v1_parser_keeps_adjacent_rooms_hidden_when_rooms_disabled() -> None: + """Adjacency conflict handling cannot override intentional transparency.""" + parser = _create_map_data_parser(MapParserConfig(show_rooms=False, map_scale=1))._image_parser + raw_data = bytes([(2 << 3) | 7, (12 << 3) | 7]) + + image, _rooms = parser.parse(raw_data, 2, 1, None) + + assert image is not None + assert [image.getpixel((x, 0)) for x in range(2)] == [(0, 0, 0, 0)] * 2 + + # We can add additional tests here in the future that actually parse valid map data diff --git a/tests/protocols/test_b01_q10_protocol.py b/tests/protocols/test_b01_q10_protocol.py index f5d916d1..5fddc41a 100644 --- a/tests/protocols/test_b01_q10_protocol.py +++ b/tests/protocols/test_b01_q10_protocol.py @@ -41,6 +41,26 @@ def test_decode_message_dps_update() -> None: assert decoded == Q10DpsUpdate(dps={B01_Q10_DP.BATTERY: 100}) +def test_decode_message_common_multi_map_list() -> None: + """A nested dpCommon map-list response is flattened into dpMultiMap.""" + message = _message( + b'{"dps":{"101":{"61":{"data":[{"id":"12345","name":"Map","timestamp":1}],"op":"list","result":1}}}}', + RoborockMessageProtocol.RPC_RESPONSE, + ) + + decoded = decode_message(message) + + assert decoded == Q10DpsUpdate( + dps={ + B01_Q10_DP.MULTI_MAP: { + "data": [{"id": "12345", "name": "Map", "timestamp": 1}], + "op": "list", + "result": 1, + } + } + ) + + def test_decode_message_map_packet() -> None: """A MAP_RESPONSE 01 01 payload decodes into a Q10MapPacket.""" message = _message(MAP_FIXTURE.read_bytes(), RoborockMessageProtocol.MAP_RESPONSE)