fix(flights): stop writing debug images on every map composite - #305
fix(flights): stop writing debug images on every map composite#305ChuckBuilds wants to merge 1 commit into
Conversation
_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.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 17 |
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.
|
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. |
…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>
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:Left-over debugging shipped as production behaviour.
Measured on the rig
debug_composite.pngdebug_cropped.pngWritten 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.
Both this and #304 branch from
mainat1.12.13and bump to1.12.14. Whichever merges second needs a re-bump to 1.12.15 and aplugins.jsonregenerate. They touch different code (display_widthproperty 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.pywalks the module's AST and asserts everysave()sits under anisEnabledFor(DEBUG)guard, and that no bare working-directory path remains.Mutation-checked:
All 10 flights suites exit 0.
🤖 Generated with Claude Code
https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW