refactor: render Q10 maps through one image path

Author: tubededentifrice

roborock/map/b01_q10_render.py

@@ -8,7 +8,7 @@
 
 It exists so the map trait stays about state management: the trait accumulates
 the pushed inputs and calls :func:`render_q10_map` once per change, holding the
-returned object rather than mutating a pile of derived fields itself. All the
+returned image rather than mutating a pile of derived fields itself. All the
 low-level pixel work (erase-zone blanking, world->pixel overlay placement, path
 drawing) and the calibration policy live here, next to the rest of the map code.
 """
@@ -66,36 +66,18 @@ class Q10MapOverlays:
     virtual_walls: Sequence[Q10Zone] = ()
 
 
-@dataclass
-class Q10MapRender:
-    """The image and projected geometry produced by Q10 map composition.
-
-    Built by :func:`render_q10_map` from one map packet, trace packet and DPS
-    overlay snapshot. Source data and intermediate calibration/layer state stay
-    on their owning packet or inside the renderer rather than being repeated on
-    this result.
-    """
-
-    image_content: bytes
-    """The rendered base map (PNG) with erase zones blanked, path not drawn."""
-
-    map_data: MapData
-    """Parsed map data: image metadata, room names, and -- once a calibration is
-    known -- the path / robot position / zones / walls placed in pixel space."""
-
-
 def render_q10_map(
     packet: Q10MapPacket,
     trace: Q10TracePacket | None,
     overlays: Q10MapOverlays,
     *,
     config: B01Q10MapParserConfig,
-) -> Q10MapRender:
-    """Compose the latest map, trace and DPS inputs into a render.
+) -> bytes:
+    """Compose the latest map, trace and DPS inputs into one PNG image.
 
     Calibration is derived from ``packet`` (layers + header calibration) and
     ``trace`` (path points). Once calibrated, erase zones are blanked out of the
-    raster and trace/overlay data is projected into ``map_data`` pixel space.
+    raster and trace/overlay data is projected and drawn in pixel space.
     Without a usable trace only the base raster is rendered. Raises
     :class:`RoborockException` if map rendering fails.
     """
