fix(ledmatrix-flights): give the Vegas map the same features as the rotation - #231
Merged
Conversation
…otation get_vegas_content() reimplemented a cut-down version of the map view rather than sharing it. Its copy drew only the map background and the aircraft dots, so three things were silently missing from the ticker: - aircraft trails, even with show_trails enabled - the white centre-position marker, which is the map's only reference point - the aircraft count and airplane icon The trail data was always there — aircraft_trails is populated in update() regardless of which display path runs — so this was purely a rendering gap. It is duplication drift rather than a deliberate simplification: _display_map gained features over time and the Vegas copy did not track them. Extracted _render_map_image() as the single source of truth, called by both _display_map and get_vegas_content. Fixing the copy in place would have left the same trap for the next feature added to the map view. Sizing needs no extra plumbing: display_width/display_height are properties over matrix, and Vegas narrows the display manager while requesting content, so _latlon_to_pixel already scales its projection to whatever width the ticker asked for. Verified at 64x32 through 512x64, including a 64px-wide render. test_vegas_map_parity.py asserts the Vegas image is byte-identical to what the rotation pushes to the display, so the two cannot diverge again, plus direct coverage of the trails, the centre marker and the untracked-trail skip. Also drops two now-unused locals from get_vegas_content. Safety harness: all sizes PASS. Module collisions: OK. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Contributor
📝 WalkthroughWalkthroughThe Vegas map renderer is centralized in ChangesVegas map parity
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 | 38 |
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/ledmatrix-flights/test_vegas_map_parity.py (1)
221-225: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover every supported panel size.
This omits required 128×64 and 256×32 coverage while testing unsupported 256×64 and 512×64 dimensions.
Proposed test matrix
- `@pytest.mark.parametrize`("width,height", [(64, 32), (128, 32), (256, 64), (512, 64)]) + `@pytest.mark.parametrize`( + "width,height", + [(64, 32), (128, 32), (128, 64), (256, 32)], + )As per coding guidelines, “Every plugin must render correctly on all supported matrix sizes: 64×32, 128×32, 128×64, and 256×32.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/ledmatrix-flights/test_vegas_map_parity.py` around lines 221 - 225, Update the width,height parameter matrix in test_renders_without_error_at_any_size to cover exactly the supported sizes 64×32, 128×32, 128×64, and 256×32; remove the unsupported 256×64 and 512×64 cases while preserving the existing render and image-size assertions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/ledmatrix-flights/manager.py`:
- Around line 2902-2903: Move map background retrieval out of the render path:
update() should refresh tile/background data through self.cache_manager, while
display() and get_vegas_content() should only read the cached background and
retain the existing black fallback when unavailable. Ensure
_get_map_background() is no longer invoked synchronously by either render
method.
---
Nitpick comments:
In `@plugins/ledmatrix-flights/test_vegas_map_parity.py`:
- Around line 221-225: Update the width,height parameter matrix in
test_renders_without_error_at_any_size to cover exactly the supported sizes
64×32, 128×32, 128×64, and 256×32; remove the unsupported 256×64 and 512×64
cases while preserving the existing render and image-size assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 05a8f722-73c6-4dd4-bda7-ee98ff7ff369
📒 Files selected for processing (5)
plugins.jsonplugins/ledmatrix-flights/CHANGELOG.mdplugins/ledmatrix-flights/manager.pyplugins/ledmatrix-flights/manifest.jsonplugins/ledmatrix-flights/test_vegas_map_parity.py
Drops an unused ImageDraw import, and replaces Cls.__new__(Cls) with a no-op-__init__ subclass for building the test shell. The __new__ form is valid Python but Codacy's Pylint reports it as a missing-cls call; the subclass reads better anyway and states the intent — bypass the real __init__, which builds fetchers, threads and caches the map renderer does not need. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/ledmatrix-flights/test_vegas_map_parity.py`:
- Around line 63-76: Update _plugin_shell and all sibling imports in this test
to load plugins/ledmatrix-flights modules under a plugin-unique module name
instead of bare top-level names such as manager. Preserve the existing
FlightTrackerPlugin behavior while ensuring imports cannot resolve modules from
another plugin; apply the same naming convention consistently to every sibling
module import in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f66f2c74-c2e1-4d4c-8883-803680f6cc2c
📒 Files selected for processing (2)
plugins.jsonplugins/ledmatrix-flights/test_vegas_map_parity.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins.json
20 tasks
ChuckBuilds
added a commit
that referenced
this pull request
Aug 3, 2026
…243) The cached map composite is already cropped to the display aspect ratio and resized to the panel before it is stored, but the memo was keyed on centre and zoom alone. Vegas narrows the display manager while it requests content, so the rotation and the ticker ask for the same view at different widths — and whichever rendered second was handed the other one's image. _render_map_image() copies that background, so the symptom is a whole frame at the wrong size, with aircraft and trails projected for the size it didn't get. This is why #231's "sizing needs no extra plumbing" only held for _latlon_to_pixel: the projection follows the display, the cached background didn't. The cache now holds one entry per size and clears them all when the centre or zoom moves. Keeping every size rather than a single slot keyed on size matters because the two paths alternate: a single slot would re-crop and re-resize on every Vegas/rotation swap. Also renames cached_map_bg -> cached_map_bgs, updating the config-reload test that asserts the cache is invalidated on change. Tests: three cases in test_vegas_map_parity.py covering the size after a narrower render, per-size cache retention, and the end-to-end frame size across a size switch. All three fail against the old memo. The existing parity tests can't catch this — they set map_bg_enabled = False, so the tile path never runs. Full suite: 21 passed, plus test_config_reload.py (15) and test_overhead_radius.py green. check_module_collisions.py: OK across 42 plugins. Claude-Session: https://claude.ai/code/session_016ZQZi3TiR77t2k5QjPix3E Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Aircraft trails never appeared on the map in Vegas scroll mode, even with
show_trailsenabled.get_vegas_content()reimplemented a cut-down version of the map view rather than sharing it. Its copy drew two things;_display_mapdrew five. Missing from the ticker:The trail data was always being collected:
aircraft_trailsis populated inupdate()regardless of which display path runs. This was purely a rendering gap.It's duplication drift rather than a deliberate simplification —
_display_mapaccumulated features over time and the Vegas copy didn't track them.Fix
Extracted
_render_map_image()as the single source of truth, called by both_display_mapandget_vegas_content.I deliberately did not just add the three missing pieces to the copy: that leaves the identical trap for the next feature added to the map view. There is now one renderer and one call site for
_get_map_background.Sizing needs no extra plumbing
Worth noting for review, since it looks like it should:
display_width/display_heightare properties overmatrix, and the core narrows the display manager while requesting Vegas content, so_latlon_to_pixelalready scales its projection to whatever width the ticker asked for. Verified from 64×32 up to 512×64, including an explicit 64px-wide render.On the test rig flights renders at 256px (it has
vegas_width_pct: 50), and the log confirms the shared path producing256x64.Testing
test_vegas_map_parity.py(17 cases). The load-bearing one asserts the Vegas image is byte-identical to what the rotation pushes to the display — that's what stops the two drifting again, rather than testing for the three features individually and missing the fourth next time. Plus direct coverage of trails on/off, the centre marker, the untracked-trail skip, and rendering at every panel size.check_module_collisions.py: OK across 41 plugins.Also drops two locals in
get_vegas_contentthat the refactor left unused.Context
Part of a set of Vegas scroll mode changes; the core-side companion is ChuckBuilds/LEDMatrix#423.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Summary by CodeRabbit
Bug Fixes
Release