docs(q7): document live map pushes and cover them in tests - #927
Open
ximex wants to merge 1 commit into
Open
Conversation
The map content trait's module docstring described a pure pull contract -- `refresh()` does I/O, `parse_map_content()` reparses, fields "are then readable" -- which no longer holds: `update_from_push()` mutates the same cached fields from the MQTT receive callback and notifies listeners, so they can change without the reader calling anything. Spell that out. `docs/DEVICES.md` only mentioned `device.b01_q7_properties` for Q7, so the push path was undiscoverable and consumers would build a polling loop for something the device streams by itself. Name the listener entry point and the fields to read. The push test checked `image_content` and `raw_api_response` but not `map_data`, the field carrying the live pose and cleaning path -- dropping its assignment kept the test green. It also never asserted that no RPC was published, which is the whole point of the push path: unlike `refresh()`, it needs no map id and no round trips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up on the Q7 (B01) live map pushes: the runtime behaviour is unchanged, this only makes the push path discoverable and pins it down in tests.
Docs
MapContentTrait's module docstring describes a pure pull contract —refresh()does I/O,parse_map_content()reparses, fields "are then readable". That no longer holds:update_from_push()writes the same cached fields from the MQTT receive callback and notifies listeners, soimage_content/map_data/raw_api_responsecan change without the reader calling anything. The docstring now says so.docs/DEVICES.mdlisted onlydevice.b01_q7_propertiesunder B01 Q7, with no hint that maps arrive on their own. A consumer reading it finds just the pull API and builds a polling loop for something the device streams by itself — so the entry point (map_content.add_update_listener(cb)) and the fields to read are now named, the way the Q10 section already does for its commands.Tests
Two gaps in
test_q7_map_content_updates_from_push:map_datawas not asserted, even though it carries the live robot pose and cleaning path._parse_and_store()sets three fields; dropping themap_dataassignment left the test green. Asserted by identity, since two emptyMapDatainstances compare equal and equality would also accept a stale or copied object.refresh()needsmap_trait.current_map_idand costs two round trips (the refresh tests assertlen(published_commands) == 2), while a push needs neither. Without the assertion the test would still pass ifupdate_from_push()orstart()started issuing RPCs.uv run pytest tests/devices/traits/b01/q7 tests/devices/rpc/test_b01_q7_channel.py→ 55 passed.🤖 Generated with Claude Code