docs: clarify Q10 archive callbacks and simplify tests

Author: hCoureau

roborock/devices/README.md

@@ -43,47 +43,38 @@ This speeds up startup time and reduces load on the Roborock cloud APIs.
 
 ## Q10 archive updates
 
-Q10 traits are push driven. `refresh_detail()` publishes a selection and returns
-when the command has been sent. It does not wait for the map response. Register
-an update listener before requesting data, and read the trait when it notifies
-you. A history listener can also fire for record-list updates; compare the
-received detail with the previous detail to distinguish archive updates.
+Archive updates use the same push-driven pattern as all other Q10 traits.
+Register an update listener and update your view's state from the trait when the
+callback runs. `refresh_detail()` publishes a selection; the device sends the
+archive independently through its push stream. A history listener can also fire
+for record-list updates, so read whichever properties your view displays on each
+callback.
 
 ```python
-import asyncio
-
 from roborock.devices.traits.b01.q10 import Q10PropertiesApi
 
 
-async def request_archive_preview(properties: Q10PropertiesApi) -> bytes | None:
-    history = properties.clean_history
-    previous_detail = history.detail
-    received = asyncio.Event()
-
-    def archive_updated() -> None:
-        if history.detail is not previous_detail:
-            received.set()
-
-    record = history.last_record  # Populated by an earlier history list push.
-    if record is None or not record.map_len:
-        return None
-    unsubscribe = history.add_update_listener(archive_updated)
-    try:
-        # This caller chooses to wait; the trait itself only publishes.
-        async with asyncio.timeout(30):
-            await history.refresh_detail(record)
-            await received.wait()
-        return history.detail_image_content
-    finally:
-        unsubscribe()
+class ArchivePreview:
+    def __init__(self, properties: Q10PropertiesApi) -> None:
+        self.history = properties.clean_history
+        self.image_content = self.history.detail_image_content
+        self._unsubscribe = self.history.add_update_listener(self._archive_updated)
+
+    def _archive_updated(self) -> None:
+        self.image_content = self.history.detail_image_content
+        # Notify your UI to redraw using the latest received image.
+
+    async def select_latest_record(self) -> None:
+        record = self.history.last_record
+        if record is not None and record.map_len:
+            await self.history.refresh_detail(record)
+
+    def close(self) -> None:
+        self._unsubscribe()
 ```
 
-For a persistent archive view, keep its listener until the view closes. The
-example is a one-shot caller that waits for its own event with a deadline; that
-waiting policy belongs to the caller. A missing response does not block later
-requests. Publication failures and cancellation propagate to the caller, and
-the publication lock is released.
-The traits do not create response-waiting tasks or futures.
+Keep the listener registered while the view is open and unsubscribe when it
+closes. The callback receives updates regardless of which client requested them.
 
 Clean-record detail packets contain no record identifier. `detail` represents
 the latest received archive, including delayed or unsolicited pushes; it cannot
@@ -104,5 +95,4 @@ map or trace in `properties.map`.
 `properties.as_dict()` includes clean-history records and path data and
 saved-map metadata. Binary map grids and PNG bytes are excluded; access images
 through `detail_image_content`. Live map and trace updates use the usual trait
-listener API without revision counters. The CLI subscribes before requesting a
-push, then waits for an update satisfying its predicate, with a 30-second limit.
+listener API.

tests/devices/traits/b01/q10/test_clean_history.py

@@ -188,24 +188,14 @@ async def test_refresh_sends_op_list(q10_api: Q10PropertiesApi, fake_channel: Fa
     )
 
 
