[hCoureau]

Expose parsed Q10 clean-record maps and historical paths, plus saved-map preview packets, through the existing push-driven trait APIs. Archive pushes update typed state and notify listeners independently of the live map; archive traits do not render or store PNGs. Live-map scale and drawable configuration remains optional.

Selections publish commands and updates arrive through trait listeners. Clean-record packets contain no record ID for attribution; saved-map packets supply a map ID without request correlation. Shared RoborockBase serialization includes history and typed path data while excluding binary map packets and grids.

Validation: focused Q10 suites passed 274 tests on Python 3.11 and 3.14. Full suites passed 2,237 tests with 53 expected failures on each; the default test configuration deselects 11 MQTT-broker tests. Pre-commit, commitlint, and wheel/source distribution builds passed.

Updated on upstream main (acd298ee, 7.19.0). GitHub CI passed, including MQTT-broker suites on Python 3.11 and 3.14. No physical-device validation was run for this revision. Related: #767.

[hCoureau]

Addressed all five review comments in f1f5e24 and pushed the changes to this branch.

The archive traits now follow the existing push/listener contract: selections return after publication, asyncio.Lock serializes sends, and received archives update the read models and notify subscribers. Clean-history data is in a RoborockBase dataclass and included in diagnostic serialization. Removed the response-waiting machinery, revision counters and unsupported clean-record request attribution. Added consumer documentation, fake-channel command/lifecycle tests and PNG-dimension tests for shared render configuration.

Validation: 1,440 passed, 53 expected failures and 92 snapshots passed on both Python 3.11 and 3.14; all pre-commit hooks and both package builds passed. The PR description now reflects the final API and protocol limitations.

[allenporter]

Note: Conducting this code review following our /review skill using Antigravity (AGY) and Gemini 4 Argon.

Thanks for updating the traits to use the push/listener pattern and extracting the RoborockBase read models!

Stepping back, I want to make sure I understand the intended consumer use case here—especially around rendering standalone PNG images (detail_image_content) on CleanHistoryTrait and MapsTrait:

1. Clean record & saved map rendering vs. live map isolation:

   • Protecting properties.map so that 03 01 (clean record archive) and 04 01 (saved map preview) packets on the push stream do not overwrite the active live map (01 01) makes complete sense.
   • However, it's surprising to see CleanHistoryTrait and MapsTrait eagerly rendering and storing their own PNG images (detail_image_content). Neither V1 (v1/clean_summary.py, v1/maps.py) nor Q7 (q7/clean_summary.py) renders map images on clean_summary or maps.
   • Because 03 01 packets carry no record ID on the wire, clean_history.detail / detail_image_content cannot even be reliably attributed to a specific Q10CleanRecord (e.g., if another client/app triggers a history push, we also synchronously run PIL rendering on the event loop for an unattributable packet).
   • How do you envision a downstream consumer (like Home Assistant) using clean_history.detail_image_content and maps.detail_image_content? If there isn't a concrete consumer need for rendering archive images right now, should we simplify this PR to just isolating properties.map (and/or exposing the parsed packet data without eager image rendering)?

2. Removing _detail_lock and lock-serialization tests:

   • In my earlier comment, asyncio.Lock was only mentioned as an alternative to the hand-rolled locking when refresh_detail() was blocking on a response future. Now that refresh_detail() is a fire-and-forget self._command.send(...) just like refresh(), we don't need _detail_lock at all—nor the send_gate / lock-serialization / cancellation tests.

[allenporter]

Sorry for the long delay on reviewing this stack—I set aside time today to go through #937, #938, #939, #934, and #935 so you have feedback across the whole set rather than getting drip-fed comments one PR at a time.

First, the Q10 reverse-engineering and wire-format work across these PRs is great—having verified byte layouts for the map tail sections, zones/virtual walls, and room settings is super valuable.

To be transparent about why these have been slow for me to get through: the core protocol and trait changes are surrounded by a lot of extra defensive machinery (locks, confirmation futures, revision/freshness tracking, runtime type checks on typed args, and hundreds of lines of tests exercising those edge cases) plus the cumulative branch stacking. That makes the diffs much larger and harder to reason about than the underlying protocol changes need to be.

For Q10, we really want the traits to stay thin and boring:

• Commands simply encode and publish to the channel (self._command.send(...)).
• Incoming pushes decode into RoborockBase / RoborockEnum models, update trait state, and notify listeners.
• No extra state machines, locks, or speculative APIs unless a downstream consumer (like Home Assistant) directly needs them.

Once we strip out that extra layer, these changes should be much smaller, independent of each other where possible (for example, #938 can target main directly without waiting on #937), and much faster for me to review and merge.

Thanks again!

[Lash-L]

Marking as a draft while Allen's comments are oustanding!

[allenporter]

Note: Conducting this code review following our /review skill using Antigravity (AGY) and Gemini 4 Argon.

Thanks for addressing the inline comments! I merged main (93c2d7e4) to resolve the conflicts with #938 and #934.

Following up on my question from the last review: how do you envision a downstream consumer (like Home Assistant) using detail_image_content on clean_history and maps? Neither V1 nor Q7 renders PNGs on those traits, and 03 01 pushes don't carry a record ID to tie the rendered image back to a specific clean record.

Isolating properties.map from 03 01 / 04 01 pushes makes complete sense, so if there isn't a concrete consumer need for archive/preview PNGs right now, I'd prefer to drop detail_image_content rendering and keep those traits lightweight.

Also left one small inline note on keeping map_parser_config optional.

[hCoureau]

Addressed in ada5119. Removed detail_image_content, automatic PNG rendering and rendering configuration from both archive traits. Typed archive maps and historical paths still update through listeners, independently of the live map. map_parser_config remains optional for live rendering, and the clean-record attribution limitation stays in the docstring. Your merge of main is preserved.