fix(flights): stop the SkyAware poll writing to the SD card - #269
Conversation
The last good payload was persisted through cache_manager on every successful poll, but it is only ever read back inside the failure branch -- by this same process, seconds later. The round trip bought nothing and cost the whole ~55KB SkyAware payload once per update_interval. Measured on a live rig: 12 rewrites a minute, around 928 MB/day, this plugin's largest single source of card wear and about two thirds of everything the service writes. It now lives in memory. The fallback also has to expire, which it previously did not in any meaningful way. Reading it back took cache_manager.get's default max_age of 300s, so a receiver outage could hand back a five-minute-old sky -- some forty miles of error for an airliner -- and the map would draw aircraft where they plainly were not. Thirty seconds is generous enough that one dropped poll never blanks the display and short enough that an outage does. Both fetch paths had their own copy of this: manager._fetch_aircraft_data (the one that runs) and fetcher.SkyAwareFetcher.fetch_raw. Both are changed, and they share one freshness bound so they cannot drift. The new test pins the contract -- no cache use on the success path, the fallback serves then expires, a cold start with nothing held shows nothing. Verified non-vacuous: it fails three checks against the previous code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 ChangesSkyAware fallback handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FlightTrackerPlugin
participant SkyAware
participant CacheManager
FlightTrackerPlugin->>SkyAware: Fetch aircraft data
SkyAware-->>FlightTrackerPlugin: Return successful payload
FlightTrackerPlugin->>FlightTrackerPlugin: Store payload and timestamp in memory
SkyAware-->>FlightTrackerPlugin: Return request failure
FlightTrackerPlugin->>FlightTrackerPlugin: Check 30-second freshness limit
FlightTrackerPlugin-->>FlightTrackerPlugin: Return recent payload or no data
FlightTrackerPlugin-->>CacheManager: Do not read or write payload cache
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 | 18 |
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.
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/manifest.json`:
- Around line 40-44: Add the required "ledmatrix_min_version" field with value
"2.0.0" to the release entry for version 1.12.8 in the manifest, and do not add
the deprecated "ledmatrix_min" field.
🪄 Autofix
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: eeac80a9-03f8-49ed-a9f1-9556e0654f7c
📒 Files selected for processing (5)
plugins.jsonplugins/ledmatrix-flights/fetcher.pyplugins/ledmatrix-flights/manager.pyplugins/ledmatrix-flights/manifest.jsonplugins/ledmatrix-flights/test_no_disk_write_on_poll.py
compatibility.py reads versions[0] and nothing else, so a newest entry without a minimum silently drops the plugin's declared floor rather than inheriting the previous one. Uses ledmatrix_min_version, the spelling in the canonical schema; the older entries' ledmatrix_min is the deprecated alias that is only still read as a fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
What
The flights plugin persisted its last good SkyAware payload through
cache_manageron every successful poll, but only ever read it back inside the failure branch — by the same process, seconds later. That payload is now held in memory.Why it matters
Measured on the dev rig:
flight_tracker_data.jsonrun.pywritesIt was the single largest source of SD-card wear on the device, and the write bought nothing — the cached copy never outlived the process that wrote it in any useful way.
Also: the fallback never really expired
Reading it back took
cache_manager.get's defaultmax_ageof 300s. So a receiver outage could hand back a five-minute-old sky and the map would draw aircraft roughly forty miles from where they actually were.It now expires after 30s — generous enough that one dropped poll (default
update_intervalis 5s) never blanks the display, short enough that a real outage does. At airliner speed 30s is about seven miles: visible on the map, but not yet a lie.Both paths
There were two independent copies of this logic, both writing the same key:
manager._fetch_aircraft_data()— the one that actually runsfetcher.SkyAwareFetcher.fetch_raw()Both are changed, and they share a single
FALLBACK_MAX_AGE_SECONDS(defined infetcher.py, imported bymanager.py) so they cannot drift apart.Tests
New
test_no_disk_write_on_poll.pypins the contract: 20 consecutive successful polls touch the cache manager neither for reads nor writes; a failed poll is covered from memory; the payload expires and is not resurrected; a cold start with nothing held returnsNonerather than something stale; recovery restores normal service.Verified non-vacuous — run against the previous
fetcher.pyit fails three checks.Also green:
check_module_collisions.py(43 plugins), and the safety harness passes all 8 sizes. The harness has no receiver, so it exercises exactly the no-data path this changes.Not included
Two things I looked at and deliberately left out of scope: the payload carries 55 fields per aircraft where the display needs about ten (a further ~5× cut to any future write), and
/var/cache/ledmatrixhas 8,900 files totalling 1.3 GB with no eviction and entries dating to January. Both are worth their own PRs.🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit
New Features
Bug Fixes