-async def test_refresh_detail_publishes_without_waiting_for_push(
+async def test_refresh_detail_sends_op_select(
     clean_history: CleanHistoryTrait,
     fake_channel: FakeB01Q10Channel,
 ) -> None:
     record = CleanRecordConverter.parse_record(RECORD_A)
     assert record is not None
-    updates = []
-    clean_history.add_update_listener(lambda: updates.append(clean_history.detail))
     await clean_history.refresh_detail(record)
     assert fake_channel.published_commands == [(B01_Q10_DP.COMMON, {"52": {"op": "select", "id": RECORD_A}})]
-    assert clean_history.detail is None
-    assert updates == []
-    detail = _detail()
-    clean_history.update_from_detail(detail)
-    assert clean_history.detail is detail
-    assert clean_history.detail_packet is detail.map
-    assert clean_history.detail_image_content is not None
-    assert updates == [detail]
 
 
 def _detail() -> Q10CleanRecordDetail:
@@ -220,68 +210,39 @@ async def test_refresh_detail_rejects_record_without_map(clean_history: CleanHis
         await clean_history.refresh_detail(record)
 
 
-async def test_missing_push_does_not_block_further_selections(
-    clean_history: CleanHistoryTrait,
-    fake_channel: FakeB01Q10Channel,
-) -> None:
-    first = CleanRecordConverter.parse_record(RECORD_A)
-    second = CleanRecordConverter.parse_record(RECORD_B)
-    assert first is not None and second is not None
-    await clean_history.refresh_detail(first)
-    await clean_history.refresh_detail(second)
-    assert [params["52"]["id"] for _, params in fake_channel.published_commands] == [RECORD_A, RECORD_B]
-    detail = _detail()
-    clean_history.update_from_detail(detail)
-    assert clean_history.detail is detail
-    # The received payload has no record identifier; no association is invented.
-    assert "detailRecord" not in clean_history.as_dict()
-
-
-@pytest.mark.parametrize("failure", ["cancel", "send_error"])
-async def test_failed_publication_releases_lock(
+async def test_refresh_detail_after_cancelled_publication(
     clean_history: CleanHistoryTrait,
     fake_channel: FakeB01Q10Channel,
-    failure: str,
 ) -> None:
     record = CleanRecordConverter.parse_record(RECORD_A)
     assert record is not None
-    if failure == "cancel":
-        fake_channel.send_gate = asyncio.Event()
-    else:
-        fake_channel.send_error = RoborockException("publish failed")
+    fake_channel.send_gate = asyncio.Event()
     request = asyncio.create_task(clean_history.refresh_detail(record))
     await fake_channel.send_started.wait()
-    if failure == "cancel":
-        request.cancel()
-    with pytest.raises(asyncio.CancelledError if failure == "cancel" else RoborockException):
+    request.cancel()
+    with pytest.raises(asyncio.CancelledError):
         await request
+    assert fake_channel.published_commands == []
+
     fake_channel.send_gate = None
-    fake_channel.send_error = None
     await clean_history.refresh_detail(record)
-    assert len(fake_channel.published_commands) == 1
-    clean_history.update_from_detail(_detail())
-    assert clean_history.detail_image_content is not None
+    assert fake_channel.published_commands == [(B01_Q10_DP.COMMON, {"52": {"op": "select", "id": RECORD_A}})]
 
 
-async def test_concurrent_selections_serialize_publication(
+async def test_refresh_detail_after_publication_error(
     clean_history: CleanHistoryTrait,
     fake_channel: FakeB01Q10Channel,
 ) -> None:
-    first = CleanRecordConverter.parse_record(RECORD_A)
-    second = CleanRecordConverter.parse_record(RECORD_B)
-    assert first is not None and second is not None
-    gate = asyncio.Event()
-    fake_channel.send_gate = gate
-    first_request = asyncio.create_task(clean_history.refresh_detail(first))
-    await fake_channel.send_started.wait()
-    fake_channel.send_started.clear()
-    second_request = asyncio.create_task(clean_history.refresh_detail(second))
-    await asyncio.sleep(0)
-    assert not fake_channel.send_started.is_set()
-    assert not second_request.done()
-    gate.set()
-    await asyncio.gather(first_request, second_request)
-    assert [params["52"]["id"] for _, params in fake_channel.published_commands] == [RECORD_A, RECORD_B]
+    record = CleanRecordConverter.parse_record(RECORD_A)
+    assert record is not None
+    fake_channel.send_error = RoborockException("publish failed")
+    with pytest.raises(RoborockException, match="publish failed"):
+        await clean_history.refresh_detail(record)
+    assert fake_channel.published_commands == []
+
+    fake_channel.send_error = None
+    await clean_history.refresh_detail(record)
+    assert fake_channel.published_commands == [(B01_Q10_DP.COMMON, {"52": {"op": "select", "id": RECORD_A}})]
 
 
 def test_received_detail_is_serializable_and_path_list_is_defensive(q10_api: Q10PropertiesApi) -> None: