Skip to content

fix(flights): stop writing debug images on every map composite - #305

Closed
ChuckBuilds wants to merge 1 commit into
mainfrom
fix/flights-debug-image-writes
Closed

fix(flights): stop writing debug images on every map composite#305
ChuckBuilds wants to merge 1 commit into
mainfrom
fix/flights-debug-image-writes

Conversation

@ChuckBuilds

Copy link
Copy Markdown
Owner

Found while auditing SD-card wear. This is the largest single write source I have measured on the rig.

What it was doing

_get_map_background() saved two PNGs every time it composed a map, with nothing gating it — no debug flag, no config check — under a comment that gives it away:

# Debug: Save composite image to see what's happening
try:
    debug_composite = Path("debug_composite.png")
    composite.save(debug_composite)
    ...

Left-over debugging shipped as production behaviour.

Measured on the rig

debug_composite.png 5.36 MB
debug_cropped.png 0.04 MB
composites in 6 hours 5
≈ per day ~108 MB

Written to an SD card, for files nothing reads, on a device whose cards have already failed twice with unreadable-block-device symptoms.

For scale: the health-record churn I fixed in the core repo was ~6,300 small writes a day. This is 108 MB of large ones from a single plugin.

It also gets worse with #304: honouring the ticker's narrower render request means composing at two widths instead of one, roughly doubling the count.

And they landed in the process's working directory — the install root — where a pre-commit hook has previously swept them into a commit (noted in #294).

The fix

Written only when debug logging is enabled, and into the plugin's own tile cache directory rather than wherever the process happens to be running. The debug aid still exists for anyone who turns debug on; it just isn't the default.

⚠️ Version collision with #304

Both this and #304 branch from main at 1.12.13 and bump to 1.12.14. Whichever merges second needs a re-bump to 1.12.15 and a plugins.json regenerate. They touch different code (display_width property vs the debug block), so there is no conflict beyond the manifest — I kept them separate because they are independent fixes and you may want one without the other.

Verification

test_no_debug_image_writes.py walks the module's AST and asserts every save() sits under an isEnabledFor(DEBUG) guard, and that no bare working-directory path remains.

Mutation-checked:

mutation result
ungate the writes 2 checks fail
restore the bare CWD path 1 check fails

All 10 flights suites exit 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

_get_map_background() saved two PNGs every time it composed a map, with
nothing gating it -- no debug flag, no config check -- under a comment reading
"Debug: Save composite image to see what's happening". Left-over debugging
shipped as production behaviour.

Measured on a live rig:

    debug_composite.png   5.36 MB
    debug_cropped.png     0.04 MB
    composites in 6h      5

That is roughly 108 MB a day written to an SD card for files nothing reads, on
a device whose cards have already failed twice with unreadable-block-device
symptoms. It also gets worse, not better, with the change that makes this
plugin honour the ticker's narrower render request: composing at two widths
instead of one roughly doubles the count.

They also landed in the process's working directory, which is the install
root, where a pre-commit hook has previously swept them into a commit.

Now written only when debug logging is enabled, and into the plugin's own tile
cache directory rather than wherever the process happens to be running.

For comparison, the health-record churn fixed in the core repo was ~6,300
small writes a day; this is 108 MB of large ones from a single plugin.

test_no_debug_image_writes.py walks the module's AST and asserts every save()
sits under an isEnabledFor(DEBUG) guard and that no bare working-directory
path remains. Mutation-checked: ungating the writes fails two checks, and
restoring the bare CWD path fails another. All 10 flights suites pass.
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6744f2cb-f7cf-4627-86ea-263affd53786


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 17 complexity

Metric Results
Complexity 17

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@ChuckBuilds

Copy link
Copy Markdown
Owner Author

Superseded by #314, which combines the four ledmatrix-flights PRs into one version bump. Every change from this PR is verified present on that branch; the branch here is untouched if you want to compare.

ChuckBuilds added a commit that referenced this pull request Aug 23, 2026
…artifacts (#314)

* fix(flights): compose the map at the width the ticker asked for

Noticed on a live rig: the flight map came back at two different sizes.

Vegas asks a plugin for a narrower render so a layout built for the full panel
does not read as sparse in the ticker. It normally delivers that by narrowing
the shared canvas for the duration of the call, which a plugin sizing itself
from matrix.width picks up with no changes of its own. But it cannot narrow
the canvas in offscreen mode -- _render_at swaps the shared canvas, which is
unsafe there -- so it only sets the hint, and notes that a plugin reading
get_vegas_render_width() still gets the narrow size while one that reads only
matrix.width "renders full width and is trimmed instead".

This plugin relied on the canvas being narrowed; its own docstring said the
projection scales "without any extra plumbing". Measured over two hours on the
rig, that assumption held 3 times out of 12:

    Native: requesting 256px instead of 512px
    Native: SUCCESS - 1 images, 512px total width      <- cropped afterwards
    Native: SUCCESS - 1 images, 256px total width

so which width came back depended on a path the plugin cannot see, and the
full-width renders composed a map twice the needed width to have most of it
thrown away.

The width property now reads the hint. That covers both paths and falls back
to the panel width outside a Vegas request, so the rotation is unaffected. The
composite cache is already keyed by (width, height) and its comment already
says "Rotation and Vegas render the same map at different widths", so this
adds a cache entry rather than invalidating one.

No plugin in the repo read get_vegas_render_width() before this, despite core
providing and documenting it. Nine other plugins currently render full-width
frames that the adapter then trims by 56-83%; they are candidates for the same
treatment but each needs its own layout thought, so this fixes the one with a
measured problem.

test_vegas_render_width.py asserts the width follows the request, returns to
the panel width afterwards, and falls back on a nonsense hint, plus that the
cache still keys on size. Mutation-checked: reverting to matrix.width fails it.

The harness sets both display_manager and _display_manager_ref deliberately:
this plugin keeps the private name while BasePlugin stores the same object
under the public one, and get_vegas_render_width()'s fallback reads the public
one. A stub with only the private name makes the fallback miss the matrix and
return its hard-coded 128, which looks exactly like the property being broken.

* fix(flights): stop writing debug images on every map composite

_get_map_background() saved two PNGs every time it composed a map, with
nothing gating it -- no debug flag, no config check -- under a comment reading
"Debug: Save composite image to see what's happening". Left-over debugging
shipped as production behaviour.

Measured on a live rig:

    debug_composite.png   5.36 MB
    debug_cropped.png     0.04 MB
    composites in 6h      5

That is roughly 108 MB a day written to an SD card for files nothing reads, on
a device whose cards have already failed twice with unreadable-block-device
symptoms. It also gets worse, not better, with the change that makes this
plugin honour the ticker's narrower render request: composing at two widths
instead of one roughly doubles the count.

They also landed in the process's working directory, which is the install
root, where a pre-commit hook has previously swept them into a commit.

Now written only when debug logging is enabled, and into the plugin's own tile
cache directory rather than wherever the process happens to be running.

For comparison, the health-record churn fixed in the core repo was ~6,300
small writes a day; this is 108 MB of large ones from a single plugin.

test_no_debug_image_writes.py walks the module's AST and asserts every save()
sits under an isEnabledFor(DEBUG) guard and that no bare working-directory
path remains. Mutation-checked: ungating the writes fails two checks, and
restoring the bare CWD path fails another. All 10 flights suites pass.

* perf(flights): stop the tracker writing 690 log lines every half hour

The flight tracker logged two unconditional INFO lines on every poll -- one
naming the aircraft count it was about to process, one summarising the result.
Polls run about every five seconds, so on a live rig that was 686 lines per
half hour, roughly 86% of the device's entire log volume, and a steady trickle
of journal writes to the SD card for a line that mostly repeated itself.

The "Processing N aircraft" line is trace: the summary immediately below
reports the same total. Demoted to debug, with lazy %-args so a disabled level
costs nothing to skip.

The summary is worth keeping, so it is now reported when it changes. Which
fields to key on mattered more than expected. Total and With-position jitter
on almost every poll as distant traffic drifts through the receiver's edge,
and keying on the whole line collapsed 343 samples to 210 -- barely worth
doing. Keying on what the plugin actually displays, aircraft in range and
aircraft tracked, collapses the same samples to 67. The jittery counts still
ride along in the message, where they cost nothing.

A 300-second heartbeat keeps a quiet sky from looking like a stalled tracker.

Measured by replaying 343 real polls captured from a running rig through the
committed logic: 686 lines become 67, a 90% reduction, with the heartbeat
never needing to fire. Safety harness passes at all eight sizes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

* chore(flights): drop the committed debug images

debug_composite.png and debug_cropped.png are output, not assets. The map
composer writes them with Path("debug_composite.png") -- a relative path, so
on an install they land in the process working directory, which is the core
checkout. A running rig accordingly carries two untracked files in its
LEDMatrix repo, and copies of them were committed here as well.

Untracked files in a checkout are harmless until upstream adds a file at the
same path, at which point the pull refuses. There is no reason to leave that
waiting.

Removed and added to .gitignore so they cannot come back. #305 is what stops
them being written at all unless debug logging is on, and puts them under the
tile cache directory rather than the CWD; this only clears what was already
committed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

---------

Co-authored-by: Claude <noreply@anthropic.com>
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