Skip to content

fix(display): a frame the preview throttle skipped still reaches the snapshot - #752

Open
ChuckBuilds wants to merge 1 commit into
mainfrom
claude/fix-soccer-first-screen
Open

ChuckBuilds wants to merge 1 commit into
mainfrom
claude/fix-soccer-first-screen

Conversation

@ChuckBuilds

Copy link
Copy Markdown
Owner

Bug

On ledpi (main ef69201, soccer-scoreboard 2.39.2), starting soccer on-demand showed 0 lit pixels in /api/v3/display/current for that mode's whole 15 s screen. The next on-demand screen rendered at once. Other plugins were not affected.

Root cause (core, not soccer)

The panel showed the card. The preview snapshot did not.

  • The snapshot is written only from DisplayManager.update_display(), at most once per write interval (snapshot_policy: 1 s with a viewer, 30 s without).
  • On-demand start runs clear() + update_display(), which writes a black frame.
  • A few ms later soccer draws its card and calls update_display(). The frame goes to the panel, but it lands inside the write interval, so the snapshot skips it.
  • Soccer's switch-mode recent/upcoming managers skip redundant redraws (return True # Already showing this game). They make no further update_display() call for the rest of the screen, so the skipped frame is never written.
  • Only soccer, afl and nrl carry that skip. Baseball, football, hockey, clock and news redraw every frame, so their next call writes the snapshot within a second.
  • The next on-demand screen worked because its first push came 15 s after the last write.

The plugin's redraw skip is a legitimate draw-once-and-hold pattern (dirty tracking exists for it). The defect is that the preview depends on a later update_display() call that may never happen.

Fix

  • DisplayManager._write_snapshot_if_due() now records a changed frame it skipped as owed (_snapshot_owed).
  • New DisplayManager.write_owed_snapshot():
    • Costs one attribute read when nothing is owed.
    • Otherwise runs the usual policy under _update_lock. The write still waits out the interval, and an unchanged frame is never re-encoded.
  • DisplayController._display_once() calls it after each frame, guarded with getattr. It covers both the 1 s and the high-FPS loops.

Tests

  • test/test_snapshot_owed_frame.py (new) uses the real DisplayManager on the emulator plus DisplayController._display_once. It replays the case: clear, then a card drawn inside the interval, then a held frame that draws nothing.
    • On origin/main the key test fails with assert 0 > 0 (snapshot black). 3 of its 5 tests fail there; all 5 pass with the fix.
    • The other tests check that the owed write waits out the interval, writes only once, and does nothing when no frame is owed.
  • Full suite vs an origin/main baseline worktree (Windows): the sorted FAILED/ERROR lists are identical (62 failed, 6 errors on both), and 5 more tests pass. test_run_loop_golden passes and its fixtures are unchanged.
  • CHANGELOG entry under Unreleased.

Side note, not changed here: soccer's per-league modes (soccer_esp.1_recent) try every enabled league's manager, not just the named one. On ledpi that mode showed the eng.1 card.

🤖 Generated with Claude Code

…snapshot

The preview snapshot (/api/v3/display/current, the web UI's live preview)
is written only from update_display(), at most once per write interval.
A changed frame pushed inside that interval was skipped and left for the
next update_display() -- which a screen that draws its card once and then
holds it never makes.

Soccer's recent/upcoming cards skip redundant redraws. After an on-demand
start, the card is pushed a few milliseconds after the start's clear wrote
a black frame, so the throttle skipped it and the preview stayed black for
the whole 15 s screen while the panel showed the card. The next screen
looked fine because its first push came after the interval had passed.

DisplayManager now records a skipped changed frame as owed, and the render
loop (_display_once) calls write_owed_snapshot() after each frame, which
writes it once the interval has passed. The cadence is unchanged, an
unchanged frame is never re-encoded, and nothing runs when no frame is owed.
Golden run-loop traces are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 50 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 02f2937c-c9ed-48be-a318-266df307567a
📥 Commits

Reviewing files that changed from the base of the PR and between 2236ff3 and 9b7217e.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/display_controller.py
  • src/display_manager.py
  • test/test_snapshot_owed_frame.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 4 complexity

Metric Results
Complexity 4

View in Codacy

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant