mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
* docs: add ARCHITECTURE and PERMISSIONS guides ARCHITECTURE.md maps the processes, the state the display and web services share through the cache, the display loop, the plugin system, the web UI and the update path, with links into the code and a where-to-start table. PERMISSIONS.md lists who owns what after install, both sudoers files (and why iptables is not granted), the polkit rule, and which scripts/fix_perms script to run as which user. Both are linked from the docs index, along with the MQTT bridge README and src/common/README.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: correct stale setup, service and troubleshooting claims - README: quick actions run systemctl on ledmatrix.service (run.py), not display_controller.py; use_short_date_format has no effect; the installer uses system pip with --break-system-packages, not a venv. - CONFIG_DEBUGGING: LEDMATRIX_DEBUG must be "true"; logs are in journald. - GETTING_STARTED, WEB_INTERFACE_GUIDE, TROUBLESHOOTING: enabling a plugin, plugin settings, brightness and Vegas settings apply without a restart; matrix hardware settings still need one. - TROUBLESHOOTING: install dependencies with sudo so the root service sees them; point permission problems at PERMISSIONS.md instead of a project-wide chown. - ADVANCED_FEATURES: real BackgroundDataService stats keys; Vegas hooks return VegasDisplayMode and None falls back to capture; cache files are 0660; fix_web_permissions.sh runs as the web user and does not touch sudoers. - STARLARK_APPS_GUIDE: only the linux-arm64 pixlet binary is downloaded. - HOW_TO_RUN_TESTS: test class examples that exist. - CLAUDE.md: PluginStoreManager, plugin_dirs.py, monorepo installs via the Trees API with ZIP fallback, requirements.txt is optional. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: mark deprecated plugin APIs and state manifest fields once Methods @deprecated("3.7.0") (the set pinned in test_deprecation.py) were shown as current API in the quick reference, API reference, advanced guide, development guide and FONT_MANAGER. Each is now marked deprecated with its replacement. FONT_MANAGER is rewritten around the current API; the override editor is gone and override methods are deprecated. Required manifest fields were stated three different ways. The API reference now has one section: the 7 schema-required fields, the 4 the store refuses without, class_name for the loader, and the 8 to set. The other guides link to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: document every src/common module and every widget - src/common/README.md covered 7 of 17 modules. It now has a table of all of them (purpose, whether plugins import it, release to floor on), a short entry each, and logging advice that matches the code. - SPORTS_UNIFICATION listed two shared modules and called sports_helpers the first; it now lists all six. - The widgets README lists all 28 registered widgets plus the support files, and absorbs the parts that only docs/widget-guide.md had (x-options.labels, x-advanced, x-display hidden, plugin-file-manager). docs/widget-guide.md is now a pointer to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(security): fix_web_permissions.sh re-hardens the root sudo helpers The script chowns the whole project to the web user. That included scripts/fix_perms/safe_plugin_rm.sh and safe_pip_install.sh -- the two helpers /etc/sudoers.d/ledmatrix_web lets the web user run as root -- so running it turned both into a root shell for whoever can edit them. It also re-grouped config_secrets.json away from ledmatrix. After the chown it now does what first_time_install.sh's Steps 11 and 11.1 do: helpers back to root:root 755, and config_secrets.json back to the web unit's User=:ledmatrix 640. Each step is non-fatal and prints the manual command if it fails. Also fixes what the script and its docs claimed: it never configured sudoers, its closing hint pointed at ./configure_web_sudo.sh (wrong path), and the README and ADVANCED_FEATURES.md said to run it with sudo, which it refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(security): validate and harden every sudoers drop-in the scripts write configure_wifi_permissions.sh copied its rules into /etc/sudoers.d/ledmatrix_wifi without `visudo -c`. A malformed drop-in makes sudo refuse every command for every user, which on a headless Pi leaves no way back in. It now checks first and leaves the installed file alone when the rules do not parse, as the other two writers do. (It already used mktemp, so that part of the review did not apply.) It also grants the two literal commands wifi_manager.py runs for NetworkManager's shared-mode dnsmasq drop-in -- `cp /tmp/ledmatrix-nm-dnsmasq.conf .../dnsmasq-shared.d/ledmatrix-captive.conf` and `rm -f` of that file. The directory's mkdir was granted, the file was not. Both are pinned in test_sudo_allowlist_covers_calls.py. configure_web_sudo.sh wrote its rules to /tmp/ledmatrix_web_sudoers_$$, a predictable name in a world-writable directory; it now uses mktemp with an EXIT trap, as first_time_install.sh does. It sets mode 440 on the installed file instead of leaving the temp file's mode, and finds visudo in /usr/sbin when that is not on the user's PATH, which skipped the check silently. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install): escape the project path in the DNS-fix and MQTT unit renderers install_dns_fix.sh and install_mqtt_bridge.sh substituted __PROJECT_ROOT_DIR__ with the raw path, while the other three renderers go through sed_escape_replacement from lib_systemd_render.sh. A checkout under a path containing `&`, `\` or `|` rendered a corrupted unit from these two only. Both now source the helper and use it, and a test checks that every placeholder substitution in scripts/install uses an escaped value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install): stop the installer scripts reporting things that are not true - first_time_install.sh printed "Password: ledmatrix123" for the setup access point. wifi_manager creates it as an open network ("No password" on the panel), so it now says so. - Step 10.1 printed "✓ WiFi management permissions configured" straight after its own failure message; install_wifi_monitor.sh printed "✓ Package installation completed" after a failed apt install. The tick now only follows success. - Step 7 printed "Web dependencies already installed ... in Step 5" in the one branch that runs because Step 5 did not install them, then created .web_deps_installed on that basis. It now warns and leaves the marker off so the next run retries, as the comment below it intends. - check_system_compatibility.sh called Debian 12 Bookworm "full compatibility confirmed" while first_time_install.sh refuses anything but Debian 13. Bookworm, older Debian and non-Debian systems are now errors. Its counters used ((X++)), which under `set -e` exits the script at the first warning or error (the expression is 0), so the check never reached its summary on any system with one. - configure_web_sudo.sh and configure_wifi_permissions.sh finished by testing `sudo -n test -f ...` and `sudo -n nmcli device status`, neither of which is granted, so they always reported a failure. They now ask `sudo -n -l` about commands the new rules do grant, which checks the rule without running anything. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(install): print the completion summary before rebooting With -y -- and so for every one-shot `curl | bash` install, which always passes -y -- first_time_install.sh ran `reboot` about 180 lines before its "Installation Complete / Web UI Access" summary. reboot returns at once, so the summary printed while the Pi was going down and the SSH session usually dropped before the web UI address could be read. The reboot block moves, unchanged, to the very end of the script. The interactive prompt now also follows the summary. Because the summary now runs before the -y reboot, its one command that could fail under `set -Eeuo pipefail` (the SSID lookup, when nmcli reports a connected device but no active network line) gets `|| true`; a missing SSID was already handled as "SSID unknown". one-shot-install.sh prints its "Next steps" after the installer returns, by which time the reboot is under way, so it now says so, and README's Quick Install mentions the automatic reboot. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore(scripts): correct wrong comments and messages, drop dead code No behaviour change except the output text noted below. - 2775 is setgid, not the sticky bit (first_time_install.sh Step 3.1, fix_plugin_permissions.sh), and root needs no "PWM hardware access" to plugin files. - The 777 comments in first_time_install.sh Step 3's fallback and fix_assets_permissions.sh said root needs it to write. Root ignores mode bits; the comments now say what 777 actually opens. The 777 itself is unchanged. - apt_remove ends in `|| true`, so Step 12's "Some packages could not be removed" branch could never run; it is gone and the helper stays non-fatal. - detect_web_service_user's comment named Step 8 for the web unit (install_service.sh installs it in Step 7.5) and now says which branch actually runs. - Step 5 described an "already installed" check that does not exist; the ACTUAL_USER comment described the re-exec backwards. - on_error printed a literal "\n" before "Common fixes:". - Dead code: one-shot-install.sh's uncalled fix_tmp_permissions, LEDMATRIX_ELEVATED=1 (never read) on the sudo re-exec, and configure_web_sudo.sh's unused PYTHON_PATH, which also made a missing python3 fatal for rules that never mention it. - start_display.sh / stop_display.sh said "for user: <you>"; the service runs as root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(fix_perms): fix_cache_permissions.sh uses setup_cache.sh's model There were two models for /var/cache/ledmatrix. setup_cache.sh (the installer's Step 2) and install_web_service.sh share it through the ledmatrix group: root:ledmatrix, 2775, files 660, which is also what DiskCache relies on to give files the directory's group. fix_cache_permissions.sh instead made it 777 and re-grouped it to the invoking user's group, undoing that. It now runs setup_cache.sh for /var/cache/ledmatrix and keeps its own handling of ~/.ledmatrix_cache. Dropped: /var/cache/ledmatrix/ placeholder_logos (nothing reads it) and the checks against the `daemon` user (no service runs as daemon). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * ci: pin actions/checkout in the Claude workflows, drop template comments claude.yml and claude-code-review.yml used actions/checkout@v4 while test.yml and release-version-check.yml pin the v4.2.2 commit SHA; they now pin the same SHA. The commented-out starter-template settings (prompt, claude_args, paths, author filter) are removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(scripts): index every script and list removal candidates New scripts/README.md gives one line per top-level script and scripts directory, marked keep, dev-only or diagnostic, and lists the eight scripts nothing in the repo refers to as candidates for removal (kept for now). The install, utils and dev READMEs now list the files they were missing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: tighten two checks that mutation testing showed were too loose - The wifi sudoers check matched `visudo -c -f "$TEMP_SUDOERS"` in the error report too, so replacing the check with `if false` still passed. It now requires the command as the condition. - The summary test never had the setup access point up, so reinstating the bogus "Password: ledmatrix123" line went unnoticed. A case with hostapd active now checks the AP is described as open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(permissions): describe the repaired fix_perms scripts and new WiFi grants Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(changelog): docs-scripts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
544 lines
32 KiB
Markdown
544 lines
32 KiB
Markdown
# Sports Code Unification — Architecture
|
||
|
||
How the nine sports scoreboard plugins converge onto shared core code **without**
|
||
becoming nine clients of a god class.
|
||
|
||
## The problem
|
||
|
||
Nine plugins (`afl`, `baseball`, `basketball`, `football`, `hockey`, `lacrosse`,
|
||
`nrl`, `soccer`, `ufc`) each ship a ~3,000-line `sports.py` descended from this
|
||
repo's former `src/base_classes/sports.py` (since removed). They have drifted
|
||
into three lineages, and only 28 of the 66 methods appearing across them are
|
||
present in all nine. One logical fix (the UTC start-time bug) cost 75 files.
|
||
|
||
Merging everything into one base class would fix the duplication and create a
|
||
worse problem: a single 2,500-line class that all nine plugins inherit, where any
|
||
change has a nine-plugin blast radius and per-sport behavior survives only as
|
||
`if self.sport == "hockey"` branches.
|
||
|
||
## Three properties, three mechanisms
|
||
|
||
These are independent concerns. Conflating them is what produces god classes.
|
||
|
||
### Upgradability — a plugin keeps working across core versions
|
||
|
||
| Rule | Mechanism |
|
||
|---|---|
|
||
| Plugin loads on a core that predates a module | Guarded import with a bundled fallback (`try: from src.X import Y / except ModuleNotFoundError: from y import Y`) |
|
||
| Plugin loads on a core that predates a *method* | Capability probing — `hasattr(SportsCore, "_detect_stale_games")` — never a version comparison. The loader's compat check is advisory-only (it logs and continues), so probing is the real protection. |
|
||
| Core changes never break a plugin's rendering | The **view-model contract**: the game dict each plugin's `_extract_game_details_common` builds is read by the shared `src/common` renderers, so its keys may be added, never renamed or removed. |
|
||
| A plugin can drop its bundled copy safely | The **sunset rule**: its manifest must floor `ledmatrix_min_version` at the first core release shipping the module (recorded in `CHANGELOG.md`) — *necessary but not sufficient*. The store enforces that floor on every registry-managed install and on both supported update paths (sideloading via `install_from_url` is not gated), but a floor cannot reach a user who never updates, so the copy also waits for the B6 gate below. |
|
||
|
||
The core API is **additive-only**. A method the plugins call is never removed or
|
||
given a new required parameter; new behavior arrives as new methods with
|
||
defaults, or as capabilities they opt into.
|
||
|
||
### Reusability — write once, nine plugins benefit
|
||
|
||
Only code that is **identical in intent across all nine** moves into the base
|
||
class. That set is small and knowable — it is exactly the methods present in every
|
||
copy today (phase B1 below). Everything else stays where it is until it earns
|
||
promotion.
|
||
|
||
### Modularity — a change to one feature cannot reach a plugin that doesn't use it
|
||
|
||
This is the property the naive merge destroys, and it is enforced structurally:
|
||
|
||
1. **Capabilities are separate modules composed by inheritance, not config
|
||
branches inside the base class.** Hockey has no celebrations, so
|
||
`HockeyLive` does not inherit `CelebrationMixin` — the celebration code is not
|
||
merely disabled for hockey, it is *not in hockey's MRO at all*. No shared
|
||
state, no dead branches, no risk. Contrast with
|
||
`if self.celebrations_enabled:` inside `SportsLive`, where a bug in
|
||
celebration code can still crash a plugin that never wanted the feature.
|
||
|
||
2. **Variant behavior is a strategy object chosen by name, not a branch.**
|
||
Live rotation exists in three dialects across the lineages; core ships all
|
||
three behind `rotation_strategy: "swrr" | "weighted" | "simple"` and a plugin
|
||
may register its own. Core never learns sport names.
|
||
|
||
3. **Sport-specific behavior is a documented override point.** The base class
|
||
declares the seam; the plugin fills it. Basketball's tournament-round parsing
|
||
and baseball's BDF sizing stay in their plugins forever — they are not
|
||
candidates for promotion, and core must never grow a branch for them.
|
||
|
||
4. **Files bound the blast radius.** Capabilities live in their own modules so a
|
||
diff shows at a glance which plugins a change can reach.
|
||
|
||
## Layering
|
||
|
||
`src/base_classes/` has been removed: no scoreboard plugin built on it. B1 and
|
||
B2 below promoted code into it (`SportsCore`, the mode classes,
|
||
`CelebrationMixin`, the rotation strategies); the override points and
|
||
capabilities sections record that design, but none of it ships in core any
|
||
more. Shared sports code lives in `src/common`:
|
||
|
||
| Module | Since | Holds |
|
||
|---|---|---|
|
||
| `sports_scroll.py` | 3.2.0 | `SportsScrollDisplay` / `SportsScrollDisplayManager` — scroll orchestration (content building stays in the plugins) |
|
||
| `sports_card.py` | 3.3.0 | Free functions for card settings, colours, favourite-team rules, dates and font sizes |
|
||
| `sports_game_renderer.py` | 3.3.0 | `SportsGameRendererMixin` — scroll/Vegas card geometry |
|
||
| `sports_shared.py` | 3.3.0 | `SportsCoreSharedMixin`, `SportsLiveSharedMixin`, `SportsRecentSharedMixin` — the sport-independent `sports.py` methods |
|
||
| `sports_helpers.py` | 3.5.0 | clamp/logo/rotation free functions and `SportsHelpersMixin`, plus the `_favorite_key` seam |
|
||
| `espn_dates.py` | 3.5.0 | ESPN date-range and `limit` workarounds |
|
||
|
||
Each is described in [src/common/README.md](../src/common/README.md).
|
||
|
||
### Converging on `src/common`
|
||
|
||
The scoreboards never built on `src/base_classes` (now removed); their own
|
||
`sports.py` copies had moved past it. So shared code now lands in hardware-free `src/common`
|
||
modules taken from the plugin copies, each a **new module** rather than growth
|
||
on an existing one: a plugin that deletes a method copy and relies on an older
|
||
module having gained it fails at runtime with an `AttributeError`, while a
|
||
missing module fails at load, where the version checks can see it.
|
||
`sports_helpers.py` is the newest (it holds `_favorite_key`, the override point
|
||
listed below, for later phases); its parity test compares every body against
|
||
the plugin copies when `LEDMATRIX_PLUGINS` points at a checkout, and
|
||
`test/test_common_is_hardware_free.py` keeps `src/common` free of
|
||
`rgbmatrix`, `src.display_manager` and `src.plugin_system`. How a plugin adopts a
|
||
module and drops its copy is documented in the plugins repo's
|
||
`docs/plugin-development/08-shared-sports-code.md`.
|
||
|
||
## Override points (the plugin-facing seam)
|
||
|
||
The base class calls these; plugins implement or override them. This table is the
|
||
contract — additions require a default implementation, removals require a
|
||
deprecation cycle.
|
||
|
||
| Hook | Purpose | Default |
|
||
|---|---|---|
|
||
| `_fetch_data()` | Sport's schedule source | abstract |
|
||
| `_extract_game_details(event)` | Sport-specific view-model fields on top of the common ones | delegates to `_extract_game_details_common` |
|
||
| `_draw_scorebug_layout(game, force_clear)` | Sport's card rendering | base layout |
|
||
| `_custom_scorebug_layout(game, draw)` | Per-sport overlay on the base layout | no-op |
|
||
| `score_phrase(points, team_abbr)` | Celebration wording (`"GOOOOAAALLL!"` vs `"TOUCHDOWN!"`). `points` is the score delta, which sports with variable-value scores use to name the play | `"<abbr> SCORES!"` — only consulted when `CelebrationMixin` is present |
|
||
| `win_phrase(team_abbr)` | Win-celebration wording | `"<abbr> WINS!"` — mixin only |
|
||
| `_favorite_key(game, side)` | Which view-model field identifies a team for favorites matching | `game["<side>_abbr"]` |
|
||
| `_config_schema_path()` | Plugin's `config_schema.json` — returning it routes `_get_layout_offset` through the `src.element_style` resolver (and gives it the defaults to compare against) | `None`, i.e. the classic inline `customization.layout` read |
|
||
| `_font_root()` | Directory to resolve `assets/fonts` against | core install root |
|
||
|
||
Two class attributes serve the same purpose for values that are per-sport
|
||
constants rather than behavior:
|
||
|
||
| Attribute | Meaning | Default |
|
||
|---|---|---|
|
||
| `FINAL_PERIOD` | Period at/after which a zero clock can mean "over" | `4` (hockey overrides to `3`) |
|
||
| `CLOCK_COUNTS_DOWN` | Whether `0:00` means "expired" | `True` (soccer/afl/nrl override to `False` — their clocks count up, so `0:00` is kickoff) |
|
||
| `COALESCE_SCORING_SEQUENCE` | Fold score increments arriving during an active celebration into that one celebration | `False` (football overrides to `True` — a touchdown lands as +6, then +1 for the extra point) |
|
||
|
||
### Why these are seams and not branches
|
||
|
||
`_favorite_key` exists because NRL abbreviations are **not unique** — "NEW" is both
|
||
Newcastle Knights and New Zealand Warriors, "CAN" both Canberra and Canterbury —
|
||
so NRL matches favorites on team ID. Flattening every plugin to abbreviations
|
||
would silently select the wrong club for NRL users. The base declares the seam,
|
||
NRL fills it, and core never learns the string `"nrl"`.
|
||
|
||
`CLOCK_COUNTS_DOWN` exists for the same reason in the opposite direction: a
|
||
soccer clock reading `0:00` means the match has not kicked off, so running the
|
||
clock-expiry branch there would evict live games.
|
||
|
||
`COALESCE_SCORING_SEQUENCE` is the third of the same kind. In football one
|
||
scoring play arrives as two score updates, so the follow-up must be folded into
|
||
the first celebration; in soccer two increments a few seconds apart are two real
|
||
goals, and folding them would swallow one. Neither default is "right" — which is
|
||
precisely why it is a declared per-sport constant rather than a hidden
|
||
assumption baked into the shared body.
|
||
|
||
## Capabilities
|
||
|
||
```
|
||
capabilities/
|
||
celebrations.py CelebrationMixin opt-in: afl, nrl, soccer, football
|
||
rotation.py RotationStrategy + registry
|
||
```
|
||
|
||
**`CelebrationMixin`** merges the two dialects the lineages grew
|
||
(`_check_for_goal`/`celebrate_opponent_goals` vs
|
||
`_check_for_score`/`celebrate_opponent_scores`). Their bodies were identical
|
||
apart from three things, each now a seam: wording (`score_phrase`), follow-up
|
||
suppression (`COALESCE_SCORING_SEQUENCE`), and team identity (`_favorite_key`,
|
||
so NRL matches on id). Both config spellings are read, so a plugin adopting the
|
||
mixin keeps working with the keys already in its published schema.
|
||
|
||
Mix it in **before** the mode class — `class SoccerLive(CelebrationMixin,
|
||
SportsLive)` — so the celebration `display()` runs first and falls through to
|
||
the scorebug via `super()`.
|
||
|
||
**Rotation strategies.** The three "dialects" turned out to be one algorithm
|
||
(Smooth Weighted Round-Robin) in two shapes: an incremental picker holding state
|
||
across calls (afl/nrl/soccer) and a precomputed per-cycle list
|
||
(football/baseball/basketball, and hockey with a different loop shape). They
|
||
agree within a cycle and differ only at the boundary — the incremental form has
|
||
no restart seam — so core ships both rather than declaring a winner:
|
||
|
||
```python
|
||
self.rotation = get_rotation_strategy("swrr", weight_for=self._live_weight)
|
||
```
|
||
|
||
`weight_for` is supplied by the host, so the *favorites* policy stays with the
|
||
plugin and `rotation.py` never learns what a favorite is. An unknown strategy
|
||
name degrades to `simple` rather than raising: the name comes from user config,
|
||
and a typo should cost the boost, not the scoreboard. When a plugin needs an
|
||
ordering that core does not ship, it calls `register_rotation_strategy` to add
|
||
its own — rather than core growing a branch for it.
|
||
|
||
`test_sports_capabilities.py` (removed with `src/base_classes`) checked each
|
||
strategy against a **verbatim transcription** of the plugin code it replaces,
|
||
over every live-game shape up to four games. That differential is what B5 deletes the bundled copies on the
|
||
strength of.
|
||
|
||
## Scroll display — where the promotion line falls
|
||
|
||
`src/common/sports_scroll.py` is deliberately *not* a superset of the ten
|
||
`scroll_display.py` copies. A method-level comparison of the eight that share a
|
||
shape (f1 and ufc are genuine forks) found a sharp split:
|
||
|
||
| Layer | Evidence | Outcome |
|
||
|---|---|---|
|
||
| Orchestration — `get_all_vegas_content_items`, `clear_all`, `get_scroll_info`, `get_dynamic_duration`, `is_complete`, `display_frame` | identical to 96–100% similar across all eight | **promoted** |
|
||
| Settings — `_get_scroll_settings` | one algorithm; the copies differ *only* in which league keys they walk | **promoted**, with the ladder as data (`SCROLL_LEAGUE_KEYS`) |
|
||
| Content — `prepare_scroll_content`, `_load_separator_icons` | 8 distinct bodies across 8 plugins (145 lines, 53% similar at worst); icons 6% | **override point, permanently** |
|
||
|
||
Same name, different job: `prepare_scroll_content` draws *this sport's* game
|
||
card. Merging the eight bodies would be the exact mistake the promotion rule
|
||
exists to prevent, so the base class raises `NotImplementedError` rather than
|
||
rendering something plausible — a base that rendered *something* would let a
|
||
plugin ship a silently blank scroll.
|
||
|
||
The one behavior the upstreamed version adds is native
|
||
`global_config['target_fps']` support. The bundled copies hardcode ~100 FPS via
|
||
`scroll_delay = 0.01` and never consult the global smooth-scrolling target;
|
||
Part A threaded it through each copy by hand, and this makes that threading
|
||
legacy compatibility rather than the mechanism.
|
||
|
||
> **Superseded.** Once presentation became frame-locked (#545) the helper
|
||
> steps a fixed whole-pixel amount per presented frame and the panel presents
|
||
> at its own refresh, so honouring `target_fps` only turned it into a speed
|
||
> multiplier (60 doubled a scoreboard's speed, 200 halved it). `sports_scroll`
|
||
> no longer reads it: the crisp-speed ladder uses the panel refresh
|
||
> (`display_manager.refresh_hz`), and speed comes from
|
||
> `scroll_settings.scroll_speed` alone. See `docs/SCROLL_PERFORMANCE.md`.
|
||
|
||
## Phases
|
||
|
||
B0–B3 are merged and shipping in core 3.2.0. Everything that remains is
|
||
**rollout**, and it splits into three phases with very different risk profiles.
|
||
The original plan folded the last two together; they are separated here because
|
||
one of them cannot break a user on an old core and the other can.
|
||
|
||
| Phase | Scope | Status | Gate |
|
||
|---|---|---|---|
|
||
| **B0** | Characterization tests, CI unit job, `element_style`, font cwd fix, CHANGELOG discipline | ✅ | — |
|
||
| **B1** | Promote the nine universal methods; convert `sports.py` → package | ✅ | Characterization suite green; no behavior change intended |
|
||
| **B2** | `CelebrationMixin` + rotation strategies as opt-in capabilities | ✅ | Non-adopters have zero new code in their MRO; strategies checked against verbatim plugin transcriptions |
|
||
| **B3** | Upstream the scroll **orchestration** layer as `src/common/sports_scroll.py`, reading `global_config['target_fps']` natively | ✅ | Content building stays per-sport |
|
||
| **B4** | Ship 3.2.0 *and* make version reporting trustworthy | ✅ | Released 2026-08-03; tag, release and `src.__version__` agree; compatibility gate merged (#428, #431, #433) |
|
||
| **B5** | Adoption — guarded core imports, all eight. **Bundled copies stay.** | ✅ | All eight adopted; harness byte-identical; see the B5 retrospective below — four shipped broken and were repaired in plugins #251 |
|
||
| **B6** | Sunset — delete the bundled copies | ✅ | Ran 2026-09-01, all eight. Floors at 3.2.0; the store refuses on all three routes in (#431/#433, #508, #510). See "B6 — what actually happened" |
|
||
|
||
### B4 — what "ship 3.2.0" actually requires
|
||
|
||
Cutting the tag is the small part. The version *number* has to become something
|
||
a floor can be trusted against, and today it is not:
|
||
|
||
- **The tag and `src.__version__` have never agreed.** `v3.1.0` was tagged
|
||
2026-05-31; `__version__` only became `"3.1.0"` on 2026-07-12 (`7f7f0d64`).
|
||
The v3.1.0 release therefore reports `__version__ = "1.0.0"`.
|
||
- **Which silences the compatibility warning entirely for that population.**
|
||
`PluginLoader._warn_if_incompatible` skips the check when the parsed core
|
||
version is below `(2, 0, 0)` — an anti-spam guard that, given the above,
|
||
matches exactly the users most likely to be behind.
|
||
- **Nothing enforces a floor anyway.** The check is advisory (it logs and
|
||
continues), and neither `StoreManager.install_plugin` nor
|
||
`StoreManager.update_plugin` compares the core version at all — `update_plugin`
|
||
compares the plugin's manifest version against the registry's
|
||
`latest_version` and nothing else.
|
||
|
||
*Fixed, in two parts.* `install_plugin` gained the gate in #431/#433, which
|
||
covers every registry-managed install and, through `_reinstall_with_rollback`,
|
||
the update path that re-downloads.
|
||
`update_plugin`'s git branch pulls in place and re-downloads nothing, so it
|
||
stayed ungated until `_gate_pulled_commit` closed it — checked after the pull
|
||
(the registry carries no floor field, so the incoming floor is unknowable
|
||
before it) and undone with `git reset --hard` to the pre-pull commit. That
|
||
route is rare in practice, since monorepo plugins install as archives; it was
|
||
closed because the sunset rule in the plugins repo's
|
||
`08-shared-sports-code.md` states as **condition 3** that the core enforces
|
||
the floor "at install/update time", and B6 rests on that being true rather
|
||
than merely written down. `install_from_url` — sideloading a plugin from a
|
||
URL — is still ungated.
|
||
|
||
So B4 is: tag and release 3.2.0; make the tag, the release, and `__version__`
|
||
agree, and keep them agreeing; reconsider the `< 2.0.0` skip; migrate manifests
|
||
from `ledmatrix_min` to `ledmatrix_min_version`; and add the install/update
|
||
compatibility gate that B6 depends on.
|
||
|
||
#### Two fields express compatibility, and the gate only reads one
|
||
|
||
`compatible_versions` is the canonical contract: `schema/manifest_schema.json`
|
||
**requires** it, all 42 published manifests carry it, and it holds semver
|
||
*ranges* — `[">=2.0.0"]` in 41 of them, `[">=1.0.0"]` in `7-segment-clock`.
|
||
`ledmatrix_min_version` is the optional per-release floor inside `versions[]`.
|
||
|
||
The gate as merged reads only the floor. Today that is harmless: no manifest
|
||
uses an upper bound, and the two fields agree everywhere except
|
||
`7-segment-clock` (`>=1.0.0` against a `2.0.0` floor). But the fields *can*
|
||
disagree, and the range syntax the schema already permits includes upper bounds
|
||
— a plugin declaring `["2.0.0 - 2.9.9"]` means "not compatible with 3.x" and
|
||
the gate would install it on 3.2.0 regardless.
|
||
|
||
**Before B6, the gate must evaluate `compatible_versions` as well**, and the
|
||
manifest migration must reconcile the two fields rather than only renaming the
|
||
floor. Deciding which wins when they disagree is part of that work; the safe
|
||
default is the more restrictive.
|
||
|
||
(The schema also deprecates a top-level `ledmatrix_version` in favour of
|
||
`compatible_versions`. No manifest still carries it, so there is nothing to
|
||
migrate there.)
|
||
|
||
### B5 — the *fallback* is safe by construction; the modern path is not
|
||
|
||
The heading matters, because the unqualified version of this claim is false and
|
||
this document proves it two sections down: four of the eight adopted plugins
|
||
shipped with scroll mode broken on a 3.2.0 core. What is safe by construction is
|
||
narrower than "adoption".
|
||
|
||
A plugin adopting core imports keeps its bundled copy and reaches it through the
|
||
guarded import (see the Upgradability table above). On a core that doesn't ship
|
||
the module the plugin falls back and behaves exactly as it does today. That
|
||
fallback compatibility — and only that — is safe by construction. On a core that
|
||
*does* ship the module, correctness is not automatic — object-level and scroll-mode validation
|
||
(building both classes and comparing, per the retrospective below) is required
|
||
to prove full behavior. There is no version of this step that breaks a user *on
|
||
an old core*, which is why it does not wait for B6's gate.
|
||
|
||
The hockey scroll-display pilot is **already validated**: adopted against a core
|
||
carrying 3.2.0, `scroll_display.py` went from 691 to 289 lines and all 16 harness
|
||
renders (8 sizes × 2 screens) came out byte-for-byte identical to the
|
||
pre-adoption run. That byte-comparison is the acceptance gate for every
|
||
adoption. The recipe and its two gotchas are in the plugins repo's
|
||
`docs/plugin-development/08-shared-sports-code.md`.
|
||
|
||
### B6 — why the sunset needs more than a version floor
|
||
|
||
Deleting a bundled copy removes the fallback, so the guarded import becomes a
|
||
hard dependency. On a core without the module the plugin raises
|
||
`ModuleNotFoundError` at load; `PluginManager.load_plugin` catches it, records
|
||
`PluginState.ERROR`, logs one line, and continues. Nothing crashes — the user
|
||
simply loses that scoreboard, with no visible explanation.
|
||
|
||
Verified against a `v3.1.0` worktree: `src/common/sports_scroll.py`,
|
||
`src/element_style.py` and the `src/base_classes/sports/` package are all absent
|
||
there, and the import fails with `exc.name == 'src.common.sports_scroll'`. Guard
|
||
sets must name that exact dotted path — `{"src"}` alone does not match it.
|
||
|
||
Combined with the B4 findings, a plugin that deletes its copy today reaches an
|
||
un-updated user through a normal store update, fails to load, and warns nobody.
|
||
**B6 therefore waits for B4's compatibility gate to have shipped and to have
|
||
been in users' hands long enough that the population running a core without it
|
||
is small.** The bundled copies cost disk space; deleting them early costs
|
||
scoreboards, silently. That trade is not close.
|
||
|
||
Before the first sunset, add a **compatibility regression test**. **Built:**
|
||
core `test/test_sports_sunset_matrix.py` (#505). It has to
|
||
cover four cases, not one — B5's safety claim and B6's failure mode are
|
||
different propositions and only the second is obvious:
|
||
|
||
| | bundled copy present | bundled copy removed |
|
||
|---|---|---|
|
||
| **pinned old core** | **loads** — this is B5's whole guarantee, that the guarded import falls back | `PluginState.ERROR`, and the recorded error names the exact missing module |
|
||
| **current core** | loads, using core code | loads, using core code |
|
||
|
||
The top-left cell is the one worth writing first: nothing in the suite currently
|
||
proves that an adopted plugin still works on a core that predates the module,
|
||
which is the entire basis for saying B5 is safe to run ahead of the gate.
|
||
|
||
Assert the old-core/removed-copy case as `PluginState.ERROR` **plus the missing
|
||
module path**, not as an uncaught exception. `PluginManager.load_plugin` catches
|
||
`ModuleNotFoundError`, so nothing propagates — a test expecting a raise would
|
||
pass for the wrong reason on a core where the module is merely broken rather
|
||
than absent. "Fails loudly" is aspirational, not what the code does today: it
|
||
fails into `ERROR` state with one log line, which is precisely why B6 needs the
|
||
gate rather than trusting the failure to be noticed.
|
||
|
||
The same suite should exercise the install/update gate, since it is the other
|
||
half of the guarantee.
|
||
|
||
### B6 — what actually happened
|
||
|
||
**Ran 2026-09-01, across all eight scoreboards.** Held from 2026-08-05 to
|
||
2026-09-01 on the argument below, which is kept because the reasoning applies to
|
||
the next module, not because it is still in force.
|
||
|
||
**The hold, and why it lifted.** The stated gate was evidence of 3.2.0 uptake —
|
||
"a few months of it being the default download, or store-side install data".
|
||
That evidence never arrived and could not: the core updates by
|
||
`git pull --rebase`, so release-asset counts cannot measure uptake, and no
|
||
store-side telemetry exists. What changed instead is that the *risk* the gate
|
||
protected against was closed directly. The store now refuses a plugin whose
|
||
floor exceeds the running core on **all three** routes in:
|
||
|
||
| route | gated by |
|
||
|---|---|
|
||
| `install_plugin` — every path that re-downloads, `_reinstall_with_rollback` included | #431, #433 |
|
||
| `update_plugin`'s git branch — pulls in place, re-downloads nothing | #508 |
|
||
| `install_from_url` — sideloading | #510 |
|
||
|
||
With all three closed a pre-3.2.0 user cannot receive a sunset plugin at all;
|
||
they keep the version they already run. The population the hold existed to
|
||
protect is protected by refusal rather than by a bundled copy — which is what
|
||
the copy was standing in for.
|
||
|
||
**What shipped.** Eight plugins, ~5,800 lines of frozen fallback deleted. Each:
|
||
copy removed, guarded import collapsed to a plain one, floor raised to 3.2.0,
|
||
`test_core_fallback.py` rewritten as `test_core_scroll.py` asserting the sunset
|
||
rather than the fallback. `scripts/check_scroll_adoption.py` gained
|
||
`sunset_violations` and a `SUNSET_PLUGINS` set naming all eight, so a
|
||
resurrected copy or a returned guard fails CI.
|
||
|
||
Delivered as plugins #346 (hockey, later folded into #351), #349 (football),
|
||
#350 (baseball), #351 (the remaining six).
|
||
|
||
**Two things found by doing it, both worth carrying forward:**
|
||
|
||
- **Only one fallback held orchestration logic the core lacked.** baseball's
|
||
`_configure_scroll_helper` reinterpreted `scroll_speed` as pixels-per-*frame*
|
||
when `speed × delay` fell outside the 0.1–5.0 window — measured, 10–20×
|
||
faster than configured for a speed between 1.0 and 5.0. Standardised onto the
|
||
core's behaviour (honour the documented px/sec, clamp) rather than preserved.
|
||
Every other difference across the eight was a docstring, an unreachable
|
||
`scroll_helper is None` guard, or an equivalent diagnostic.
|
||
- **Two tests had been leaning on the guard without anyone knowing.**
|
||
`soccer/test_live_screens.py` stubbed `src` in a way that shadowed the core,
|
||
so its guarded import fell back and the test had been exercising the *frozen
|
||
copy* rather than the shipping class since B5. Before sunsetting anything else
|
||
that carries a guarded core import, grep for tests that stub `src`.
|
||
|
||
**The floor-raising traps still apply** to any future sunset: four plugins
|
||
declare their floor top-level, where editing `versions[0]` is a silent no-op,
|
||
and the floor has three live spellings (`min_ledmatrix_version`,
|
||
`requires.min_ledmatrix_version`, `versions[].ledmatrix_min_version`, plus
|
||
deprecated `ledmatrix_min`). See
|
||
`src/plugin_system/compatibility.py:declared_min_version` for the resolution
|
||
order any floor-raising tool must reproduce — and note the name is **inverted**
|
||
between the top level and `versions[]`.
|
||
|
||
**Still not adopted, deliberately:** `data_sources.py`, `game_renderer.py` and
|
||
`base_odds_manager.py`. The standing decision held them until B6 closed; it now
|
||
has, so they can be reconsidered — with B5's lesson applied, which is to build
|
||
the object and diff rendered output rather than trust a static check.
|
||
|
||
### B5 retrospective — what the adoption actually cost
|
||
|
||
Recorded because it is the evidence behind the two decisions above, and because
|
||
"the adoption went fine" is not what happened.
|
||
|
||
**Four of the eight shipped with scroll mode broken** on a 3.2.0 core, and were
|
||
repaired in plugins-repo #251. The restructure lifted the content methods
|
||
verbatim but left the state they read off `self` behind: separator-icon
|
||
constants (hockey, basketball, lacrosse) and the game-renderer cache (afl).
|
||
hockey/basketball/lacrosse could not construct the scroll display at all; afl
|
||
raised inside `prepare_scroll_content`, which the core base *catches*, so its
|
||
only symptom was scroll mode silently drawing nothing.
|
||
|
||
Three things are worth carrying forward:
|
||
|
||
- **The bundled fallback did not protect anyone from this.** The break was on
|
||
the modern path, which the fallback never touches. Carrying the second copy
|
||
bought nothing against the actual defect while creating the divergence that
|
||
produced it. That is an argument *for* B6, not against it.
|
||
- **Every gate was green.** The safety harness renders the scoreboard screens,
|
||
not scroll mode; `test_core_fallback.py` checked that methods existed and that
|
||
their *globals* resolved, and `self.NHL_SEPARATOR_ICON` is an attribute read,
|
||
invisible to an AST scan for `Name` loads. The fix was to stop reasoning about
|
||
source and **build the object**: construct both classes on both paths, compare
|
||
the separator icons they end up with, and assert the adopted class ends up
|
||
with every instance attribute the bundled one sets.
|
||
- **Test what the change touches, not what is convenient to render.** Scroll
|
||
mode had no coverage because the harness could not reach it. A comparison
|
||
harness that renders the same games through both paths and diffs the pixels
|
||
needs no per-sport knowledge of the right answer, only that adopting core code
|
||
did not change it.
|
||
|
||
**The ledger.** Before adoption, eight duplicated copies totalled 5,685 lines.
|
||
After adoption plus the frozen legacy copies it was 10,610; removing the dead
|
||
inline duplication (plugins #252) brought it to roughly 8,620. B6 would take it
|
||
to about 3,300 including the shared core module — some 2,400 fewer than before
|
||
this project started. **Until B6 runs, the adoption is net negative on disk**,
|
||
and its one delivered user-visible gain was that adopted plugins honoured the
|
||
global `target_fps` instead of hardcoding ~100 FPS (since withdrawn: see the
|
||
note under the B3 design above).
|
||
|
||
### Decision: stop adopting further modules until B6 closes
|
||
|
||
`data_sources.py` (9 copies), `game_renderer.py` (8) and `base_odds_manager.py`
|
||
are the obvious next candidates. **Do not adopt them yet.** Each adoption adds
|
||
carrying cost — a second copy to keep in step — against a payoff that is
|
||
contingent on B6, and B6 is gated on an installed base we cannot currently
|
||
measure. Consolidate what is already committed; revisit when B6 does.
|
||
|
||
## What's next
|
||
|
||
Steps 1–5 of the original plan are **done**: 3.2.0 is tagged and published with
|
||
a version number CI now asserts (#428), the compatibility gate is in
|
||
`install_plugin` and reads `compatible_versions` as well as the floor
|
||
(#431, #433), the newest manifest entry is required to use `ledmatrix_min_version`
|
||
(plugins #244), and all eight plugins have adopted the scroll orchestration
|
||
(plugins #245–#249, repaired in #251, tidied in #252).
|
||
|
||
What actually remains, smallest first:
|
||
|
||
1. **Soak the adoptions on hardware.** football and hockey have been run on a
|
||
live rig through real games; baseball was watched through one earlier. The
|
||
rest are proven by harness, unit tests and pixel comparison. Out-of-season
|
||
sports cannot be soaked until their season starts. When you do, **check the
|
||
rig's `*_display_mode` first** — a board in `switch` mode will happily load a
|
||
sunset plugin and tell you nothing about the scroll code the sunset changed.
|
||
2. **Cut 3.3.0.** Not required by B6 — its floors are 3.2.0, which is released —
|
||
but `calendar` 1.2.3 floors at 3.3.0 for the device-authorization endpoints
|
||
that landed after 3.2.0, so it is un-installable until the release exists.
|
||
3. **Reconsider the held modules** (`data_sources.py`, `game_renderer.py`,
|
||
`base_odds_manager.py`) now that the sunset has closed. `game_renderer.py` is
|
||
the largest single duplication left: ~11,500 lines across eight plugins, with
|
||
~36,500 more in the eight `sports.py`. The `src/base_classes/sports/`
|
||
package promoted in B1/B2 was never imported by a plugin and has been
|
||
removed, so the plugin copies are the only starting point.
|
||
|
||
## How to keep this project healthy
|
||
|
||
Lessons this migration paid for, worth applying beyond it:
|
||
|
||
- **A version number is a promise; keep it in one place.** Three different
|
||
answers to "what version am I on" (tag, release, `__version__`) is what made
|
||
the floor untrustworthy. Assert their agreement mechanically.
|
||
- **Advisory checks protect nobody.** If a rule matters, enforce it where the
|
||
action happens — the install path, not a log line the user will never read.
|
||
If it doesn't matter enough to enforce, don't write the rule.
|
||
- **Prefer failures that are loud and early.** A plugin that dies at load with
|
||
one journal line is indistinguishable, to a user, from a plugin that was never
|
||
installed. Surface plugin health in the UI.
|
||
- **Keep the two repos' rules in sync deliberately.** The sunset rule lives in
|
||
both this file and the plugins repo's
|
||
`docs/plugin-development/08-shared-sports-code.md`. When one changes, change
|
||
the other in the same PR — drift between them is how a contributor ends up
|
||
following a rule that was superseded.
|
||
- **Measure before and after, on real hardware.** Byte-identical harness renders
|
||
and a device soak caught what unit tests could not. Reserve "it should be
|
||
fine" for things you have actually looked at.
|
||
|
||
## Rules for contributors
|
||
|
||
- **Promote on evidence, not intuition.** A method moves to core when every copy
|
||
has it and they agree on intent. Otherwise it stays in the plugins.
|
||
- **Never add a sport name to core.** If core needs to know which sport it is,
|
||
the design is wrong — add an override point instead.
|
||
- **A capability that is not opted into must not execute.** If you find yourself
|
||
writing `if self.<capability>_enabled` inside a base class, it belongs in a
|
||
mixin.
|
||
- **Touch the view-model keys only additively.** The shared `src/common`
|
||
renderers read them.
|
||
- **Every promotion lands with the characterization suite green**, and every
|
||
pilot adoption lands with that plugin's harness and golden suites green.
|