diff --git a/.github/workflows/module-collisions.yml b/.github/workflows/module-collisions.yml index 2f948012..237ebbbd 100644 --- a/.github/workflows/module-collisions.yml +++ b/.github/workflows/module-collisions.yml @@ -25,6 +25,7 @@ on: - 'scripts/test_check_scroll_adoption.py' - 'scripts/check_sports_display_contract.py' - 'scripts/test_check_sports_display_contract.py' + - 'scripts/test_scroll_mode_is_reachable.py' # Without this, a PR that only edits this workflow matches no path and # the workflow never runs against its own change. - '.github/workflows/module-collisions.yml' @@ -61,3 +62,10 @@ jobs: - name: Test the sports display()-contract gate if: always() run: python scripts/test_check_sports_display_contract.py + # A scoreboard offering *_display_mode: "scroll" must actually reach its + # scroll renderer. football's dispatch sat in a method nothing called, so + # the setting silently kept switching cards -- and both existing tests + # exercised the card renderer directly, never the path that reaches it. + - name: Check scroll mode is reachable from display() + if: always() + run: python scripts/test_scroll_mode_is_reachable.py diff --git a/plugins.json b/plugins.json index 6ee646c5..e6b5b272 100644 --- a/plugins.json +++ b/plugins.json @@ -240,7 +240,7 @@ "last_updated": "2026-09-02", "verified": true, "screenshot": "", - "latest_version": "3.4.2" + "latest_version": "3.5.0" }, { "id": "geochron", diff --git a/plugins/football-scoreboard/manager.py b/plugins/football-scoreboard/manager.py index 814c4425..a6fb265f 100644 --- a/plugins/football-scoreboard/manager.py +++ b/plugins/football-scoreboard/manager.py @@ -192,6 +192,7 @@ def __init__( # Track current scroll state self._scroll_active: Dict[str, bool] = {} # {game_type: is_active} self._scroll_prepared: Dict[str, bool] = {} # {game_type: is_prepared} + self._scroll_active_league: Dict[str, str] = {} # {game_type: league currently prepared} # Enable high-FPS mode for scroll display (allows 100+ FPS scrolling) # This signals to the display controller to use high-FPS loop (8ms = 125 FPS) @@ -306,6 +307,7 @@ def on_config_change(self, new_config: Dict[str, Any]) -> None: self.enable_scrolling = self._scroll_manager is not None self._scroll_active = {} self._scroll_prepared = {} + self._scroll_active_league = {} # Rebuild rotation modes and reset cycling state. self.modes = self._get_available_modes() @@ -1485,6 +1487,112 @@ def _display_switch_mode_fallback(self, display_mode: str, mode_type: str, force return False + def _display_league_scroll_mode(self, league: str, mode_type: str, force_clear: bool) -> bool: + """ + Display scrolling content for a single league/mode combination (e.g. NFL + Recent configured for scroll instead of switch). + + The underlying ScrollDisplayManager keeps one active scroll session per + game_type (mode_type), shared across leagues, so we track which league's + content is currently prepared and force a re-prepare when that changes + (e.g. rotation switches from nfl_recent to ncaa_fb_recent and both are + set to scroll). + + Args: + league: League ID ('nfl' or 'ncaa_fb') + mode_type: Mode type ('live', 'recent', or 'upcoming') + force_clear: Whether to force clear display + + Returns: + True if content was displayed, False otherwise + """ + display_mode = f"{league}_{mode_type}" + self._current_display_league = league + self._current_display_mode_type = mode_type + + if not self._scroll_manager: + self.logger.warning( + f"Scroll mode requested for {display_mode} but scroll manager not available; " + "falling back to switch mode" + ) + manager = self._get_league_manager_for_mode(league, mode_type) + if not manager: + return False + success, _ = self._try_manager_display(manager, force_clear, display_mode, mode_type, None) + return success + + needs_prepare = ( + not self._scroll_prepared.get(mode_type, False) + or self._scroll_active_league.get(mode_type) != league + ) + + if needs_prepare: + manager = self._get_league_manager_for_mode(league, mode_type) + if not manager: + self.logger.debug(f"No manager available for {league} {mode_type}") + return False + + self._ensure_manager_updated(manager) + + live_priority_active = ( + mode_type == 'live' + and (self.nfl_live_priority or self.ncaa_fb_live_priority) + and self.has_live_content() + ) + + games = self._get_games_from_manager(manager, mode_type) + for game in games: + game['league'] = league + if not isinstance(game.get('status'), dict): + game['status'] = {} + if 'state' not in game['status']: + state_map = {'live': 'in', 'recent': 'post', 'upcoming': 'pre'} + game['status']['state'] = state_map.get(mode_type, 'pre') + + if live_priority_active: + games = [g for g in games if g.get('is_live', False) and not g.get('is_final', False)] + + if not games: + self.logger.debug(f"No games to scroll for {display_mode}") + self._scroll_prepared[mode_type] = False + self._scroll_active[mode_type] = False + return False + + rankings = self._get_rankings_cache() + + success = self._scroll_manager.prepare_and_display(games, mode_type, [league], rankings) + + if success: + self._scroll_prepared[mode_type] = True + self._scroll_active[mode_type] = True + self._scroll_active_league[mode_type] = league + self.logger.info( + f"[Football Scroll] Started scrolling {len(games)} {mode_type} games from {league}" + ) + else: + self._scroll_prepared[mode_type] = False + self._scroll_active[mode_type] = False + return False + + if self._scroll_active.get(mode_type, False): + displayed = self._scroll_manager.display_frame(mode_type) + + if displayed: + if self._scroll_manager.is_complete(mode_type): + self.logger.info(f"[Football Scroll] Cycle complete for {display_mode}") + self._scroll_prepared[mode_type] = False + self._scroll_active[mode_type] = False + self._dynamic_cycle_complete = True + + return True + else: + self._scroll_prepared[mode_type] = False + self._scroll_active[mode_type] = False + self._scroll_active_league.pop(mode_type, None) + return False + + return False + def _display_league_mode(self, league: str, mode_type: str, force_clear: bool) -> bool: """ Display a specific league/mode combination (e.g., NFL Recent, NCAA FB Upcoming). @@ -1510,15 +1618,28 @@ def _display_league_mode(self, league: str, mode_type: str, force_clear: bool) - self.logger.debug(f"League {league} is disabled, skipping") return False + # If this league/mode is configured for scroll display, delegate to the + # scroll manager instead of the switch/flip behavior below. + # + # This check used to live only in _display_external_mode(), which nothing + # calls: manifest.json registers granular modes only (nfl_recent, + # ncaa_fb_live, ...), and display() routes every one of those straight + # here. So _display_scroll_mode() was unreachable and setting + # *_display_mode: "scroll" silently kept switching cards. The unit tests + # missed it because they call _should_use_scroll_mode() directly rather + # than going through display(). + if self._get_display_mode(league, mode_type) == 'scroll': + return self._display_league_scroll_mode(league, mode_type, force_clear) + # Get manager for this league/mode combination manager = self._get_league_manager_for_mode(league, mode_type) if not manager: self.logger.debug(f"No manager available for {league} {mode_type}") return False - + # Create display mode name for tracking display_mode = f"{league}_{mode_type}" - + # Set display context for dynamic duration tracking self._current_display_league = league self._current_display_mode_type = mode_type @@ -2526,15 +2647,28 @@ def get_cycle_duration(self, display_mode: str = None) -> Optional[float]: # Parse granular mode name if applicable (e.g., "nfl_recent", "ncaa_fb_upcoming") league = None if "_" in display_mode and not display_mode.startswith("football_"): - # Granular mode: extract league - parts = display_mode.split("_", 1) - if len(parts) == 2: - potential_league, potential_mode_type = parts - if potential_league in self._league_registry and potential_mode_type == mode_type: - league = potential_league - - # Check if scroll mode is active for this mode type - if self._should_use_scroll_mode(mode_type) and self._scroll_manager: + # Granular mode: extract league. Match against the registry rather + # than split("_", 1) -- that splits "ncaa_fb_recent" into + # ("ncaa", "fb_recent"), leaving league unset, so the per-league + # scroll check below would fall back to the any-enabled-league one + # and hand NCAA FB a scroll duration while NFL is the league set to + # scroll. Registry matching also survives any future league whose + # id contains an underscore. + for league_id in self._league_registry: + if display_mode == f"{league_id}_{mode_type}": + league = league_id + break + + # Check if scroll mode is active for this mode type. For a granular + # per-league mode, only that league's display_mode setting counts -- + # otherwise a league set to 'switch' could inherit another league's + # scroll duration just because that other league is set to 'scroll'. + is_scroll_mode = ( + self._get_display_mode(league, mode_type) == 'scroll' + if league + else self._should_use_scroll_mode(mode_type) + ) + if is_scroll_mode and self._scroll_manager: # Get dynamic duration from scroll manager scroll_duration = self._scroll_manager.get_dynamic_duration(mode_type) if scroll_duration > 0: @@ -2804,7 +2938,26 @@ def is_cycle_complete(self) -> bool: # Check if scroll mode is active for the current display mode if self._current_active_display_mode: mode_type = self._extract_mode_type(self._current_active_display_mode) - if mode_type and self._should_use_scroll_mode(mode_type) and self._scroll_manager: + + # Parse granular mode name if applicable (e.g. "nfl_recent", "ncaa_fb_upcoming") + league = None + display_mode = self._current_active_display_mode + if "_" in display_mode and not display_mode.startswith("football_"): + # Use startswith checks to correctly handle multi-underscore league IDs + if display_mode.startswith("ncaa_fb_"): + league = "ncaa_fb" + elif display_mode.startswith("nfl_"): + league = "nfl" + + # For a granular per-league mode, only that league's display_mode + # setting counts -- otherwise a league set to 'switch' could report + # completion based on another league's scroll state. + is_scroll_mode = ( + self._get_display_mode(league, mode_type) == 'scroll' + if league + else self._should_use_scroll_mode(mode_type) + ) + if mode_type and is_scroll_mode and self._scroll_manager: # For scroll mode, check ScrollHelper's completion status is_complete = self._scroll_manager.is_complete(mode_type) self.logger.info(f"is_cycle_complete() [scroll mode]: display_mode={self._current_active_display_mode}, returning {is_complete}") diff --git a/plugins/football-scoreboard/manifest.json b/plugins/football-scoreboard/manifest.json index 6999be34..a75c76e5 100644 --- a/plugins/football-scoreboard/manifest.json +++ b/plugins/football-scoreboard/manifest.json @@ -1,7 +1,7 @@ { "id": "football-scoreboard", "name": "Football Scoreboard", - "version": "3.4.2", + "version": "3.5.0", "update_interval": 60, "author": "ChuckBuilds", "class_name": "FootballScoreboardPlugin", @@ -25,6 +25,12 @@ "ncaa_fb_live" ], "versions": [ + { + "released": "2026-09-03", + "version": "3.5.0", + "changelog": "Scroll display mode now actually scrolls: the dispatch lived in an uncalled method, so the setting did nothing", + "ledmatrix_min_version": "3.3.0" + }, { "version": "3.4.2", "released": "2026-09-02", diff --git a/plugins/football-scoreboard/test_granular_league_parsing.py b/plugins/football-scoreboard/test_granular_league_parsing.py new file mode 100755 index 00000000..21782a5d --- /dev/null +++ b/plugins/football-scoreboard/test_granular_league_parsing.py @@ -0,0 +1,131 @@ +#!/usr/bin/env python3 +"""A granular mode must resolve to its own league, including ncaa_fb. + +get_cycle_duration() extracted the league with display_mode.split("_", 1), +which turns "ncaa_fb_recent" into ("ncaa", "fb_recent"). "ncaa" is not in the +league registry, so league stayed None and the per-league scroll check fell +back to the any-enabled-league one -- handing NCAA FB a scroll duration on the +strength of NFL being set to scroll. + +The bug only showed for the league whose id contains an underscore, so nfl_* +behaved correctly throughout and none of the plugin's other tests noticed. +""" +import logging +import sys +from pathlib import Path +from unittest.mock import Mock + +plugin_dir = Path(__file__).parent +sys.path.insert(0, str(plugin_dir)) +sys.path.insert(0, str(plugin_dir.parent.parent)) + +logging.basicConfig(level=logging.CRITICAL) + +SCROLL_SENTINEL = 987.0 + + +def _display_manager(): + dm = Mock() + dm.display_width = dm.width = 128 + dm.display_height = dm.height = 32 + matrix = Mock() + matrix.width, matrix.height = 128, 32 + dm.matrix = matrix + for name in ("clear", "set_image", "show"): + setattr(dm, name, Mock()) + return dm + + +def _config(nfl_mode, ncaa_mode): + def league(mode): + return { + "enabled": True, + "favorite_teams": [], + "display_modes": { + "show_live": True, "show_recent": True, "show_upcoming": True, + "live_display_mode": mode, + "recent_display_mode": mode, + "upcoming_display_mode": mode, + }, + } + return { + "enabled": True, "display_duration": 15, "game_display_duration": 5, + "timezone": "UTC", "nfl": league(nfl_mode), "ncaa_fb": league(ncaa_mode), + } + + +def _plugin(nfl_mode, ncaa_mode): + from manager import FootballScoreboardPlugin + + pm = Mock() + pm.get_plugin = Mock(return_value=None) + plugin = FootballScoreboardPlugin( + plugin_id="football-scoreboard", + config=_config(nfl_mode, ncaa_mode), + display_manager=_display_manager(), + cache_manager=Mock(), + plugin_manager=pm, + ) + # A scroll duration that is unmistakable if the scroll branch is taken. + scroll = Mock() + scroll.get_dynamic_duration = Mock(return_value=SCROLL_SENTINEL) + plugin._scroll_manager = scroll + return plugin + + +def test_per_league_scroll_duration(): + """nfl=scroll, ncaa_fb=switch: only nfl_* may take the scroll duration.""" + plugin = _plugin(nfl_mode="scroll", ncaa_mode="switch") + + nfl = plugin.get_cycle_duration("nfl_recent") + ncaa = plugin.get_cycle_duration("ncaa_fb_recent") + + ok = True + if nfl != SCROLL_SENTINEL: + print(f"[FAIL] nfl_recent is set to scroll but got {nfl}, " + f"expected {SCROLL_SENTINEL}") + ok = False + if ncaa == SCROLL_SENTINEL: + print("[FAIL] ncaa_fb_recent is set to switch but received the scroll " + "duration -- the league did not resolve (split('_', 1) bug)") + ok = False + if ok: + print("[OK] nfl_recent scrolls, ncaa_fb_recent does not") + return ok + + +def test_reversed(): + """The mirror case: ncaa_fb=scroll, nfl=switch.""" + plugin = _plugin(nfl_mode="switch", ncaa_mode="scroll") + + nfl = plugin.get_cycle_duration("nfl_recent") + ncaa = plugin.get_cycle_duration("ncaa_fb_recent") + + ok = True + if ncaa != SCROLL_SENTINEL: + print(f"[FAIL] ncaa_fb_recent is set to scroll but got {ncaa}, " + f"expected {SCROLL_SENTINEL}") + ok = False + if nfl == SCROLL_SENTINEL: + print("[FAIL] nfl_recent is set to switch but received the scroll duration") + ok = False + if ok: + print("[OK] ncaa_fb_recent scrolls, nfl_recent does not") + return ok + + +def main(): + results = [ + ("per-league scroll duration", test_per_league_scroll_duration()), + ("reversed", test_reversed()), + ] + failed = [name for name, ok in results if not ok] + if failed: + print(f"\n[FAIL] {', '.join(failed)}") + return 1 + print("\nAll checks passed") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_scroll_mode_is_reachable.py b/scripts/test_scroll_mode_is_reachable.py new file mode 100755 index 00000000..8d8ab8a0 --- /dev/null +++ b/scripts/test_scroll_mode_is_reachable.py @@ -0,0 +1,141 @@ +#!/usr/bin/env python3 +"""A scoreboard configured for scroll must actually reach its scroll renderer. + +football-scoreboard shipped with `*_display_mode: "scroll"` doing nothing. The +scroll dispatch lived in `_display_external_mode()`, which nothing calls: +manifest.json registers granular modes only (nfl_recent, ncaa_fb_live, ...) and +`display()` routes every one of those to `_display_league_mode()`, which had no +scroll check. `_display_scroll_mode()` was defined, tested, and unreachable. + +Every existing test missed it, in the same way: + + * test_scroll_mode.py calls `_should_use_scroll_mode("recent")` directly + * scripts/test_scroll_card_renders.py renders `render_game_card` directly + +Both prove the card renderer works. Neither proves the display path ever asks +for it. On hardware the panel switched cards while the config said scroll, and +the journal showed no scroll image from the plugin at all. + +So this checks reachability rather than rendering: from `display()`, following +calls through the manager, is any scroll-rendering method reachable? It is a +static call-graph walk -- no data, no panel, no live games -- because the modes +that expose the bug need live fixtures the harness does not have. +""" +import ast +import json +import os +import sys + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +PLUGINS = os.path.join(ROOT, "plugins") + +#: Known-broken, recorded rather than hidden. +#: +#: ufc-scoreboard has the same defect football had -- config_schema.json offers +#: live/recent/upcoming_display_mode, and is_cycle_complete() consults +#: _should_use_scroll_mode(), but display() never mentions scrolling, so setting +#: "scroll" changes nothing on the panel. It is excluded here because unlike +#: football it has no scroll renderer to wire up: football's _display_scroll_mode +#: existed and was merely unreachable, whereas ufc would need the prepare/display +#: path written from scratch. That is a feature, not a repair, so it is not +#: bundled with this fix. +#: +#: Removing an entry from this list must make the gate pass, never fail. +KNOWN_MISSING_SCROLL = {"ufc-scoreboard"} + + +def scroll_render_methods(fns): + """Methods that render a scroll frame (not merely decide about scrolling).""" + return {n for n in fns if "scroll" in n.lower() and "display" in n.lower()} + + +def reachable_from(fns, entry): + seen, stack = set(), [entry] + while stack: + cur = stack.pop() + if cur in seen or cur not in fns: + continue + seen.add(cur) + for node in ast.walk(fns[cur]): + if isinstance(node, ast.Call) and isinstance(node.func, ast.Attribute): + stack.append(node.func.attr) + return seen + + +def check(plugin): + """Return (status, detail). status: 'pass' | 'fail' | 'skip'.""" + manager = os.path.join(PLUGINS, plugin, "manager.py") + if not os.path.isfile(manager): + return "skip", "no manager.py" + + with open(manager, encoding="utf-8") as fh: + src = fh.read() + try: + tree = ast.parse(src) + except SyntaxError as exc: + return "fail", f"cannot parse manager.py: {exc}" + + fns = {n.name: n for n in ast.walk(tree) + if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))} + if "display" not in fns: + return "skip", "no display() entry point" + + renderers = scroll_render_methods(fns) + if not renderers: + return "skip", "plugin has no scroll renderer" + + # Only meaningful if the plugin actually offers a scroll setting. + schema = os.path.join(PLUGINS, plugin, "config_schema.json") + if os.path.isfile(schema): + with open(schema, encoding="utf-8") as fh: + if "_display_mode" not in fh.read(): + return "skip", "no *_display_mode setting" + + reached = renderers & reachable_from(fns, "display") + if reached: + return "pass", ", ".join(sorted(reached)) + return "fail", (f"defined but unreachable from display(): " + f"{', '.join(sorted(renderers))}") + + +def main(): + if not os.path.isdir(PLUGINS): + print("[skip] no plugins/ directory") + return 2 + + failures, checked = [], 0 + for plugin in sorted(os.listdir(PLUGINS)): + if not os.path.isdir(os.path.join(PLUGINS, plugin)): + continue + status, detail = check(plugin) + if status == "skip": + continue + checked += 1 + if status == "fail": + if plugin in KNOWN_MISSING_SCROLL: + print(f" [known] {plugin}: {detail}") + continue + failures.append(f"{plugin}: {detail}") + elif plugin in KNOWN_MISSING_SCROLL: + failures.append( + f"{plugin}: now reaches its scroll renderer -- " + f"remove it from KNOWN_MISSING_SCROLL") + + if not checked: + print("[skip] no scoreboard with a scroll renderer found") + return 2 + + if failures: + print("[FAIL] scroll mode is configurable but unreachable:") + for f in failures: + print(f" {f}") + print("\nA granular mode routed to _display_league_mode() must check the " + "league's display_mode and delegate to the scroll renderer.") + return 1 + + print(f"[pass] {checked} scoreboard(s): scroll renderer reachable from display()") + return 0 + + +if __name__ == "__main__": + sys.exit(main())