refactor: move Q10 refresh into map trait

Author: tubededentifrice

roborock/cli.py

@@ -626,7 +626,7 @@ def on_update() -> None:
 
     unsub = add_source_listener(on_update)
     try:
-        await properties.refresh()
+        await properties.map.refresh()
         await asyncio.wait_for(updated, timeout=timeout)
         return True
     except TimeoutError:

roborock/data/b01_q10/b01_q10_containers.py

@@ -83,11 +83,32 @@ def start_datetime(self) -> datetime.datetime | None:
         return None
 
 
+@dataclass
+class Q10MapInfo(RoborockBase):
+    """A saved map reported by ``dpMultiMap``.
+
+    Q10 firmware represents the map identifier as a string on the wire. The
+    value is sent back unchanged in a subsequent ``{"op": "get"}`` request.
+    """
+
+    id: str
+    name: str | None = None
+    timestamp: int | None = None
+
+
 @dataclass
 class dpMultiMap(RoborockBase):
+    """Response envelope for the Q10 ``dpMultiMap`` data point."""
+
     op: str
     result: int
-    data: list
+    data: list[Q10MapInfo] = field(default_factory=list)
+
+    @property
+    def current_map_id(self) -> str | None:
+        """Return the first saved-map identifier, if one was reported."""
+        first = next((map_info for map_info in self.data if isinstance(map_info, Q10MapInfo) and map_info.id), None)
+        return first.id if first else None
 
 
 @dataclass

roborock/devices/traits/b01/q10/__init__.py

@@ -2,12 +2,10 @@
 
 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
 
@@ -40,25 +38,6 @@
 ]
 
 _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 not isinstance(map_info, dict):
-            continue
-        map_id = map_info.get("id")
-        if isinstance(map_id, int) and not isinstance(map_id, bool):
-            return map_id
-        if isinstance(map_id, str) and map_id:
-            return map_id
-    return None
 
 
 class Q10PropertiesApi(Trait):
@@ -121,7 +100,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.command)
         self.clean_history = CleanHistoryTrait(self.command)
         # Read-model traits updated from the device's DPS push stream.
         self._updatable_traits = [
@@ -136,9 +115,6 @@ 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."""
@@ -156,42 +132,9 @@ async def close(self) -> None:
 
     async def refresh(self) -> None:
         """Refresh all traits."""
-        # 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.
+        # Sending REQUEST_DPS causes the device to publish its ordinary status
+        # values. Map refreshes have their own cadence through ``map.refresh()``.
         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
-            sent = False
-            try:
-                await self.command.send(
-                    B01_Q10_DP.COMMON,
-                    {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}},
-                )
-                sent = True
-            finally:
-                if not sent and self._map_list_request_token is token:
-                    self._map_list_request_token = None
-                    self._map_list_requested_at = None
 
     async def _subscribe_loop(self) -> None:
         """Persistent loop dispatching decoded messages to the read-model traits."""
@@ -202,9 +145,8 @@ 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.
-        A ``dpMultiMap`` list response completes the asynchronous request flow
-        started by :meth:`request_map`; other DPS updates feed the read-model
-        traits.
+        Map-list DPS responses are handed to the map trait; other DPS updates
+        feed the read-model traits.
         """
         if isinstance(message, Q10MapPacket):
             self.map.update_from_map_packet(message)
@@ -216,39 +158,7 @@ async def _handle_message(self, message: Q10Message) -> None:
             # only updates the fields that it is responsible for.
             for trait in self._updatable_traits:
                 trait.update_from_dps(message.dps)
-            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
+            await self.map.update_from_dps(message.dps)
 
 
 def create(channel: B01Q10Channel) -> Q10PropertiesApi:

roborock/devices/traits/b01/q10/map.py

@@ -21,6 +21,7 @@
 from roborock.callbacks import CallbackList
 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_containers import dpMultiMap
 from roborock.devices.traits.common import DpsDataConverter, TraitUpdateListener
 from roborock.exceptions import RoborockException
 from roborock.map.b01_q10_map_parser import (
@@ -33,6 +34,7 @@
 from roborock.map.b01_q10_overlays import parse_virtual_wall_blob, parse_zone_blob
 from roborock.map.b01_q10_render import Q10MapOverlays, render_q10_map
 
+from .command import CommandTrait
 from .common import UpdatableTrait
 
 _LOGGER = logging.getLogger(__name__)
@@ -72,29 +74,73 @@ def update_from_dps(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None:
         self._notify_update()
 
 
-class MapContentTrait(TraitUpdateListener):
+@dataclass
+class MapListDps(RoborockBase):
+    """Typed ``dpMultiMap`` state delivered through the Q10 DPS stream."""
+
+    multi_map: dpMultiMap | None = field(default=None, metadata={"dps": B01_Q10_DP.MULTI_MAP})
+
+
+class MapContentTrait(MapListDps, 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.
     """
 
+    _CONVERTER = DpsDataConverter.from_dataclass(MapListDps)
+
     def __init__(
         self,
         map_dps: MapDpsTrait,
+        command: CommandTrait | None = None,
         *,
         map_parser_config: B01Q10MapParserConfig | None = None,
     ) -> None:
+        MapListDps.__init__(self)
         TraitUpdateListener.__init__(self, logger=_LOGGER)
         self._config = map_parser_config or B01Q10MapParserConfig()
         self._map_dps = map_dps
+        self._command = command
         self._map_packet: Q10MapPacket | None = None
         self._trace_packet: Q10TracePacket | None = None
         self._image_content: bytes | None = None
         self._map_packet_callbacks: CallbackList[None] = CallbackList(_LOGGER)
         self._trace_packet_callbacks: CallbackList[None] = CallbackList(_LOGGER)
         self._map_dps.add_update_listener(self._map_dps_updated)
 
+    async def refresh(self) -> None:
+        """Request the current saved map independently of general status."""
+        if self._command is None:
+            raise ValueError("Trait is read-only; no command channel was provided")
+        await self._command.send(
+            B01_Q10_DP.COMMON,
+            {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}},
+        )
+
+    async def update_from_dps(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None:
+        """Request map content when a typed ``dpMultiMap`` list response arrives."""
+        if not self._CONVERTER.update_from_dps(self, decoded_dps):
+            return
+        if self._command is None or self.multi_map is None or self.multi_map.op != "list" or self.multi_map.result != 1:
+            return
+        if (map_id := self.multi_map.current_map_id) is None:
+            _LOGGER.debug("Q10 map list response did not contain a 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)
+
     @property
     def image_content(self) -> bytes | None:
         """The composed map PNG, if the latest map rendered successfully."""