refactor: split Q10 map list and content

Author: tubededentifrice

roborock/cli.py

@@ -608,14 +608,11 @@ async def _await_q10_map_push(
     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 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 a packet-specific listener, send the request, and wait for a newly
-    pushed update to satisfy ``predicate``. Returns whether it did within
-    ``timeout``.
+    """Request Q10 map content and wait for a fresh map or trace packet.
+
+    A Q10 needs a saved-map ID before it can request content. The map list and
+    content have independent refresh schedules, so the list is requested only
+    when no ID is stored. The content then arrives as a later ``MAP_RESPONSE``.
     """
     loop = asyncio.get_running_loop()
     updated: asyncio.Future[None] = loop.create_future()
@@ -626,8 +623,23 @@ def on_update() -> None:
 
     unsub = add_source_listener(on_update)
     try:
-        await properties.map.refresh()
-        await asyncio.wait_for(updated, timeout=timeout)
+        async with asyncio.timeout(timeout):
+            if properties.maps.current_map_id is None:
+                map_list_updated: asyncio.Future[None] = loop.create_future()
+
+                def on_map_list_update() -> None:
+                    if properties.maps.current_map_id is not None and not map_list_updated.done():
+                        map_list_updated.set_result(None)
+
+                unsub_maps = properties.maps.add_update_listener(on_map_list_update)
+                try:
+                    await properties.maps.refresh()
+                    if properties.maps.current_map_id is None:
+                        await map_list_updated
+                finally:
+                    unsub_maps()
+            await properties.map.refresh()
+            await updated
         return True
     except TimeoutError:
         return allow_cached_on_timeout and predicate()

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

@@ -17,6 +17,7 @@
 from .do_not_disturb import DoNotDisturbTrait
 from .dust_collection import DustCollectionTrait
 from .map import MapContentTrait, MapDpsTrait
+from .maps import MapsTrait
 from .network_info import NetworkInfoTrait
 from .remote import RemoteTrait
 from .status import StatusTrait
@@ -32,6 +33,7 @@
     "DoNotDisturbTrait",
     "DustCollectionTrait",
     "MapContentTrait",
+    "MapsTrait",
     "NetworkInfoTrait",
     "SoundVolumeTrait",
     "StatusTrait",
@@ -79,6 +81,9 @@ class Q10PropertiesApi(Trait):
     map: MapContentTrait
     """Composed map image plus caller-facing map and trace data."""
 
+    maps: MapsTrait
+    """Saved-map list metadata."""
+
     _map_dps: MapDpsTrait
     """Private source of restricted zones and virtual walls received through DPS."""
 
@@ -100,7 +105,8 @@ def __init__(self, channel: B01Q10Channel) -> None:
         self.network_info = NetworkInfoTrait()
         self.consumable = ConsumableTrait()
         self._map_dps = MapDpsTrait()
-        self.map = MapContentTrait(self._map_dps, self.command)
+        self.maps = MapsTrait(self.command)
+        self.map = MapContentTrait(self._map_dps, self.maps, self.command)
         self.clean_history = CleanHistoryTrait(self.command)
         # Read-model traits updated from the device's DPS push stream.
         self._updatable_traits = [
@@ -113,6 +119,7 @@ def __init__(self, channel: B01Q10Channel) -> None:
             self.consumable,
             self.clean_history,
             self._map_dps,
+            self.maps,
         ]
         self._subscribe_task: asyncio.Task[None] | None = None
 
@@ -133,20 +140,19 @@ async def close(self) -> None:
     async def refresh(self) -> None:
         """Refresh all traits."""
         # Sending REQUEST_DPS causes the device to publish its ordinary status
-        # values. Map refreshes have their own cadence through ``map.refresh()``.
+        # values. Map-list and map-content refreshes have separate schedules.
         await self.command.send(B01_Q10_DP.REQUEST_DPS, params={})
 
     async def _subscribe_loop(self) -> None:
         """Persistent loop dispatching decoded messages to the read-model traits."""
         async for message in self._channel.subscribe_stream():
-            await self._handle_message(message)
+            self._handle_message(message)
 
-    async def _handle_message(self, message: Q10Message) -> None:
+    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.
-        Map-list DPS responses are handed to the map trait; other DPS updates
-        feed the read-model traits.
+        Map-list DPS responses and other DPS updates feed the read-model traits.
         """
         if isinstance(message, Q10MapPacket):
             self.map.update_from_map_packet(message)
@@ -158,7 +164,6 @@ 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.map.update_from_dps(message.dps)
 
 
 def create(channel: B01Q10Channel) -> Q10PropertiesApi:

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

@@ -6,11 +6,10 @@
 * trace packets are decoded from trace-protocol responses;
 * restricted zones and virtual walls arrive as ordinary DPS values.
 
-``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;
-calibration, path placement and overlay placement remain inside the renderer.
+``MapDpsTrait`` owns the low-level DPS read model. ``MapContentTrait`` uses a
+stored ID from ``MapsTrait`` only when it requests content. It combines map,
+trace, and overlay state through the pure functions in
+:mod:`roborock.map.b01_q10_render`. Map-list updates do not refresh content.
 """
 
 import logging
@@ -21,7 +20,6 @@
 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 (
@@ -36,6 +34,7 @@
 
 from .command import CommandTrait
 from .common import UpdatableTrait
+from .maps import MapsTrait
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -74,33 +73,27 @@ def update_from_dps(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None:
         self._notify_update()
 
 
-@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):
+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.
+    :class:`MapDpsTrait` whenever any of those three sources changes. The
+    :class:`MapsTrait` supplies a stored ID only when this trait requests
+    content.
     """
 
-    _CONVERTER = DpsDataConverter.from_dataclass(MapListDps)
-
     def __init__(
         self,
         map_dps: MapDpsTrait,
-        command: CommandTrait | None = None,
+        maps: MapsTrait,
+        command: CommandTrait,
         *,
         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._maps = maps
         self._command = command
         self._map_packet: Q10MapPacket | None = None
         self._trace_packet: Q10TracePacket | None = None
@@ -110,37 +103,21 @@ def __init__(
         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")
+        """Request content for the first map in the latest saved-map list."""
+        if (map_id := self._maps.current_map_id) is None:
+            raise RoborockException("Cannot request Q10 map content before the map list is available")
+        # Map lists and map content can change at different times. Reuse the
+        # stored ID so a content refresh does not also refresh the list.
         await self._command.send(
             B01_Q10_DP.COMMON,
-            {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}},
+            {
+                str(B01_Q10_DP.MULTI_MAP.code): {
+                    "op": "get",
+                    "id": map_id,
+                }
+            },
         )
 
-    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."""

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

@@ -0,0 +1,58 @@
+"""Trait for Q10 saved-map list data."""
+
+import logging
+from dataclasses import dataclass, field
+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_containers import dpMultiMap
+from roborock.devices.traits.common import DpsDataConverter
+
+from .command import CommandTrait
+from .common import UpdatableTrait
+
+_LOGGER = logging.getLogger(__name__)
+
+
+@dataclass
+class Maps(RoborockBase):
+    """Saved-map list data from the Q10 DPS stream."""
+
+    multi_map: dpMultiMap | None = field(default=None, metadata={"dps": B01_Q10_DP.MULTI_MAP})
+
+    @property
+    def current_map_id(self) -> str | None:
+        """Return the first saved-map ID for a content request, if available."""
+        if self.multi_map is None or self.multi_map.op != "list" or self.multi_map.result != 1:
+            return None
+        return self.multi_map.current_map_id
+
+
+class MapsTrait(Maps, UpdatableTrait):
+    """Request and store the Q10 saved-map list."""
+
+    _CONVERTER = DpsDataConverter.from_dataclass(Maps)
+    _command: CommandTrait
+
+    def __init__(self, command: CommandTrait) -> None:
+        """Initialize the saved-map list trait."""
+        Maps.__init__(self)
+        UpdatableTrait.__init__(self, command, _LOGGER)
+        self._command = command
+
+    async def refresh(self) -> None:
+        """Request a new saved-map list from the device."""
+        await self._command.send(
+            B01_Q10_DP.COMMON,
+            {str(B01_Q10_DP.MULTI_MAP.code): {"op": "list"}},
+        )
+
+    def update_from_dps(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None:
+        """Store a successful saved-map list response."""
+        response = decoded_dps.get(B01_Q10_DP.MULTI_MAP)
+        # DP 61 also carries map-content acknowledgements. Ignore them so they
+        # cannot replace a usable map list with an unrelated response.
+        if not isinstance(response, dict) or response.get("op") != "list" or response.get("result") != 1:
+            return
+        super().update_from_dps(decoded_dps)