fix(ledmatrix-flights): give the Vegas map the same features as the rotation - #231
fix(ledmatrix-flights): give the Vegas map the same features as the rotation#231ChuckBuilds wants to merge 2 commits into
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
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 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}
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
| # Get map background if enabled | ||
| map_bg = self._get_map_background(self.center_lat, self.center_lon) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Move tile fetching out of the render path.
_get_map_background() can synchronously fetch map tiles on cache misses, and this helper runs from both display() and get_vegas_content(). A cold cache can therefore stall a rotation/Vegas frame. Refresh tile/background data in update() through self.cache_manager; render from cached data or use the existing black fallback.
As per coding guidelines, “Fetch or refresh network data in update(), never in display(), and use the shared self.cache_manager for network-fetched data.”
🤖 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/manager.py` around lines 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.
Source: Coding guidelines
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
2 participants
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