fix: live priority now interrupts long display durations (#196) - #298
Conversation
…ions (#196) _check_live_priority() was only called once per main loop iteration, before entering the display duration loop. With dynamic duration enabled, the loop could run for 60-120+ seconds without ever checking if a favorite team's live game started — so the display stayed on leaderboard, weather, etc. while the live game played. Now both the high-FPS and normal FPS display loops check for live priority every ~30 seconds (throttled to avoid overhead). When live content is detected, the loop breaks immediately and switches to the live game mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughPeriodic live-priority checks (~30s) were added to both the high-FPS dynamic loop and the standard display loop in the display controller. If a different live mode is detected while on-demand is inactive, the controller updates the active mode, sets a force-change flag, attempts to align the mode index, and exits the current loop to apply priority immediately. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/display_controller.py`:
- Around line 1866-1875: The live-priority check sets self.current_display_mode
and self.force_change but doesn't update the rotation pointer, so subsequent
rotation will overwrite the live mode; after detecting live_mode in the block
that calls self._check_live_priority() (the branch that sets
self.current_display_mode, self.force_change and breaks), also set
self.current_mode_index to the index of live_mode in the sequence used for
rotation (e.g., find live_mode in self.display_modes or the list that drives
rotation) or to a sentinel value that prevents the rotator advancing; ensure you
handle the case where live_mode is not found (fallback to 0 or leave unchanged)
so the rotation logic uses the correct index going forward.
- Around line 1793-1802: When detecting live priority in the high-FPS loop
(where _check_live_priority() returns a live_mode and you set
self.current_display_mode and self.force_change), also update
self.current_mode_index to the index of that live_mode so subsequent rotation
logic using active_mode doesn't overwrite it; mirror the pattern used earlier
where current_mode_index is set when live is chosen (use the same lookup
mechanism that maps modes to indices), ensuring this update happens before the
break and only when live_mode != active_mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
The live priority break set current_display_mode but not current_mode_index, so the post-loop rotation logic (which checks the old active_mode) would overwrite the live mode on the next advance. Now both loops also set current_mode_index to match the live mode, mirroring the existing pattern at the top of the main loop (line 1385). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/display_controller.py`:
- Around line 1795-1797: The current throttle uses (elapsed % 30.0) <
display_interval which can miss the narrow window; replace it with an explicit
timestamp check (e.g., add self.next_live_priority_check_at or
self.last_live_priority_check) and compare time.time() >=
next_live_priority_check_at before calling self._check_live_priority(), then
update next_live_priority_check_at = time.time() + 30.0 (or set
last_live_priority_check = time.time()) after the check; apply the same change
to the analogous block around the _check_live_priority calls at the other
location (lines 1871-1874) and ensure the condition still respects
self.on_demand_active and display_interval where relevant.
- Around line 1797-1806: The live-priority branch currently only breaks the
inner render loop so later outer-loop logic (rotation advancement and
remaining-duration handling) can still run and override the new live mode;
modify the branch in the block handling live_mode (the one that sets
self.current_display_mode, self.force_change and self.current_mode_index) so
that after setting those fields you also short-circuit the outer flow—e.g. set a
dedicated flag (like self._live_preempted = True) or call/return to exit the
enclosing loop/function so no further rotation or duration logic for the old
active_mode executes; apply the same change to the analogous block around lines
1874-1883 that checks live priority.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…tion Two issues fixed: 1. The modulo-based throttle (elapsed % 30.0 < display_interval) could miss the narrow 8ms window due to timing jitter. Replaced with an explicit timestamp check (_next_live_priority_check) that fires reliably every 30 seconds. 2. After breaking out of the display loop for live priority, the post-loop code (remaining-duration sleep and rotation advancement) would still run and overwrite the live mode. Now a continue skips directly to the next main loop iteration when current_display_mode was changed during the loop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/display_controller.py (1)
1794-1811: Consider computingelapsedfromnowto avoid redundanttime.time()calls.At 125 FPS, calling
time.time()twice per iteration is harmless but slightly wasteful. Computingnowfirst and derivingelapsedfrom it is cleaner.♻️ Optional: Reduce time.time() calls
- elapsed = time.time() - start_time now = time.time() + elapsed = now - start_time if not self.on_demand_active and now >= self._next_live_priority_check:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/display_controller.py` around lines 1794 - 1811, Compute now once and derive elapsed from it to avoid duplicate time.time() calls: replace the current two calls to time.time() by setting now = time.time() first, then elapsed = now - start_time, and keep the rest of the logic using now and elapsed (affecting the block that checks self._next_live_priority_check, calls self._check_live_priority(), and updates self.current_display_mode, self.force_change, and self.current_mode_index from self.available_modes).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/display_controller.py`:
- Around line 1794-1811: Compute now once and derive elapsed from it to avoid
duplicate time.time() calls: replace the current two calls to time.time() by
setting now = time.time() first, then elapsed = now - start_time, and keep the
rest of the logic using now and elapsed (affecting the block that checks
self._next_live_priority_check, calls self._check_live_priority(), and updates
self.current_display_mode, self.force_change, and self.current_mode_index from
self.available_modes).
Uh oh!
There was an error while loading. Please reload this page.
…367) When the display loop breaks early because current_display_mode changed (on-demand activation, live priority, etc.), it would fall through to the "honour minimum duration" sleep for the *previous* mode — blocking for up to that mode's full display_duration (default 30s) without polling on-demand requests or re-checking the mode. New modes could sit unrendered for up to 30s, or get clobbered by a queued stop request before ever displaying. This guard was added in #298 to fix#196 (live priority not interrupting long display durations) and was accidentally dropped in #330 as collateral damage of an unrelated time.monotonic() -> time.time() cleanup in the same diff hunk. Restoring it fixes both the original #196 regression and a new symptom found via the on-air MQTT plugin, where ON/OFF toggles could be delayed by up to 30s or missed entirely depending on timing within the previous mode's display cycle. Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Fixes#196 — Live priority games not taking over the display when dynamic duration is enabled.
_check_live_priority()was only called once per main loop iteration, before entering the display duration loop. With dynamic duration, the loop runs for 60-120+ seconds without checking if a favorite team's live game started._check_live_priority()every ~30 seconds (throttled). When live content is detected, the loop breaks immediately and switches to the live game mode._poll_on_demand_requests()), which are already checked inside both loops.Test plan
live_priorityfor a sport plugin with an active live gamelive_prioritydisabled, verify no interruption occurs🤖 Generated with Claude Code
Summary by CodeRabbit