Files
planeMapper/_bmad-output/implementation-artifacts/2-6-stateful-renderer-and-display-interface.md
T
Matt Edholm 709053debf review(2-6): story 2-6 passes all ACs — mark done, add deferred items
Code review of stateful Renderer and DisplayInterface: all 10 review
criteria pass with no code changes required. Add two deferred items for
WaveshareDisplay NotImplementedError stub (wired in 2-7) and pixel-space
trail staleness on re-provisioning. Mark story done in story file and
sprint-status.yaml.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-22 23:35:13 -04:00

144 lines
7.6 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Story 2.6: Stateful Renderer & Display Interface
Status: done
## Story
As the radar loop,
I want a stateful `Renderer` owning the in-memory tile composite and per-aircraft trail history, and a `DisplayInterface` protocol with `WaveshareDisplay` (SPI) and `NullDisplay` (tests),
So that the render pipeline is fully isolated, testable without hardware, and trail history persists across cycles.
## Acceptance Criteria
AC1: **Given** a `Renderer` initialised with a loaded base map **When** `renderer.render(aircraft_list)` is called **Then** it returns a `PIL.Image` (800×480) with base map, airspace outlines, home marker, and all aircraft drawn
AC2: **Given** an aircraft appears in two consecutive calls to `renderer.render()` **When** the second call is made **Then** its previous position appears as a trail dot **And** trail length never exceeds `TRAIL_MAX_DOTS` (5)
AC3: **Given** an aircraft was present last cycle but absent from current list **When** `renderer.render()` is called **Then** the aircraft does not appear on display **And** its trail history is retained in `dict[str, deque]` for when it reappears
AC4: **Given** a `NullDisplay` **When** `display.show(image)` is called **Then** it logs image dimensions at DEBUG level and returns without error
AC5: **Given** the `test_pipeline.py` smoke test (`FileFixtureFetcher → Renderer → NullDisplay`) **When** one full cycle runs **Then** it completes without exception and the returned image is 800×480
## Tasks / Subtasks
- [x] Task 1: Update `NullDisplay` in `src/planemapper/display.py` (AC: #4)
- [x] 1.1 Add `import logging` and `log = logging.getLogger(__name__)`
- [x] 1.2 Update `NullDisplay.show()` to log at DEBUG level:
```python
log.debug("NullDisplay.show: %dx%d", image.width, image.height)
```
- [x] 1.3 Add `WaveshareDisplay` stub:
```python
class WaveshareDisplay:
def show(self, image: Image.Image) -> None:
raise NotImplementedError
```
- [x] Task 2: Implement `Renderer` in `src/planemapper/renderer/renderer.py` (AC: #1, #2, #3)
- [x] 2.1 Replace `# stub` with full implementation
- [x] 2.2 Imports:
```python
import collections
from PIL import Image
from planemapper.models import Aircraft
from planemapper.constants import TRAIL_MAX_DOTS
from planemapper.renderer.projection import MapBounds, project
from planemapper.renderer.airspace import draw_airspace
from planemapper.renderer.overlay import draw_home_marker
from planemapper.renderer.aircraft import draw_aircraft
```
- [x] 2.3 Implement `__init__` with `base_map`, `bounds`, `_trails` dict:
```python
def __init__(
self,
base_map: Image.Image,
bounds: MapBounds,
) -> None:
self._base_map = base_map
self._bounds = bounds
self._trails: dict[str, collections.deque[tuple[int, int]]] = {}
```
- [x] 2.4 Implement `render(aircraft_list)` with full pipeline:
```python
def render(self, aircraft_list: list[Aircraft]) -> Image.Image:
image = self._base_map.copy()
draw_airspace(image, self._bounds)
draw_home_marker(image, self._bounds)
for aircraft in aircraft_list:
pos = project(aircraft.lat, aircraft.lon, self._bounds)
trail = self._trails.get(aircraft.icao, collections.deque())
draw_aircraft(image, aircraft, pos, trail)
trail.appendleft(pos)
while len(trail) > TRAIL_MAX_DOTS:
trail.pop()
self._trails[aircraft.icao] = trail
return image
```
- [x] Task 3: Write tests in `tests/test_renderer.py` (AC: #1, #2, #3)
- [x] 3.1 Replace placeholder with full test module
- [x] 3.2 Test AC1: create white 800×480 RGB base map, call `render([])` with empty aircraft list, assert returned image is `PIL.Image.Image` with size `(800, 480)`
- [x] 3.3 Test AC2: call `render()` twice with same aircraft; after second call, assert `renderer._trails` has an entry keyed on the aircraft's ICAO
- [x] 3.4 Test AC3: call `render()` with one aircraft, then call `render([])` with empty list; assert `renderer._trails` still has the aircraft ICAO entry
- [x] Task 4: Write/update pipeline smoke test in `tests/test_pipeline.py` (AC: #5)
- [x] 4.1 Replace placeholder with full smoke test:
- Use `FileFixtureFetcher(Path("tests/fixtures/aircraft_sample.json"))`
- Create a fake 800×480 white RGB base map (`Image.new("RGB", (800, 480), "white")`)
- Create `Renderer(base_map, bounds)` where `bounds = MapBounds(53.0, -6.0, 100.0)`
- Monkeypatch `planemapper.renderer.airspace.AIRSPACE_PATH` to `tests/fixtures/airspace_sample.geojson`
- Call `renderer.render(fetcher.fetch())`
- Assert returned image is `PIL.Image.Image` with size `(800, 480)`
- Call `NullDisplay().show(image)` — assert no exception
- [x] 4.2 Ensure monkeypatch of `AIRSPACE_PATH` is applied before `render()` is called
- [x] Task 5: Run quality gates
- [x] 5.1 `python -m pytest tests/` — all tests pass
- [x] 5.2 `python -m ruff check .` — zero violations
- [x] 5.3 `python -m ruff format --check .` — no formatting issues
## Implementation Notes
### `Renderer` class — trail management detail
Trail entries are `(x, y)` pixel positions stored most-recent-first (index 0). After drawing each aircraft:
1. `trail.appendleft(pos)` — prepend current position
2. `while len(trail) > TRAIL_MAX_DOTS: trail.pop()` — trim oldest entries from the right
3. `self._trails[aircraft.icao] = trail` — write back (no-op if already the same deque object, but keeps the dict consistent)
Aircraft absent from `aircraft_list` are not iterated, so their trail deque remains in `self._trails` untouched, ready to resume when the aircraft reappears.
### `WaveshareDisplay` stub
This story adds only the stub. The real SPI driver implementation (using the Waveshare Python library) is deferred to story 2-7. The stub class must satisfy the `DisplayInterface` protocol structurally — it has a `show(self, image: Image.Image) -> None` method — but raises `NotImplementedError` so it cannot be called accidentally in tests.
### `NullDisplay` logging
`NullDisplay` lives in `src/planemapper/display.py`. The module-level logger uses `__name__` (`planemapper.display`). Log format: `"NullDisplay.show: %dx%d"` with `image.width` and `image.height` as positional args (uses `%`-style lazy formatting, not f-strings, to avoid string construction overhead when DEBUG is not enabled).
### `render()` pipeline order
The pipeline order is significant:
1. Copy base map — ensures each cycle starts from the clean pre-rendered tile composite
2. Draw airspace outlines — static geometry, drawn once per cycle over the base copy
3. Draw home marker — static overlay
4. Draw aircraft (with trails) — dynamic, per-cycle
All drawing mutates the `image` copy in-place. `self._base_map` is never mutated.
### Test fixtures
`tests/test_renderer.py` does not need any fixture files — it constructs a minimal white `Image.new("RGB", (800, 480), "white")` base map and uses a handcrafted `Aircraft` object. Use `unittest.mock.patch` or `pytest` monkeypatch to suppress `draw_airspace` and `draw_home_marker` if they have external dependencies (e.g. airspace file path), or monkeypatch `AIRSPACE_PATH` as done in the pipeline smoke test.
### Existing files affected
| File | Change |
|---|---|
| `src/planemapper/display.py` | Add logging; update `NullDisplay.show()`; add `WaveshareDisplay` stub |
| `src/planemapper/renderer/renderer.py` | Replace `# stub` with full `Renderer` class |
| `tests/test_renderer.py` | Replace placeholder with AC1/AC2/AC3 tests |
| `tests/test_pipeline.py` | Replace placeholder with end-to-end smoke test |