Skip to content

refactor: reuse shared grid layers in Q7 parser#882

Draft
tubededentifrice wants to merge 5 commits into
Python-roborock:mainfrom
tubededentifrice:agent/q7-map-layers
Draft

refactor: reuse shared grid layers in Q7 parser#882
tubededentifrice wants to merge 5 commits into
Python-roborock:mainfrom
tubededentifrice:agent/q7-map-layers

Conversation

@tubededentifrice

@tubededentifrice tubededentifrice commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Keeps the Q7 shared-grid reuse from #848 as a small parser-internal refactor. The draft no longer expands the Q7 trait API and remains independent of the Q10 map stack.

Changes

  • Classify Q7 SCMap cells into the shared background, wall, and floor classes.
  • Build GridLayers once inside B01MapParser.parse from the already-parsed protobuf payload.
  • Render the existing composed Q7 image from those internal canonical layers.
  • Keep MapContentTrait unchanged: callers continue to receive only image_content, map_data, and raw_api_response.
  • Remove speculative Q7 calibration; no current Q7 renderer or caller consumes it.

Review feedback addressed

  • Layers and calibration are no longer exposed as public trait fields.
  • The trait no longer reparses the raw map payload.
  • Shared decomposition stays behind the existing parser interface and is used directly by rendering.

Validation

  • focused Q7 parser and trait suite — 39 passed
  • pytest -q — 792 passed, 86 snapshots passed
  • pre-commit run --show-diff-on-failure --color=always --all-files — all hooks passed, including Ruff and mypy

@allenporter allenporter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is draft still and not ready but giving early feedback in the name of quick iteration

map_data: MapData | None = None
"""Parsed map data (metadata for points on the map)."""

layers: GridLayers | None = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These don't seem like data points we want or need to expose to callers, and are more like internal map rendering details.

self.image_content = parsed_data.image_content
self.map_data = parsed_data.map_data
self.raw_api_response = raw_payload
try:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once parsed, these don't appear to be used by anything.

is there a specifc reason we're parsing the raw payload again? I would expect this to all be behind the existing self._map_parser.parse call above where the map parser parses the raw content and pulls out useful data points.

Is it because in q10 these come as a separate channel, therefore we need to change the map interface from working in raw payload to be raw payload + layers + calibration data?

@tubededentifrice tubededentifrice changed the title feat: expose shared map layers for Q7 refactor: reuse shared grid layers in Q7 parser Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants