fix(scroll): stop timing the idle gap between scrolls as a frame (#582)

ScrollHelper.last_frame_time was set once in __init__ and thereafter only
at the end of log_frame_rate(). Nothing re-armed it when a scroll began, so
the first frame of every scroll was timed against the last frame of the
*previous* one and the whole idle period between them was recorded as a
single frame.

Measured over 3 hours on a 256x64 Pi 4, that produced 31 windows reading

    Scroll frame stats - 0.0 fps over 1 frames | median 136776.02ms
    p95 136776.02ms max 136776.02ms min 136776.02ms | stalls 0 (0.0%)

and -- worse, because it is not obviously wrong -- put the same gap in the
max field of otherwise healthy windows, where the worst values were 537s
and 604s. It also counted as one stall per scroll start: at ~500 frames to
a window that is ~0.2%, against measured stall rates of 0.07-0.16%. The
stall rate is the number used to judge whether a scroll change worked, and
it was the same order of magnitude as its own artefact.

The first frame of a scroll has no predecessor, so it has no frame time.
last_frame_time is now None until one is rendered, and reset_scroll() puts
it back -- the same treatment last_update_time already gets three lines
above, for the same reason. reset_scroll() alone is not enough, because the
scrollers actually emitting these lines never call it, so a sample at or
past the 5s log interval is dropped as well: nothing that renders a scroll
takes that long over one frame. Seeding also restarts the window timer, or
the boundary is already overdue when the second frame arrives and every
scroll opens by reporting a window of exactly one frame. A window whose
samples were all dropped now logs nothing rather than reporting the gap.

docs/SCROLL_PERFORMANCE.md documented the diagnostic in terms of a
"Frame time: N ms" line that 6031e705 replaced with the aggregate, so its
grep matched nothing on any rig. The section now describes the line that is
actually emitted, reads duplicate frames off skips and a below-median
result rather than a 2ms mode, and adds a command that ranks every scroller
by p95 -- verified against 3 hours of journal, where it reproduces
src.base_odds_manager at p95 44.08ms against 10.19ms for the two scrollers
already on src/common/scroll_config.py.

requirements.txt still offered scipy for the sub-pixel interpolation path
deleted in #570. Installing it has no effect; the entry says so.


Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

CHANGELOG.md

@@ -19,6 +19,30 @@ accepts both, but the store flags the old spelling as deprecated
 
 ## Unreleased
 
+## 3.4.0
+
+Plugin-facing changes since 3.3.0 (tag `v3.3.1`) not covered further down:
+
+- `BasePlugin.get_update_interval()` (#555) — return seconds to override the
+  manifest's `update_interval` at runtime (e.g. poll fast only while a game is
+  live), or `None` to keep it. Clamped to at least 5 seconds; a raising or
+  non-numeric return is ignored. Called every scheduling tick, so keep it
+  cheap. Older cores never call it. See `docs/PLUGIN_API_REFERENCE.md`.
+- `src.common.scroll_config` (#523) — turns a plugin's scroll config into a
+  configured `ScrollHelper` in one place, replacing per-plugin resolution that
+  disagreed between tickers, and warns when a speed won't advance whole pixels
+  per panel refresh. Floor on 3.4.0 to import it.
+- **Skins are marked unsupported.** No current scoreboard plugin builds on
+  `src.base_classes`, so the skin hook (`SportsCore._render_game`) never runs.
+  The web UI no longer shows the Visual Skin dropdown, the store hides and
+  refuses `"type": "skin"` entries, and `GET /api/v3/skins` reports
+  `"supported": false`. Saved `skin` config values still load and save.
+  `src/skin_system/` is unchanged.
+- **Web preview size** now comes from `src/display_geometry.py`, the same
+  computation `DisplayManager` uses: double-sided setups preview one screen,
+  and a missing `chain_length` defaults to 2 everywhere (the Starlark magnify
+  default and the sync handshake used 1).
+
 **Per-element display customization, and the last mile of it into the web UI.**
 A user can set the font, size, colour, position, visibility and alignment of
 individual display elements per plugin -- and, where a plugin has display
@@ -109,6 +133,8 @@ Removed:
 
 ## 3.3.0
 
+Historical note: tag `v3.3.0` reports `__version__` "3.2.0" and tag `v3.3.1` reports "3.3.0", so a "3.3.0" floor is effectively `v3.3.1`, the first release shipping `src/common/sports_shared.py`.
+
 **The release the sports scoreboards floor on to delete their bundled copies.**
 3.2.0 shipped the unified sports library and made `ledmatrix_min_version`
 enforceable; this ships the last three shared modules and completes the store

CLAUDE.md

@@ -6,7 +6,7 @@
 - `config/config.json` — User plugin configuration (persists across plugin reinstalls)
 - `plugin-repos/` — **Default** plugin install directory used by the
   Plugin Store, set by `plugin_system.plugins_directory` in
-  `config.json` (default per `config/config.template.json:167`).
+  `config.json` (default per `config/config.template.json`).
   Not gitignored.
 - `plugins/` — Legacy/dev plugin location. Gitignored (`plugins/*`).
   Used by `scripts/dev/dev_plugin_setup.sh` for symlinks. The plugin
@@ -23,7 +23,7 @@
 - Each plugin needs: `manifest.json`, `config_schema.json`, `manager.py`, `requirements.txt`
 - Plugin instantiation args: `plugin_id, config, display_manager, cache_manager, plugin_manager`
 - Config schemas use JSON Schema Draft-7
-- Display dimensions: always read dynamically from `self.display_manager.matrix.width/height`
+- Display dimensions: always read dynamically from `self.display_manager.width/height` — not `display_manager.matrix.width/height`, because `matrix` is `None` when hardware init fails (the properties fall back to the canvas size)
 - Secrets: namespaced by plugin id in `config/config_secrets.json`, declared
   via `"x-secret": true` in the plugin's config schema, and deep-merged into
   the plugin's config dict at load time — plugins read them with plain
@@ -45,9 +45,12 @@
 - Plugin configs stored in `config/config.json`, NOT in plugin directories — safe across reinstalls
 - Third-party plugins can use their own repo URL with empty `plugin_path`
 
-## Skin System (visual overlays for sports scoreboards)
+## Skin System (visual overlays for sports scoreboards) — NOT SUPPORTED YET
+- Skins do not render with the current scoreboard plugins: the only hook is `SportsCore._render_game()` in `src/base_classes/sports/core.py`, and no current scoreboard plugin (monorepo or third-party registry) builds on `src.base_classes`
+- So core doesn't offer them: no Visual Skin dropdown (`get_plugin_schema` skips `inject_skin_selector`), the store hides/refuses `"type": "skin"` entries, `GET /api/v3/skins` reports `"supported": false`. Switch: `SKINS_RENDER_SUPPORTED` in `src/skin_system/__init__.py`
+- Stored `skin` / `skin_options` config values must keep loading and saving (base schema allows them; form saves deep-merge over the stored section)
 - Skins live in `skins/<skin-id>/` (skin.json + skin.py), NOT in plugin dirs — plugin reinstall deletes plugin dirs
-- Core: `src/skin_system/` (ScoreboardSkin, SkinContext, runtime); hook: `SportsCore._render_game()` in `src/base_classes/sports/core.py`
+- Core: `src/skin_system/` (ScoreboardSkin, SkinContext, runtime); keep it and its tests
 - Skins render onto `ctx.canvas` only; fallback to built-in renderer on `False`/exception (3 strikes disables for session)
 - View-model guaranteed keys are frozen (see `test/test_skin_system.py::TestViewModelContract`) — renaming keys in `_extract_game_details_common` or sport extractors breaks published skins
 - Validate skins headlessly: `python scripts/validate_skin.py --skin <id>`; docs: `docs/SKIN_SYSTEM.md`, `docs/CREATING_SKINS.md`

README.md

@@ -463,13 +463,12 @@ For plugin development, check out the [Hello World Plugin](https://github.com/Ch
 
 ### Visual Skins for Scoreboards
 
-Want a different look for a sports scoreboard without forking the plugin?
+**Not supported yet.** Skins are meant to restyle a sports scoreboard's
-**Skins** restyle the live/recent/upcoming screens while the plugin keeps
+live/recent/upcoming screens without forking the plugin, but the current
-handling data, scheduling, caching, and vegas mode. Install one with
+scoreboard plugins don't render them: a selected skin has no effect. The web
-`git clone <skin repo> skins/<skin-id>`, select it in the plugin's config,
+UI doesn't offer skin install or selection for that reason. The skin system
-and you're done — see [docs/SKIN_SYSTEM.md](docs/SKIN_SYSTEM.md) (how it
+and its docs stay in place for when scoreboards adopt it; see
-works) and [docs/CREATING_SKINS.md](docs/CREATING_SKINS.md) (build your own,
+[docs/SKIN_SYSTEM.md](docs/SKIN_SYSTEM.md) for why.
-including a ready-made Claude Code prompt).
 
 2. **Built-in Managers Deprecated**: The built-in managers (hockey, football, stocks, etc.) are now deprecated and have been moved to the plugin system. **You must install replacement plugins from the Plugin Store** in the web interface instead. The plugin system provides the same functionality with better maintainability and extensibility.
 </details>

docs/ADAPTIVE_LAYOUT.md

@@ -223,8 +223,8 @@ The harness already renders every plugin at a spread of sizes (now
 including 96x48):
 
 ```bash
-python scripts/check_plugin.py <plugin-dir> --sizes 64x32,128x32,96x48,128x64,256x64
+python scripts/check_plugin.py --plugin <plugin-id> --sizes 64x32,128x32,96x48,128x64,256x64
-python scripts/render_plugin.py <plugin-dir> --width 96 --height 48
+python scripts/render_plugin.py --plugin <plugin-id> --width 96 --height 48
 ```
 
 `BoundsCheckingDisplayManager` flags right/bottom overflow and now records

docs/CREATING_SKINS.md

@@ -1,5 +1,14 @@
 # Creating Skins
 
+> **Not supported yet: skins don't render with the current scoreboard
+> plugins.** The only render hook is `SportsCore._render_game()` in
+> `src/base_classes/sports/core.py`, and no current scoreboard (monorepo or
+> third-party) builds on `src.base_classes`, so a skin you build here passes
+> `validate_skin.py` but never appears on the matrix. The web UI and Plugin
+> Store don't offer skins for that reason. Details:
+> [SKIN_SYSTEM.md](SKIN_SYSTEM.md#status-not-supported-yet). The guide below
+> stays accurate for the skin API itself.
+
 A skin restyles a sports scoreboard (live / recent / upcoming) without
 forking the plugin: the plugin keeps fetching data, scheduling, caching, and
 doing vegas mode; your skin only draws. Architecture background:
@@ -19,7 +28,9 @@ panel sizes with **no hardware, no network, no running service**, saves PNGs
 (plus 4x previews) to `skin_renders/`, and fails loudly on errors. Iterate:
 edit → validate → look at the PNGs.
 
-To see it on your matrix, add to your plugin's section in `config/config.json`:
+To select it, add to your plugin's section in `config/config.json` (this is
+stored and validated, but has no visible effect until a scoreboard uses the
+skin hook — see the note at the top):
 
 ```json
 "baseball-scoreboard": {
@@ -28,8 +39,8 @@ To see it on your matrix, add to your plugin's section in `config/config.json`:
 }
 ```
 
-or pick it from the **Visual Skin** dropdown in the web UI (it appears once a
+The web UI's **Visual Skin** dropdown is hidden while skins are unsupported.
-matching skin is installed). `"skin"` also accepts a per-mode mapping:
+`"skin"` also accepts a per-mode mapping:
 `{"live": "my-skin", "recent": "built-in"}`.
 
 ## The manifest (`skin.json`)
@@ -234,8 +245,9 @@ Tips that keep Claude (and you) out of trouble:
       dev machine
 
 Distribute by publishing the directory as a git repo (users
-`git clone <repo> skins/<id>`), or submit it to the plugin registry as an
+`git clone <repo> skins/<id>`). Registry entries with `"type": "skin"` are
-entry with `"type": "skin"` (see [SKIN_SYSTEM.md](SKIN_SYSTEM.md) §Distribution).
+hidden and refused by the Plugin Store while skins are unsupported (see
+[SKIN_SYSTEM.md](SKIN_SYSTEM.md) §Distribution).
 
 **Trust note:** a skin is Python running inside the display service — the
 same trust level as a plugin. Review code before installing skins from

docs/PLUGIN_API_REFERENCE.md

@@ -36,7 +36,11 @@ self.enabled            # Boolean enabled status
 
 #### `update() -> None`
 
-Fetch/update data for this plugin. Called based on `update_interval` specified in the plugin's manifest.
+Fetch/update data for this plugin. Called on the plugin's update interval:
+the value `get_update_interval()` returns when it returns a number, otherwise
+the static interval: the `update_interval` in the plugin's manifest, else
+`update_interval` in the plugin's section of `config.json`, else 60 seconds
+(see [`get_update_interval()`](#get_update_interval---optionalfloat) below).
 
 **Example**:
 ```python
@@ -109,6 +113,46 @@ Called when plugin is enabled.
 
 Called when plugin is disabled.
 
+#### `get_update_interval() -> Optional[float]`
+
+How often this plugin wants `update()` called right now, in seconds. The
+manifest's `update_interval` is one static number; override this when the
+right cadence depends on state only the plugin knows, e.g. poll every 15s
+while a game is live and fall back to the manifest value otherwise.
+
+**Returns**: seconds as a number, or `None` (the default) for no opinion.
+
+How `PluginManager` (`_get_plugin_update_interval` in
+`src/plugin_system/plugin_manager.py`) resolves the interval on each
+scheduling tick:
+
+1. It calls `get_update_interval()`. A number wins over everything below.
+   Values under `PluginManager.MIN_DYNAMIC_UPDATE_INTERVAL` (5 seconds) are
+   raised to it.
+2. If the hook returns `None`, raises, or returns something that isn't a
+   finite number (a `bool`, a string, NaN, infinity), it is ignored and the
+   static interval applies: the manifest's `update_interval`, else
+   `update_interval` in the plugin's section of `config.json`, else 60
+   seconds.
+
+The static value is cached per plugin until the plugin is loaded or
+unloaded again, so editing `update_interval` in config takes effect on the
+next reload. The hook's return value is never cached: it is called on every
+tick of the display loop, so keep it to attribute reads (no config lookups,
+no I/O, no locks a fetch might hold) and don't let it raise.
+
+**Example**:
+```python
+def get_update_interval(self):
+    # Fast while something is live, manifest default otherwise.
+    if any(m.live_games for m in self._live_managers):
+        return self.config.get("live_update_interval", 15)
+    return None
+```
+
+Added in core 3.4.0; older cores never call it, so a plugin that relies on
+it should floor `ledmatrix_min_version` at `3.4.0`.
+
 #### `get_display_duration() -> float`
 
 Get display duration for this plugin. Can be overridden for dynamic durations.

docs/PLUGIN_ARCHITECTURE_SPEC.md

@@ -189,7 +189,9 @@ class BasePlugin(ABC):
     def update(self) -> None:
         """
         Fetch/update data for this plugin.
-        Called based on update_interval in manifest.
+        Called every get_update_interval() seconds when that returns a
+        number, otherwise at the static interval: the manifest's
+        update_interval, else the plugin config's update_interval, else 60s.
         """
         pass
     
@@ -204,6 +206,21 @@ class BasePlugin(ABC):
         """
         pass
     
+    def get_update_interval(self) -> Optional[float]:
+        """
+        Seconds until update() should run again, decided at runtime.
+        Return None (the default) to use the static interval.
+
+        PluginManager._get_plugin_update_interval calls this on every
+        scheduling tick. A number overrides the manifest and is clamped up
+        to PluginManager.MIN_DYNAMIC_UPDATE_INTERVAL (5s); None, a raise,
+        or a non-finite/non-numeric value falls back to the manifest's
+        update_interval, then the plugin config's update_interval, then
+        60s. The static value is cached until the plugin reloads; the hook
+        is not cached, so it must be cheap and must not raise.
+        """
+        return None
+    
     def get_display_duration(self) -> float:
         """
         Get the display duration for this plugin instance.

docs/PLUGIN_DEVELOPMENT_GUIDE.md

@@ -10,11 +10,11 @@ This guide explains how to set up a development workflow for plugins that are ma
 > scale. Existing plugins keep their classic rendering unless they adopt
 > those APIs; nothing migrates automatically.
 
-> **Just want a different look for an existing sports scoreboard?** You may
+> **Want a different look for an existing sports scoreboard?** Skins are
-> not need a plugin at all — a **skin** restyles the live/recent/upcoming
+> meant for that, but they are **not supported yet**: the current scoreboard
-> rendering while the plugin keeps handling data, scheduling, caching, and
+> plugins don't render them (see [SKIN_SYSTEM.md](SKIN_SYSTEM.md#status-not-supported-yet)).
-> vegas mode, in ~100 lines of drawing code. See
+> For now, change the look through the plugin's own display settings or its
-> [CREATING_SKINS.md](CREATING_SKINS.md).
+> code.
 
 ## Overview
 

docs/README.md

@@ -56,8 +56,8 @@ Going deeper:
 - [ADVANCED_FEATURES.md](ADVANCED_FEATURES.md) — Vegas scroll, on-demand display,
   cache management, background services, permissions
 - [FONT_MANAGER.md](FONT_MANAGER.md) — font system
-- [SKIN_SYSTEM.md](SKIN_SYSTEM.md) — skin architecture for sports scoreboards
+- [SKIN_SYSTEM.md](SKIN_SYSTEM.md) — skin architecture for sports scoreboards (not supported yet: current scoreboards don't render skins)
-- [CREATING_SKINS.md](CREATING_SKINS.md) — writing and validating a skin
+- [CREATING_SKINS.md](CREATING_SKINS.md) — writing and validating a skin (same caveat)
 
 ## Reference
 

docs/SCROLL_PERFORMANCE.md

@@ -203,26 +203,61 @@ configured speed, so position stays proportional to real time.
 
 ## Diagnosing a juddery scroller
 
-**`Avg FPS` will lie to you.** It is a 100-frame moving average, and a 2 ms
+**An average will lie to you.** A 2 ms duplicate frame and a 21 ms double-wait
-duplicate frame plus a 21 ms double-wait average to exactly 10 ms. A ticker
+mean exactly 10 ms, so a ticker stalling on half its frames still averages to a
-that is stalling on half its frames still reports a healthy `100.0`.
+healthy 100 fps. The stats line reports the tail for that reason — read the
+percentiles, not the fps.
 
-Look at the **distribution** instead:
+Every scroller emits one line every 5 seconds covering *every* frame in that
+window, tagged with the plugin it came from:
 
 ```bash
-journalctl -u ledmatrix --since "-10min" --no-pager \
+journalctl -u ledmatrix --since "-10min" --no-pager | grep "Scroll frame stats"
-  | grep -oE "Frame time: [0-9.]+ms" | awk '{print $3}' | sed 's/ms//' \
+```
-  | awk '{printf "%.0f\n", $1}' | sort -n | uniq -c
+
+```
+[Plugin: news] Scroll frame stats - 100.0 fps over 501 frames | median 10.00ms
+p95 10.11ms max 12.03ms min 7.98ms | stalls 0 (0.0%) skips 0 (0.0%)
 ```
 
 Reading it, on a 100 Hz panel:
 
 | you see | it means |
 |---|---|
-| everything at 10 ms | healthy |
+| median 10 ms, p95 within ~0.5 ms of it | healthy — locked to the panel |
-| a mode at ~2 ms | **duplicate frames** — the swap was skipped because the image did not change. The scroller is advancing less than one pixel per frame. |
+| p95 or max at 20/30/50 ms | frames missing refreshes — per-frame work is overrunning, or a background thread is holding the GIL |
-| a mode at 20/30/50 ms | frames missing refreshes — per-frame work is overrunning, or a background thread is holding the GIL |
+| non-zero **skips**, or a median *below* 10 ms | **duplicate frames** — the swap was skipped because the image did not change, so the frame never waited on vsync. The scroller is advancing less than one pixel per frame. |
-| `Avg FPS` above 100 | duplicates present, unless the scroll cycle has completed and is idling |
+| non-zero **stalls** | frames past 1.5× the median, which is the measure of judder that survives averaging |
+
+`stalls` and `skips` are both counted against that window's own median, so they
+stay meaningful on a panel running at any refresh rate.
+
+To rank every scroller at once rather than reading lines one at a time:
+
+```bash
+journalctl -u ledmatrix --since "-3h" --no-pager | grep "Scroll frame stats" \
+  | sed -E 's/.*- (\S+) - (\[Plugin: [^]]+\] )?Scroll.*median ([0-9.]+)ms p95 ([0-9.]+)ms.*/\1 \3 \4/' \
+  | awk '$2 < 1000 {n[$1]++; m[$1]+=$2; p[$1]+=$3} END {for (k in n)
+        printf "%-28s %5d windows  median %6.2fms  p95 %6.2fms\n", k, n[k], m[k]/n[k], p[k]/n[k]}' \
+  | sort -k7 -rn
+```
+
+The `$2 < 1000` guard drops windows whose median is a whole second or more.
+Those are not frames. Until the idle-gap fix in `log_frame_rate()`, the first
+frame of every scroll was timed against the end of the *previous* scroll, so
+the gap between them was recorded as one enormous sample — it landed in the
+`max` field of otherwise healthy windows and counted as one stall per scroll,
+roughly 0.2% at 500 frames to a window, which is the same order as the real
+stall rates it sat beside. Current builds emit none, but the guard costs
+nothing and keeps the command honest against older journals.
+
+A scroller whose p95 sits several times its median is the one to fix, and it is
+usually the one doing the most per-frame work rather than the one configured
+worst. Measured over 20 minutes with two scrollers set identically at 100 px/s,
+the leaderboard held 10 ms flat while the odds ticker spent ~20% of its frames
+on duplicates. Same settings, different render cost: odds does more per-frame
+work, and more variably, so it is first to land a frame that advances less than
+a whole pixel. Check the render path before the config.
 
 Then confirm what the plugin actually loaded — config edits do not always reach
 the running code:

docs/SKIN_SYSTEM.md

@@ -1,5 +1,35 @@
 # Skin System Architecture
 
+## Status: not supported yet
+
+**Skins don't render with the current scoreboard plugins.** The skin system
+below works in isolation (it loads, validates and renders skins in
+`scripts/validate_skin.py` and `test/test_skin_system.py`), but nothing on a
+running display calls it:
+
+- The only render hook is `SportsCore._render_game()` in
+  `src/base_classes/sports/core.py`.
+- None of the current scoreboard plugins build on `src.base_classes`. The
+  official scoreboards in the `ledmatrix-plugins` monorepo, and the
+  third-party scoreboards in the plugin registry, carry their own sports and
+  rendering code (with the shared `src/common/sports_*` helpers) and never
+  reach `SportsCore._render_game()`.
+
+So a skin can be dropped into `skins/` and named in a plugin's config, but the
+scoreboard keeps drawing its built-in layout. Until a scoreboard adopts the
+hook, core does not offer skins to users:
+
+- The plugin config page shows no **Visual Skin** dropdown.
+- The Plugin Store hides registry entries with `"type": "skin"` and refuses
+  to install one (`POST /api/v3/plugins/install` answers 400 with the reason).
+- `GET /api/v3/skins` still lists what is in `skins/`, with
+  `"supported": false` and a `message`.
+- A config that already contains `"skin"` / `"skin_options"` still loads,
+  validates and saves unchanged; the value is simply unused.
+
+The rest of this document describes the design as built, for whoever wires a
+scoreboard to it.
+
 Skins are user-installable **visual overlays** for the sports scoreboards.
 A skin replaces only the *look* of a scoreboard — the host plugin keeps doing
 data fetching, scheduling, caching, dedup, live-priority takeover, and vegas
@@ -32,8 +62,10 @@ crashing) simply restores the built-in look.
 
 ## The render funnel
 
-Every sports scoreboard (baseball, football, basketball, hockey — anything
+A sports scoreboard built on the `src/base_classes/sports/` package
-built on the `src/base_classes/sports/` package, `core.py`) renders through exactly one seam:
+(`core.py`) renders through exactly one seam. No current scoreboard plugin is
+built on it (see [Status](#status-not-supported-yet)), so for them this seam is
+never reached:
 `SportsCore._render_game(game, force_clear)`.
 
 1. The mode class's `display()` (live, `SportsUpcoming`, `SportsRecent`)
@@ -135,22 +167,26 @@ Inside the plugin's own config section in `config/config.json`:
 `"built-in"` means the stock renderer. Because this rides the plugin's config
 section, it persists across plugin reinstalls like every other setting.
 
-The web UI shows a **Visual Skin** dropdown for plugins that have matching
+`SchemaManager.inject_skin_selector` can add a **Visual Skin** enum to the
-skins installed: `SchemaManager.inject_skin_selector` adds an enum to the
+*served* schema for plugins with matching skins installed. While skins are
-*served* schema only. Validation never sees the enum — so a config that
+unsupported the plugin schema endpoint does not call it, so the dropdown is
-references an uninstalled skin stays valid (rendering just falls back), and
+not shown. Validation never sees the enum either way: the base schema allows
-the currently-configured value is always kept selectable. `GET /api/v3/skins`
+any `skin` value, so a config that references an uninstalled skin stays valid.
-lists installed skins (optionally filtered by `?plugin_id=`).
+`GET /api/v3/skins` lists installed skins (optionally filtered by
+`?plugin_id=`) and reports `"supported": false`.
 
 ## Distribution
 
 - **Manual:** `git clone <skin repo> skins/<skin-id>` — that's the whole
   install. No manifest bumps, no `update_registry.py`; skins are not monorepo
   plugins.
-- **Store:** registry entries with `"type": "skin"` install through the same
+- **Store (disabled while unsupported):** registry entries with
-  `plugins.json` pipeline; `PluginStoreManager` routes them to `skins/`,
+  `"type": "skin"` are hidden from the store list and refused on install.
-  validates `skin.json` (including the API major version) instead of
+  `PluginStoreManager._install_skin_from_info` is kept: once
-  `manifest.json`, and never installs dependencies — skins are render-only
+  `SKINS_RENDER_SUPPORTED` in `src/skin_system/__init__.py` is true, such
+  entries install through the same `plugins.json` pipeline, land in `skins/`,
+  are validated against `skin.json` (including the API major version) instead
+  of `manifest.json`, and never install dependencies — skins are render-only
   (stdlib + PIL + the provided context, no third-party packages in v1).
 
 ## Trust model

first_time_install.sh

@@ -125,6 +125,72 @@ fi
 # Get the home directory of the actual user
 USER_HOME=$(eval echo ~$ACTUAL_USER)
 
+# --- rpi-rgb-led-matrix checkout helpers -------------------------------------
+# Run git as whoever owns the project directory. Run as root against a
+# user-owned repo, git refuses it ("dubious ownership"), and anything it does
+# create — such as .git/modules/<submodule> — ends up root-owned, locking the
+# user out of their own checkout. A root-owned install keeps running as root.
+_rgb_repo_owner() {
+    stat -c %U "$PROJECT_ROOT_DIR" 2>/dev/null || echo root
+}
+
+_git_as_repo_owner() {
+    local owner
+    owner=$(_rgb_repo_owner)
+    if [ "$(id -u)" = "0" ] && [ "$owner" != "root" ] && command -v sudo >/dev/null 2>&1; then
+        sudo -u "$owner" -H git "$@"
+    else
+        git "$@"
+    fi
+}
+
+# Earlier installer versions ran the submodule git commands as root, leaving
+# root-owned files the repo owner (and so _git_as_repo_owner) cannot write.
+_reclaim_rgb_checkout() {
+    local owner path
+    owner=$(_rgb_repo_owner)
+    if [ "$(id -u)" != "0" ] || [ "$owner" = "root" ]; then
+        return 0
+    fi
+    for path in "$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master" "$PROJECT_ROOT_DIR/.git/modules/rpi-rgb-led-matrix-master"; do
+        if [ -e "$path" ]; then
+            chown -R "$owner:" "$path" 2>/dev/null || true
+        fi
+    done
+}
+
+# `git pull` on the main repo never moves an existing submodule checkout, so a
+# submodule bump (e.g. the ARMv6 build fix for Pi Zero/1) would never reach a
+# device installed before it. Move the checkout forward to the pinned commit —
+# but never backward or sideways: a user who ran `git submodule update --remote`
+# is newer than the pin and is left alone. Never fatal.
+_sync_rgb_submodule() {
+    local sub="$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master" pinned current
+    if [ ! -f "$PROJECT_ROOT_DIR/.gitmodules" ] || ! grep -q "rpi-rgb-led-matrix" "$PROJECT_ROOT_DIR/.gitmodules" \
+        || [ ! -e "$sub/.git" ]; then
+        return 0
+    fi
+    if ! pinned=$(_git_as_repo_owner -C "$PROJECT_ROOT_DIR" rev-parse "HEAD:rpi-rgb-led-matrix-master" 2>/dev/null) \
+        || [ -z "$pinned" ]; then
+        return 0
+    fi
+    current=$(_git_as_repo_owner -C "$sub" rev-parse HEAD 2>/dev/null) || current=""
+    if [ "$current" = "$pinned" ]; then
+        return 0
+    fi
+    if [ -n "$current" ] && _git_as_repo_owner -C "$sub" cat-file -e "${pinned}^{commit}" 2>/dev/null \
+        && ! _git_as_repo_owner -C "$sub" merge-base --is-ancestor "$current" "$pinned" 2>/dev/null; then
+        echo "rpi-rgb-led-matrix-master is at ${current:0:7}, not behind the pinned ${pinned:0:7}; leaving it as is"
+        return 0
+    fi
+    echo "Updating rpi-rgb-led-matrix-master to the pinned commit ${pinned:0:7}..."
+    if ! _git_as_repo_owner -C "$PROJECT_ROOT_DIR" submodule update --init --recursive rpi-rgb-led-matrix-master; then
+        echo "⚠ Could not update rpi-rgb-led-matrix-master to the pinned commit; building the existing checkout"
+    fi
+    return 0
+}
+# --- end rpi-rgb-led-matrix checkout helpers ---------------------------------
+
 # Determine the Project Root Directory (where this script is located)
 PROJECT_ROOT_DIR=$(cd "$(dirname "$0")" && pwd)
 
@@ -1038,8 +1104,9 @@ else
     # so git clone doesn't fail with "destination path already exists".
     _clone_rpi_rgb() {
         rm -rf "$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master"
-        git clone https://github.com/hzeller/rpi-rgb-led-matrix.git rpi-rgb-led-matrix-master
+        _git_as_repo_owner clone https://github.com/hzeller/rpi-rgb-led-matrix.git rpi-rgb-led-matrix-master
     }
+    _reclaim_rgb_checkout
     if [ ! -d "$PROJECT_ROOT_DIR/rpi-rgb-led-matrix-master" ]; then
         echo "rpi-rgb-led-matrix-master not found. Initializing git submodule..."
         cd "$PROJECT_ROOT_DIR"
@@ -1047,7 +1114,7 @@ else
         # Try to initialize submodule if .gitmodules exists
         if [ -f "$PROJECT_ROOT_DIR/.gitmodules" ] && grep -q "rpi-rgb-led-matrix" "$PROJECT_ROOT_DIR/.gitmodules"; then
             echo "Initializing rpi-rgb-led-matrix submodule..."
-            if ! retry git submodule update --init --recursive rpi-rgb-led-matrix-master; then
+            if ! retry _git_as_repo_owner submodule update --init --recursive rpi-rgb-led-matrix-master; then
                 echo "⚠ Submodule init failed, cloning directly from GitHub..."
                 retry _clone_rpi_rgb
             fi
@@ -1066,12 +1133,14 @@ else
             cd "$PROJECT_ROOT_DIR"
             rm -rf rpi-rgb-led-matrix-master
             if [ -f "$PROJECT_ROOT_DIR/.gitmodules" ] && grep -q "rpi-rgb-led-matrix" "$PROJECT_ROOT_DIR/.gitmodules"; then
-                retry git submodule update --init --recursive rpi-rgb-led-matrix-master
+                retry _git_as_repo_owner submodule update --init --recursive rpi-rgb-led-matrix-master
             else
                 retry _clone_rpi_rgb
             fi
         fi
 
+        _sync_rgb_submodule
+
         # Add temporary swap on low-memory devices so the compiler survives.
         CURRENT_STEP="Prepare the low-memory build environment"
         if [ "$LOWMEM_AVAILABLE" = "1" ] && [ "$SKIP_SWAP" != "1" ]; then
@@ -1416,6 +1485,9 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_ROOT_DIR/display_controll
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/start_display.sh
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/stop_display.sh
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_plugin_rm.sh *
+# Install a requirements.txt as root via vetted helper, so packages are visible
+# to root-run ledmatrix.service (not just the web interface's own user).
+$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_pip_install.sh *
 EOF
 if [ -n "$JOURNALCTL_PATH" ]; then
     cat >> /tmp/ledmatrix_web_sudoers << EOF
@@ -1623,6 +1695,19 @@ chmod 755 "$PROJECT_ROOT_DIR/scripts/install/install_service.sh" "$PROJECT_ROOT_
 # Re-apply special permissions for config directory (lost during normalization)
 chmod 2775 "$PROJECT_ROOT_DIR/config" || true
 
+# Harden the sudo-granted helper scripts: root-owned, not writable by the web
+# user (matches scripts/install/configure_web_sudo.sh). The sudoers rules in
+# Step 10 run these as root, so a user-owned copy is a root shell for whoever
+# can edit it. This must come after Step 11's project-wide chown to
+# $ACTUAL_USER, which would otherwise hand them straight back.
+for helper in safe_plugin_rm.sh safe_pip_install.sh; do
+    HELPER_PATH="$PROJECT_ROOT_DIR/scripts/fix_perms/$helper"
+    if [ -f "$HELPER_PATH" ]; then
+        chown root:root "$HELPER_PATH" || echo "⚠ Could not set ownership on $HELPER_PATH"
+        chmod 755 "$HELPER_PATH" || echo "⚠ Could not set permissions on $HELPER_PATH"
+    fi
+done
+
 echo "✓ Project file permissions normalized"
 echo ""
 

requirements.txt

@@ -43,10 +43,14 @@ packaging>=23.0,<27.0
 # full feature set, or skip them for a minimal install.
 # ───────────────────────────────────────────────────────────────────────
 #
-# scipy             — sub-pixel interpolation in
+# scipy             — nothing, as of #570. It was listed for the sub-pixel
-#                     src/common/scroll_helper.py for smoother
+#                     interpolation path in src/common/scroll_helper.py, but
-#                     scrolling. Falls back to a simpler shift algorithm.
+#                     get_visible_portion never consulted HAS_SCIPY, so that
-#   pip install 'scipy>=1.10.0,<2.0.0'
+#                     path was dead before it was deleted. The blend that
+#                     replaced it is numpy-only. Do not install it expecting
+#                     smoother scrolling: sub-pixel blending is off by default
+#                     because it reads worse on a coarse panel, not because it
+#                     is missing a library. See docs/SCROLL_PERFORMANCE.md.
 #
 # psutil            — per-plugin resource monitoring in
 #                     src/plugin_system/resource_monitor.py. The monitor

scripts/dev/vegas_audit.py

@@ -72,12 +72,8 @@ def load_main_config(path: Path) -> Dict[str, Any]:
 
 def display_size_from_config(config: Dict[str, Any]) -> tuple:
     """Derive the logical ticker size the way DisplayManager does."""
-    hw = config.get('display', {}).get('hardware', {})
+    from src.display_geometry import logical_size
-    cols = int(hw.get('cols', 64))
+    return logical_size(config)
-    chain = int(hw.get('chain_length', 1))
-    rows = int(hw.get('rows', 32))
-    parallel = int(hw.get('parallel', 1))
-    return cols * chain, rows * parallel
 
 
 def enabled_plugin_ids(config: Dict[str, Any]) -> List[str]:

scripts/scroll_speeds.py

@@ -59,10 +59,13 @@ def build_options(hardware, refresh_override=None):
     from rgbmatrix import RGBMatrixOptions
 
     o = RGBMatrixOptions()
-    o.rows = int(hardware.get("rows", 32))
+    from src.display_geometry import (
-    o.cols = int(hardware.get("cols", 64))
+        DEFAULT_CHAIN_LENGTH, DEFAULT_COLS, DEFAULT_PARALLEL, DEFAULT_ROWS,
-    o.chain_length = int(hardware.get("chain_length", 1))
+    )
-    o.parallel = int(hardware.get("parallel", 1))
+    o.rows = int(hardware.get("rows", DEFAULT_ROWS))
+    o.cols = int(hardware.get("cols", DEFAULT_COLS))
+    o.chain_length = int(hardware.get("chain_length", DEFAULT_CHAIN_LENGTH))
+    o.parallel = int(hardware.get("parallel", DEFAULT_PARALLEL))
     o.brightness = int(hardware.get("brightness", 80))
     o.hardware_mapping = hardware.get("hardware_mapping", "regular")
     o.pwm_bits = int(hardware.get("pwm_bits", 11))

skins/README.md

@@ -1,5 +1,10 @@
 # skins/
 
+> **Not supported yet.** The current scoreboard plugins don't render skins,
+> so a skin placed here and selected in config has no effect, and the web UI
+> and Plugin Store don't offer them. See
+> [docs/SKIN_SYSTEM.md](../docs/SKIN_SYSTEM.md#status-not-supported-yet).
+
 User-installable **visual skins** for the sports scoreboards. Each
 subdirectory is one skin:
 
@@ -10,10 +15,10 @@ skins/<skin-id>/
   preview.png   # optional
 ```
 
-- Install a skin: `git clone <skin repo> skins/<skin-id>` (or via the Plugin
+- Install a skin: `git clone <skin repo> skins/<skin-id>`. The Plugin Store
-  Store for registry entries with `"type": "skin"`).
+  refuses registry entries with `"type": "skin"` while skins don't render.
 - Select it: set `"skin": "<skin-id>"` in the plugin's section of
-  `config/config.json`, or use the web UI's Visual Skin dropdown.
+  `config/config.json`. The web UI no longer shows a Visual Skin dropdown.
 - Build one: start from `example-classic-baseball/` and read
   [docs/CREATING_SKINS.md](../docs/CREATING_SKINS.md). Validate with
   `python scripts/validate_skin.py --skin <skin-id>`.

src/__init__.py

@@ -4,5 +4,5 @@ LEDMatrix Display System
 Core source package for the LED Matrix Display project.
 """
 
-__version__ = "3.3.0"
+__version__ = "3.4.0"
 

src/common/scroll_helper.py

@@ -30,6 +30,12 @@ except ImportError:
     HAS_SCIPY = False
 
 
+# How often the frame-stats line is emitted, and therefore also the ceiling
+# on a believable frame time: a scroll that renders at all cannot take this
+# long over one frame, so a sample this large is an idle gap between scrolls.
+FPS_LOG_INTERVAL = 5.0
+
+
 def frame_stats(frame_times: list) -> Dict[str, Any]:
     """Summary statistics over one window of frame durations (seconds).
 
@@ -158,9 +164,11 @@ class ScrollHelper:
         self.last_progress_log_time: Optional[float] = None
         self.progress_log_interval = 5.0  # seconds
         
-        # Frame rate tracking
+        # Frame rate tracking. last_frame_time is None until the first frame
+        # of a scroll is rendered -- see log_frame_rate() for why timing from
+        # construction (or from the end of the previous scroll) is wrong.
         self.frame_count = 0
-        self.last_frame_time = time.time()
+        self.last_frame_time: Optional[float] = None
         self.last_fps_log_time = time.time()
         self.frame_times = []
         # Every frame time since the last stats line, so the 5s summary can
@@ -764,6 +772,10 @@ class ScrollHelper:
         # Reset last_update_time to prevent large delta_time on next update
         # This ensures smooth scrolling after reset without jumping ahead
         self.last_update_time = now
+        # Same reasoning for the frame-rate clock: the first frame after a
+        # reset has no predecessor in this scroll, and timing it against the
+        # last frame of the previous one measures the idle gap between them.
+        self.last_frame_time = None
         self.logger.debug("Scroll position reset")
 
     def reset(self) -> None:
@@ -962,11 +974,37 @@ class ScrollHelper:
         Log frame rate statistics for performance monitoring.
         """
         current_time = time.time()
-        
+
+        # The first frame of a scroll has no predecessor, so it has no frame
+        # time. Measuring one anyway records the whole idle gap since the last
+        # scroll as a single frame: on hardware that produced windows reading
+        # "0.0 fps over 1 frames | median 136776.02ms", and -- worse, because
+        # it is not obviously wrong -- put that gap in the max field of
+        # otherwise healthy windows and counted it as one stall per scroll.
+        # At ~500 frames to a window that is ~0.2%, which is the same order as
+        # the real stall rates being measured, so the number could not be
+        # trusted at all. Seed the clock and take no sample.
+        if self.last_frame_time is None:
+            self.last_frame_time = current_time
+            # Restart the window with the scroll. Otherwise the boundary is
+            # already long overdue when the second frame arrives, and the new
+            # scroll opens by reporting a window of exactly one frame.
+            self.last_fps_log_time = current_time
+            return
+
         # Calculate instantaneous frame time
         frame_time = current_time - self.last_frame_time
+
+        # A caller that scrolls without ever calling reset_scroll() never arms
+        # the sentinel above, so catch the same gap by its size. Nothing that
+        # renders a scroll produces a frame longer than the log interval; a
+        # sample that large is an idle period, not a frame.
+        if frame_time >= FPS_LOG_INTERVAL:
+            self.last_frame_time = current_time
+            return
+
         self.frame_times.append(frame_time)
-        
+
         # Keep only last 100 frames for average
         if len(self.frame_times) > 100:
             self.frame_times.pop(0)
@@ -979,17 +1017,21 @@ class ScrollHelper:
         # duplicate and a 21ms double-wait mean exactly 10ms). Chasing scroll
         # judder needs the tail, so keep the window and report percentiles.
         self._window.append(frame_time)
-        
+
         # Log FPS every 5 seconds to avoid spam
-        if current_time - self.last_fps_log_time >= 5.0:
+        if current_time - self.last_fps_log_time >= FPS_LOG_INTERVAL:
-            self.logger.info(
+            # An empty window means every sample in this interval was dropped
-                "Scroll frame stats - %s",
+            # as an idle gap. There is nothing to report, and reporting the
-                format_frame_stats(self._window or [frame_time]),
+            # gap itself is the bug above.
-            )
+            if self._window:
+                self.logger.info(
+                    "Scroll frame stats - %s",
+                    format_frame_stats(self._window),
+                )
             self.last_fps_log_time = current_time
             self.frame_count = 0
             self._window = []
-        
+
         self.last_frame_time = current_time
         self.frame_count += 1
     

src/common/sync_manager.py

@@ -32,6 +32,8 @@ from typing import Callable, Optional
 import numpy as np
 from PIL import Image
 
+from src.display_geometry import DEFAULT_CHAIN_LENGTH
+
 # Raw-frame wire format: 8-byte magic + 4-byte header + raw RGB pixels
 # Much faster than PNG: no encode/decode, negligible CPU, same UDP packet size
 _RAW_MAGIC = b'SYNC_RAW'
@@ -194,7 +196,7 @@ class DisplaySyncManager:
         local_cols = hw.get("cols", 64)
         peer_rows = int(msg.get("rows", 0))
         peer_cols = int(msg.get("cols", 0))
-        peer_chain = int(msg.get("chain", 1))
+        peer_chain = int(msg.get("chain", DEFAULT_CHAIN_LENGTH))
 
         compatible = peer_rows == local_rows and peer_cols == local_cols
 
@@ -589,7 +591,7 @@ class DisplaySyncManager:
             "t": "hello",
             "rows": hw.get("rows", 32),
             "cols": hw.get("cols", 64),
-            "chain": hw.get("chain_length", 1),
+            "chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH),
         }).encode("utf-8")
         heartbeat = json.dumps({"t": "hb"}).encode("utf-8")
         dest = ("<broadcast>", self.port)
@@ -660,7 +662,7 @@ class DisplaySyncManager:
             "port": self.port,
             "local_rows": hw.get("rows", 32),
             "local_cols": hw.get("cols", 64),
-            "local_chain": hw.get("chain_length", 1),
+            "local_chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH),
         }
 
         if self.role == SyncRole.STANDALONE:

src/display_geometry.py

@@ -0,0 +1,144 @@
+"""Display size from config: the one computation every caller shares.
+
+``DisplayManager`` sizes its canvas from ``display.hardware`` plus
+``display.double_sided``. The web preview, the Starlark magnify default and
+the multi-display sync handshake used to re-derive that size themselves,
+each with its own defaults (``chain_length`` fell back to 2 in one place and
+1 in three others) and none of them applying double-sided mode. They now all
+call this module.
+
+Kept free of hardware imports on purpose: the web interface imports it, and
+``display_manager`` pulls in ``rgbmatrix``.
+"""
+
+import logging
+from typing import Any, Dict, Mapping, Optional, Tuple
+
+logger = logging.getLogger(__name__)
+
+# Match config/config.template.json's display.hardware block.
+DEFAULT_ROWS = 32
+DEFAULT_COLS = 64
+DEFAULT_CHAIN_LENGTH = 2
+DEFAULT_PARALLEL = 1
+
+
+def _display(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]:
+    # A hand-edited config.json can hold anything here; treat a non-mapping
+    # like a missing block so callers get the defaults, not AttributeError.
+    display = (config or {}).get('display')
+    return display if isinstance(display, Mapping) else {}
+
+
+def _hardware(config: Optional[Mapping[str, Any]]) -> Mapping[str, Any]:
+    hw = _display(config).get('hardware')
+    return hw if isinstance(hw, Mapping) else {}
+
+
+def physical_size(config: Optional[Mapping[str, Any]]) -> Tuple[int, int]:
+    """Width and height of the whole panel chain, in pixels.
+
+    ``cols * chain_length`` by ``rows * parallel``. Raises ``ValueError`` or
+    ``TypeError`` on a non-numeric value, as ``DisplayManager`` does; callers
+    decide their own fallback.
+
+    A non-finite value (``Infinity``, which Python's JSON parser accepts in a
+    hand-edited config.json) raises ``ValueError`` too, not ``OverflowError``,
+    so every caller's existing fallback catches it.
+    """
+    hw = _hardware(config)
+    try:
+        rows = int(hw.get('rows', DEFAULT_ROWS))
+        cols = int(hw.get('cols', DEFAULT_COLS))
+        chain_length = int(hw.get('chain_length', DEFAULT_CHAIN_LENGTH))
+        parallel = int(hw.get('parallel', DEFAULT_PARALLEL))
+    except OverflowError as e:
+        raise ValueError(f"display.hardware size is not finite: {e}") from e
+    return max(1, cols * chain_length), max(1, rows * parallel)
+
+
+def resolve_double_sided(physical_width: int, physical_height: int,
+                         ds_config: Dict[str, Any],
+                         quiet: bool = False) -> Optional[Dict[str, Any]]:
+    """Validate the ``display.double_sided`` config against the physical size.
+
+    Returns a dict ``{copies, axis, logical_width, logical_height}`` when the
+    feature is enabled and the physical panel divides evenly into ``copies``
+    along the chosen axis, otherwise ``None`` (single-screen behaviour). Bad
+    config is logged and disabled rather than raised — a misconfigured panel
+    should still light up.
+
+    Only pixels are checked, not whole panels: ``chain_length`` and
+    ``parallel`` don't say which axis a panel lies on once an orientation
+    ``Rotate:`` or U-mapper ``pixel_mapper_config`` rearranges the chain.
+
+    ``quiet`` suppresses the log lines, for callers that run on every web
+    request and would otherwise repeat them on each poll.
+    """
+    def _log(level, *args):
+        if not quiet:
+            logger.log(level, *args)
+
+    if not isinstance(ds_config, dict) or not ds_config.get('enabled', False):
+        return None
+
+    copies = ds_config.get('copies', 2)
+    if not isinstance(copies, int) or copies < 2:
+        _log(logging.WARNING,
+             "double_sided: 'copies' must be an integer >= 2 (got %r); "
+             "disabling double-sided mode", copies)
+        return None
+
+    axis = ds_config.get('axis', 'horizontal')
+    if axis not in ('horizontal', 'vertical'):
+        _log(logging.WARNING,
+             "double_sided: 'axis' must be 'horizontal' or 'vertical' "
+             "(got %r); defaulting to 'horizontal'", axis)
+        axis = 'horizontal'
+
+    # Horizontal splits the chain (panels side by side); vertical splits the
+    # parallel outputs (panels stacked). The split axis must divide evenly.
+    if axis == 'horizontal':
+        if physical_width % copies != 0:
+            _log(logging.WARNING,
+                 "double_sided: physical width %d is not divisible by copies "
+                 "%d; disabling double-sided mode", physical_width, copies)
+            return None
+        logical_width = physical_width // copies
+        logical_height = physical_height
+    else:
+        if physical_height % copies != 0:
+            _log(logging.WARNING,
+                 "double_sided: physical height %d is not divisible by copies "
+                 "%d; disabling double-sided mode", physical_height, copies)
+            return None
+        logical_width = physical_width
+        logical_height = physical_height // copies
+
+    _log(logging.INFO,
+         "double_sided enabled: %d copies on %s axis — logical screen %dx%d "
+         "tiled across physical %dx%d", copies, axis, logical_width,
+         logical_height, physical_width, physical_height)
+    return {
+        'copies': copies,
+        'axis': axis,
+        'logical_width': logical_width,
+        'logical_height': logical_height,
+    }
+
+
+def logical_size(config: Optional[Mapping[str, Any]],
+                 quiet: bool = True) -> Tuple[int, int]:
+    """The size plugins draw at and the web preview shows.
+
+    The physical size, divided by ``double_sided.copies`` along its axis when
+    double-sided mode is enabled and valid — the same answer
+    ``DisplayManager.width``/``height`` give.
+    """
+    width, height = physical_size(config)
+    ds = resolve_double_sided(width, height,
+                              _display(config).get('double_sided') or {},
+                              quiet=quiet)
+    if ds is not None:
+        return ds['logical_width'], ds['logical_height']
+    return width, height

src/display_manager.py

@@ -35,6 +35,10 @@ from contextlib import contextmanager
 from pathlib import Path
 from PIL import Image, ImageDraw, ImageFont
 from src.common.font_layout import crisp_size, load_truetype, resolve_asset_path
+from src.display_geometry import (
+    DEFAULT_CHAIN_LENGTH, DEFAULT_COLS, DEFAULT_PARALLEL, DEFAULT_ROWS,
+    physical_size, resolve_double_sided,
+)
 import threading
 import time
 from collections import OrderedDict
@@ -123,62 +127,10 @@ class _LogicalMatrix:
         setattr(object.__getattribute__(self, "_matrix"), name, value)
 
 
-def _resolve_double_sided(physical_width: int, physical_height: int,
+# Moved to src/display_geometry.py so the web preview, Starlark magnify and
-                          ds_config: Dict[str, Any]) -> Optional[Dict[str, Any]]:
+# sync handshake compute the display size exactly as DisplayManager does
-    """Validate the ``display.double_sided`` config against the physical size.
+# without importing rgbmatrix. Aliased here for existing callers.
-
+_resolve_double_sided = resolve_double_sided
-    Returns a dict ``{copies, axis, logical_width, logical_height}`` when the
-    feature is enabled and the physical panel divides evenly into ``copies``
-    along the chosen axis, otherwise ``None`` (single-screen behaviour). Bad
-    config is logged and disabled rather than raised — a misconfigured panel
-    should still light up.
-    """
-    if not isinstance(ds_config, dict) or not ds_config.get('enabled', False):
-        return None
-
-    copies = ds_config.get('copies', 2)
-    if not isinstance(copies, int) or copies < 2:
-        logger.warning(
-            "double_sided: 'copies' must be an integer >= 2 (got %r); "
-            "disabling double-sided mode", copies)
-        return None
-
-    axis = ds_config.get('axis', 'horizontal')
-    if axis not in ('horizontal', 'vertical'):
-        logger.warning(
-            "double_sided: 'axis' must be 'horizontal' or 'vertical' "
-            "(got %r); defaulting to 'horizontal'", axis)
-        axis = 'horizontal'
-
-    # Horizontal splits the chain (panels side by side); vertical splits the
-    # parallel outputs (panels stacked). The split axis must divide evenly.
-    if axis == 'horizontal':
-        if physical_width % copies != 0:
-            logger.warning(
-                "double_sided: physical width %d is not divisible by copies "
-                "%d; disabling double-sided mode", physical_width, copies)
-            return None
-        logical_width = physical_width // copies
-        logical_height = physical_height
-    else:
-        if physical_height % copies != 0:
-            logger.warning(
-                "double_sided: physical height %d is not divisible by copies "
-                "%d; disabling double-sided mode", physical_height, copies)
-            return None
-        logical_width = physical_width
-        logical_height = physical_height // copies
-
-    logger.info(
-        "double_sided enabled: %d copies on %s axis — logical screen %dx%d "
-        "tiled across physical %dx%d", copies, axis, logical_width,
-        logical_height, physical_width, physical_height)
-    return {
-        'copies': copies,
-        'axis': axis,
-        'logical_width': logical_width,
-        'logical_height': logical_height,
-    }
 
 
 class DisplayManager:
@@ -327,10 +279,10 @@ class DisplayManager:
             runtime_config = self.config.get('display', {}).get('runtime', {})
             
             # Basic hardware settings
-            options.rows = hardware_config.get('rows', 32)
+            options.rows = hardware_config.get('rows', DEFAULT_ROWS)
-            options.cols = hardware_config.get('cols', 64)
+            options.cols = hardware_config.get('cols', DEFAULT_COLS)
-            options.chain_length = hardware_config.get('chain_length', 2)
+            options.chain_length = hardware_config.get('chain_length', DEFAULT_CHAIN_LENGTH)
-            options.parallel = hardware_config.get('parallel', 1)
+            options.parallel = hardware_config.get('parallel', DEFAULT_PARALLEL)
             options.hardware_mapping = hardware_config.get('hardware_mapping', 'adafruit-hat-pwm')
             
             # Performance and stability settings
@@ -421,13 +373,7 @@ class DisplayManager:
             # Create a fallback image for web preview using configured dimensions when available
             self.matrix = None
             try:
-                hardware_config = self.config.get('display', {}).get('hardware', {}) if self.config else {}
+                fallback_width, fallback_height = physical_size(self.config)
-                rows = int(hardware_config.get('rows', 32))
-                cols = int(hardware_config.get('cols', 64))
-                chain_length = int(hardware_config.get('chain_length', 2))
-                parallel = int(hardware_config.get('parallel', 1))
-                fallback_width = max(1, cols * chain_length)
-                fallback_height = max(1, rows * parallel)
                 # Mirror double-sided in fallback so the preview shows one screen.
                 ds_config = self.config.get('display', {}).get('double_sided', {}) if self.config else {}
                 ds = _resolve_double_sided(fallback_width, fallback_height, ds_config)

src/plugin_system/store_manager.py

@@ -1338,9 +1338,16 @@ class PluginStoreManager:
             self.logger.error(f"Plugin not found in registry: {plugin_id}")
             return False
 
-        # Visual skins share the registry but install to skins/, not to a
+        # Visual skins share the registry. _install_skin_from_info can put one
-        # plugin directory (docs/SKIN_SYSTEM.md)
+        # in skins/, but no current scoreboard plugin renders skins, so the
+        # store refuses them rather than installing something that does
+        # nothing (docs/SKIN_SYSTEM.md). Manual installs under skins/ and
+        # uninstall_skin are unaffected.
         if (plugin_info.get('type') or 'plugin') == 'skin':
+            from src.skin_system import SKINS_RENDER_SUPPORTED, SKINS_UNSUPPORTED_MESSAGE
+            if not SKINS_RENDER_SUPPORTED:
+                self.logger.error(f"Not installing skin {plugin_id}: {SKINS_UNSUPPORTED_MESSAGE}")
+                return False
             return self._install_skin_from_info(plugin_id, plugin_info, branch)
 
         repo_url = plugin_info.get('repo')

src/skin_system/__init__.py

@@ -6,7 +6,19 @@ upcoming) while the host plugin keeps doing data fetching, scheduling,
 caching, live priority, and vegas mode. See docs/SKIN_SYSTEM.md.
 """
 
-from src.skin_system.skin_base import (
+# Skins are not offered to users yet. The only render hook is
+# SportsCore._render_game in src/base_classes/sports/core.py, and none of the
+# current scoreboard plugins (monorepo or third-party) build on
+# src.base_classes, so a selected skin never draws. The web UI and store
+# read these instead of offering install/selection; stored "skin" config
+# values still load and save. See docs/SKIN_SYSTEM.md.
+SKINS_RENDER_SUPPORTED = False
+SKINS_UNSUPPORTED_MESSAGE = (
+    "Skins aren't supported yet: the current scoreboard plugins don't render "
+    "them. Installed skins and saved skin settings are kept but have no effect."
+)
+
+from src.skin_system.skin_base import (  # noqa: E402
     SKIN_API_VERSION,
     VIEW_MODEL_VERSION,
     ScoreboardSkin,

test/test_display_geometry.py

@@ -0,0 +1,144 @@
+"""The web preview must be the size DisplayManager actually renders at.
+
+Before src/display_geometry.py, the preview endpoints computed
+``cols * chain_length`` by ``rows * parallel`` themselves, ignored
+``display.double_sided`` (so a double-sided panel previewed two screens side
+by side), and the ``chain_length`` fallback was 2 in DisplayManager but 1 in
+the Starlark magnify default and the sync handshake.
+"""
+
+import json
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+from flask import Flask
+
+from src.display_geometry import (
+    DEFAULT_CHAIN_LENGTH, DEFAULT_COLS, DEFAULT_PARALLEL, DEFAULT_ROWS,
+    logical_size, physical_size,
+)
+
+REPO_ROOT = Path(__file__).resolve().parents[1]
+
+
+def _config(hardware=None, double_sided=None):
+    display = {'hardware': hardware or {}}
+    if double_sided is not None:
+        display['double_sided'] = double_sided
+    return {'display': display}
+
+
+def test_defaults_match_the_config_template():
+    template = json.loads(
+        (REPO_ROOT / 'config' / 'config.template.json').read_text(encoding='utf-8'))
+    hw = template['display']['hardware']
+    assert (DEFAULT_ROWS, DEFAULT_COLS, DEFAULT_CHAIN_LENGTH, DEFAULT_PARALLEL) == (
+        hw['rows'], hw['cols'], hw['chain_length'], hw['parallel'])
+
+
+def test_missing_hardware_uses_the_template_defaults():
+    assert physical_size({}) == (128, 32)
+    assert logical_size({}) == (128, 32)
+
+
+def test_parallel_multiplies_height():
+    cfg = _config({'rows': 32, 'cols': 64, 'chain_length': 2, 'parallel': 2})
+    assert physical_size(cfg) == (128, 64)
+
+
+def test_double_sided_horizontal_previews_one_screen():
+    cfg = _config({'rows': 32, 'cols': 64, 'chain_length': 4, 'parallel': 1},
+                  {'enabled': True, 'copies': 2, 'axis': 'horizontal'})
+    assert physical_size(cfg) == (256, 32)
+    assert logical_size(cfg) == (128, 32)
+
+
+def test_double_sided_vertical_splits_parallel():
+    cfg = _config({'rows': 32, 'cols': 64, 'chain_length': 2, 'parallel': 2},
+                  {'enabled': True, 'copies': 2, 'axis': 'vertical'})
+    assert logical_size(cfg) == (128, 32)
+
+
+def test_double_sided_that_does_not_divide_falls_back_to_physical():
+    cfg = _config({'rows': 32, 'cols': 64, 'chain_length': 2, 'parallel': 1},
+                  {'enabled': True, 'copies': 3, 'axis': 'horizontal'})
+    assert logical_size(cfg) == (128, 32)
+
+
+def test_disabled_double_sided_is_ignored():
+    cfg = _config({'chain_length': 4}, {'enabled': False, 'copies': 2})
+    assert logical_size(cfg) == (256, 32)
+
+
+@pytest.mark.parametrize('display', ['oops', ['a'], 1, {'hardware': 'oops'},
+                                     {'hardware': ['a']}])
+def test_non_mapping_display_config_uses_the_defaults(display):
+    assert physical_size({'display': display}) == (128, 32)
+    assert logical_size({'display': display}) == (128, 32)
+
+
+@pytest.mark.parametrize('key', ['rows', 'cols', 'chain_length', 'parallel'])
+def test_infinite_hardware_value_raises_value_error(key):
+    # json.loads accepts Infinity; int(inf) raises OverflowError, which the
+    # Starlark magnify default and the preview stream did not catch (HTTP 500).
+    cfg = _config({key: json.loads('Infinity')})
+    with pytest.raises(ValueError):
+        physical_size(cfg)
+    with pytest.raises(ValueError):
+        logical_size(cfg)
+
+
+@pytest.fixture
+def display_client(monkeypatch):
+    from web_interface.blueprints.api_v3 import api_v3
+    config_manager = MagicMock()
+    monkeypatch.setattr(api_v3, 'config_manager', config_manager, raising=False)
+    app = Flask(__name__)
+    app.config['TESTING'] = True
+    app.register_blueprint(api_v3, url_prefix='/api/v3')
+    with app.test_client() as client:
+        yield client, config_manager
+
+
+def test_display_current_reports_the_logical_size(display_client):
+    client, config_manager = display_client
+    config_manager.load_config.return_value = _config(
+        {'rows': 32, 'cols': 64, 'chain_length': 4, 'parallel': 2},
+        {'enabled': True, 'copies': 2, 'axis': 'horizontal'})
+
+    data = client.get('/api/v3/display/current').get_json()['data']
+
+    assert (data['width'], data['height']) == (128, 64)
+
+
+def test_display_current_defaults_chain_length_like_display_manager(display_client):
+    client, config_manager = display_client
+    config_manager.load_config.return_value = _config({'rows': 32, 'cols': 64})
+
+    data = client.get('/api/v3/display/current').get_json()['data']
+
+    assert (data['width'], data['height']) == (64 * DEFAULT_CHAIN_LENGTH, 32)
+
+
+def test_display_current_falls_back_to_the_shared_default(display_client):
+    client, config_manager = display_client
+    config_manager.load_config.side_effect = ValueError('unreadable')
+
+    data = client.get('/api/v3/display/current').get_json()['data']
+
+    assert (data['width'], data['height']) == logical_size({}) == (128, 32)
+
+
+def test_preview_callers_do_not_rederive_the_size():
+    """Every preview/size caller goes through display_geometry, so none of
+    them can drift back to a private chain_length default."""
+    for rel in ('web_interface/app.py',
+                'web_interface/blueprints/api_v3/display.py',
+                'web_interface/blueprints/api_v3/__init__.py',
+                'src/common/sync_manager.py',
+                'src/display_manager.py'):
+        source = (REPO_ROOT / rel).read_text(encoding='utf-8')
+        assert "get('chain_length', 1)" not in source, rel
+        assert 'get("chain_length", 1)' not in source, rel
+        assert 'cols * chain_length' not in source, rel

test/test_install_rgb_checkout.py

@@ -0,0 +1,238 @@
+"""
+Tests for the rpi-rgb-led-matrix checkout helpers in first_time_install.sh, and
+for the contract between scripts/install/one-shot-install.sh and that script.
+
+Background: the submodule was pinned to a commit whose RP1 backend emits the
+ARMv7-only `dmb ishst` instruction, so the build failed on every ARMv6 board
+(Pi Zero / Zero W / Pi 1). Bumping the pin fixes fresh installs, but `git pull`
+never moves an existing submodule checkout, and earlier installers ran the
+submodule git commands as root, leaving .git/modules/<submodule> root-owned and
+the user locked out of their own checkout.
+
+These cover everything that runs without root. The ownership-repair path
+(root, a second user, chown) mutates the system, so -- as with the swap helpers
+in test_install_lowmem.py -- it is exercised manually instead.
+"""
+
+import os
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+INSTALLER = ROOT / "first_time_install.sh"
+ONE_SHOT = ROOT / "scripts" / "install" / "one-shot-install.sh"
+
+BEGIN = "# --- rpi-rgb-led-matrix checkout helpers"
+END = "# --- end rpi-rgb-led-matrix checkout helpers"
+SUB = "rpi-rgb-led-matrix-master"
+
+pytestmark = pytest.mark.skipif(
+    not sys.platform.startswith("linux"), reason="exercises the installer's bash under Linux"
+)
+
+
+def helper_block() -> str:
+    text = INSTALLER.read_text(encoding="utf-8")
+    assert text.count(BEGIN) == 1 and text.count(END) == 1, "helper block markers missing or duplicated"
+    return text[text.index(BEGIN): text.index(END)]
+
+
+def git(*args: str, cwd: Path, env: dict) -> str:
+    result = subprocess.run(["git", *args], cwd=cwd, env=env, capture_output=True, text=True)
+    assert result.returncode == 0, f"git {' '.join(args)} failed: {result.stderr}"
+    return result.stdout.strip()
+
+
+@pytest.fixture
+def git_env(tmp_path):
+    config = tmp_path / "gitconfig"
+    config.write_text(
+        "[user]\n\tname = t\n\temail = t@t\n"
+        "[protocol \"file\"]\n\tallow = always\n"
+        "[init]\n\tdefaultBranch = master\n"
+        "[advice]\n\tdetachedHead = false\n",
+        encoding="utf-8",
+    )
+    return {
+        "PATH": "/usr/bin:/bin:/usr/sbin:/sbin",
+        "HOME": str(tmp_path),
+        "GIT_CONFIG_GLOBAL": str(config),
+        "GIT_CONFIG_NOSYSTEM": "1",
+    }
+
+
+@pytest.fixture
+def upstream(tmp_path, git_env):
+    """A stand-in for hzeller/rpi-rgb-led-matrix with commits A -> B -> C."""
+    up = tmp_path / "upstream"
+    up.mkdir()
+    git("init", "-q", ".", cwd=up, env=git_env)
+    commits = {}
+    for name, filename in (("A", "Makefile"), ("B", "b"), ("C", "c")):
+        (up / filename).write_text(name, encoding="utf-8")
+        git("add", ".", cwd=up, env=git_env)
+        git("commit", "-qm", name, cwd=up, env=git_env)
+        commits[name] = git("rev-parse", "HEAD", cwd=up, env=git_env)
+    return up, commits
+
+
+@pytest.fixture
+def make_project(tmp_path, git_env, upstream):
+    """make_project(checkout) -> project pinned at B, submodule checked out at `checkout`."""
+    up, commits = upstream
+
+    def _make(checkout: str) -> Path:
+        project = tmp_path / f"project-{checkout}"
+        project.mkdir()
+        git("init", "-q", ".", cwd=project, env=git_env)
+        git("submodule", "add", "-q", str(up), SUB, cwd=project, env=git_env)
+        git("checkout", "-q", commits["B"], cwd=project / SUB, env=git_env)
+        git("add", SUB, cwd=project, env=git_env)
+        git("commit", "-qm", "pin", cwd=project, env=git_env)
+        git("checkout", "-q", commits[checkout], cwd=project / SUB, env=git_env)
+        return project
+
+    return _make
+
+
+def run_helpers(snippet: str, project: Path, env: dict) -> subprocess.CompletedProcess:
+    """Run a snippet against the helpers under the installer's own strict mode."""
+    script = (
+        "set -Eeuo pipefail\n"
+        "trap 'echo ERR_TRAP_FIRED >&2; exit 99' ERR\n"
+        f"{helper_block()}\n"
+        f'PROJECT_ROOT_DIR="{project}"\n'
+        f"{snippet}\n"
+    )
+    return subprocess.run(["bash", "-c", script], capture_output=True, text=True, env=env)
+
+
+def head_of(project: Path, env: dict) -> str:
+    return git("rev-parse", "HEAD", cwd=project / SUB, env=env)
+
+
+class TestInstallerStructure:
+    def test_installer_is_syntactically_valid(self):
+        result = subprocess.run(["bash", "-n", str(INSTALLER)], capture_output=True, text=True)
+        assert result.returncode == 0, result.stderr
+
+    def test_helper_block_defines_the_helpers(self):
+        block = helper_block()
+        for name in ("_rgb_repo_owner", "_git_as_repo_owner", "_reclaim_rgb_checkout", "_sync_rgb_submodule"):
+            assert re.search(rf"^{name}\(\) \{{", block, re.M), f"{name} is not defined in the helper block"
+
+    def test_every_called_underscore_function_is_defined_before_use(self):
+        # A renamed helper with a stale call site aborts the installer with
+        # "command not found" under set -e -- the users this protects are the
+        # ones least able to recover from that.
+        lines = INSTALLER.read_text(encoding="utf-8").splitlines()
+        defined_at = {}
+        for number, line in enumerate(lines, 1):
+            match = re.match(r"\s*(_[A-Za-z0-9_]+)\(\) \{", line)
+            if match:
+                defined_at.setdefault(match.group(1), number)
+        # A name in command position; `_name=value` is an assignment and
+        # `_name() {` a definition, so neither counts as a call.
+        call = re.compile(r"(?:^\s*|\bretry\s+|\bif\s+!?\s*|&&\s*|\|\|\s*|\$\()(_[A-Za-z0-9_]+)\b(?!\(\)|=|\[)")
+        for number, line in enumerate(lines, 1):
+            if line.lstrip().startswith("#"):
+                continue
+            for name in call.findall(line):
+                if name.startswith("_") and not re.match(r"_[A-Z0-9_]+$", name):  # skip $_VARS
+                    assert name in defined_at, f"line {number} calls undefined function {name}"
+                    # Functions defined inside another function body run later;
+                    # top-level calls must come after the definition.
+                    if not line.startswith((" ", "\t")):
+                        assert defined_at[name] < number, f"line {number} calls {name} before it is defined"
+
+
+class TestSyncSubmodule:
+    def test_checkout_behind_the_pin_is_moved_forward(self, make_project, upstream, git_env):
+        project = make_project("A")
+        result = run_helpers("_reclaim_rgb_checkout; _sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert head_of(project, git_env) == upstream[1]["B"]
+
+    def test_checkout_newer_than_the_pin_is_left_alone(self, make_project, upstream, git_env):
+        # What `git submodule update --remote` -- advice the installer itself
+        # prints for Pi 5 -- leaves behind. Must never be rolled back.
+        project = make_project("C")
+        result = run_helpers("_sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert head_of(project, git_env) == upstream[1]["C"]
+        assert "leaving it as is" in result.stdout
+
+    def test_checkout_at_the_pin_is_a_silent_no_op(self, make_project, upstream, git_env):
+        project = make_project("B")
+        result = run_helpers("_sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert result.stdout == ""
+        assert head_of(project, git_env) == upstream[1]["B"]
+
+    def test_unreachable_pin_warns_and_keeps_the_checkout(self, make_project, upstream, git_env):
+        project = make_project("A")
+        ghost = "0123456789abcdef0123456789abcdef01234567"
+        git("update-index", "--cacheinfo", f"160000,{ghost},{SUB}", cwd=project, env=git_env)
+        git("commit", "-qm", "ghost pin", cwd=project, env=git_env)
+        result = run_helpers("_sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert "ERR_TRAP_FIRED" not in result.stderr
+        assert "Could not update" in result.stdout
+        assert head_of(project, git_env) == upstream[1]["A"]
+
+    def test_local_files_blocking_the_checkout_are_not_clobbered(self, make_project, git_env):
+        project = make_project("A")
+        (project / SUB / "b").write_text("mine", encoding="utf-8")  # B adds a tracked `b`
+        result = run_helpers("_sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert (project / SUB / "b").read_text(encoding="utf-8") == "mine"
+
+    def test_directory_without_git_metadata_is_a_no_op(self, make_project, git_env):
+        project = make_project("A")
+        subprocess.run(["rm", "-rf", str(project / SUB / ".git"), str(project / ".git" / "modules")], check=True)
+        result = run_helpers("_sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert result.stdout == ""
+
+    def test_project_without_the_submodule_configured_is_a_no_op(self, tmp_path, git_env):
+        project = tmp_path / "plain"
+        (project / SUB).mkdir(parents=True)
+        result = run_helpers("_sync_rgb_submodule", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert result.stdout == ""
+
+
+class TestOwnershipHelpersWithoutRoot:
+    @pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="covers the non-root path")
+    def test_reclaim_is_a_no_op_and_git_runs_directly(self, make_project, git_env):
+        project = make_project("B")
+        result = run_helpers("_reclaim_rgb_checkout; _git_as_repo_owner --version", project, git_env)
+        assert result.returncode == 0, result.stderr
+        assert result.stdout.startswith("git version")
+
+
+class TestOneShotContract:
+    """one-shot-install.sh clones the repo and hands off to first_time_install.sh."""
+
+    def test_one_shot_invokes_the_installer_at_the_repo_root(self):
+        one_shot = ONE_SHOT.read_text(encoding="utf-8")
+        assert "first_time_install.sh -y" in one_shot
+        assert INSTALLER.is_file()
+
+    def test_installer_accepts_everything_the_one_shot_passes(self):
+        one_shot = ONE_SHOT.read_text(encoding="utf-8")
+        installer = INSTALLER.read_text(encoding="utf-8")
+        assert re.search(r"^\s*-y\|--yes\)", installer, re.M), "installer no longer parses -y"
+        for var in sorted(set(re.findall(r"\b(LEDMATRIX_[A-Z_]+)=", one_shot))):
+            assert var in installer, f"one-shot passes {var}, which first_time_install.sh never reads"
+
+    def test_installer_initialises_the_submodule_itself(self):
+        # The one-shot clones without --recurse-submodules, so the installer is
+        # the only thing that ever fetches the matrix library.
+        assert "--recurse-submodules" not in ONE_SHOT.read_text(encoding="utf-8")
+        installer = INSTALLER.read_text(encoding="utf-8")
+        assert re.search(rf"submodule update --init --recursive {SUB}", installer)

test/test_scroll_helper.py

@@ -387,9 +387,81 @@ class TestFrameStatsPercentiles:
 
     def test_log_frame_rate_emits_the_line_and_clears_the_window(self, helper):
         helper._window = [0.010] * 20
+        helper.last_frame_time = time.time()  # arm the clock; see below
         helper.last_fps_log_time = 0.0  # force the 5s boundary
         with patch.object(helper.logger, "info") as info:
             helper.log_frame_rate()
         assert info.called
         assert "Scroll frame stats" in info.call_args[0][0]
         assert helper._window == []
+
+
+class TestIdleGapIsNotAFrame:
+    """The first frame of a scroll has no predecessor, so timing one measures
+    the idle gap since the last scroll rather than a frame.
+
+    Left in, that gap lands in the max field of otherwise healthy windows and
+    counts as one stall per scroll start -- about 0.2% at 500 frames to a
+    window, which is the same order as the real stall rates it sits next to.
+    The stall rate is the number used to judge whether a scroll change worked,
+    so it has to be clean.
+    """
+
+    def test_clock_starts_unarmed(self, helper):
+        assert helper.last_frame_time is None
+
+    def test_first_frame_seeds_the_clock_without_sampling(self, helper):
+        helper.log_frame_rate()
+        assert helper.last_frame_time is not None
+        assert helper._window == []
+        assert helper.frame_times == []
+
+    def test_second_frame_is_sampled(self, helper):
+        helper.log_frame_rate()
+        helper.log_frame_rate()
+        assert len(helper._window) == 1
+
+    def test_reset_scroll_disarms_the_clock(self, helper):
+        helper.log_frame_rate()
+        assert helper.last_frame_time is not None
+        helper.reset_scroll()
+        assert helper.last_frame_time is None
+
+    def test_gap_longer_than_the_log_interval_is_dropped(self, helper):
+        """Covers callers that scroll without ever calling reset_scroll()."""
+        helper.last_frame_time = time.time() - 137.0  # a real observed gap
+        helper.log_frame_rate()
+        assert helper._window == []
+        assert helper.frame_times == []
+
+    def test_a_gap_does_not_reach_the_stats_line(self, helper):
+        helper.last_frame_time = time.time() - 137.0
+        helper.last_fps_log_time = 0.0  # the 5s boundary is also due
+        with patch.object(helper.logger, "info") as info:
+            helper.log_frame_rate()
+        assert not info.called, "the idle gap was reported as a frame"
+
+    def test_window_of_only_gaps_logs_nothing(self, helper):
+        """A window whose every sample was dropped has nothing to report --
+        and reporting the gap itself is the bug this guards."""
+        helper.last_fps_log_time = 0.0
+        with patch.object(helper.logger, "info") as info:
+            helper.log_frame_rate()   # seeds
+            helper.last_frame_time = time.time() - 137.0
+            helper.log_frame_rate()   # dropped
+        assert not info.called
+
+    def test_seeding_restarts_the_window_timer(self, helper):
+        """A new scroll should not open by reporting a one-frame window."""
+        helper.last_fps_log_time = 0.0  # boundary long overdue
+        helper.log_frame_rate()         # seeds
+        with patch.object(helper.logger, "info") as info:
+            helper.log_frame_rate()     # first real frame
+        assert not info.called
+        assert len(helper._window) == 1
+
+    def test_a_normal_frame_still_counts(self, helper):
+        helper.last_frame_time = time.time() - 0.010
+        helper.log_frame_rate()
+        assert len(helper._window) == 1
+        assert helper._window[0] == pytest.approx(0.010, abs=0.005)

test/test_web_sudoers_installers_agree.py

@@ -0,0 +1,131 @@
+"""The two installers that write /etc/sudoers.d/ledmatrix_web must agree.
+
+first_time_install.sh (Step 10, a heredoc) and scripts/install/configure_web_sudo.sh
+(a block of echo lines) each generate the web user's sudo allow-list. They
+drifted: configure_web_sudo.sh granted scripts/fix_perms/safe_pip_install.sh
+but first_time_install.sh did not, so on a device set up only by the first-time
+installer permission_utils.install_requirements_file could not use the root
+wrapper and fell back to a user-level install that root-run ledmatrix.service
+may not see (and the auto-update rollback reported its reinstall as failed).
+
+This compares the granted command sets after normalising the spellings that
+differ between the files but expand identically at install time:
+$WEB_USER/$ACTUAL_USER, $PROJECT_ROOT/$PROJECT_ROOT_DIR, and the helper-path
+variables configure_web_sudo.sh defines ($SAFE_RM_PATH, ...).
+
+It also checks that every fix_perms helper granted via sudo is hardened to
+root:root in both scripts -- and, in first_time_install.sh, after Step 11's
+project-wide chown to the user, which would otherwise undo it.
+"""
+import re
+from pathlib import Path
+
+ROOT = Path(__file__).resolve().parent.parent
+FIRST_TIME = ROOT / "first_time_install.sh"
+CONFIGURE = ROOT / "scripts" / "install" / "configure_web_sudo.sh"
+
+#: Grants that intentionally exist in only one installer, as normalised
+#: commands. There are none today; add one here with a reason rather than
+#: loosening the comparison.
+ONLY_IN_FIRST_TIME = frozenset()
+ONLY_IN_CONFIGURE = frozenset()
+
+
+def _text(path):
+    return path.read_text(encoding="utf-8", errors="replace")
+
+
+def _web_sudoers_section(path):
+    """The part of the script that writes the ledmatrix_web allow-list.
+
+    first_time_install.sh also writes other files later (WiFi permissions are
+    delegated to a separate script, but keep this robust against future
+    additions), so restrict it to Step 10.
+    """
+    text = _text(path)
+    if path == FIRST_TIME:
+        start = text.index('CURRENT_STEP="Configure passwordless sudo access"')
+        end = text.index('CURRENT_STEP="Configure WiFi management permissions"')
+        return text[start:end]
+    return text
+
+
+def _variables(text):
+    """Simple NAME="..." assignments, so $SAFE_RM_PATH can be expanded."""
+    return {m.group(1): m.group(2)
+            for m in re.finditer(r'^\s*([A-Z_]+)="([^"$]*\$[^"]*)"\s*$', text, re.M)}
+
+
+def _normalise(command, variables):
+    for _ in range(3):  # helper paths reference $PROJECT_ROOT
+        command = re.sub(r"\$\{?([A-Z][A-Z0-9_]*)\}?",
+                         lambda m: variables.get(m.group(1), m.group(0)), command)
+    command = command.replace("$PROJECT_ROOT_DIR", "$PROJECT_ROOT")
+    return " ".join(command.split())
+
+
+def _grants(path):
+    """{(tags, command)} for every ledmatrix_web rule the script writes."""
+    section = _web_sudoers_section(path)
+    variables = _variables(_text(path))
+    grants = set()
+    for line in section.splitlines():
+        m = re.search(r'\$(?:WEB_USER|ACTUAL_USER) ALL=\(ALL\) (NOPASSWD:(?:NOEXEC:)?)\s*(.*)$',
+                      line)
+        if not m:
+            continue
+        command = m.group(2).rstrip().rstrip('"').rstrip()
+        grants.add((m.group(1), _normalise(command, variables)))
+    return grants
+
+
+def test_both_installers_generate_rules():
+    # Guards against the parser silently matching nothing in either file.
+    assert len(_grants(FIRST_TIME)) >= 15
+    assert len(_grants(CONFIGURE)) >= 15
+
+
+def test_installers_grant_the_same_commands():
+    first = _grants(FIRST_TIME)
+    configure = _grants(CONFIGURE)
+    only_first = {c for c in first - configure if c[1] not in ONLY_IN_FIRST_TIME}
+    only_configure = {c for c in configure - first if c[1] not in ONLY_IN_CONFIGURE}
+    assert not only_first and not only_configure, (
+        "ledmatrix_web sudoers drift between installers:\n"
+        f"  only in first_time_install.sh: {sorted(only_first)}\n"
+        f"  only in configure_web_sudo.sh: {sorted(only_configure)}")
+
+
+def test_pip_install_helper_is_granted():
+    wanted = ("NOPASSWD:", "$BASH_PATH $PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh *")
+    assert wanted in _grants(FIRST_TIME)
+    assert wanted in _grants(CONFIGURE)
+
+
+def _granted_helpers():
+    helpers = set()
+    for _, command in _grants(FIRST_TIME) | _grants(CONFIGURE):
+        m = re.search(r"scripts/fix_perms/([\w.-]+\.sh)", command)
+        if m:
+            helpers.add(m.group(1))
+    return helpers
+
+
+def test_every_granted_helper_is_hardened_in_configure_web_sudo():
+    text = _text(CONFIGURE)
+    variables = _variables(text)
+    hardened = {Path(_normalise(m.group(2), variables)).name
+                for m in re.finditer(r"sudo (chown root:root|chmod 755) \"?([^\"\s]+)", text)
+                if m.group(1).startswith("chown")}
+    assert _granted_helpers() <= hardened, _granted_helpers() - hardened
+
+
+def test_every_granted_helper_is_hardened_in_first_time_install_after_chown():
+    text = _text(FIRST_TIME)
+    project_chown = text.index('-exec chown -h "$ACTUAL_USER:$ACTUAL_USER"')
+    loop = re.search(r"for helper in ([^;]+); do\n(.*?)\ndone", text, re.S)
+    assert loop, "no helper-hardening loop in first_time_install.sh"
+    assert "chown root:root" in loop.group(2) and "chmod 755" in loop.group(2)
+    assert loop.start() > project_chown, (
+        "helper hardening runs before Step 11's project-wide chown, which undoes it")
+    assert _granted_helpers() <= set(loop.group(1).split())

web_interface/app.py

@@ -748,18 +748,13 @@ def display_preview_generator():
         except OSError:
             pass  # display side treats a missing marker as "no viewer"
     
-    # Get display dimensions from config
+    # Get display dimensions from config: the logical size DisplayManager
+    # renders at, so double-sided setups preview one screen
+    from src.display_geometry import logical_size
     try:
-        main_config = config_manager.load_config()
+        width, height = logical_size(config_manager.load_config())
-        cols = main_config.get('display', {}).get('hardware', {}).get('cols', 64)
+    except (KeyError, TypeError, ValueError, AttributeError, ConfigError):
-        chain_length = main_config.get('display', {}).get('hardware', {}).get('chain_length', 2)
+        width, height = logical_size({})
-        rows = main_config.get('display', {}).get('hardware', {}).get('rows', 32)
-        parallel = main_config.get('display', {}).get('hardware', {}).get('parallel', 1)
-        width = cols * chain_length
-        height = rows * parallel
-    except (KeyError, TypeError, ValueError, ConfigError):
-        width = 128
-        height = 64
     
     while True:
         try:

web_interface/blueprints/api_v3/__init__.py

@@ -1693,11 +1693,14 @@ def _standalone_render_starlark_app(app_id: str) -> Tuple[bool, int, Optional[st
 
     magnify = plugin_config.get('magnify')
     if magnify is None:
-        hw = full_config.get('display', {}).get('hardware', {})
+        # The size DisplayManager renders at (shared defaults, double-sided
-        cols = hw.get('cols', 64)
+        # applied), so the Pixlet render matches the screen it lands on
-        chain = hw.get('chain_length', 1)
+        from src.display_geometry import logical_size
-        rows = hw.get('rows', 32)
+        try:
-        magnify = max(1, min(8, int(min((cols * chain) / 64, rows / 32))))
+            width, height = logical_size(full_config)
+        except (TypeError, ValueError):
+            width, height = 64, 32
+        magnify = max(1, min(8, int(min(width / 64, height / 32))))
     else:
         try:
             magnify = max(1, min(8, int(magnify)))

web_interface/blueprints/api_v3/display.py

@@ -25,23 +25,14 @@ def get_display_current():
 
         snapshot_path = "/tmp/led_matrix_preview.png"
 
-        # Get display dimensions from config
+        # Get display dimensions from config: the logical size DisplayManager
+        # renders at, so double-sided setups preview one screen
+        from src.display_geometry import logical_size
         try:
-            if api_v3.config_manager:
+            config = api_v3.config_manager.load_config() if api_v3.config_manager else {}
-                main_config = api_v3.config_manager.load_config()
+            width, height = logical_size(config)
-                hardware_config = main_config.get('display', {}).get('hardware', {})
-                cols = hardware_config.get('cols', 64)
-                chain_length = hardware_config.get('chain_length', 2)
-                rows = hardware_config.get('rows', 32)
-                parallel = hardware_config.get('parallel', 1)
-                width = cols * chain_length
-                height = rows * parallel
-            else:
-                width = 128
-                height = 64
         except Exception:
-            width = 128
+            width, height = logical_size({})
-            height = 64
 
         # Try to read snapshot file
         image_data = None

web_interface/blueprints/api_v3/misc.py

@@ -154,9 +154,15 @@ def list_skins():
     """List installed visual skins (docs/SKIN_SYSTEM.md).
 
     Optional ?plugin_id=... filters to skins matching that plugin.
+
+    The response carries ``supported: false`` and a ``message``: the current
+    scoreboard plugins don't render skins, so a client must not present
+    these as selectable.
     """
     try:
-        from src.skin_system import skin_runtime
+        from src.skin_system import (
+            SKINS_RENDER_SUPPORTED, SKINS_UNSUPPORTED_MESSAGE, skin_runtime,
+        )
 
         plugin_id = request.args.get('plugin_id')
         if plugin_id:
@@ -181,7 +187,10 @@ def list_skins():
                 'modes': manifest.get('modes', []),
                 'has_preview': bool(preview and (skin_dir / preview).is_file()),
             })
-        return jsonify({'status': 'success', 'data': {'skins': payload}})
+        data = {'skins': payload, 'supported': SKINS_RENDER_SUPPORTED}
+        if not SKINS_RENDER_SUPPORTED:
+            data['message'] = SKINS_UNSUPPORTED_MESSAGE
+        return jsonify({'status': 'success', 'data': data})
     except Exception as e:
         logger.error('Error in list_skins', exc_info=True)
         return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500

web_interface/blueprints/api_v3/plugins.py

@@ -1256,6 +1256,16 @@ def install_plugin():
         plugin_id = data['plugin_id']
         branch = data.get('branch')  # Optional branch parameter
 
+        # A registry skin would install but never render with the current
+        # scoreboards; refuse it with the reason rather than a generic failure
+        try:
+            registry_entry = api_v3.plugin_store_manager.get_registry_info(plugin_id)
+        except Exception:
+            registry_entry = None
+        if isinstance(registry_entry, dict) and (registry_entry.get('type') or 'plugin') == 'skin':
+            from src.skin_system import SKINS_UNSUPPORTED_MESSAGE
+            return jsonify({'status': 'error', 'message': SKINS_UNSUPPORTED_MESSAGE}), 400
+
         # Install the plugin
         # Log the plugins directory being used for debugging
         plugins_dir = api_v3.plugin_store_manager.plugins_dir
@@ -1456,9 +1466,13 @@ def get_registry_from_url():
         registry = api_v3.plugin_store_manager.fetch_registry_from_url(repo_url)
 
         if registry:
+            # Skins aren't offered: current scoreboards don't render them
             return jsonify({
                 'status': 'success',
-                'plugins': registry.get('plugins', []),
+                'plugins': [
+                    p for p in registry.get('plugins', [])
+                    if not (isinstance(p, dict) and (p.get('type') or 'plugin') == 'skin')
+                ],
                 'registry_url': repo_url
             })
         else:
@@ -1573,6 +1587,10 @@ def list_plugin_store():
         # Format plugins for the web interface
         formatted_plugins = []
         for plugin in plugins:
+            # Registry skins install but never render with the current
+            # scoreboards, so the store doesn't offer them
+            if (plugin.get('type') or 'plugin') == 'skin':
+                continue
             formatted_plugins.append({
                 'id': plugin.get('id'),
                 'name': plugin.get('name'),
@@ -2649,18 +2667,11 @@ def get_plugin_schema():
         schema = schema_mgr.load_schema(plugin_id, use_cache=True)
 
         if schema:
-            # Offer installed visual skins as a dropdown (returns a copy;
+            # No "Visual Skin" dropdown: the current scoreboard plugins don't
-            # the cached schema and validation are never enum-restricted)
+            # render skins (src.skin_system.SKINS_UNSUPPORTED_MESSAGE), so
-            try:
+            # schema_mgr.inject_skin_selector is deliberately not called. A
-                current_skin = None
+            # stored "skin" value is unaffected: validation still allows it
-                if api_v3.config_manager:
+            # and a form save deep-merges over the stored section, keeping it.
-                    config = api_v3.config_manager.load_config()
-                    current_skin = config.get(plugin_id, {}).get('skin')
-                injected = schema_mgr.inject_skin_selector(schema, plugin_id, current_skin)
-                if isinstance(injected, dict):
-                    schema = injected
-            except Exception:
-                logger.debug('Skin selector injection failed for %s', plugin_id, exc_info=True)
             return jsonify({'status': 'success', 'data': {'schema': schema}})
 
         # Return a simple default schema if file not found