Skip to content

feat: align Q10 map colors with V1 - #902

Merged
allenporter merged 3 commits into
Python-roborock:mainfrom
tubededentifrice:vc/q10-v1-palette
Jul 30, 2026
Merged

feat: align Q10 map colors with V1#902
allenporter merged 3 commits into
Python-roborock:mainfrom
tubededentifrice:vc/q10-v1-palette

Conversation

@tubededentifrice

@tubededentifrice tubededentifrice commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

First rendering slice extracted from #900. The Q10 base raster now follows the V1 map palette, including transparent out-of-map pixels, while both map formats share the same deterministic adjacent-room color policy.

Changes

  • render floor, wall, room, and outside pixels with the V1 palette
  • make pixels outside the map transparent
  • assign room colors deterministically while avoiding the same color for adjacent rooms where possible
  • reset V1 palette state per render so repeated and concurrent renders stay independent
  • keep this slice limited to map parsing/rasterization; it has no trait or transport changes

Before / after

The same synthetic occupancy grid is rendered on main and on this branch.
The checkerboard in the after image denotes transparency.

Before After
Before: Q10-specific colors and opaque outside pixels After: V1 colors and transparent outside pixels

Validation

  • pytest -q — 882 passed, 86 snapshots passed
  • pre-commit run --files ... — all hooks passed

Stack

  1. This PR: base raster and room palette
  2. refactor: render Q10 overlays with V1 #903: V1 overlay renderer
  3. fix: show docked Q10 robot on map #904: docked-state composition

The transport correction is independent in #901. #900 remains the aggregate real-device/Home Assistant before-and-after reference.

@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.

Looks good to me, just some small nits

Comment thread roborock/map/map_parser.py Outdated
room_colors = adjacency_aware_room_colors(raw_data, width, self._room_palette, room_id)
for number, color in room_colors.items():
self._room_palette.cached_room_colors[number] = color
self._room_palette.cached_room_colors[str(number)] = color

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 dont see any code that uses strings for this. Can you explain what's going on here?

otherwise i think tis can be:

self._room_palette.cached_room_colors.update(room_colors)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ColorsPalette.get_room_color accepts str | int and intentionally caches both key forms. Since the cache is pre-populated, updating only the integer entries would leave the corresponding string entries at their original colors, so get_room_color("12") could disagree with get_room_color(12) after recoloring. I kept both assignments, made the cache contract explicit in a comment, and added a regression assertion for the two key forms in e44a7ae.

Comment thread roborock/map/map_parser.py Outdated
Comment on lines +131 to +132
self._room_palette.cached_room_colors.clear()
self._room_palette.cached_room_colors.update(self._base_room_colors)

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.

Does this work as a one-liner?

Suggested change
self._room_palette.cached_room_colors.clear()
self._room_palette.cached_room_colors.update(self._base_room_colors)
self._room_palette.cached_room_colors = {**self._base_room_colors}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That one-liner does not work because cached_room_colors is a getter-only property; assigning to it raises AttributeError. The clear/update pair intentionally resets the existing dictionary in place. I introduced a local variable and added a comment in e44a7ae to make that constraint clearer.

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.

Ah ok. So we're mutating a property meant to be readonly. This seems to me like it is abusing this API, and not really whats intended by returning a readonly property.

I want to get this improvement in, but I don't think this is sustainable. We can discuss further in discord how we want to fix this.

@allenporter
allenporter merged commit dbb6c46 into Python-roborock:main Jul 30, 2026
7 checks passed
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