flights: stop writing debug PNGs on every map rebuild - #321
Conversation
_get_map_background() saved two images unconditionally:
# Debug: Save composite image to see what's happening
composite.save(Path("debug_composite.png"))
cropped.save(Path("debug_cropped.png"))
No flag, no log-level check -- every map composite wrote ~27KB into the
process's working directory. On a Pi that is SD-card wear for nothing,
and the files land in whatever directory the service was started from,
which for a normal install is the checkout itself: two untracked PNGs
appearing in `git status` on the device.
I found them by accident. They turned up as untracked files in the
plugins repo while I was running tests, timestamped minutes earlier.
Kept rather than deleted, behind self.logger.isEnabledFor(logging.DEBUG)
-- they are genuinely useful when working on the map, and debug logging
is the control that already exists for "I want to see what is happening".
Verified: the harness leaves no PNGs behind now, and both wrong-guard
mutations fail the new test -- `if True:` and gating on an unrelated flag
(map_bg_enabled) each fail 2 of 3. One test pins the premise that the
saves still exist, so if they are ever removed outright the file says so
instead of passing vacuously.
9 plugin tests pass, harness 8/8, no module collisions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
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 | 20 |
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.
Three PRs are open at once, each bumping a different plugin, and each
carrying its own regenerated plugins.json. The first to merge is clean;
the others then conflict on that file -- confirmed by merging all three
into a scratch branch:
#319 merged cleanly
#320 CONFLICT in plugins.json
#321 CONFLICT in plugins.json
The file is generated, and .github/workflows/update-registry.yml already
regenerates and commits it on any push to main touching
plugins/*/manifest.json -- which every one of these PRs does. So carrying
it here buys nothing and costs a conflict per concurrent PR.
Reverted to main's copy. The manifest bump stays, so the registry still
updates -- just on main, once, instead of three times in conflict.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
Three PRs are open at once, each bumping a different plugin, and each
carrying its own regenerated plugins.json. The first to merge is clean;
the others then conflict on that file -- confirmed by merging all three
into a scratch branch:
#319 merged cleanly
#320 CONFLICT in plugins.json
#321 CONFLICT in plugins.json
The file is generated, and .github/workflows/update-registry.yml already
regenerates and commits it on any push to main touching
plugins/*/manifest.json -- which every one of these PRs does. So carrying
it here buys nothing and costs a conflict per concurrent PR.
Reverted to main's copy. The manifest bump stays, so the registry still
updates -- just on main, once, instead of three times in conflict.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
Removed Carrying it here bought nothing and cost a conflict per concurrent PR. With three open at once, each bumping a different plugin, only the first could merge cleanly. Confirmed by merging all three into a scratch branch: The manifest bump stays, so the registry still updates — once, on |
|
Closing as redundant — #314 already fixes this, and fixes it better. I found the unconditional
So #314 supersedes this entirely, and the two would have conflicted on the same region of Nothing here is worth salvaging into #314; the only thing this PR had that #314 doesn't is a regression test, and I'd rather not bolt a test onto someone else's consolidation PR unasked. Happy to add one as a follow-up once #314 lands, if you want the guard pinned. |
#320) * music: download album art on the polling thread, not the render thread display() fetched album art inline whenever the cached image was empty: if image_currently_in_cache and art_url_currently_in_cache == target: ...use cache... else: fetched_image = self._fetch_and_resize_image(target, size) # HTTP and the polling thread deliberately emptied that cache on every track change -- it set self.album_art_image = None and moved on. So the invalidation happened off-thread while the refill happened on the render thread, and the panel blocked for an HTTP round trip every time the track changed. The poller already knew the URL had changed. It now downloads there. _fetch_and_resize_image is split so the two halves can run in different places: _fetch_album_art_bytes does the network, _render_album_art decodes and fits. Only the second runs in display(), which is also where the target size is finally known -- that size dependency is presumably why the fetch ended up there in the first place. Both polling paths prefetch (_process_ytm_data_update and _poll_music_data); a grep for one would have missed the other. The bytes are stored with the URL they came from, so a stale prefetch cannot be shown for a newly-started track, and the original inline fetch stays as a first-paint fallback so nothing goes blank before the poller catches up. Verified by mutation -- each guard fails independently: prefetch call removed -> 1 failed display() ignores the bytes -> 1 failed URL check dropped -> 1 failed 43 tests pass (38 on main plus 5 new), harness 8/8, no module collisions. Not yet seen on a panel: both rigs are unreachable, so this has the harness renders behind it but no hardware run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * chore: leave plugins.json to main's registry workflow Three PRs are open at once, each bumping a different plugin, and each carrying its own regenerated plugins.json. The first to merge is clean; the others then conflict on that file -- confirmed by merging all three into a scratch branch: #319 merged cleanly #320 CONFLICT in plugins.json #321 CONFLICT in plugins.json The file is generated, and .github/workflows/update-registry.yml already regenerates and commits it on any push to main touching plugins/*/manifest.json -- which every one of these PRs does. So carrying it here buys nothing and costs a conflict per concurrent PR. Reverted to main's copy. The manifest bump stays, so the registry still updates -- just on main, once, instead of three times in conflict. 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>
2 participants
The bug
_get_map_background()saved two images unconditionally:No flag, no log-level check. Every map composite wrote ~27KB into the process's working directory — on a Pi that's SD-card wear for nothing, and the files land wherever the service was started from. For a normal install that's the checkout itself, so two untracked PNGs show up in
git statuson the device.There was no config option, so nobody could turn them off.
How I found it
By accident. They appeared as untracked files in the plugins repo while I was running tests, timestamped minutes earlier — I was looking at something else entirely and noticed the clutter.
The fix
Kept, not deleted — they're genuinely useful when working on the map. Now behind
self.logger.isEnabledFor(logging.DEBUG), which is the control that already exists for "show me what's happening", rather than a new flag nobody would find.Verification
The harness now leaves no PNGs behind. Both wrong-guard mutations fail the new test:
One test pins the premise that the saves still exist, so if they're ever removed outright the file says so rather than passing vacuously.
run_plugin_tests.py ledmatrix-flights: 9 passed. Harness 8/8. No module collisions.This is the only plugin doing it — I checked all 43 for
.save()of a debug path.