fix(soccer): eliminate flicker from redundant redraws in switch mode (v1.5.2)#58
Merged
ChuckBuilds merged 4 commits intomainfrom Mar 27, 2026
Merged
fix(soccer): eliminate flicker from redundant redraws in switch mode (v1.5.2)#58ChuckBuilds merged 4 commits intomainfrom
ChuckBuilds merged 4 commits intomainfrom
Conversation
…v1.0.1) When the YouTube API returns 400/401/403, the plugin now logs a clear message: "YouTube API key is invalid or expired. Update your API key in Settings > Secrets." instead of dumping the raw HTTP error. Also renders "YT: Update API Key" on the LED display so the user sees the problem without checking logs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…1.5.2) The display controller calls display() every 1 second in the standard loop. Each call was re-creating full PIL images, loading logos, drawing text, and pushing to the matrix — even when the displayed game hadn't changed. This caused visible flicker on the LED panel. Now Recent and Upcoming display() methods track the last rendered game ID and skip _draw_scorebug_layout entirely when force_clear=False and the same game is still showing. The cache is invalidated when the game list changes from an update or when the game switches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 0 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughTwo plugins receive enhancements: soccer-scoreboard adds render-state tracking to skip redundant redraws, and youtube-stats adds API key error detection with on-matrix error display. Both plugins have version bumps. Changes
Sequence Diagram(s)sequenceDiagram
participant D as display()
participant G as _get_channel_stats()
participant A as YouTube API
participant E as _show_error_on_display()
participant M as LED Matrix
D->>G: Request channel stats
G->>A: Fetch stats (API call)
alt API Success
A-->>G: Return stats
G->>G: Clear _api_key_error
G-->>D: Return stats
D->>D: Render stats normally
else API Error (400/401/403)
A-->>G: Error response
G->>G: Set _api_key_error
G-->>D: Return None
D->>D: Check _api_key_error
D->>E: Log & show error
E->>M: Render centered error text
M-->>E: (success/failure)
E->>E: Swallow render errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/soccer-scoreboard/sports.py (1)
1290-1299:⚠️ Potential issue | 🟠 MajorCache invalidation misses same-ID data updates.
Invalidation only runs when the set of IDs changes. If game payload changes (e.g., time/status/odds/order) while IDs stay the same, stale data can persist because the fast-path at Line 1620 and Line 2152 keeps skipping redraws.
💡 Proposed fix (refresh list and invalidate once when IDs are unchanged)
@@ - elif self.games_list: - self.current_game = self.games_list[ - self.current_game_index - ] # Update data + elif self.games_list: + # IDs unchanged, but payload/order may have changed. + # Refresh backing list and force one redraw. + self.games_list = team_games + self.current_game_index = min( + self.current_game_index, len(self.games_list) - 1 + ) + self.current_game = self.games_list[self.current_game_index] + self._last_rendered_game_id = None @@ - elif self.games_list: - # List content is same, just update data for current game - self.current_game = self.games_list[self.current_game_index] + elif self.games_list: + # IDs unchanged, but payload/order may have changed. + # Refresh backing list and force one redraw. + self.games_list = team_games + self.current_game_index = min( + self.current_game_index, len(self.games_list) - 1 + ) + self.current_game = self.games_list[self.current_game_index] + self._last_rendered_game_id = NoneAlso applies to: 1819-1828
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/soccer-scoreboard/sports.py` around lines 1290 - 1299, The code only resets self.games_list and self._last_rendered_game_id when the set of IDs changes, which misses payload-only updates; change the logic inside the with self._games_lock block to also detect when the payload changed even if IDs are equal (e.g., compare team_games != self.games_list or compute a stable checksum of game entries) and in that case assign self.games_list = team_games and set self._last_rendered_game_id = None to force a redraw; update the same pattern wherever the ID-only comparison is used (references: new_game_ids, current_game_ids, self.games_list, self._last_rendered_game_id).
🧹 Nitpick comments (3)
plugins/youtube-stats/manager.py (2)
300-318: Consider addingexc_infofor debugging broad exception catches.The defensive exception handling is appropriate for hardware rendering, but logging
exc_info=Truewould help diagnose unexpected failures during development without cluttering production logs (since it's at debug level).Suggested improvement
except Exception as e: - self.logger.debug(f"Could not show error on display: {e}") + self.logger.debug(f"Could not show error on display: {e}", exc_info=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/youtube-stats/manager.py` around lines 300 - 318, In _show_error_on_display, update the broad exception logging to include exception info so failures during rendering are easier to diagnose: when catching Exception in _show_error_on_display, call self.logger.debug with the same message but pass exc_info=True (or include the exception object) so the stacktrace is captured; keep the handler behavior (swallowing the exception) and debug log level unchanged.
161-167: Simplify dictionary access.The key existence check can be simplified using
dict.get().Suggested simplification
- if 'items' in data and data['items']: + if data.get('items'): channel = data['items'][0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/youtube-stats/manager.py` around lines 161 - 167, Replace explicit key existence checks for 'items' with dict.get to simplify and make access safer: use data.get('items') and get the first item with something like items = data.get('items') and channel = items[0] if items else None; then build stats using channel.get('snippet', {}).get('title') and channel.get('statistics', {}).get('subscriberCount') / 'viewCount' with safe int conversion (handle None/defaults) so that the block around variables data, channel, and stats is concise and resilient to missing keys.plugins/youtube-stats/manifest.json (1)
1-35: Missingversionsarray in manifest.The manifest lacks the
versionsarray required by coding guidelines. This array should track version history withreleased,version, andledmatrix_minfields.Consider whether
1.0.1is the correct version bump—the new_show_error_on_display()feature could arguably warrant a MINOR bump (1.1.0) per the guideline for "new display modes/functionality."As per coding guidelines: "Add the new version FIRST (most recent at top) to the
versionsarray inmanifest.json, with fields:released(date),version(semver), andledmatrix_min(minimum LEDMatrix version)."Suggested manifest structure
{ "id": "youtube-stats", "name": "YouTube Stats", - "version": "1.0.1", + "version": "1.1.0", "author": "ChuckBuilds", + "versions": [ + { + "released": "2026-03-27", + "version": "1.1.0", + "ledmatrix_min": "1.0.0" + }, + { + "released": "2026-01-01", + "version": "1.0.0", + "ledmatrix_min": "1.0.0" + } + ], "description": "Display YouTube channel statistics including subscriber count, total views, and channel name on your LED matrix",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/youtube-stats/manifest.json` around lines 1 - 35, The manifest is missing the required "versions" array; add a top-level "versions" array to manifest.json with the newest entry first containing the fields "released" (ISO date string), "version" (semver) and "ledmatrix_min" (minimum LEDMatrix version), and include the new feature bump—either keep "version": "1.0.1" or bump to "1.1.0" to reflect the new _show_error_on_display() functionality—then update the top-level "version" field to match the newest entry in the "versions" array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@plugins/soccer-scoreboard/sports.py`:
- Around line 1290-1299: The code only resets self.games_list and
self._last_rendered_game_id when the set of IDs changes, which misses
payload-only updates; change the logic inside the with self._games_lock block to
also detect when the payload changed even if IDs are equal (e.g., compare
team_games != self.games_list or compute a stable checksum of game entries) and
in that case assign self.games_list = team_games and set
self._last_rendered_game_id = None to force a redraw; update the same pattern
wherever the ID-only comparison is used (references: new_game_ids,
current_game_ids, self.games_list, self._last_rendered_game_id).
---
Nitpick comments:
In `@plugins/youtube-stats/manager.py`:
- Around line 300-318: In _show_error_on_display, update the broad exception
logging to include exception info so failures during rendering are easier to
diagnose: when catching Exception in _show_error_on_display, call
self.logger.debug with the same message but pass exc_info=True (or include the
exception object) so the stacktrace is captured; keep the handler behavior
(swallowing the exception) and debug log level unchanged.
- Around line 161-167: Replace explicit key existence checks for 'items' with
dict.get to simplify and make access safer: use data.get('items') and get the
first item with something like items = data.get('items') and channel = items[0]
if items else None; then build stats using channel.get('snippet',
{}).get('title') and channel.get('statistics', {}).get('subscriberCount') /
'viewCount' with safe int conversion (handle None/defaults) so that the block
around variables data, channel, and stats is concise and resilient to missing
keys.
In `@plugins/youtube-stats/manifest.json`:
- Around line 1-35: The manifest is missing the required "versions" array; add a
top-level "versions" array to manifest.json with the newest entry first
containing the fields "released" (ISO date string), "version" (semver) and
"ledmatrix_min" (minimum LEDMatrix version), and include the new feature
bump—either keep "version": "1.0.1" or bump to "1.1.0" to reflect the new
_show_error_on_display() functionality—then update the top-level "version" field
to match the newest entry in the "versions" array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53dbbb42-ca69-481c-96cb-a72763f14a6c
📒 Files selected for processing (4)
plugins/soccer-scoreboard/manifest.jsonplugins/soccer-scoreboard/sports.pyplugins/youtube-stats/manager.pyplugins/youtube-stats/manifest.json
The elif branches that handle same-IDs-but-refreshed-data were updating self.current_game but not clearing _last_rendered_game_id, so payload changes (e.g. corrected final scores) wouldn't trigger a redraw until the next game switch or force_clear. Now both Upcoming and Recent update paths clear _last_rendered_game_id when the game list is refreshed, even if the set of IDs is unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…dling Co-Authored-By: Claude Opus 4.6 (1M context) <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
1 participant
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.
Summary
display()every 1 second in the standard loop. Each call to_draw_scorebug_layoutwas re-creating full PIL images from scratch (new Image, logo loading/resizing, text drawing,update_display()) even when the displayed game hadn't changed. This caused visible flicker on the LED matrix.SportsRecent.display()andSportsUpcoming.display()now track the last rendered game ID (_last_rendered_game_id). Whenforce_clear=Falseand the same game is still showing, the full redraw is skipped entirely — the existing frame stays on the matrix.force_clear=Trueis passed.Test plan
game_display_durationseconds, display updates)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements