chore: mark skins unsupported, fix stale docs and preview size, prepare 3.4.0 (#580)

* chore: mark skins unsupported, fix stale docs and preview size, prepare 3.4.0

Skins: no current scoreboard plugin builds on src.base_classes, so the only
skin hook (SportsCore._render_game) never runs. The plugin schema endpoint no
longer injects the Visual Skin dropdown, the store hides and refuses
"type": "skin" registry entries, and GET /api/v3/skins reports
supported: false with a message. Stored skin config still loads and saves.
src/skin_system/ and its tests are unchanged apart from the support flag.

Docs: check_plugin.py/render_plugin.py examples use --plugin; document
BasePlugin.get_update_interval() and its interaction with the manifest
update_interval; CLAUDE.md drops the stale template line number and
recommends display_manager.width/height.

Preview size: new src/display_geometry.py holds the size computation and
defaults DisplayManager uses (double-sided applied, chain_length default 2).
The web preview, /display/current, Starlark magnify default, sync handshake
and two dev scripts use it.

Release: __version__ 3.4.0, CHANGELOG 3.4.0 section plus a 3.3.0 tag note.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix: address CodeRabbit review on #580

- Preview fallbacks (SSE stream and /display/current) use logical_size({})
  (128x32, the shared default) instead of a hard-coded 128x64.
- display_geometry treats a non-mapping display/hardware block as missing,
  so a malformed config.json falls back to defaults instead of raising
  AttributeError (which turned the Starlark render into an HTTP 500).
- Docs: the static update interval falls back manifest -> plugin config
  -> 60s, in both the API reference and the architecture spec.

Not taken: validating double_sided copies against chain_length/parallel.
An orientation Rotate: or U-mapper pixel mapper decides which axis panels
lie on, so the counts would reject working setups (the existing
vertical-split test is one).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(display_geometry): a non-finite hardware size raises ValueError, not OverflowError

CodeRabbit flagged the Starlark magnify default in
_standalone_render_starlark_app for truthy non-mapping display values. That
case was already handled by a9e1bd0b (_display/_hardware treat a non-mapping
block as missing, covered by test_non_mapping_display_config_uses_the_defaults),
and the magnify it produces from the 128x32 defaults is the same as from 64x32.

Checking the same path found one input that still escaped: Python's JSON
parser accepts Infinity, and int(inf) raises OverflowError, which neither the
Starlark path (TypeError, ValueError) nor the preview stream in app.py caught,
so a hand-edited "rows": Infinity returned HTTP 500. physical_size now raises
ValueError for it, matching its documented contract, so every caller's
existing fallback applies. DisplayManager already caught Exception.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

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/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,
-                          ds_config: Dict[str, Any]) -> 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.
-    """
-    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,
-    }
+# Moved to src/display_geometry.py so the web preview, Starlark magnify and
+# sync handshake compute the display size exactly as DisplayManager does
+# without importing rgbmatrix. Aliased here for existing callers.
+_resolve_double_sided = resolve_double_sided
 
 
 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.cols = hardware_config.get('cols', 64)
-            options.chain_length = hardware_config.get('chain_length', 2)
-            options.parallel = hardware_config.get('parallel', 1)
+            options.rows = hardware_config.get('rows', DEFAULT_ROWS)
+            options.cols = hardware_config.get('cols', DEFAULT_COLS)
+            options.chain_length = hardware_config.get('chain_length', DEFAULT_CHAIN_LENGTH)
+            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 {}
-                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)
+                fallback_width, fallback_height = physical_size(self.config)
                 # 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
-        # plugin directory (docs/SKIN_SYSTEM.md)
+        # Visual skins share the registry. _install_skin_from_info can put one
+        # 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,