@@ -118,8 +100,9 @@ def render_q10_map(
     if calibration is not None and trace is not None:
         _place_trace(map_data, calibration, trace)
         _place_overlays(map_data, calibration, overlays)
+        return _draw_map_content(parsed.image_content, map_data, config=config)
 
-    return Q10MapRender(image_content=parsed.image_content, map_data=map_data)
+    return parsed.image_content
 
 
 def solve_q10_calibration(
@@ -233,23 +216,17 @@ def to_area(zone: Q10Zone) -> Area | None:
     map_data.walls = walls or None
 
 
-def draw_path_on_map(
-    render: Q10MapRender,
+def _draw_map_content(
+    image_content: bytes,
+    map_data: MapData,
     *,
     config: B01Q10MapParserConfig,
     line_color: tuple[int, int, int, int] = (235, 64, 52, 255),
     position_color: tuple[int, int, int, int] = (255, 211, 0, 255),
 ) -> bytes:
-    """Draw the projected ``MapData`` content onto the base map PNG.
-
-    Returns a fresh PNG; the base raster in
-    :attr:`Q10MapRender.image_content` is left untouched.
-    """
-    if render.map_data.path is None:
-        raise RoborockException("No calibration available; a cleaning path must be captured during a clean")
-
+    """Draw projected map content onto a base PNG and return a fresh PNG."""
     scale = config.map_scale
-    base = Image.open(io.BytesIO(render.image_content)).convert("RGBA")
+    base = Image.open(io.BytesIO(image_content)).convert("RGBA")
 
     def to_image(point: Point) -> tuple[float, float]:
         return (point.x * scale, point.y * scale)
@@ -261,8 +238,8 @@ def to_image(point: Point) -> tuple[float, float]:
 
     # No-go (blue) and no-mop (magenta) zones beneath the path.
     for areas, fill, outline in (
-        (render.map_data.no_go_areas or [], (0, 120, 255, 70), (0, 80, 200, 255)),
-        (render.map_data.no_mopping_areas or [], (255, 0, 200, 70), (200, 0, 160, 255)),
+        (map_data.no_go_areas or [], (0, 120, 255, 70), (0, 80, 200, 255)),
+        (map_data.no_mopping_areas or [], (255, 0, 200, 70), (200, 0, 160, 255)),
     ):
         for area in areas:
             polygon = [
@@ -274,20 +251,20 @@ def to_image(point: Point) -> tuple[float, float]:
             draw.polygon(polygon, fill=fill, outline=outline)
 
     # Virtual walls (line segments, not polygons) drawn over the zones.
-    for wall in render.map_data.walls or []:
+    for wall in map_data.walls or []:
         draw.line(
             [(wall.x0 * scale, wall.y0 * scale), (wall.x1 * scale, wall.y1 * scale)],
             fill=(255, 64, 64, 255),
             width=max(2, scale),
         )
 
-    for path in render.map_data.path.path if render.map_data.path else []:
+    for path in map_data.path.path if map_data.path else []:
         if len(path) >= 2:
             draw.line([to_image(point) for point in path], fill=line_color, width=max(1, scale // 2))
-    if render.map_data.charger is not None:
-        dx, dy = to_image(render.map_data.charger)
+    if map_data.charger is not None:
+        dx, dy = to_image(map_data.charger)
         draw.ellipse([dx - scale, dy - scale, dx + scale, dy + scale], outline=(40, 200, 40, 255), width=2)
-    robot_position = render.map_data.vacuum_position
+    robot_position = map_data.vacuum_position
     if robot_position is not None:
         cx, cy = to_image(robot_position)
         radius = scale

tests/map/test_b01_q10_render.py

@@ -9,10 +9,8 @@
 from dataclasses import replace
 from pathlib import Path
 
-import pytest
 from PIL import Image
 
-from roborock.exceptions import RoborockException
 from roborock.map.b01_grid_layers import GridCalibration
 from roborock.map.b01_q10_map_parser import (
     B01Q10MapParserConfig,
@@ -32,9 +30,7 @@
 from roborock.map.b01_q10_render import (
     _Q10_RESOLUTIONS,
     Q10MapOverlays,
-    Q10MapRender,
     _erased_cells,
-    draw_path_on_map,
     render_q10_map,
     solve_q10_calibration,
 )
@@ -58,7 +54,7 @@ def _render(
     *,
     trace: Q10TracePacket | None = None,
     overlays: Q10MapOverlays | None = None,
-) -> Q10MapRender:
+) -> bytes:
     return render_q10_map(
         packet if packet is not None else _packet(),
         trace,
@@ -96,44 +92,35 @@ def _calibrated_inputs(*, heading: int = 0) -> tuple[Q10MapPacket, Q10TracePacke
 
 def test_render_base_map_without_calibration() -> None:
     """Without a calibration only the base raster is produced."""
-    render = _render()
-    assert render.image_content[:8] == b"\x89PNG\r\n\x1a\n"
-    assert render.map_data is not None
-    # Overlays are world-coordinate only, so nothing is placed yet.
-    assert render.map_data.path is None
+    image = _render()
+    assert image[:8] == b"\x89PNG\r\n\x1a\n"
 
 
-def test_render_places_path_and_position() -> None:
-    """The map and trace derive calibration, path and position together."""
-    packet, trace = _calibrated_inputs(heading=45)
-    render = _render(packet, trace=trace)
+def test_render_draws_path_and_position() -> None:
+    """The map and trace derive calibration and draw the robot position."""
+    packet, trace = _calibrated_inputs()
+    image = _render(packet, trace=trace)
     calibration = solve_q10_calibration(packet, trace)
     assert calibration is not None
-    assert render.map_data.path is not None
-    assert render.map_data.vacuum_position is not None
     assert trace.robot_position is not None
-    expected = calibration.world_to_pixel(trace.robot_position.x, trace.robot_position.y)
-    assert (render.map_data.vacuum_position.x, render.map_data.vacuum_position.y) == expected
-    assert render.map_data.vacuum_position.a == -45
+    px, py = calibration.world_to_pixel(trace.robot_position.x, trace.robot_position.y)
+    image_position = (round(px * CONFIG.map_scale), round(py * CONFIG.map_scale))
+    rendered = Image.open(io.BytesIO(image)).convert("RGBA")
+    assert rendered.size == (8 * 4, 6 * 4)
+    assert rendered.getpixel(image_position) == (255, 211, 0, 255)
 
 
-def test_render_places_zones_and_charger() -> None:
-    """Decoded no-go / no-mop zones become pixel-space MapData areas + charger."""
+def test_render_draws_zones_and_virtual_walls() -> None:
+    """Decoded DPS overlays are included in the composed image."""
     packet, trace = _calibrated_inputs()
     zones = [
         Q10Zone(type=ZONE_TYPE_NO_GO, vertices=[(0, 0), (4, 0), (4, 4), (0, 4)]),
         Q10Zone(type=ZONE_TYPE_NO_MOP, vertices=[(1, 1), (2, 1), (2, 2), (1, 2)]),
     ]
     walls = [Q10Zone(type=ZONE_TYPE_VIRTUAL_WALL, vertices=[(0, 0), (4, 0)])]
-    render = _render(packet, trace=trace, overlays=Q10MapOverlays(zones=zones, virtual_walls=walls))
-    assert len(render.map_data.no_go_areas or []) == 1
-    assert len(render.map_data.no_mopping_areas or []) == 1
-    assert len(render.map_data.walls or []) == 1
-    calibration = solve_q10_calibration(packet, trace)
-    assert calibration is not None
-    assert render.map_data.charger is not None
-    expected = calibration.world_to_pixel(trace.points[0].x, trace.points[0].y)
-    assert (render.map_data.charger.x, render.map_data.charger.y) == expected
+    base = _render(packet, trace=trace)
+    rendered = _render(packet, trace=trace, overlays=Q10MapOverlays(zones=zones, virtual_walls=walls))
+    assert rendered != base
 
 
 def test_render_applies_erase_zones() -> None:
@@ -150,7 +137,7 @@ def test_render_applies_erase_zones() -> None:
     render = _render(replace(packet, erase_zones=[erase_zone]), trace=trace)
 
     assert len(cells) == packet.layers.width * packet.layers.height
-    assert render.image_content != base.image_content  # re-rendered
+    assert render != base
 
 
 def test_render_partial_erase() -> None:
@@ -167,55 +154,29 @@ def test_render_partial_erase() -> None:
     render = _render(replace(packet, erase_zones=[erase_zone]), trace=trace)
 
     assert 0 < len(cells) < packet.layers.width * packet.layers.height
-    assert render.image_content != base.image_content
-
-
-def test_draw_path_on_map_requires_projected_path() -> None:
-    """Drawing fails clearly when the source streams could not calibrate."""
-    with pytest.raises(RoborockException, match="No calibration available"):
-        draw_path_on_map(_render(), config=CONFIG)
-
-
-def test_draw_path_on_map_draws_position() -> None:
-    """The robot position is drawn at the mapped pixel."""
-    packet, trace = _calibrated_inputs()
-    render = _render(packet, trace=trace)
-    png = draw_path_on_map(
-        render,
-        config=CONFIG,
-        position_color=(255, 211, 0, 255),
-    )
-    img = Image.open(io.BytesIO(png)).convert("RGBA")
-    position = render.map_data.vacuum_position
-    assert position is not None
-    image_position = (round(position.x * CONFIG.map_scale), round(position.y * CONFIG.map_scale))
-    assert img.size == (8 * 4, 6 * 4)
-    assert img.getpixel(image_position) == (255, 211, 0, 255)
+    assert render != base
 
 
-def test_draw_path_on_map_draws_heading_indicator() -> None:
+def test_render_draws_heading_indicator() -> None:
     """A known heading draws a facing tick from the robot marker.
 
     With heading 0 (= +x world) and the identity-ish calibration, the tick
     extends to the right of the robot pixel; with the marker at image (12, 12)
     the tick covers pixels at x > 12 along y == 12.
     """
     packet, trace = _calibrated_inputs(heading=0)
-    render = _render(packet, trace=trace)
-    png = draw_path_on_map(
-        render,
-        config=CONFIG,
-        position_color=(255, 211, 0, 255),
-    )
-    img = Image.open(io.BytesIO(png)).convert("RGBA")
-    position = render.map_data.vacuum_position
-    assert position is not None
-    cx = round(position.x * CONFIG.map_scale)
-    cy = round(position.y * CONFIG.map_scale)
+    image = _render(packet, trace=trace)
+    calibration = solve_q10_calibration(packet, trace)
+    assert calibration is not None
+    assert trace.robot_position is not None
+    px, py = calibration.world_to_pixel(trace.robot_position.x, trace.robot_position.y)
+    cx = round(px * CONFIG.map_scale)
+    cy = round(py * CONFIG.map_scale)
+    rendered = Image.open(io.BytesIO(image)).convert("RGBA")
     # Tick runs +x from the marker (4 * radius = 16 px at scale 4).
-    assert img.getpixel((cx + 8, cy)) == (255, 211, 0, 255)
+    assert rendered.getpixel((cx + 8, cy)) == (255, 211, 0, 255)
     # ...and not behind it (the marker is a small disc; sample well to the left)
-    assert img.getpixel((cx - 8, cy)) != (255, 211, 0, 255)
+    assert rendered.getpixel((cx - 8, cy)) != (255, 211, 0, 255)
 
 
 def test_solve_q10_calibration_uses_header_origin_with_short_path() -> None: