Merge remote-tracking branch 'origin/main' into claude/frame-timing-harness

# Conflicts:
#	CHANGELOG.md

src/adaptive_images.py

@@ -29,13 +29,9 @@ from typing import Any, Optional, Tuple
 
 from PIL import Image
 
-# The one Pillow >= 9.1 compat shim (replaces the per-plugin copies).
-try:
-    RESAMPLE_LANCZOS = Image.Resampling.LANCZOS
-    RESAMPLE_NEAREST = Image.Resampling.NEAREST
-except AttributeError:  # Pillow < 9.1
-    RESAMPLE_LANCZOS = Image.LANCZOS
-    RESAMPLE_NEAREST = Image.NEAREST
+# Re-exported by src.common for plugins, which import them from there.
+RESAMPLE_LANCZOS = Image.Resampling.LANCZOS
+RESAMPLE_NEAREST = Image.Resampling.NEAREST
 
 FIT_MODES = ("contain", "cover", "fill_height", "stretch")
 

src/background_data_service.py

@@ -131,19 +131,14 @@ class BackgroundDataService:
         
         # Thread management
         self.executor = ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix="BackgroundData")
-        # cache_key -> request_id for fetches currently in flight. Submitting
-        # the same key twice used to start two identical fetches: request_id
-        # carries a millisecond timestamp, so every submit looked new, and
-        # active_requests is keyed by it rather than by what is being fetched.
-        # On a real board the season-schedule key is requested by both the
-        # Recent and the Upcoming manager, which miss the cache in the same
-        # millisecond and each download and parse the same payload.
+        # cache_key -> request_id for fetches currently in flight, so a second
+        # submit for the same key joins the running fetch instead of starting
+        # another. It is the normal case: a sport's Recent and Upcoming
+        # managers miss the cache for the same season schedule together.
         self._inflight_by_cache_key: Dict[str, str] = {}
-        # request_id was sport_year_milliseconds, which is not unique: two
-        # submits inside the same millisecond produced the SAME id, so one
-        # silently replaced the other in active_requests and completed_requests.
-        # Rare before, but dedupe hands this id back to every joiner as their
-        # handle for get_result(), so it has to be unique. A counter is enough.
+        # Makes every request_id unique. The id also carries a millisecond
+        # timestamp, but two submits can share a millisecond, and a joiner
+        # uses the id as its handle for get_result().
         self._request_seq = itertools.count()
         self.active_requests: Dict[str, FetchRequest] = {}
         self.completed_requests: Dict[str, FetchResult] = {}
@@ -186,9 +181,9 @@ class BackgroundDataService:
         This ensures Recent/Upcoming managers and background service
         use the same cache keys.
         """
-        # Same format as CacheManager.generate_sport_cache_key(). This used to
-        # build a whole CacheManager to call it -- config load, cache-dir
-        # probing with test writes -- on every submit without a cache_key.
+        # Same format as CacheManager.generate_sport_cache_key(), built here
+        # rather than by constructing a CacheManager (config load, cache-dir
+        # probing) on every submit without a cache_key.
         if date_str is None:
             date_str = datetime.now(pytz.utc).strftime('%Y%m%d')
         return f"{sport}_{date_str}"
@@ -331,10 +326,8 @@ class BackgroundDataService:
         
         try:
             with self._lock:
-                # A request cancelled while it sat in the executor queue must
-                # stay cancelled. Overwriting the status here undid the cancel
-                # outright: the worker went on to download, cache and call back
-                # for work the caller had already withdrawn.
+                # A request cancelled while it sat in the executor queue stays
+                # cancelled: no download, no cache write, no callback.
                 if request.status == FetchStatus.CANCELLED:
                     cancelled_before_start = True
                 else:
@@ -463,10 +456,9 @@ class BackgroundDataService:
             logger.error(f"Failed to fetch {request.sport} {request.year} data: {error_msg}")
             
             with self._lock:
-                # Don't relabel a cancelled request. The callback gate in the
-                # finally block only suppresses CANCELLED, so promoting it to
-                # FAILED here delivered an error callback for a fetch nobody
-                # was waiting on any more.
+                # A cancelled request stays CANCELLED even when its fetch
+                # failed: the finally block skips callbacks only for
+                # CANCELLED, and nobody is waiting on this fetch any more.
                 if request.status != FetchStatus.CANCELLED:
                     request.status = FetchStatus.FAILED
                 request.error = error_msg
@@ -526,20 +518,13 @@ class BackgroundDataService:
                 except Exception as e:
                     logger.error(f"Error in callback for request {request.id}: {e}")
 
-            # Released AFTER the loop, not inside it. Every callback here holds
-            # the same FetchResult, so releasing per-delivery handed the first
-            # one the data and every joiner `result.data is None` -- which is
-            # not a quiet degradation: they read `result.data.get('events')` and
-            # raise AttributeError, which this very loop catches and logs, so
-            # the symptom was one ERROR line and a manager that silently never
-            # got its schedule. Deduplication is the normal case, not a corner:
-            # a sport's recent, upcoming and live managers all ride one season
-            # fetch.
+            # Released after the loop, never inside it: every callback holds
+            # the same FetchResult (a sport's recent, upcoming and live
+            # managers usually share one fetch), so a release between
+            # deliveries would hand the later ones `result.data is None`.
             #
-            # Guarded on `callbacks`, because a request submitted without one
-            # has no other way to collect its payload than polling get_result().
-            # The old per-delivery release got that right by accident: an empty
-            # list never entered the loop body.
+            # Only when there were callbacks: a request submitted without one
+            # collects its payload by polling get_result().
             if callbacks:
                 self._release_payload(result)
                 request.result = None
@@ -721,9 +706,6 @@ class BackgroundDataService:
                 'completed_requests_count': len(self.completed_requests),
                 'max_completed_requests': self._max_completed_requests,
                 'completed_requests_usage_percent': (len(self.completed_requests) / self._max_completed_requests * 100) if self._max_completed_requests > 0 else 0,
-                # Nothing is queued outside the executor; kept for callers
-                # that read the key.
-                'queue_size': 0,
                 'last_cleanup': self._last_completed_requests_cleanup,
                 'cleanup_interval': self._completed_requests_cleanup_interval
             }
@@ -793,27 +775,6 @@ class BackgroundDataService:
             
             return removed_count
     
-    def clear_completed_requests(self, older_than_hours: int = 24):
-        """
-        Clear completed requests older than specified time.
-        
-        Args:
-            older_than_hours: Clear requests older than this many hours
-        """
-        cutoff_time = time.time() - (older_than_hours * 3600)
-        
-        with self._lock:
-            to_remove = []
-            for request_id, result in self.completed_requests.items():
-                if result.completed_at < cutoff_time:
-                    to_remove.append(request_id)
-            
-            for request_id in to_remove:
-                del self.completed_requests[request_id]
-            
-            if to_remove:
-                logger.info(f"Cleared {len(to_remove)} old completed requests")
-    
     def shutdown(self, wait: bool = True):
         """
         Shutdown the background data service.

src/backup_manager.py

@@ -83,14 +83,25 @@ BUNDLED_FONTS: frozenset[str] = frozenset({
 _CONFIG_REL = Path("config/config.json")
 _SECRETS_REL = Path("config/config_secrets.json")
 _WIFI_REL = Path("config/wifi_config.json")
-# Sits in config/ next to the three above and is pure user state — a
-# YouTube Music session that has to be re-authenticated by hand if lost.
-# It was omitted from backups, so a restore silently signed the user out.
+# A YouTube Music session: pure user state that has to be re-authenticated by
+# hand if lost, so a restore must bring it back.
 _YTM_REL = Path("config/ytm_auth.json")
 _FONTS_REL = Path("assets/fonts")
 _PLUGIN_UPLOADS_REL = Path("assets/plugins")
 _STATE_REL = Path("data/plugin_state.json")
 
+#: The sections that are one file each: (section name, path, the
+#: RestoreOptions flag that restores it). create, preview, validate and
+#: restore all walk this table. ytm_auth follows restore_wifi: it is
+#: device-local auth like the Wi-Fi settings, and a toggle of its own for one
+#: file would be noise in the restore dialog.
+_SINGLE_FILE_SECTIONS: Tuple[Tuple[str, Path, str], ...] = (
+    ("config", _CONFIG_REL, "restore_config"),
+    ("secrets", _SECRETS_REL, "restore_secrets"),
+    ("wifi", _WIFI_REL, "restore_wifi"),
+    ("ytm_auth", _YTM_REL, "restore_wifi"),
+)
+
 MANIFEST_NAME = "manifest.json"
 PLUGINS_MANIFEST_NAME = "plugins.json"
 
@@ -140,34 +151,18 @@ class RestoreResult:
 # ---------------------------------------------------------------------------
 
 
-def _ledmatrix_version(project_root: Path) -> str:
-    """Best-effort version string for the current install."""
-    version_file = project_root / "VERSION"
-    if version_file.exists():
-        try:
-            return version_file.read_text(encoding="utf-8").strip() or "unknown"
-        except OSError:
-            pass
-    head_file = project_root / ".git" / "HEAD"
-    if head_file.exists():
-        try:
-            head = head_file.read_text(encoding="utf-8").strip()
-            if head.startswith("ref: "):
-                ref = head[5:]
-                ref_path = project_root / ".git" / ref
-                if ref_path.exists():
-                    return ref_path.read_text(encoding="utf-8").strip()[:12] or "unknown"
-            return head[:12] or "unknown"
-        except OSError:
-            pass
-    return "unknown"
+def _ledmatrix_version() -> str:
+    """The release of the running core (``src.__version__``), recorded in the
+    manifest so a restore can tell which release wrote the backup."""
+    from src import __version__
+    return __version__
 
 
-def _build_manifest(contents: List[str], project_root: Path) -> Dict[str, Any]:
+def _build_manifest(contents: List[str]) -> Dict[str, Any]:
     return {
         "schema_version": SCHEMA_VERSION,
         "created_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"),
-        "ledmatrix_version": _ledmatrix_version(project_root),
+        "ledmatrix_version": _ledmatrix_version(),
         "hostname": socket.gethostname(),
         "contents": contents,
     }
@@ -178,13 +173,34 @@ def _build_manifest(contents: List[str], project_root: Path) -> Dict[str, Any]:
 # ---------------------------------------------------------------------------
 
 
+def _plugins_directory(project_root: Path) -> Path:
+    """The plugin install directory: ``plugin_system.plugins_directory`` from
+    config/config.json (relative to ``project_root`` unless absolute), or
+    ``plugin-repos`` when the config does not say or cannot be read."""
+    configured: Any = None
+    try:
+        with (project_root / _CONFIG_REL).open("r", encoding="utf-8") as f:
+            config = json.load(f)
+        if isinstance(config, dict):
+            plugin_system = config.get("plugin_system")
+            if isinstance(plugin_system, dict):
+                configured = plugin_system.get("plugins_directory")
+    except (OSError, json.JSONDecodeError):
+        pass
+    if not isinstance(configured, str) or not configured.strip():
+        configured = "plugin-repos"
+    path = Path(configured)
+    return path if path.is_absolute() else project_root / path
+
+
 def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]:
     """
     Return a list of currently-installed plugins suitable for the backup
     manifest. Each entry has ``plugin_id`` and ``version``.
 
-    Reads ``data/plugin_state.json`` if present; otherwise walks the plugin
-    directory and reads each ``manifest.json``.
+    Reads ``data/plugin_state.json`` if present, then adds any plugin it
+    does not list from the ``manifest.json`` files in the configured plugin
+    directory (see :func:`_plugins_directory`).
     """
     plugins: Dict[str, Dict[str, Any]] = {}
 
@@ -206,8 +222,7 @@ def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]:
         except (OSError, json.JSONDecodeError) as e:
             logger.warning("Could not read plugin_state.json: %s", e)
 
-    # Fall back to scanning plugin-repos/ for manifests.
-    plugins_root = project_root / "plugin-repos"
+    plugins_root = _plugins_directory(project_root)
     if plugins_root.exists():
         for entry in sorted(plugins_root.iterdir()):
             if not entry.is_dir():
@@ -298,19 +313,10 @@ def create_backup(
     tmp_path = zip_path.with_suffix(".zip.tmp")
     try:
         with zipfile.ZipFile(tmp_path, "w", compression=zipfile.ZIP_DEFLATED) as zf:
-            # Config files.
-            if (project_root / _CONFIG_REL).exists():
-                zf.write(project_root / _CONFIG_REL, _CONFIG_REL.as_posix())
-                contents.append("config")
-            if (project_root / _SECRETS_REL).exists():
-                zf.write(project_root / _SECRETS_REL, _SECRETS_REL.as_posix())
-                contents.append("secrets")
-            if (project_root / _WIFI_REL).exists():
-                zf.write(project_root / _WIFI_REL, _WIFI_REL.as_posix())
-                contents.append("wifi")
-            if (project_root / _YTM_REL).exists():
-                zf.write(project_root / _YTM_REL, _YTM_REL.as_posix())
-                contents.append("ytm_auth")
+            for section, rel, _flag in _SINGLE_FILE_SECTIONS:
+                if (project_root / rel).exists():
+                    zf.write(project_root / rel, rel.as_posix())
+                    contents.append(section)
 
             # User-uploaded fonts.
             user_fonts = iter_user_fonts(project_root)
@@ -338,7 +344,7 @@ def create_backup(
                 contents.append("plugins")
 
             # Manifest goes last so that `contents` reflects what we actually wrote.
-            manifest = _build_manifest(contents, project_root)
+            manifest = _build_manifest(contents)
             zf.writestr(MANIFEST_NAME, json.dumps(manifest, indent=2))
 
         os.replace(tmp_path, zip_path)
@@ -352,15 +358,16 @@ def create_backup(
 def preview_backup_contents(project_root: Path) -> Dict[str, Any]:
     """Return a summary of what ``create_backup`` would include."""
     project_root = Path(project_root).resolve()
-    return {
-        "has_config": (project_root / _CONFIG_REL).exists(),
-        "has_secrets": (project_root / _SECRETS_REL).exists(),
-        "has_wifi": (project_root / _WIFI_REL).exists(),
-        "has_ytm_auth": (project_root / _YTM_REL).exists(),
+    preview: Dict[str, Any] = {
+        f"has_{section}": (project_root / rel).exists()
+        for section, rel, _flag in _SINGLE_FILE_SECTIONS
+    }
+    preview.update({
         "user_fonts": [p.name for p in iter_user_fonts(project_root)],
         "plugin_uploads": len(iter_plugin_uploads(project_root)),
         "plugins": list_installed_plugins(project_root),
-    }
+    })
+    return preview
 
 
 # ---------------------------------------------------------------------------
@@ -431,15 +438,10 @@ def validate_backup(zip_path: Path) -> Tuple[bool, str, Dict[str, Any]]:
                     {},
                 )
 
-            detected: List[str] = []
-            if _CONFIG_REL.as_posix() in names:
-                detected.append("config")
-            if _SECRETS_REL.as_posix() in names:
-                detected.append("secrets")
-            if _WIFI_REL.as_posix() in names:
-                detected.append("wifi")
-            if _YTM_REL.as_posix() in names:
-                detected.append("ytm_auth")
+            detected: List[str] = [
+                section for section, rel, _flag in _SINGLE_FILE_SECTIONS
+                if rel.as_posix() in names
+            ]
             if any(n.startswith(_FONTS_REL.as_posix() + "/") for n in names):
                 detected.append("fonts")
             if any(
@@ -584,55 +586,18 @@ def restore_backup(
             result.errors.append("Failed to extract backup")
             return result
 
-        # Main config.
-        if options.restore_config and (tmp_dir / _CONFIG_REL).exists():
+        for section, rel, flag in _SINGLE_FILE_SECTIONS:
+            if not (tmp_dir / rel).exists():
+                continue
+            if not getattr(options, flag):
+                result.skipped.append(section)
+                continue
             try:
-                _copy_file(tmp_dir / _CONFIG_REL, project_root / _CONFIG_REL)
-                result.restored.append("config")
+                _copy_file(tmp_dir / rel, project_root / rel)
+                result.restored.append(section)
             except OSError as e:
-                logger.error("[Backup] Failed to restore config.json: %s", e, exc_info=True)
-                result.errors.append("Failed to restore config.json")
-        elif (tmp_dir / _CONFIG_REL).exists():
-            result.skipped.append("config")
-
-        # Secrets.
-        if options.restore_secrets and (tmp_dir / _SECRETS_REL).exists():
-            try:
-                _copy_file(tmp_dir / _SECRETS_REL, project_root / _SECRETS_REL)
-                result.restored.append("secrets")
-            except OSError as e:
-                logger.error(
-                    "[Backup] Failed to restore config_secrets.json: %s", e, exc_info=True
-                )
-                result.errors.append("Failed to restore config_secrets.json")
-        elif (tmp_dir / _SECRETS_REL).exists():
-            result.skipped.append("secrets")
-
-        # WiFi.
-        if options.restore_wifi and (tmp_dir / _WIFI_REL).exists():
-            try:
-                _copy_file(tmp_dir / _WIFI_REL, project_root / _WIFI_REL)
-                result.restored.append("wifi")
-            except OSError as e:
-                logger.error(
-                    "[Backup] Failed to restore wifi_config.json: %s", e, exc_info=True
-                )
-                result.errors.append("Failed to restore wifi_config.json")
-        elif (tmp_dir / _WIFI_REL).exists():
-            result.skipped.append("wifi")
-
-        # YouTube Music session. Follows restore_wifi rather than getting its
-        # own flag: it is device-local auth in the same sense, and a separate
-        # toggle for one file would be noise in the restore dialog.
-        if options.restore_wifi and (tmp_dir / _YTM_REL).exists():
-            try:
-                _copy_file(tmp_dir / _YTM_REL, project_root / _YTM_REL)
-                result.restored.append("ytm_auth")
-            except OSError as e:
-                logger.error("[Backup] Failed to restore ytm_auth.json: %s", e, exc_info=True)
-                result.errors.append("Failed to restore ytm_auth.json")
-        elif (tmp_dir / _YTM_REL).exists():
-            result.skipped.append("ytm_auth")
+                logger.error("[Backup] Failed to restore %s: %s", rel.name, e, exc_info=True)
+                result.errors.append(f"Failed to restore {rel.name}")
 
         # User fonts — skip anything that collides with a bundled font.
         tmp_fonts = tmp_dir / _FONTS_REL

src/base_odds_manager.py

@@ -18,6 +18,8 @@ import requests
 import json
 from typing import Dict, Any, Optional, List
 
+from src.common.api_helper import DEFAULT_HTTP_HEADERS
+
 
 class BaseOddsManager:
     """
@@ -45,22 +47,15 @@ class BaseOddsManager:
         self.logger = logging.getLogger(__name__)
         self.base_url = "https://sports.core.api.espn.com/v2/sports"
 
-        # This path used a bare requests.get, so it identified itself as
-        # python-requests/x.y -- the one thing ESPN is known to reject. Around
-        # 2026-08-04 it began 403ing browser strings and bare custom tokens
-        # alike; what it accepts is a token with a URL that says who is
-        # calling. Every other ESPN caller in the tree already sends this
-        # (src/common/api_helper.py); the odds path was simply missed, and it is the one whose failures cost
-        # the caller its whole update budget.
+        # Core's shared headers: ESPN rejects requests' default User-Agent
+        # (see api_helper.USER_AGENT), and a rejected odds request costs the
+        # calling plugin its update budget.
         #
         # Deliberately no retry adapter, unlike api_helper: retries multiply
         # request_timeout, which is set to 5s precisely to stay inside that
         # budget. One try, then the cooldown below.
         self.session = requests.Session()
-        self.session.headers.update({
-            'User-Agent': 'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)',
-            'Accept': 'application/json',
-        })
+        self.session.headers.update(DEFAULT_HTTP_HEADERS)
         
         # Configuration with defaults
         self.update_interval = 3600  # 1 hour default
@@ -72,7 +67,6 @@ class BaseOddsManager:
         self.request_timeout = 5
         # Set when a request fails; until then, skip the network entirely.
         self._skip_network_until = 0.0
-        self.cache_ttl = 1800       # 30 minutes default
         
         # Load configuration if available
         if config_manager:
@@ -89,12 +83,10 @@ class BaseOddsManager:
             
             self.update_interval = odds_config.get('update_interval', self.update_interval)
             self.request_timeout = odds_config.get('timeout', self.request_timeout)
-            self.cache_ttl = odds_config.get('cache_ttl', self.cache_ttl)
-            
+
             self.logger.debug(f"BaseOddsManager configuration loaded: "
                             f"update_interval={self.update_interval}s, "
-                            f"timeout={self.request_timeout}s, "
-                            f"cache_ttl={self.cache_ttl}s")
+                            f"timeout={self.request_timeout}s")
                             
         except Exception as e:
             self.logger.warning(f"Failed to load BaseOddsManager configuration: {e}")
@@ -172,15 +164,12 @@ class BaseOddsManager:
             odds_data = self._extract_espn_data(raw_data)
             if odds_data:
                 self.logger.info(f"Successfully extracted odds data: {odds_data}")
-            else:
-                self.logger.debug("No odds data available for this game")
-            
-            if odds_data:
                 self.cache_manager.set(cache_key, odds_data, ttl=interval)
                 self.logger.info(f"Saved odds data to cache for {cache_key} with TTL {interval}s")
             else:
                 self.logger.debug(f"No odds data available for {cache_key}")
-                # Cache the fact that no odds are available to avoid repeated API calls
+                # Cache the absence too, so the game is not re-requested
+                # on every update until the interval passes.
                 self.cache_manager.set(cache_key, {"no_odds": True}, ttl=interval)
             
             return odds_data

src/cache_manager.py

@@ -32,7 +32,6 @@ from typing import Any, Dict, List, Optional
 import logging
 import threading
 import tempfile
-from src.exceptions import CacheError
 from src.cache.memory_cache import MemoryCache, default_max_size
 from src.cache.disk_cache import DiskCache
 from src.cache.cache_strategy import CacheStrategy
@@ -272,12 +271,9 @@ class CacheManager:
         # Update memory cache first
         self._memory_cache_component.set(key, data)
         
-        # Save to disk cache
-        try:
-            self._disk_cache_component.set(key, data)
-        except CacheError:
-            # Disk cache errors are already logged and raised by DiskCache
-            raise
+        # DiskCache logs a failed write and raises CacheError, which the
+        # caller gets as is.
+        self._disk_cache_component.set(key, data)
 
     def load_cache(self, key: str) -> Optional[Dict[str, Any]]:
         """Load data from cache with memory caching."""

src/common/README.md

@@ -1,62 +1,242 @@
-# Common Utilities
+# src/common
 
-This directory contains reusable utilities and helpers for LEDMatrix plugins and core modules.
+Helpers shared by core and plugins. This page lists every module, what it is
+for, and whether plugins are expected to import it.
 
-## Adaptive Layout & Images (`src/adaptive_layout.py`, `src/adaptive_images.py`)
+Rules for the package:
 
-The recommended way to lay out plugins that render legibly on **any** panel
-size (64x32 through 256x128+) without hand-tuned coordinates. Re-exported
-from `src.common` for convenience; canonical import paths are
-`src.adaptive_layout` / `src.adaptive_images`.
+- Every module must import without display hardware: nothing here may import
+  `src.display_manager` or `src.plugin_system` at module level
+  ([`test/test_common_is_hardware_free.py`](../../test/test_common_is_hardware_free.py)).
+  That keeps plugins that use it loadable by the web preview,
+  `scripts/check_plugin.py` and tests on a laptop.
+- A plugin that imports a module added in a given core release must declare
+  that release as its minimum (`ledmatrix_min_version` in the manifest's
+  `versions` entry). The "Since" column gives the release; "—" means it
+  predates 3.1.0, "n/a" that plugins should not import it.
+- `from src.common import ...` re-exports `APIHelper`, `ScrollHelper`,
+  `LogoHelper`, `TextHelper`, `scroll_config` (plus `ScrollSettings`,
+  `configure_scroll`, `resolve_scroll_settings`, `refresh_hz_from_config`) and
+  the adaptive layout names below ([`__init__.py`](__init__.py)).
+
+## Summary
+
+| Module | For | Plugins import it? | Since |
+|---|---|---|---|
+| [`api_helper`](#api_helper) | HTTP GET/POST with caching and rate limiting | Yes | — |
+| [`bdf_font`](#bdf_font) | Load and draw BDF bitmap fonts | Yes, if drawing BDF text directly | Unreleased |
+| [`espn_dates`](#espn_dates) | Fetch ESPN scoreboards across a date range | Yes (scoreboards) | 3.5.0 |
+| [`font_layout`](#font_layout) | Reproducible TrueType loading, crisp sizes | Yes | 3.4.0 |
+| [`logo_helper`](#logo_helper) | Load, resize and cache team logos | Yes | — |
+| [`path_safety`](#path_safety) | Turn request-supplied names into safe paths | No, core-internal | n/a |
+| [`permission_utils`](#permission_utils) | File modes and shared-group ownership | Rarely | — |
+| [`scroll_config`](#scroll_config) | Plugin scroll config → configured `ScrollHelper` | Yes (scrollers) | 3.4.0 |
+| [`scroll_helper`](#scroll_helper) | Pre-rendered horizontal scrolling | Yes | — |
+| [`snapshot_policy`](#snapshot_policy) | When to write the web preview frame | No, core-internal | n/a |
+| [`sports_card`](#sports_card) | Scoreboard card settings, colours, fonts, dates | Yes (scoreboards) | 3.3.0 |
+| [`sports_game_renderer`](#sports_game_renderer) | Scoreboard scroll/Vegas card geometry | Yes (scoreboards) | 3.3.0 |
+| [`sports_helpers`](#sports_helpers) | Small helpers every scoreboard `sports.py` copies | Yes (scoreboards) | 3.5.0 |
+| [`sports_scroll`](#sports_scroll) | Scoreboard scroll-display orchestration | Yes (scoreboards) | 3.2.0 |
+| [`sports_shared`](#sports_shared) | Sport-independent `sports.py` methods | Yes (scoreboards) | 3.3.0 |
+| [`sync_manager`](#sync_manager) | Leader/follower sync between two displays | No, core-internal | n/a |
+| [`text_helper`](#text_helper) | Outlined text, wrapping, measurement | Yes | — |
+
+The four `sports_*` mixin and card modules hold code the scoreboard plugins
+used to carry as identical copies. Each module docstring lists what a host
+class must provide. The plan behind them is in
+[docs/SPORTS_UNIFICATION.md](../../docs/SPORTS_UNIFICATION.md).
+
+## Adaptive layout and images
+
+`src/adaptive_layout.py` and `src/adaptive_images.py` live outside this
+package but are re-exported from `src.common`. They are the recommended way
+to lay out a plugin that renders legibly on any panel size. Every
+`BasePlugin` already has `self.layout`, `self.draw_fit()` and
+`self.draw_image()`:
 
 ```python
-# Every BasePlugin already has self.layout and the draw helpers:
 regs = scoreboard_regions(self.layout.bounds, ctx=self.layout)
 self.draw_image(away_logo, regs.away_slot, mode="fill_height",
                 crop_to_ink=True, cache_key=f"logo:{abbr}")
 self.draw_fit(score_text, regs.score_area)     # largest crisp font that fits
-self.draw_fit(status, regs.status_band)
 ```
 
-Key pieces: `Region` (rect algebra: bands/columns/splits/offset),
-font ladders (`LADDER_GRID`, `LADDER_ARCADE` — discrete crisp sizes, never
-fractional scaling), `LayoutContext` (`fit_text`, `fit_image`, `by_tier`,
-`px`), and composite carvers `scoreboard_regions()` / `media_row()`.
-Full guide: [docs/ADAPTIVE_LAYOUT.md](../../docs/ADAPTIVE_LAYOUT.md).
+Key pieces: `Region`, the font ladders `LADDER_GRID` / `LADDER_ARCADE`,
+`LayoutContext` (`fit_text`, `fit_image`, `by_tier`, `px`), and
+`scoreboard_regions()` / `media_row()`. Guide:
+[docs/ADAPTIVE_LAYOUT.md](../../docs/ADAPTIVE_LAYOUT.md).
 
-## API Helpers (`api_helper.py`)
+## Modules
 
-Utilities for making HTTP requests and handling API responses.
+### api_helper
 
-## Logo Helpers (`logo_helper.py`)
+[`api_helper.py`](api_helper.py). `APIHelper(cache_manager=None, ...)`:
+`get()` and `post()` with retries, optional caching through the cache
+manager, and a minimum interval between requests (`set_rate_limit()`). Also has
+`fetch_espn_scoreboard()`, `fetch_espn_standings()` and
+`fetch_espn_rankings()`.
 
-Utilities for loading and managing team logos.
+### bdf_font
 
-## Text Helpers (`text_helper.py`)
+[`bdf_font.py`](bdf_font.py). The one BDF loader and rasterizer.
+`load_bdf_face(path, size)` returns `(face, realised_px)`, falling back to
+the file's native strike when it has none at `size`;
+`draw_bdf_text(draw, text, x, y, face, color)` draws top-left anchored onto a
+PIL `ImageDraw` the same way the panel does. `read_bdf_native_size(path)`
+and `clear_face_cache()` round it out. Faces are cached per thread (FreeType
+faces are not thread-safe). `DisplayManager`, `FontManager`, `element_style`
+and the plugin test harness all use it. Most plugins get BDF text through
+`display_manager.draw_text()` or `FontManager` and never import this.
 
-Utilities for text processing and formatting.
+### espn_dates
 
-## BDF Fonts (`bdf_font.py`)
+[`espn_dates.py`](espn_dates.py). ESPN's site API rejects `dates=` ranges
+and truncates results when `limit` is above 500. `fetch_espn_scoreboard()`
+splits a range into month and day requests ESPN accepts and merges the
+results; `espn_date_chunks()`, `fetch_espn_date_chunks()`,
+`clamp_espn_limit()` and `merge_scoreboard_payloads()` are the pieces.
+Scoreboard plugins also bundle a copy for older cores.
 
-The one way to load and draw BDF bitmap fonts. `load_bdf_face(path, size)`
-returns `(face, realised_px)`, falling back to the file's native strike when
-it has none at `size`; `draw_bdf_text(draw, text, x, y, face, color)` draws
-top-left anchored onto a PIL `ImageDraw` exactly as the panel does.
-`DisplayManager`, `FontManager`, `element_style` and the plugin test harness
-all go through it.
+### font_layout
 
-## Scroll Helpers (`scroll_helper.py`)
+[`font_layout.py`](font_layout.py). `load_truetype(path, size)` is
+`ImageFont.truetype` with PIL's Basic layout engine pinned, so text lays out
+the same whether or not the host Pillow has libraqm; use it for anything
+drawn to the panel or compared against a golden image. `crisp_size()` gives
+the size a bundled face renders on whole pixels at. `resolve_asset_path()`
+resolves `assets/fonts/...` against the install root rather than the
+working directory.
 
-Utilities for scrolling text on the display.
+### logo_helper
 
-## Permission Utilities (`permission_utils.py`)
+[`logo_helper.py`](logo_helper.py). `LogoHelper(display_width,
+display_height, ...)`: `load_logo()`, `load_logo_with_download()`,
+`get_logo_variations()`, `normalize_abbreviation()`, with an in-memory cache.
 
-Helpers for ensuring directory permissions and ownership are correct
-when running as a service (used by `CacheManager` to set up its
-persistent cache directory).
+### path_safety
 
-## Best Practices
+[`path_safety.py`](path_safety.py). Core-internal, used by web handlers that
+open files named in a request. `safe_path_component(value)` returns the
+value if it is one harmless path segment, else `None`;
+`resolve_under(base, *parts)` returns the resolved path, or `None` if a part
+is unsafe or the result would leave `base`; `safe_relative_parts()` splits a
+relative path the same way. Both return the sanitised value rather than a
+boolean, so a caller cannot check one string and open another.
 
-1. **Use centralized logging**: Import from `src.logging_config` instead of creating loggers directly
-2. **Reuse utilities**: Check existing utilities before creating new ones
-3. **Document additions**: Add documentation when adding new utilities
+### permission_utils
+
+[`permission_utils.py`](permission_utils.py). The modes and ownership that
+let the root display service and the web user share files:
+`ensure_directory_permissions()`, `ensure_file_permissions()`, the
+`get_*_mode()` functions, `ensure_shared_group_ownership()`,
+`sudo_remove_directory()` and `install_requirements_file()` (the sudo
+`safe_pip_install.sh` path). `ConfigManager`, `CacheManager` and the store
+already call these; a plugin needs them only when it creates its own files
+outside the cache. See [docs/PERMISSIONS.md](../../docs/PERMISSIONS.md).
+
+### scroll_config
+
+[`scroll_config.py`](scroll_config.py). `configure(scroll_helper,
+plugin_config=, global_config=, display_manager=, plugin_logger=)` reads a
+plugin's scroll settings, snaps the speed to a whole number of pixels per
+panel refresh, puts the helper in fixed-step mode and returns
+`ScrollSettings`. Pass `settings.frame_hold` to
+`display_manager.set_scrolling_state(True, frame_hold=...)` or the scroll
+runs too fast. `resolve()` does the calculation without touching a helper.
+See [docs/SCROLL_PERFORMANCE.md](../../docs/SCROLL_PERFORMANCE.md).
+
+### scroll_helper
+
+[`scroll_helper.py`](scroll_helper.py). `ScrollHelper(display_width,
+display_height, logger=None)`: build a wide image once
+(`create_scrolling_image()` or `set_scrolling_image()`), then per frame
+`update_scroll_position()` and `get_visible_portion()`;
+`is_scroll_complete()`, `calculate_dynamic_duration()` and
+`get_dynamic_duration()` for timing. Configure it with `scroll_config`
+rather than the `set_*` methods. Vegas mode reads a plugin's
+`scroll_helper` image when the plugin has no `get_vegas_content()`.
+
+### snapshot_policy
+
+[`snapshot_policy.py`](snapshot_policy.py). Core-internal. `decide()`
+tells `DisplayManager` whether to write `/tmp/led_matrix_preview.png`, only
+touch its mtime, or skip, based on whether a browser is watching the preview.
+The web health check reads the file's age.
+
+### sports_card
+
+[`sports_card.py`](sports_card.py). Free functions taking `config`, `fonts`
+and `logger` explicitly: card options (`scroll_card_option()`,
+`vs_text()`, `upcoming_center_mode()`), colours (`element_color()`,
+`font_color()`, `score_color_for()`, `recent_score_color()`), favourite-team
+rules (`favorite_teams_for()`, `side_is_favorite()`, `favorite_result()`),
+dates (`format_game_date()`, `format_game_time()`, `card_tzinfo()`) and font
+sizes (`schema_font_size()`, `resolve_font_size()`). A plugin keeps its own
+method and delegates the body.
+
+### sports_game_renderer
+
+[`sports_game_renderer.py`](sports_game_renderer.py).
+`SportsGameRendererMixin`: the scroll/Vegas card geometry (centre gap, logo
+slot, layout offsets, upcoming-card date and time). No `__init__` and no
+state; add it as a base class of the plugin's game renderer and override
+what differs.
+
+### sports_helpers
+
+[`sports_helpers.py`](sports_helpers.py). Free functions `clamp_window()`,
+`clamp_seconds()`, `logo_needs_refresh()`, `spread_weighted_order()`, and
+`SportsHelpersMixin` with the scoreboards' `_mode_customization`,
+`_setting_int`, `_reset_dwell_on_reentry`, `_next_switch_index`,
+`_odds_color` and `_upcoming_date_and_time_text` under their existing names.
+Nothing in core uses it.
+
+### sports_scroll
+
+[`sports_scroll.py`](sports_scroll.py). `SportsScrollDisplay` and
+`SportsScrollDisplayManager`: the scroll-display orchestration the
+scoreboards share (Vegas items, dynamic duration, frame loop), paced through
+`scroll_config`. Subclasses supply `prepare_scroll_content()` and set
+`SCROLL_LEAGUE_KEYS`; see the module docstring for an example.
+
+### sports_shared
+
+[`sports_shared.py`](sports_shared.py). `SportsCoreSharedMixin`,
+`SportsLiveSharedMixin`, `SportsRecentSharedMixin`: the `sports.py` methods
+that were identical in every scoreboard (game selection and rotation,
+fonts, colours, dates, the switch-mode upcoming card). The docstring lists
+the attributes the host class must have and the three methods deliberately
+left out.
+
+### sync_manager
+
+[`sync_manager.py`](sync_manager.py). Core-internal. `DisplaySyncManager`
+links two displays as leader and follower (`sync.role` in config) over UDP
+port 5765, plus TCP on the next port for scroll images. The leader drives the
+scroll and sends the follower its part of each frame; a follower falls back
+to its own plugins when the leader goes quiet. Rows and columns must match.
+Created by `DisplayController`; works with any plugin.
+
+### text_helper
+
+[`text_helper.py`](text_helper.py). `TextHelper(font_dir=None, ...)`:
+`load_fonts()`, `draw_text_with_outline()`, `get_text_width()`,
+`get_text_dimensions()`, `center_text()`, `wrap_text()`,
+`draw_multiline_text()`, `create_text_image()`.
+
+## Logging
+
+Modules here create their logger with `logging.getLogger(__name__)`, which is
+the same logger `src.logging_config.get_logger(__name__)` returns. The helper
+classes (`APIHelper`, `LogoHelper`, `ScrollHelper`, `TextHelper`) and
+`espn_dates` take an optional `logger`. In a plugin, pass `self.logger`: it is
+created by `get_logger(..., plugin_id=...)` in `BasePlugin`, so messages carry
+the plugin id.
+
+## Adding a module
+
+- Keep it importable without hardware (see the test above).
+- Give it a module docstring that says what it is for and, if it is a mixin,
+  what the host class must provide.
+- Add it to the table on this page and, if plugins may import it, to the
+  CHANGELOG with the release to floor on.

src/common/api_helper.py

@@ -1,8 +1,9 @@
 """
 API Helper
 
-Handles HTTP requests, caching, and ESPN API integration for LED matrix plugins.
-Extracted from LEDMatrix core to provide reusable functionality for plugins.
+HTTP requests, response caching and ESPN fetch helpers for plugins
+(``from src.common import APIHelper``), plus the headers every core request
+sends (:data:`USER_AGENT`, :data:`DEFAULT_HTTP_HEADERS`).
 """
 
 import logging
@@ -36,13 +37,20 @@ DEFAULT_HTTP_HEADERS: Mapping[str, str] = MappingProxyType({
 
 class APIHelper:
     """
-    Helper class for HTTP requests, caching, and ESPN API integration.
-    
-    Provides functionality for:
-    - HTTP requests with retry logic and timeouts
-    - Response caching with TTL support
-    - ESPN API integration for sports data
-    - Request rate limiting and throttling
+    HTTP requests with retries, response caching and ESPN helpers.
+
+    - Requests go through one ``requests.Session`` that retries GET, HEAD
+      and OPTIONS on 429 and 5xx with exponential backoff, and sends
+      :data:`DEFAULT_HTTP_HEADERS`.
+    - Consecutive requests from one helper are spaced at least
+      ``set_rate_limit()`` seconds apart (1 second by default). A cache hit
+      does not count.
+    - With a ``cache_manager``, :meth:`get` caches the parsed JSON under
+      ``cache_key`` for ``cache_ttl`` seconds. The lifetime is stored with
+      the entry, so CacheManager honours it on every later read, whatever
+      max_age that read asks for.
+    - Failed requests are logged and return None; nothing here raises for a
+      network or HTTP error.
     """
     
     def __init__(self, cache_manager=None, default_timeout: int = 30,
@@ -73,13 +81,7 @@ class APIHelper:
         self.session.mount("https://", adapter)
         self.session.mount("http://", adapter)
         
-        # Default headers
-        self.session.headers.update({
-            'User-Agent': USER_AGENT,
-            'Accept': 'application/json',
-            'Accept-Language': 'en-US,en;q=0.9',
-            'Connection': 'keep-alive'
-        })
+        self.session.headers.update({**DEFAULT_HTTP_HEADERS, 'Connection': 'keep-alive'})
         
         # Rate limiting
         self._last_request_time = 0
@@ -102,9 +104,8 @@ class APIHelper:
         Returns:
             Response data as dictionary or None if request fails
         """
-        # Check cache first
         if cache_key and self.cache_manager:
-            cached = self._get_from_cache(cache_key)
+            cached = self._get_from_cache(cache_key, cache_ttl)
             if cached is not None:
                 self.logger.debug(f"Using cached response for {cache_key}")
                 return cached
@@ -268,33 +269,31 @@ class APIHelper:
         Args:
             key: Cache key
             data: Data to cache
-            ttl: Time-to-live in seconds (ignored - CacheManager doesn't support TTL)
+            ttl: Seconds the entry stays valid. Stored with the entry, so
+                it applies to every later read of ``key``.
         """
-        if self.cache_manager:
-            self.cache_manager.set(key, data)
+        self._set_cache(key, data, ttl)
     
     def get_cache(self, key: str) -> Optional[Any]:
         """
         Get cached data.
-        
+
         Args:
             key: Cache key
-            
+
         Returns:
-            Cached data or None if not found
+            Cached data, or None if there is none or it has expired. An
+            entry written with a ttl (set_cache, get) expires after that ttl;
+            one written without expires after CacheManager's default max_age.
         """
-        if self.cache_manager:
-            return self.cache_manager.get(key)
-        return None
+        return self._get_from_cache(key)
     
     def clear_cache(self, pattern: Optional[str] = None) -> None:
         """
         Clear cache data.
 
-        Uses CacheManager's real surface (clear_cache / delete /
-        list_cache_files); safely no-ops on managers without it. The old
-        implementation guarded on a nonexistent ``clear`` method, so it
-        silently never cleared anything.
+        Uses CacheManager's clear_cache(), or list_cache_files() and delete()
+        for a pattern. A cache manager without those methods is left alone.
 
         Args:
             pattern: Optional substring to match cache keys; only matching
@@ -315,21 +314,22 @@ class APIHelper:
                     "cannot clear by pattern")
         elif hasattr(self.cache_manager, 'clear_cache'):
             self.cache_manager.clear_cache()
-        elif hasattr(self.cache_manager, 'clear'):
-            self.cache_manager.clear()
         else:
             self.logger.debug("Cache manager exposes no clear method; no-op")
     
-    def _get_from_cache(self, key: str) -> Optional[Any]:
-        """Get data from cache."""
-        if self.cache_manager:
+    def _get_from_cache(self, key: str, max_age: Optional[int] = None) -> Optional[Any]:
+        """Cached data for ``key``, or None. ``max_age`` only matters for an
+        entry stored without a ttl; one stored with a ttl uses that."""
+        if not self.cache_manager:
+            return None
+        if max_age is None:
             return self.cache_manager.get(key)
-        return None
-    
-    def _set_cache(self, key: str, data: Any, ttl: int) -> None:
-        """Set data in cache."""
+        return self.cache_manager.get(key, max_age=max_age)
+
+    def _set_cache(self, key: str, data: Any, ttl: Optional[int]) -> None:
+        """Store ``data`` under ``key`` for ``ttl`` seconds."""
         if self.cache_manager:
-            self.cache_manager.set(key, data)
+            self.cache_manager.set(key, data, ttl=ttl)
     
     def _enforce_rate_limit(self) -> None:
         """Enforce rate limiting between requests."""

src/common/font_layout.py

@@ -67,10 +67,11 @@ _INSTALL_ROOT = Path(__file__).resolve().parents[2]
 def resolve_asset_path(relative_path: str) -> str:
     """Resolve a repo-relative asset path independently of the process cwd.
 
-    Prefers the path as given — so an absolute path is returned untouched and
-    behaviour is unchanged wherever the cwd already happened to be the install
-    root — then the install root derived above, then the original string so a
-    caller that wants to raise and fall back still can.
+    In order: an absolute path that exists is returned untouched; otherwise
+    ``relative_path`` under the install root derived above, if that exists;
+    otherwise ``relative_path`` unchanged, so a caller that wants to raise
+    and fall back still can. The cwd is never consulted, so a relative path
+    means the same file whichever directory the process started in.
 
     Without the fallback, any process started outside the install root (the
     plugin safety harness, a manual ``python run.py`` from ``$HOME``, a unit

src/common/logo_helper.py

@@ -11,7 +11,7 @@ from pathlib import Path
 from typing import Dict, List, Optional, Union
 
 import requests
-from PIL import Image
+from PIL import Image, ImageDraw
 from src.common.api_helper import USER_AGENT
 from src.common.permission_utils import (
     ensure_directory_permissions,
@@ -32,35 +32,14 @@ from src.common.permission_utils import (
 # trade for not re-warning about a file nobody is going to add.
 MISSING_LOGO_RECHECK_SECONDS = 3600.0
 
-#: Bounds on a user-supplied logo scale. Wide enough to be useful, closed
-#: enough that a typo cannot ask for a 4000px image on a 64px panel.
-MIN_LOGO_SCALE = 0.05
-MAX_LOGO_SCALE = 8.0
-
-
-def _usable_scale(scale) -> float:
-    """A scale that can be applied, or 1.0.
-
-    Anything unusable -- None, a string, zero, a negative, NaN, infinity --
-    means "as shipped", because the alternative is a blank panel from a
-    mistyped number.
-    """
-    try:
-        value = float(scale)
-    except (TypeError, ValueError):
-        return 1.0
-    if value != value or value in (float('inf'), float('-inf')):
-        return 1.0
-    if value < MIN_LOGO_SCALE or value > MAX_LOGO_SCALE:
-        return 1.0
-    return value
-
-
-
 # Well above any real team logo; bounds what a remote URL can write to disk.
 # The cap for every logo download: src.logo_downloader.fetch_logo uses it too.
 MAX_LOGO_BYTES = 10 * 1024 * 1024
 
+#: A logo's default bounding box, as a multiple of the panel's width and
+#: height, when the caller gives no max_width / max_height.
+DEFAULT_LOGO_BOX_FACTOR = 1.5
+
 
 class LogoHelper:
     """
@@ -119,12 +98,16 @@ class LogoHelper:
         Args:
             team_abbr: Team abbreviation for caching
             logo_path: Path to the logo file
-            max_width: Maximum width (defaults to display_width * 1.5)
-            max_height: Maximum height (defaults to display_height * 1.5)
+            max_width: Maximum width (default display_width *
+                DEFAULT_LOGO_BOX_FACTOR)
+            max_height: Maximum height (default display_height *
+                DEFAULT_LOGO_BOX_FACTOR)
             scale: User's size multiplier for this image, from
-                ``customization.layout.<element>.scale``. 1.0 is untouched and
-                takes exactly the path it always did. Callers hold the config,
-                so they resolve the element name; this only applies the number.
+                ``customization.layout.<element>.scale``; 1.0 leaves the box
+                as is. Callers hold the config, so they resolve the element
+                name; this only applies the number, clamped to
+                src.element_style's MIN_ELEMENT_SCALE..MAX_ELEMENT_SCALE.
+                A value that is not a finite positive number means 1.0.
 
         Returns:
             PIL Image object or None if loading fails
@@ -138,10 +121,13 @@ class LogoHelper:
         # key is size-qualified — a panel-size change must not return a
         # logo resized for the old dimensions.
         if max_width is None:
-            max_width = int(self.display_width * 1.5)
+            max_width = int(self.display_width * DEFAULT_LOGO_BOX_FACTOR)
         if max_height is None:
-            max_height = int(self.display_height * 1.5)
-        scale = _usable_scale(scale)
+            max_height = int(self.display_height * DEFAULT_LOGO_BOX_FACTOR)
+        # Imported here: src.element_style imports src.common (for bdf_font),
+        # whose __init__ imports this module.
+        from src.element_style import coerce_scale
+        scale = coerce_scale(scale, 1.0)
         if scale != 1.0:
             max_width = max(1, int(round(max_width * scale)))
             max_height = max(1, int(round(max_height * scale)))
@@ -374,9 +360,9 @@ class LogoHelper:
         nobody asked to grow would change every existing render.
         """
         if max_width is None:
-            max_width = int(self.display_width * 1.5)
+            max_width = int(self.display_width * DEFAULT_LOGO_BOX_FACTOR)
         if max_height is None:
-            max_height = int(self.display_height * 1.5)
+            max_height = int(self.display_height * DEFAULT_LOGO_BOX_FACTOR)
 
         # Only resize if necessary
         if logo.width <= max_width and logo.height <= max_height:
@@ -429,31 +415,26 @@ class LogoHelper:
                                max_width: Optional[int] = None,
                                max_height: Optional[int] = None) -> Optional[Image.Image]:
         """
-        Create a placeholder logo with team abbreviation.
-        
+        A stand-in for a logo that could not be loaded or downloaded: a
+        translucent grey box with a light outline, filling the logo box.
+        No text is drawn; ``team_abbr`` is only used in log messages.
+
         Args:
-            team_abbr: Team abbreviation to display
-            max_width: Maximum width
-            max_height: Maximum height
-            
+            team_abbr: Team the placeholder stands in for
+            max_width: Width (default display_width * DEFAULT_LOGO_BOX_FACTOR)
+            max_height: Height (default display_height * DEFAULT_LOGO_BOX_FACTOR)
+
         Returns:
-            PIL Image with placeholder logo
+            The RGBA placeholder, or None if it could not be created
         """
         try:
             if max_width is None:
-                max_width = int(self.display_width * 1.5)
+                max_width = int(self.display_width * DEFAULT_LOGO_BOX_FACTOR)
             if max_height is None:
-                max_height = int(self.display_height * 1.5)
+                max_height = int(self.display_height * DEFAULT_LOGO_BOX_FACTOR)
             
-            # Create placeholder image
             placeholder = Image.new('RGBA', (max_width, max_height), (0, 0, 0, 0))
-            
-            # This would require a font, so we'll create a simple colored rectangle
-            # In a real implementation, you'd want to add text rendering here
-            from PIL import ImageDraw
             draw = ImageDraw.Draw(placeholder)
-            
-            # Draw a simple rectangle with team abbreviation
             draw.rectangle([0, 0, max_width-1, max_height-1], 
                           fill=(100, 100, 100, 200), outline=(200, 200, 200, 255))
             

src/common/permission_utils.py

@@ -241,7 +241,8 @@ def get_assets_dir_mode() -> int:
     Return permission mode for asset directories.
     
     Returns:
-        Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable directories
+        Permission mode: 0o2775 (rwxrwsr-x): group-writable, and setgid so
+        entries created in it take the directory's group
     """
     return 0o2775  # rwxrwsr-x (setgid + group writable)
 
@@ -251,7 +252,8 @@ def get_config_dir_mode() -> int:
     Return permission mode for config directory.
     
     Returns:
-        Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable directories
+        Permission mode: 0o2775 (rwxrwsr-x): group-writable, and setgid so
+        entries created in it take the directory's group
     """
     return 0o2775  # rwxrwsr-x (setgid + group writable)
 
@@ -271,7 +273,8 @@ def get_plugin_dir_mode() -> int:
     Return permission mode for plugin directories.
     
     Returns:
-        Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable directories
+        Permission mode: 0o2775 (rwxrwsr-x): group-writable, and setgid so
+        entries created in it take the directory's group
     """
     return 0o2775  # rwxrwsr-x (setgid + group writable)
 
@@ -281,7 +284,8 @@ def get_cache_dir_mode() -> int:
     Return permission mode for cache directories.
 
     Returns:
-        Permission mode: 0o2775 (rwxrwxr-x + sticky bit) for group-writable cache directories
+        Permission mode: 0o2775 (rwxrwsr-x): group-writable, and setgid so
+        entries created in it take the directory's group
     """
     return 0o2775  # rwxrwsr-x (setgid + group writable)
 

src/common/snapshot_policy.py

@@ -5,7 +5,7 @@ serves two consumers with different needs:
 
 - The web UI's live preview (SSE reader in web_interface/app.py) wants
   fresh frames — but only while a browser is actually watching.
-- The health check (web_interface/blueprints/api_v3.py, hardware status)
+- The health check (web_interface/blueprints/api_v3/misc.py, hardware status)
   uses the file's AGE as a liveness proxy: age >= 60s reads as degraded.
 
 PNG-encoding every frame at 5 fps forever — identical frames, no viewers —
@@ -27,7 +27,7 @@ Policy:
   TOUCH_INTERVAL so the health check (60s threshold) never degrades.
 
 If any constant here changes, re-check the health threshold in
-api_v3.py (get_hardware_status) — TOUCH_INTERVAL must stay well under it.
+api_v3/misc.py (get_hardware_status) — TOUCH_INTERVAL must stay well under it.
 """
 
 from enum import Enum
@@ -37,7 +37,7 @@ VIEWER_INTERVAL = 0.2
 # Snapshot cadence with no viewers — cheap freshness for page-open (seconds).
 IDLE_INTERVAL = 30.0
 # Max age of the last write/touch before bumping mtime for the health
-# check. MUST stay well under api_v3's 60s degraded threshold.
+# check. MUST stay well under get_hardware_status's 60s degraded threshold.
 TOUCH_INTERVAL = 20.0
 # A viewer marker older than this no longer counts as a live viewer.
 VIEWER_MARKER_FRESH_SEC = 5.0

src/common/sports_card.py

@@ -101,11 +101,10 @@ def element_color(config: Optional[Dict[str, Any]], element: str,
                   mode: Optional[str] = None):
     """Per-element text colour from customization.<element>.text_color.
 
-    Delegated rather than reimplemented: there were two copies of this
-    read and three of the offset read, and the shared one also resolves
-    the element under the names plugins actually use (the layout block
-    says `score` where the style block says `score_text`) and honours a
-    per-mode override. Hex strings are still accepted.
+    Delegates to src.element_style.element_color, which also resolves the
+    element under the names plugins actually use (the layout block says
+    `score` where the style block says `score_text`) and honours a per-mode
+    override. Hex strings are accepted.
     """
     from src.element_style import element_color as _shared
     return _shared(config, element, default, mode)
@@ -125,12 +124,11 @@ def resolve_font_color(config: Optional[Dict[str, Any]],
     One object can legitimately belong to several elements -- a size resolver
     can land two of them on the same face, and a BDF face cannot be un-shared
     at all because ``freetype.Face`` objects cannot be rebuilt from a path.
-    Those draws used to go out white, which is how an element rendered in any
-    of the 32 shipped bitmap fonts could silently lose a colour the user had
-    set. So ambiguity is now narrowed before it is given up on: among the
+    Ambiguity is therefore narrowed before it is given up on: among the
     elements sharing a face, a single configured colour is the only thing the
     user can have meant, and several that agree mean the same thing. Only a
-    genuine disagreement falls back to *default*.
+    genuine disagreement falls back to *default* -- otherwise an element
+    drawn in any of the shipped bitmap fonts could lose a colour the user set.
 
     The element vocabulary is a parameter because the two callers disagree
     about it -- the mixin's map says ``team_text`` where this module's says
@@ -483,7 +481,7 @@ def unshare_element_fonts(logger, fonts, element_for_font=None):
     with identical metrics, so nothing about the rendering changes; only
     the ability to tell two elements apart does. Faces that cannot be
     rebuilt (a BDF loaded through freetype.Face, anything without a usable
-    path) are left shared, and their draws stay white as before.
+    path) are left shared; resolve_font_color then picks their colour.
 
     *element_for_font* names the font keys to consider, in order (the first
     holder of a face keeps it); it defaults to this module's
@@ -491,10 +489,8 @@ def unshare_element_fonts(logger, fonts, element_for_font=None):
     which names different keys -- see ``resolve_font_color`` for why the two
     vocabularies are kept apart.
     """
-    try:
-        from src.common.font_layout import load_truetype as _load
-    except ImportError:  # pragma: no cover
-        return fonts
+    # Looked up at call time so tests can spy on the pinned loader.
+    from src.common.font_layout import load_truetype
     if element_for_font is None:
         element_for_font = ELEMENT_FOR_FONT
     seen = {}
@@ -509,7 +505,7 @@ def unshare_element_fonts(logger, fonts, element_for_font=None):
         if not path or not size:
             continue
         try:
-            fonts[key] = _load(path, size)
+            fonts[key] = load_truetype(path, size)
         except (OSError, ValueError, TypeError):
             logger.debug(
                 "Could not un-share the %s face; it keeps the default colour", key)

src/common/sports_game_renderer.py

@@ -56,21 +56,13 @@ class SportsGameRendererMixin:
 
     # ---- geometry ------------------------------------------------------
 
-    # Non-finite settings are rejected before any int()/round(): "inf" reaches
-    # these from config as a float or a string, passes an `isinstance` plus
-    # `>= 0` check unharmed, and then raises OverflowError out of int() --
-    # which the old `except (TypeError, ValueError)` did not catch, so it
-    # aborted the whole card render. Present in all eight plugins before this
-    # moved to the core; fixing it here fixes it in all eight.
-
     def _score_reserve_width(self) -> int:
-        """Centre strip the score actually needs, measured rather than assumed.
+        """Centre strip the score needs: the width of _SCORE_PROBE in the
+        score font plus a gutter each side, or 0 if it cannot be measured.
 
-        The gap was derived from the card width alone (width x
-        CENTER_GAP_RATIO, clamped to CENTER_GAP_MAX_PX) while the score's size
-        comes from config and the element-style resolver. Nothing compared the
-        two, so any score wider than the clamp was drawn over the logos.
-        Measuring it keeps the strip wide enough for whatever font is in play.
+        Measured rather than derived from the card width, because the score's
+        size comes from config and the element-style resolver: a strip sized
+        from the width alone lets a large score run over the logos.
         """
         try:
             probe = ImageDraw.Draw(Image.new("RGB", (4, 4)))
@@ -87,6 +79,10 @@ class SportsGameRendererMixin:
         the card width between the configurable min and max. 0 restores
         edge-to-edge logos.
         """
+        # Non-finite settings are rejected before any int()/round(): "inf"
+        # arrives from config as a float or a string, passes the isinstance
+        # and >= 0 checks, and int() then raises OverflowError, which would
+        # abort the whole card render.
         configured = self._scroll_card_option("center_gap")
         if (isinstance(configured, (int, float))
                 and math.isfinite(configured) and configured >= 0):
@@ -110,10 +106,9 @@ class SportsGameRendererMixin:
     def _logo_slot_width(self) -> int:
         """Per-side logo slot, leaving the center gap clear.
 
-        No longer capped at display_height: the card is sized as two
-        full-height logos plus the measured gap, so what is left after the gap
-        is exactly the logo's share. The cap was what froze the logos at 46px
-        on the old flat 128px card.
+        Not capped at display_height: the card is sized as two full-height
+        logos plus the measured gap, so what is left after the gap is exactly
+        the logo's share. At least 8 px.
         """
         available = (self.display_width - self._center_gap_width()) // 2
         return max(8, available)
@@ -130,9 +125,8 @@ class SportsGameRendererMixin:
         """X/Y nudge for one element, from customization.layout.
 
         Same block the full-screen scorebug reads (sports.py
-        _get_layout_offset), so a nudge configured in the web UI now moves
-        the element on the scroll/Vegas card too -- previously the schema
-        advertised these offsets but this renderer ignored them.
+        _get_layout_offset), so a nudge configured in the web UI moves the
+        element on the scroll/Vegas card as well as on the scorebug.
         """
         from src.element_style import layout_offset
         return layout_offset(self.config, element, axis, default,

src/common/sports_scroll.py

@@ -487,9 +487,9 @@ class SportsScrollDisplayManager:
             )
         except Exception:
             # prepare_scroll_content is subclass-implemented and builds cards
-            # straight from feed data, which is exactly where this PR's other
-            # crashes came from. One sport's bad payload must not take down the
-            # shared orchestration for the others.
+            # straight from feed data, so it can raise on a malformed payload.
+            # One sport's bad payload must not take down the shared
+            # orchestration for the others.
             self.logger.exception(
                 "Error preparing scroll content for game_type=%s", game_type
             )

src/common/sports_shared.py

@@ -97,11 +97,11 @@ from datetime import datetime, timedelta, timezone
 from typing import Any, ClassVar, Dict, List, Optional, Tuple
 
 import pytz
-from src.common.espn_dates import fetch_espn_scoreboard
+from src.common.espn_dates import ESPN_MAX_LIMIT, fetch_espn_scoreboard
 import requests
-from PIL import Image, ImageDraw, ImageFont
+from PIL import Image, ImageDraw
 from src.common import sports_card as _card
-from src.common.font_layout import load_truetype
+from src.common.font_layout import load_truetype, resolve_asset_path
 
 logger = logging.getLogger(__name__)
 
@@ -132,36 +132,16 @@ def _resolve_font_path(path: str) -> str:
     load raises, the caller falls back, and the scoreboard renders in PIL's
     default face instead of the pixel font it was laid out for.
 
-    Resolution order matches the core's own resolver: the path as given
-    first, so behaviour is unchanged wherever it already worked and a
-    configured absolute path is returned untouched, then the core install
-    root, then the original string so callers still raise and fall back
-    exactly as they do today.
+    Resolution order: the path as given, relative to the cwd, when it
+    exists -- the order the scoreboards' own sports.py copies used, so a
+    process running from another checkout keeps that checkout's fonts --
+    then :func:`src.common.font_layout.resolve_asset_path` (the install
+    root), which returns the original string when neither exists so callers
+    still raise and fall back.
     """
     if os.path.exists(path):
         return path
-    try:
-        import src.font_manager as _core_fonts
-
-        # The core grew this resolver in ChuckBuilds/LEDMatrix#425. Use it
-        # when it is there so both repos stay on one definition of "install
-        # root"; older cores fall through to the equivalent derivation below.
-        manager = getattr(_core_fonts, "FontManager", None)
-        resolver = getattr(manager, "_resolve_asset_path", None)
-        if resolver is not None:
-            resolved = resolver(path)
-            if resolved and os.path.exists(resolved):
-                return resolved
-        root = os.path.dirname(os.path.dirname(os.path.abspath(_core_fonts.__file__)))
-        candidate = os.path.join(root, path)
-        if os.path.exists(candidate):
-            return candidate
-    except (ImportError, AttributeError, OSError):
-        # No core on the path (standalone tooling), a core laid out
-        # differently, or an unreadable install. Returning the original keeps
-        # the caller's existing fallback intact.
-        return path
-    return path
+    return resolve_asset_path(path)
 
 
 class SportsCoreSharedMixin:
@@ -201,9 +181,6 @@ class SportsCoreSharedMixin:
     #: How long to stay quiet between ranking-coverage warnings.
     _RANKING_COVERAGE_SECONDS: ClassVar[int] = 60 * 60
 
-    def _get_season_schedule_dates(self) -> tuple[str, str]:
-        return "", ""
-
     def _draw_scorebug_layout(self, game: Dict, force_clear: bool = False) -> None:
         """Placeholder draw method - subclasses should override."""
         # This base method will be simple, subclasses provide specifics
@@ -889,7 +866,10 @@ class SportsCoreSharedMixin:
         draw.text((x, y), text, font=font, fill=fill)
 
     def _should_log(self, warning_type: str, cooldown: int = 60) -> bool:
-        """Check if we should log a warning based on cooldown period."""
+        """True at most once per ``cooldown`` seconds, for rate-limiting a
+        warning. The cooldown is shared by every warning on this manager:
+        ``warning_type`` is part of the signature scoreboards inherit, but
+        does not give each type its own cooldown."""
         current_time = time.time()
         if current_time - self._last_warning_time > cooldown:
             self._last_warning_time = current_time
@@ -904,8 +884,6 @@ class SportsCoreSharedMixin:
         try:
             # Fetch current week and next few days for immediate display
             now = datetime.now(pytz.utc)
-            immediate_events = []
-
             start_date = now - timedelta(days=self.schedule_lookback_days)
             end_date = now + timedelta(days=self.schedule_lookahead_days)
             date_str = f"{start_date.strftime('%Y%m%d')}-{end_date.strftime('%Y%m%d')}"
@@ -913,7 +891,7 @@ class SportsCoreSharedMixin:
             data = fetch_espn_scoreboard(
                 self.session,
                 url,
-                params={"dates": date_str, "limit": 1000},
+                params={"dates": date_str, "limit": ESPN_MAX_LIMIT},
                 headers=self.headers,
                 timeout=10,
                 logger=self.logger,

src/common/sync_manager.py

@@ -32,7 +32,7 @@ from typing import Callable, Optional
 import numpy as np
 from PIL import Image
 
-from src.display_geometry import DEFAULT_CHAIN_LENGTH
+from src.display_geometry import DEFAULT_CHAIN_LENGTH, DEFAULT_COLS, DEFAULT_ROWS
 
 # 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
@@ -75,9 +75,12 @@ class FollowerState(Enum):
 class DisplaySyncManager:
     """
     Core sync manager.  Instantiated by DisplayController based on config['sync'].
-    Leader sends compressed PNG frames to the follower after each render cycle.
-    Follower renders received frames; returns to own plugin stack when leader
-    goes offline.
+
+    The leader sends each rendered frame to the follower over UDP as raw RGB
+    bytes (send_frame), and for Vegas scrolling sends the whole scroll image
+    once per cycle as a PNG over TCP on port + 1 (send_scroll_image), then
+    only the scroll position. The follower draws what it receives and goes
+    back to its own plugins when the leader stops sending.
     """
 
     def __init__(
@@ -192,8 +195,8 @@ class DisplaySyncManager:
 
     def _handle_hello(self, msg: dict, sender_ip: str) -> None:
         hw = self._hw_config
-        local_rows = hw.get("rows", 32)
-        local_cols = hw.get("cols", 64)
+        local_rows = hw.get("rows", DEFAULT_ROWS)
+        local_cols = hw.get("cols", DEFAULT_COLS)
         peer_rows = int(msg.get("rows", 0))
         peer_cols = int(msg.get("cols", 0))
         peer_chain = int(msg.get("chain", DEFAULT_CHAIN_LENGTH))
@@ -469,16 +472,23 @@ class DisplaySyncManager:
         """Record a decoded leader frame and enter follower mode if needed."""
         with self._frame_lock:
             self._latest_frame = img
+        self._enter_follower_mode(sender_ip)
+
+    def _enter_follower_mode(self, sender_ip: str) -> bool:
+        """Note that the leader at ``sender_ip`` just sent something, and
+        switch from standalone to follower mode if not already following.
+        Returns True if this call made the switch."""
         self._last_leader_frame_time = time.time()
         self._leader_ip = sender_ip
-
-        if self._follower_state == FollowerState.STANDALONE:
-            self._follower_state = FollowerState.FOLLOWER
-            self.logger.info(
-                "Sync: leader active at %s — switching to follower mode",
-                sender_ip,
-            )
-            self.write_status_file()
+        if self._follower_state != FollowerState.STANDALONE:
+            return False
+        self._follower_state = FollowerState.FOLLOWER
+        self.logger.info(
+            "Sync: leader active at %s — switching to follower mode",
+            sender_ip,
+        )
+        self.write_status_file()
+        return True
 
     def _follower_recv_loop(self) -> None:
         while self._running:
@@ -559,15 +569,7 @@ class DisplaySyncManager:
                                 # back from. Treat it as malformed.
                                 raise ValueError(f"non-finite scroll x: {msg['x']!r}")
                             self._latest_scroll_x = scroll_x
-                            self._last_leader_frame_time = time.time()
-                            self._leader_ip = sender_ip
-                            if self._follower_state == FollowerState.STANDALONE:
-                                self._follower_state = FollowerState.FOLLOWER
-                                self.logger.info(
-                                    "Sync: leader active at %s — switching to follower mode",
-                                    sender_ip,
-                                )
-                                self.write_status_file()
+                            if self._enter_follower_mode(sender_ip):
                                 fire_new_cycle = True  # build initial scroll image
                         elif t == "nc":
                             # Leader started a new scroll cycle — rebuild local image
@@ -589,8 +591,8 @@ class DisplaySyncManager:
         hw = self._hw_config
         hello = json.dumps({
             "t": "hello",
-            "rows": hw.get("rows", 32),
-            "cols": hw.get("cols", 64),
+            "rows": hw.get("rows", DEFAULT_ROWS),
+            "cols": hw.get("cols", DEFAULT_COLS),
             "chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH),
         }).encode("utf-8")
         heartbeat = json.dumps({"t": "hb"}).encode("utf-8")
@@ -660,8 +662,8 @@ class DisplaySyncManager:
         base = {
             "role": self.role.value,
             "port": self.port,
-            "local_rows": hw.get("rows", 32),
-            "local_cols": hw.get("cols", 64),
+            "local_rows": hw.get("rows", DEFAULT_ROWS),
+            "local_cols": hw.get("cols", DEFAULT_COLS),
             "local_chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH),
         }
 

src/common/text_helper.py

@@ -10,7 +10,7 @@ from pathlib import Path
 from typing import Dict, List, Optional, Tuple, Union
 
 from PIL import Image, ImageDraw, ImageFont
-from src.common.font_layout import load_truetype
+from src.common.font_layout import load_truetype, resolve_asset_path
 
 # Shared throwaway draw surface for measuring text without a target canvas.
 _measure_draw = ImageDraw.Draw(Image.new("RGB", (1, 1)))
@@ -18,13 +18,14 @@ _measure_draw = ImageDraw.Draw(Image.new("RGB", (1, 1)))
 
 class TextHelper:
     """
-    Helper class for text rendering with outlines and font management.
-    
-    Provides functionality for:
-    - Loading and managing fonts
-    - Drawing text with outlines for better readability
-    - Calculating text dimensions and positioning
-    - Managing font resources
+    Font loading, outlined text and text measurement for plugins.
+
+    - :meth:`load_fonts` loads TrueType fonts from ``font_dir`` (the install's
+      assets/fonts by default) with the layout engine pinned
+      (font_layout.load_truetype). Each (file, size) is loaded once per helper
+      and reused; a missing or unloadable file becomes PIL's default font.
+    - :meth:`draw_text_with_outline` and friends draw onto a caller's
+      ``ImageDraw``; the measuring methods need no canvas.
     """
     
     def __init__(self, font_dir: Optional[Union[str, Path]] = None, 
@@ -33,22 +34,26 @@ class TextHelper:
         Initialize the TextHelper.
         
         Args:
-            font_dir: Directory containing font files (defaults to assets/fonts)
+            font_dir: Directory containing font files. Defaults to the
+                install's assets/fonts, whatever the process cwd is.
             logger: Optional logger instance
         """
         self.logger = logger or logging.getLogger(__name__)
-        self.font_dir = Path(font_dir) if font_dir else Path("assets/fonts")
+        self.font_dir = Path(font_dir) if font_dir else Path(resolve_asset_path("assets/fonts"))
+        # "<path>:<size>" -> loaded font; see load_fonts.
         self._font_cache: Dict[str, ImageFont.ImageFont] = {}
     
     def load_fonts(self, font_config: Optional[Dict[str, Dict]] = None) -> Dict[str, ImageFont.ImageFont]:
         """
         Load fonts for different text elements.
-        
+
         Args:
-            font_config: Custom font configuration dictionary
-            
+            font_config: ``{name: {"file": <file in font_dir>, "size": <px>}}``;
+                defaults to the scoreboard set in _get_default_font_config.
+
         Returns:
-            Dictionary mapping font names to PIL ImageFont objects
+            Dictionary mapping font names to PIL ImageFont objects. A font
+            already loaded by this helper at the same size is reused.
         """
         if font_config is None:
             font_config = self._get_default_font_config()
@@ -61,9 +66,13 @@ class TextHelper:
                 size = config['size']
                 
                 if font_path.exists():
-                    font = load_truetype(str(font_path), size)
+                    cache_key = f"{font_path}:{size}"
+                    font = self._font_cache.get(cache_key)
+                    if font is None:
+                        font = load_truetype(str(font_path), size)
+                        self._font_cache[cache_key] = font
+                        self.logger.debug(f"Loaded font: {font_name} ({font_path}, size {size})")
                     fonts[font_name] = font
-                    self.logger.debug(f"Loaded font: {font_name} ({font_path}, size {size})")
                 else:
                     # Fallback to default font
                     font = ImageFont.load_default()
@@ -115,12 +124,7 @@ class TextHelper:
         Returns:
             Width in pixels
         """
-        try:
-            return int(_measure_draw.textlength(text, font=font))
-        except AttributeError:
-            # Fallback for older PIL versions
-            bbox = _measure_draw.textbbox((0, 0), text, font=font)
-            return bbox[2] - bbox[0]
+        return int(_measure_draw.textlength(text, font=font))
     
     def get_text_height(self, text: str, font: ImageFont.ImageFont) -> int:
         """

src/config_manager.py

@@ -16,8 +16,10 @@ additionally keeps rotating backups in ``config/backups/``.
 Plugin configuration
 --------------------
 Plugin configs are stored inside ``config.json`` under the plugin's ID key
-and survive plugin reinstalls.  Use :meth:`ConfigManager.update_plugin_config`
-to write plugin settings; never write directly to the plugin directory.
+and survive plugin reinstalls. Write them by saving the whole config with
+:meth:`ConfigManager.save_config_atomic` (or
+:meth:`ConfigManager.save_raw_file_content`); never write settings into the
+plugin directory, which a reinstall deletes.
 
 Hot-reload
 ----------
@@ -127,13 +129,10 @@ class ConfigManager:
         # Update in-memory config if save was successful
         if result.status == SaveResultStatus.SUCCESS:
             self.config = new_config_data
-            # In-memory config now matches what was just written; refresh
-            # the load signature so the fast path stays valid. NOTE: the
-            # in-memory copy includes merged secrets; the on-disk file has
-            # them stripped — the fast path returning self.config preserves
-            # exactly the pre-cache behavior (load-after-save also returned
-            # the secret-merged self.config only after re-reading secrets;
-            # here secrets file is unchanged, so contents are equivalent).
+            # In-memory config now matches what was just written, so the
+            # load_config fast path may return it. It still carries the
+            # merged secrets that were stripped on disk; that matches a full
+            # reload, because the secrets file was not changed by the save.
             self._loaded_sig = self._files_signature()
             self.logger.info(f"Configuration successfully saved atomically to {os.path.abspath(self.config_path)}")
         elif result.status == SaveResultStatus.ROLLED_BACK:
@@ -253,11 +252,11 @@ class ConfigManager:
             return self.config
             
         except FileNotFoundError as e:
-            if str(e).find('config_secrets.json') == -1:  # Only raise if main config is missing
-                error_msg = f"Configuration file not found at {os.path.abspath(self.config_path)}"
-                self.logger.error(error_msg, exc_info=True)
-                raise ConfigError(error_msg, config_path=self.config_path) from e
-            return self.config
+            # Only config.json can get here: a missing or unreadable secrets
+            # file is handled where it is read.
+            error_msg = f"Configuration file not found at {os.path.abspath(self.config_path)}"
+            self.logger.error(error_msg, exc_info=True)
+            raise ConfigError(error_msg, config_path=self.config_path) from e
         except json.JSONDecodeError as e:
             error_msg = f"Error parsing configuration file {os.path.abspath(self.config_path)}"
             self.logger.error(error_msg, exc_info=True)
@@ -320,10 +319,9 @@ class ConfigManager:
         A missing secrets file is fine (nothing to strip). But a file that
         EXISTS and cannot be read or parsed means stripping is impossible —
         and the in-memory config being saved has secrets deep-merged into it,
-        so proceeding would write them into config.json in plaintext. That
-        was the historical behavior; it is now a hard refusal. The save
-        raises so the caller (and user) fixes the secrets file instead of
-        silently leaking its contents into the world-readable main config.
+        so proceeding would write them into config.json in plaintext. The
+        save raises instead, so the caller (and user) fixes the secrets file
+        rather than leaking its contents into the world-readable main config.
         """
         if not os.path.exists(self.secrets_path):
             return {}
@@ -465,9 +463,8 @@ class ConfigManager:
                 # Merge template defaults into current config
                 self._merge_template_defaults(self.config, template_config)
                 
-                # Save migrated config using atomic save to preserve permissions
-                # Use atomic save to preserve file permissions
-                # Note: save_config_atomic handles secrets internally
+                # save_config_atomic strips the merged secrets back out and
+                # keeps the file's owner and mode.
                 result = self.save_config_atomic(
                     new_config_data=self.config,
                     create_backup=False,  # Already created backup above
@@ -600,20 +597,16 @@ class ConfigManager:
 
             self.logger.info(f"{file_type.capitalize()} configuration successfully saved to {os.path.abspath(path_to_save)}")
             
-            # If we just saved the main config or secrets, the merged self.config might be stale.
-            # Reload it to reflect the new state.
-            # Note: We wrap this in try-except because reload failures (e.g., migration errors)
-            # should not cause the save operation to fail - the file was saved successfully.
-            if file_type == "main" or file_type == "secrets":
-                try:
-                    self.load_config()
-                except Exception as reload_error:
-                    # Log the reload error but don't fail the save operation
-                    # The file was saved successfully, reload is just for in-memory consistency
-                    self.logger.warning(
-                        f"Configuration file saved successfully, but reload failed: {reload_error}. "
-                        f"The file on disk is valid, but in-memory config may be stale."
-                    )
+            # The merged self.config is now stale; reload it. A reload failure
+            # (a migration error, say) is logged, not raised: the file itself
+            # was saved.
+            try:
+                self.load_config()
+            except Exception as reload_error:
+                self.logger.warning(
+                    f"Configuration file saved successfully, but reload failed: {reload_error}. "
+                    f"The file on disk is valid, but in-memory config may be stale."
+                )
 
         except PermissionError as e:
             # Provide helpful error message with fix instructions
@@ -670,7 +663,7 @@ class ConfigManager:
         try:
             # Load current configs
             main_config = self.get_raw_file_content('main')
-            secrets_config = self.get_raw_file_content('secrets') if os.path.exists(self.secrets_path) else {}
+            secrets_config = self.get_raw_file_content('secrets')  # {} when there is no file
             
             # Remove plugin from main config
             if plugin_id in main_config:
@@ -703,7 +696,7 @@ class ConfigManager:
         try:
             # Load current configs
             main_config = self.get_raw_file_content('main')
-            secrets_config = self.get_raw_file_content('secrets') if os.path.exists(self.secrets_path) else {}
+            secrets_config = self.get_raw_file_content('secrets')  # {} when there is no file
             
             valid_set = set(valid_plugin_ids)
             

src/dynamic_team_resolver.py

@@ -21,6 +21,8 @@ import time
 import requests
 from typing import Dict, List
 
+from src.common.api_helper import DEFAULT_HTTP_HEADERS
+
 logger = logging.getLogger(__name__)
 
 class DynamicTeamResolver:
@@ -141,7 +143,9 @@ class DynamicTeamResolver:
             self.logger.info("Fetching fresh NCAA Football rankings from ESPN API")
             rankings_url = "https://site.api.espn.com/apis/site/v2/sports/football/college-football/rankings"
             
-            response = requests.get(rankings_url, timeout=self.request_timeout)
+            # ESPN rejects requests' default User-Agent; see api_helper.USER_AGENT.
+            response = requests.get(rankings_url, headers=dict(DEFAULT_HTTP_HEADERS),
+                                    timeout=self.request_timeout)
             response.raise_for_status()
             data = response.json()
             

src/element_style.py

@@ -90,6 +90,17 @@ def _cache_put(key: Tuple[str, int], value: Tuple[Any, int]) -> None:
 # Config keys a style element block carries, in schema/UI order.
 _STYLE_KEYS = ('font', 'font_size', 'text_color', 'visible', 'align')
 
+# Title of every generated `layout` (x/y offset) group in the config form.
+_LAYOUT_TITLE = 'Layout Offsets'
+
+#: Bounds on a user-set ``customization.layout.<element>.scale``. They are the
+#: Scale field's minimum and maximum in the generated schema, and every reader
+#: (coerce_scale, element_scale, LogoHelper.load_logo) clamps to them, so the
+#: web form and the renderer agree. Below 0.1 a logo is a dot; ten times a
+#: panel-sized box is already far off the panel.
+MIN_ELEMENT_SCALE = 0.1
+MAX_ELEMENT_SCALE = 10.0
+
 
 @dataclass(frozen=True)
 class ElementStyle:
@@ -301,7 +312,7 @@ def expand_style_elements(schema: Dict[str, Any]) -> Dict[str, Any]:
         if layout_props:
             layout = props.setdefault('layout', {
                 'type': 'object',
-                'title': 'Layout Offsets',
+                'title': _LAYOUT_TITLE,
                 'description': 'Pixel offsets applied to each element '
                                '(positive x moves right, positive y moves down)',
                 'x-advanced': True,
@@ -345,16 +356,14 @@ def _element_block_from_spec(element_key: str,
             'type': 'string',
             'title': 'Font Family',
             'x-advanced': True,
-            # The core already ships this widget and the config form already
-            # allowlists it; without the hint the field rendered as a bare
-            # text box the user had to type a filename into.
+            # The core's font picker; without the hint the form renders a
+            # bare text box the user has to type a filename into.
             'x-widget': 'font-selector',
         }
         # A bitmap font ignores font_size and renders at its own baked-in
         # size, so the size ceiling has to be enforced when picking the
         # font, not when setting the size.
-        max_size = (size_spec or {}).get('max') if isinstance(
-            spec.get('size'), dict) else None
+        max_size = size_spec.get('max') if size_spec else None
         if isinstance(max_size, (int, float)):
             font_prop['x-options'] = {'maxFixedSize': max_size}
         if 'default' in font_spec:
@@ -457,8 +466,8 @@ def _offset_block_from_spec(element_key: str,
             'title': 'Scale',
             'description': 'Size multiplier; 1 is the shipped size.',
             'default': 1.0,
-            'minimum': 0.1,
-            'maximum': 10.0,
+            'minimum': MIN_ELEMENT_SCALE,
+            'maximum': MAX_ELEMENT_SCALE,
             'x-advanced': True,
         }
         if isinstance(scale_spec, dict):
@@ -539,7 +548,7 @@ def _modes_block(declaration: Dict[str, Any],
         if layout_props:
             element_props['layout'] = {
                 'type': 'object',
-                'title': 'Layout Offsets',
+                'title': _LAYOUT_TITLE,
                 'x-advanced': True,
                 'additionalProperties': False,
                 'properties': layout_props,
@@ -680,7 +689,7 @@ def _modes_block_from_properties(props: Dict[str, Any], element_keys: list,
         if layout_props:
             element_props['layout'] = {
                 'type': 'object',
-                'title': 'Layout Offsets',
+                'title': _LAYOUT_TITLE,
                 'x-advanced': True,
                 'additionalProperties': False,
                 'properties': layout_props,
@@ -1010,12 +1019,15 @@ def _coerce_align(value: Any) -> Optional[str]:
     return None
 
 
-def _coerce_scale(value: Any, default: float) -> float:
-    """A positive size multiplier, or ``default``.
+def coerce_scale(value: Any, default: float = 1.0) -> float:
+    """A usable size multiplier: ``value`` clamped to
+    [MIN_ELEMENT_SCALE, MAX_ELEMENT_SCALE], or ``default``.
 
-    Clamped rather than merely validated: a scale of 0 or a negative one is
-    a zero-or-inverted image, and the panel is 32 pixels tall -- a typo
-    should cost a wrong size, not a crash inside PIL.
+    ``default`` is returned for anything that is not a finite positive number
+    (None, a bool, a string, 0, a negative, NaN, infinity): those are typos,
+    and a typo should cost the shipped size, not a blank or inverted image or
+    a crash inside PIL. A positive number outside the range is a real request
+    for "smaller" or "bigger", so it is clamped rather than ignored.
     """
     if isinstance(value, bool) or value is None:
         return default
@@ -1023,9 +1035,9 @@ def _coerce_scale(value: Any, default: float) -> float:
         scale = float(value)
     except (TypeError, ValueError):
         return default
-    if scale <= 0:
+    if not math.isfinite(scale) or scale <= 0:
         return default
-    return min(scale, 10.0)
+    return min(max(scale, MIN_ELEMENT_SCALE), MAX_ELEMENT_SCALE)
 
 
 def _coerce_offset(value: Any, default: int, element_key: str,
@@ -1194,7 +1206,7 @@ def element_scale(config: Any, element_key: str, default: float = 1.0,
     try:
         value = _element_field(config, element_key, 'scale', mode,
                                in_layout=True)
-        return default if value is None else _coerce_scale(value, default)
+        return default if value is None else coerce_scale(value, default)
     except Exception as e:
         logger.warning("Error reading scale for %s: %s", element_key, e)
         return default
@@ -1371,12 +1383,6 @@ class ElementStyleResolver:
             return {}
         return _lookup_element(block.get('layout'), element_key)
 
-    @classmethod
-    def _layout_axis(cls, block: Dict[str, Any], element_key: str,
-                     axis: str) -> Any:
-        """``block['layout'][element][axis]``, or None if absent anywhere."""
-        return cls._layout_element(block, element_key).get(axis)
-
 
     # -- resolution internals -----------------------------------------------
 
@@ -1497,7 +1503,7 @@ class ElementStyleResolver:
             user_forced_color=bool(color_forced),
             visible=_coerce_bool(visible, True),
             align=_coerce_align(align),
-            scale=_coerce_scale(scale, 1.0),
+            scale=coerce_scale(scale, 1.0),
         )
 
     def _classic_style(self, classic_font: str, classic_size: int,

src/error_aggregator.py

@@ -26,6 +26,19 @@ from src.exceptions import LEDMatrixError
 from src.redaction import redact_credentials
 
 
+def _format_trace(error: BaseException) -> str:
+    """The traceback carried by ``error`` itself.
+
+    Callers often record an exception after its ``except`` block has ended,
+    or from another thread than the one that raised it (plugin_executor runs
+    plugins on worker threads), where ``traceback.format_exc()`` has nothing
+    to report. The exception object keeps its own ``__traceback__``, so the
+    trace is built from that. An exception that was created but never raised
+    has no traceback, and the result is just its type and message.
+    """
+    return "".join(traceback.format_exception(type(error), error, error.__traceback__))
+
+
 @dataclass
 class ErrorRecord:
     """Record of a single error occurrence."""
@@ -145,8 +158,8 @@ class ErrorAggregator:
         with self._lock:
             error_type = type(error).__name__
 
-            # Extract additional context from LEDMatrixError subclasses
-            error_context = context or {}
+            # A copy, so the caller's dict is not changed behind its back.
+            error_context = dict(context) if context else {}
             if isinstance(error, LEDMatrixError) and error.context:
                 error_context.update(error.context)
 
@@ -157,7 +170,7 @@ class ErrorAggregator:
                 context=error_context,
                 plugin_id=plugin_id,
                 operation=operation,
-                stack_trace=traceback.format_exc()
+                stack_trace=_format_trace(error)
             )
 
             # Add record (with size limit)

src/font_manager.py

@@ -40,6 +40,11 @@ from pathlib import Path
 from PIL import ImageFont
 from src.common.bdf_font import load_bdf_face, read_bdf_native_size
 from src.common.font_layout import load_truetype, resolve_asset_path
+from src.common.permission_utils import (
+    ensure_directory_permissions,
+    get_assets_dir_mode,
+    get_config_dir_mode,
+)
 from typing import Dict, Tuple, Optional, Union, Any, List
 from src.deprecation import deprecated
 
@@ -57,7 +62,6 @@ class FontManager:
     
     def __init__(self, config: Dict[str, Any]):
         self.config = config
-        self.fonts_config = config.get("fonts", {})
         
         # Font discovery and catalog
         self.font_catalog: Dict[str, str] = {}  # family_name -> file_path
@@ -73,10 +77,8 @@ class FontManager:
         # Plugin font management
         self.plugin_fonts: Dict[str, Dict[str, Any]] = {}  # plugin_id -> font_manifest
         self.plugin_font_catalogs: Dict[str, Dict[str, str]] = {}  # plugin_id -> {family_name -> file_path}
-        self.font_metadata: Dict[str, Dict[str, Any]] = {}  # family_name -> metadata
-        self.font_dependencies: Dict[str, List[str]] = {}  # family_name -> [required_families]
 
-        # Manager font registration - NEW for manager-centric model
+        # Fonts managers and plugins report using (register_manager_font).
         self.manager_fonts: Dict[str, Dict[str, Any]] = {}  # manager_id -> {element_key: {family, size_px, color}}
         self.detected_fonts: Dict[str, Dict[str, Any]] = {}  # element_key -> {family, size_px, color, manager_id, usage_count}
         # Bumped when a manager's registered families change (not when one
@@ -88,13 +90,10 @@ class FontManager:
         self.temp_font_dir = Path(tempfile.gettempdir()) / "ledmatrix_fonts"
         self.temp_font_dir.mkdir(exist_ok=True)
 
-        # Performance monitoring
+        # Counters behind get_performance_stats().
         self.performance_stats = {
-            "font_load_times": {},
             "cache_hits": 0,
             "cache_misses": 0,
-            "render_times": {},
-            "total_renders": 0,
             "failed_loads": 0,
             "start_time": time.time()
         }
@@ -105,9 +104,6 @@ class FontManager:
             "four_by_six": "assets/fonts/4x6-font.ttf",
             "five_by_seven": "assets/fonts/5x7.bdf",
             "tom_thumb": "assets/fonts/tom-thumb.bdf"
-            # Note: cozette_bdf removed - font file not available
-            # To re-enable: download cozette.bdf from https://github.com/the-moonwitch/Cozette
-            # and add: "cozette_bdf": "assets/fonts/cozette.bdf"
         }
         
         # Size tokens for convenience
@@ -116,7 +112,10 @@ class FontManager:
         }
         
         # Font overrides storage (for manual overrides)
-        self.font_overrides_file = "config/font_overrides.json"
+        # Under the install root's config/ (which always exists), not the
+        # cwd: the file itself may not exist yet, and resolve_asset_path
+        # hands back a missing path unchanged.
+        self.font_overrides_file = os.path.join(resolve_asset_path("config"), "font_overrides.json")
         self.font_overrides: Dict[str, Dict[str, Any]] = {}
 
         # Bumped whenever cached font objects are invalidated, so holders of
@@ -128,7 +127,6 @@ class FontManager:
     def reload_config(self, new_config: Dict[str, Any]):
         """Reload configuration and refresh font catalog."""
         self.config = new_config
-        self.fonts_config = new_config.get("fonts", {})
         self.font_cache.clear()  # Clear cache to force reload
         self.metrics_cache.clear()  # Clear metrics cache
         self.cache_generation += 1
@@ -136,7 +134,6 @@ class FontManager:
         logger.info("FontManager configuration reloaded successfully")
 
     # ==================== Manager Font Registration ====================
-    # NEW: Support for managers to register their font choices dynamically
 
     def register_manager_font(self, manager_id: str, element_key: str, 
                              family: str, size_px: int, color: Optional[Tuple[int, int, int]] = None):
@@ -209,16 +206,23 @@ class FontManager:
 
     # ==================== Plugin Font Management ====================
 
-    def register_plugin_fonts(self, plugin_id: str, font_manifest: Dict[str, Any]) -> bool:
+    def register_plugin_fonts(self, plugin_id: str, font_manifest: Dict[str, Any],
+                              plugin_dir: Optional[Union[str, Path]] = None) -> bool:
         """
         Register fonts for a specific plugin.
 
         Args:
             plugin_id: Unique identifier for the plugin
-            font_manifest: Font manifest from plugin's manifest.json
+            font_manifest: The ``fonts`` block of the plugin's manifest.json
+            plugin_dir: The plugin's directory, which ``plugin://`` sources
+                are relative to. PluginManager passes the directory it loaded
+                the plugin from. When omitted, the plugin is looked up in the
+                configured ``plugin_system.plugins_directory`` and then in
+                ``plugins/``.
 
         Returns:
-            True if registration successful, False otherwise
+            True if the manifest was valid (individual fonts that fail to load
+            are logged and skipped), False otherwise
         """
         try:
             # Validate font manifest structure
@@ -235,7 +239,7 @@ class FontManager:
             # Process font definitions
             fonts = font_manifest.get("fonts", [])
             for font_def in fonts:
-                if self._register_plugin_font(plugin_id, font_def):
+                if self._register_plugin_font(plugin_id, font_def, plugin_dir):
                     logger.info(f"Successfully registered font {font_def.get('family')} for plugin {plugin_id}")
 
             logger.info(f"Registered {len(fonts)} fonts for plugin {plugin_id}")
@@ -270,7 +274,8 @@ class FontManager:
 
         return True
 
-    def _register_plugin_font(self, plugin_id: str, font_def: Dict[str, Any]) -> bool:
+    def _register_plugin_font(self, plugin_id: str, font_def: Dict[str, Any],
+                              plugin_dir: Optional[Union[str, Path]] = None) -> bool:
         """Register a single font from a plugin."""
         try:
             family = font_def["family"]
@@ -284,7 +289,7 @@ class FontManager:
             elif source.startswith("plugin://"):
                 # Relative to plugin directory
                 relative_path = source.replace("plugin://", "")
-                font_path = self._resolve_plugin_font_path(plugin_id, relative_path)
+                font_path = self._resolve_plugin_font_path(plugin_id, relative_path, plugin_dir)
             else:
                 # Absolute or relative path
                 font_path = source
@@ -298,14 +303,6 @@ class FontManager:
             self.plugin_font_catalogs[plugin_id][family] = font_path
             self.font_catalog[namespaced_family] = font_path
 
-            # Store metadata
-            if "metadata" in font_def:
-                self.font_metadata[namespaced_family] = font_def["metadata"]
-
-            # Store dependencies
-            if "dependencies" in font_def:
-                self.font_dependencies[namespaced_family] = font_def["dependencies"]
-
             logger.info(f"Registered plugin font: {namespaced_family} -> {font_path}")
             return True
 
@@ -367,11 +364,15 @@ class FontManager:
             return '.zip'
         return '.ttf'  # default
 
-    def _resolve_plugin_font_path(self, plugin_id: str, relative_path: str) -> Optional[str]:
-        """Resolve a plugin-relative font path."""
-        # Assume plugins are in a 'plugins' directory
-        plugin_dir = Path("plugins") / plugin_id
-        font_path = plugin_dir / relative_path
+    def _resolve_plugin_font_path(self, plugin_id: str, relative_path: str,
+                                  plugin_dir: Optional[Union[str, Path]] = None) -> Optional[str]:
+        """Resolve a ``plugin://`` font path against the plugin's directory."""
+        if plugin_dir is None:
+            plugin_dir = self._find_plugin_dir(plugin_id)
+            if plugin_dir is None:
+                logger.error(f"Plugin font {relative_path}: directory for plugin {plugin_id} not found")
+                return None
+        font_path = Path(plugin_dir) / relative_path
 
         if font_path.exists():
             return str(font_path)
@@ -379,6 +380,18 @@ class FontManager:
         logger.error(f"Plugin font not found: {font_path}")
         return None
 
+    def _find_plugin_dir(self, plugin_id: str) -> Optional[Path]:
+        """The installed directory of ``plugin_id``, for callers of
+        register_plugin_fonts that do not pass one: the configured plugins
+        directory (relative paths are relative to the install root), then the
+        legacy ``plugins/`` directory."""
+        # Imported here: src.plugin_system's package import loads PluginManager.
+        from src.plugin_system.plugin_dirs import resolve_plugin_dir
+
+        configured = (self.config.get("plugin_system") or {}).get("plugins_directory") or "plugin-repos"
+        search_dirs = [Path(resolve_asset_path(configured)), Path(resolve_asset_path("plugins"))]
+        return resolve_plugin_dir(plugin_id, search_dirs, prefix=True)
+
     @deprecated("3.7.0")
     def unregister_plugin_fonts(self, plugin_id: str) -> bool:
         """Unregister all fonts for a plugin."""
@@ -390,8 +403,6 @@ class FontManager:
                         namespaced_family = f"{plugin_id}::{family}"
                         if namespaced_family in self.font_catalog:
                             del self.font_catalog[namespaced_family]
-                        if namespaced_family in self.font_metadata:
-                            del self.font_metadata[namespaced_family]
                     
                     del self.plugin_font_catalogs[plugin_id]
 
@@ -442,8 +453,6 @@ class FontManager:
         Returns:
             Resolved font object
         """
-        start_time = time.time()
-        
         try:
             # Check for manual overrides first
             if element_key in self.font_overrides:
@@ -460,14 +469,7 @@ class FontManager:
                 if plugin_id in self.plugin_font_catalogs and family in self.plugin_font_catalogs[plugin_id]:
                     family = f"{plugin_id}::{family}"
 
-            # Get the font
-            font = self.get_font(family, size_px)
-            
-            # Record performance
-            duration = time.time() - start_time
-            self._record_performance_metric("resolve", f"{family}_{size_px}", duration)
-            
-            return font
+            return self.get_font(family, size_px)
 
         except Exception as e:
             logger.error(f"Error resolving font for {element_key}: {e}", exc_info=True)
@@ -491,7 +493,6 @@ class FontManager:
             return self.font_cache[cache_key]
 
         self.performance_stats["cache_misses"] += 1
-        start_time = time.time()
 
         # Load font
         font_path = self.font_catalog.get(family)
@@ -506,15 +507,13 @@ class FontManager:
                 else:
                     font = load_truetype(font_path, size_px)
             except Exception as e:
+                # The one log line for a failed load: _load_bdf_font lets
+                # its error propagate to here.
                 logger.error(f"Error loading font {font_path}: {e}")
                 self.performance_stats["failed_loads"] += 1
                 font = ImageFont.load_default()
 
-        # Cache and record performance
         self.font_cache[cache_key] = font
-        duration = time.time() - start_time
-        self.performance_stats["font_load_times"][cache_key] = duration
-        
         return font
 
     def _load_bdf_font(self, font_path: str, size_px: int) -> freetype.Face:
@@ -524,11 +523,7 @@ class FontManager:
         rather than failing over to PIL's default font, a different typeface
         (see :func:`src.common.bdf_font.load_bdf_face`).
         """
-        try:
-            return load_bdf_face(font_path, size_px)[0]
-        except Exception as e:
-            logger.error(f"Error loading BDF font {font_path}: {e}")
-            raise
+        return load_bdf_face(font_path, size_px)[0]
 
     def get_native_bdf_size(self, family: str) -> Optional[int]:
         """The one true pixel size of a BDF family in the catalog, or None
@@ -723,11 +718,6 @@ class FontManager:
     def _save_overrides(self):
         """Save current font overrides to file."""
         try:
-            from pathlib import Path
-            from src.common.permission_utils import (
-                ensure_directory_permissions,
-                get_config_dir_mode
-            )
             font_overrides_path = Path(self.font_overrides_file)
             ensure_directory_permissions(font_overrides_path.parent, get_config_dir_mode())
             with open(self.font_overrides_file, 'w') as f:
@@ -754,12 +744,6 @@ class FontManager:
         """Get available size tokens."""
         return self.size_tokens.copy()
 
-    def _record_performance_metric(self, operation: str, font_key: str, duration: float):
-        """Record a performance metric."""
-        if operation not in self.performance_stats:
-            self.performance_stats[operation] = {}
-        self.performance_stats[operation][font_key] = duration
-
     @deprecated("3.7.0")
     def get_performance_stats(self) -> Dict[str, Any]:
         """Get performance statistics."""
@@ -789,7 +773,8 @@ class FontManager:
 
     @deprecated("3.7.0")
     def add_font(self, font_file_path: str, family_name: str) -> bool:
-        """Add a new font to the catalog."""
+        """Add ``font_file_path`` to the catalog as ``family_name``. The file
+        stays where it is; only assets/fonts is created if it is missing."""
         try:
             # Validate font file
             if not os.path.exists(font_file_path):
@@ -801,13 +786,7 @@ class FontManager:
                 logger.warning(f"Font family '{family_name}' already exists")
                 return False
 
-            # Copy font to assets/fonts directory
-            from pathlib import Path
-            from src.common.permission_utils import (
-                ensure_directory_permissions,
-                get_assets_dir_mode
-            )
-            fonts_dir = Path("assets/fonts")
+            fonts_dir = Path(resolve_asset_path("assets/fonts"))
             ensure_directory_permissions(fonts_dir, get_assets_dir_mode())
 
             # Add to catalog

src/logo_downloader.py

@@ -16,7 +16,7 @@ import json
 from typing import Dict, List, Optional, Tuple
 from pathlib import Path
 from PIL import Image, ImageDraw, ImageFont, UnidentifiedImageError
-from src.common.font_layout import load_truetype
+from src.common.font_layout import load_truetype, resolve_asset_path
 from PIL.PngImagePlugin import PngInfo
 from requests.adapters import HTTPAdapter
 from urllib3.util.retry import Retry
@@ -370,22 +370,15 @@ class LogoDownloader:
     
     @staticmethod
     def get_logo_filename_variations(abbreviation: str) -> list:
-        """Get possible filename variations for a team abbreviation."""
-        variations = []
+        """Filenames a logo for ``abbreviation`` may be stored under: the
+        upper-cased abbreviation as given, then its normalize_abbreviation()
+        form (``TA&M.png``, then ``TAANDM.png``)."""
         original = abbreviation.upper()
         normalized = LogoDownloader.normalize_abbreviation(abbreviation)
-        
-        # Add original and normalized versions
-        variations.extend([f"{original}.png", f"{normalized}.png"])
-        
-        # Special handling for known cases
-        if original == 'TA&M':
-            # TA&M has a file named TA&M.png, but normalize creates TAANDM.png
-            variations = [f"{original}.png", f"{normalized}.png"]
-        
-        return variations
+        return [f"{original}.png", f"{normalized}.png"]
     
-    # Allowlist for league names used in filesystem paths: alphanumerics, underscores, dashes only
+    # Allowlist for a league name or code that goes into a filesystem path or
+    # an ESPN URL: lower-case alphanumerics, underscores and dashes only.
     _SAFE_LEAGUE_RE = re.compile(r'^[a-z0-9_-]+$')
 
     def get_logo_directory(self, league: str) -> str:
@@ -462,15 +455,12 @@ class LogoDownloader:
             logger.error(f"Unexpected error downloading logo for {team_abbreviation}: {e}")
             return False
 
-    # Allowlist for the league_code segment interpolated into ESPN API URLs
-    _SAFE_LEAGUE_CODE_RE = re.compile(r'^[a-z0-9_-]+$')
-
     def _resolve_api_url(self, league: str) -> Optional[str]:
         """Resolve the ESPN API teams URL for a league, with dynamic fallback for custom soccer leagues."""
         api_url = self.API_ENDPOINTS.get(league)
         if not api_url and league.startswith('soccer_'):
             league_code = league[len('soccer_'):]
-            if not self._SAFE_LEAGUE_CODE_RE.match(league_code):
+            if not self._SAFE_LEAGUE_RE.match(league_code):
                 logger.warning(f"Rejecting unsafe league_code for ESPN URL construction: {league_code!r}")
                 return None
             api_url = f'https://site.api.espn.com/apis/site/v2/sports/soccer/{league_code}/teams'
@@ -501,7 +491,8 @@ class LogoDownloader:
             return None
     
     def fetch_single_team(self, league: str, team_id: str) -> Optional[Dict]:
-        """Fetch team data from ESPN API for a specific league."""
+        """Fetch one team's record (``<teams endpoint>/<team_id>``) from the
+        ESPN API; None on any request or parse failure."""
         api_url = self._resolve_api_url(league)
         if not api_url:
             logger.error(f"No API endpoint configured for league: {league}")
@@ -520,7 +511,7 @@ class LogoDownloader:
             logger.error(f"Error fetching team data for {team_id} in {league}: {e}")
             return None
         except json.JSONDecodeError as e:
-            logger.error(f"Error parsing JSON response for{team_id} in {league}: {e}")
+            logger.error(f"Error parsing JSON response for {team_id} in {league}: {e}")
             return None
     
     def extract_teams_from_data(self, data: Dict, league: str) -> List[Dict[str, str]]:
@@ -626,42 +617,6 @@ class LogoDownloader:
         # Default to FBS for unknown conferences
         return 'FBS'
     
-    def _get_team_name_variations(self, abbreviation: str) -> List[str]:
-        """Generate common variations of a team abbreviation for matching."""
-        variations = set()
-        abbr = abbreviation.upper()
-        variations.add(abbr)
-        
-        # Add normalized version
-        variations.add(self.normalize_abbreviation(abbr))
-        
-        # Common substitutions
-        substitutions = {
-            '&': ['AND', 'A'],
-            'A&M': ['TAMU', 'TA&M', 'TEXASAM'],
-            'STATE': ['ST', 'ST.'],
-            'UNIVERSITY': ['U', 'UNIV'],
-            'COLLEGE': ['C', 'COL'],
-            'TECHNICAL': ['TECH', 'T'],
-            'NORTHERN': ['NORTH', 'N'],
-            'SOUTHERN': ['SOUTH', 'S'],
-            'EASTERN': ['EAST', 'E'],
-            'WESTERN': ['WEST', 'W']
-        }
-        
-        # Apply substitutions
-        for original, replacements in substitutions.items():
-            if original in abbr:
-                for replacement in replacements:
-                    variations.add(abbr.replace(original, replacement))
-                    variations.add(abbr.replace(original, ''))  # Remove the word entirely
-        
-        # Add common abbreviations for Texas A&M
-        if 'A&M' in abbr or 'TAMU' in abbr:
-            variations.update(['TAMU', 'TA&M', 'TEXASAM', 'TEXAS_A&M', 'TEXAS_AM'])
-        
-        return list(variations)
-    
     def download_missing_logos_for_league(self, league: str, force_download: bool = False) -> Tuple[int, int]:
         """Download missing logos for a specific league."""
         logger.info(f"Starting logo download for league: {league}")
@@ -794,7 +749,9 @@ class LogoDownloader:
             return False
         try:
             logo_url = data["team"]["logos"][0]["href"]
-        except KeyError:
+        except (KeyError, IndexError, TypeError):
+            # A team without logos comes back with an empty list.
+            logger.debug(f"No logo URL for team {team_id} in {league}")
             return False
         # Download the logo
         success = self.download_logo(logo_url, logo_path, team_abbreviation)
@@ -824,24 +781,39 @@ class LogoDownloader:
         logger.info(f"Overall logo download results: {total_downloaded} downloaded, {total_failed} failed")
         return results
     
-    def create_placeholder_logo(self, team_abbreviation: str, logo_dir: str) -> bool:
-        """Create a placeholder logo when real logo cannot be downloaded."""
+    def create_placeholder_logo(self, team_abbreviation: str, logo_dir: str,
+                                filepath: Optional[Path] = None) -> bool:
+        """Write a grey placeholder with the team abbreviation on it, for when
+        the real logo cannot be downloaded.
+
+        Args:
+            team_abbreviation: Drawn on the placeholder.
+            logo_dir: Directory for the file when ``filepath`` is not given;
+                the file is then ``<normalize_abbreviation(abbr)>.png``.
+            filepath: The exact path to write, which is where the caller will
+                look for the logo. ``logo_dir`` is ignored when it is given.
+
+        Returns:
+            True if the placeholder was written, False otherwise (logged).
+        """
+        if filepath is not None:
+            filepath = Path(filepath)
+            logo_dir = str(filepath.parent)
         try:
-            # Ensure the logo directory exists
             if not self.ensure_logo_directory(logo_dir):
                 logger.error(f"Failed to create logo directory: {logo_dir}")
                 return False
-            
-            filename = f"{self.normalize_abbreviation(team_abbreviation)}.png"
-            filepath = Path(logo_dir) / filename
 
-            # Create a simple placeholder logo
-            logo = Image.new('RGBA', (64, 64), (100, 100, 100, 255))  # Gray background
+            if filepath is None:
+                filename = f"{self.normalize_abbreviation(team_abbreviation)}.png"
+                filepath = Path(logo_dir) / filename
+
+            logo = Image.new('RGBA', PLACEHOLDER_SIZE, PLACEHOLDER_BG)
             draw = ImageDraw.Draw(logo)
             
             # Try to load a font, fallback to default
             try:
-                font = load_truetype("assets/fonts/PressStart2P-Regular.ttf", 12)
+                font = load_truetype(resolve_asset_path("assets/fonts/PressStart2P-Regular.ttf"), 12)
             except (OSError, IOError):
                 try:
                     font = ImageFont.load_default()
@@ -855,8 +827,8 @@ class LogoDownloader:
                 bbox = draw.textbbox((0, 0), text, font=font)
                 text_width = bbox[2] - bbox[0]
                 text_height = bbox[3] - bbox[1]
-                x = (64 - text_width) // 2
-                y = (64 - text_height) // 2
+                x = (PLACEHOLDER_SIZE[0] - text_width) // 2
+                y = (PLACEHOLDER_SIZE[1] - text_height) // 2
                 draw.text((x, y), text, font=font, fill=(255, 255, 255, 255))
             else:
                 # Fallback without font
@@ -963,14 +935,19 @@ def download_missing_logo(league: str, team_id: str, team_abbreviation: str, log
     Convenience function to download a missing team logo.
     
     Args:
-        team_abbreviation: Team abbreviation (e.g., 'UGA', 'BAMA', 'TA&M')
         league: League identifier (e.g., 'ncaa_fb', 'nfl')
-        logo_path: Full path to where the logo should be saved
+        team_id: ESPN team id, used to look up the logo URL when
+            ``logo_url`` is not given
+        team_abbreviation: Team abbreviation (e.g., 'UGA', 'BAMA', 'TA&M')
+        logo_path: Where the logo (or placeholder) is written; relative paths
+            are relative to the install root
         logo_url: Optional direct URL to the logo
         create_placeholder: Whether to create a placeholder if download fails
         
     Returns:
-        True if logo exists or was successfully downloaded, False otherwise
+        True if a logo or placeholder is at ``logo_path`` afterwards (it was
+        already there, was downloaded, or a placeholder was written),
+        False otherwise.
     """
     downloader = shared_downloader()
 
@@ -1011,7 +988,7 @@ def download_missing_logo(league: str, team_id: str, team_abbreviation: str, log
             time.sleep(0.1)  # Small delay
         if not success and create_placeholder:
             logger.info(f"Creating placeholder logo for {team_abbreviation}")
-            success = downloader.create_placeholder_logo(team_abbreviation, logo_dir)
+            success = downloader.create_placeholder_logo(team_abbreviation, logo_dir, filepath=filepath)
         return success
 
     success = downloader.download_missing_logo_for_team(league, team_id, team_abbreviation, logo_path)
@@ -1019,7 +996,7 @@ def download_missing_logo(league: str, team_id: str, team_abbreviation: str, log
     if not success and create_placeholder:
         logger.info(f"Creating placeholder logo for {team_abbreviation}")
         # Create placeholder as fallback
-        success = downloader.create_placeholder_logo(team_abbreviation, logo_dir)
+        success = downloader.create_placeholder_logo(team_abbreviation, logo_dir, filepath=filepath)
     
     if success:
         logger.info(f"Successfully handled logo for {team_abbreviation}")

src/plugin_system/base_plugin.py

@@ -11,7 +11,6 @@ Stability: Stable - maintains backward compatibility
 from abc import ABC, abstractmethod
 from enum import Enum
 from typing import Dict, Any, Optional, List
-import logging
 import os
 import sys
 from src.logging_config import get_logger
@@ -511,104 +510,53 @@ class BasePlugin(ABC):
         """
         Get the display duration for this plugin instance.
 
-        Automatically detects duration from:
-        1. self.display_duration instance variable (if exists)
-        2. self.config.get("display_duration", 15.0) (fallback)
+        Uses, in order, the first positive number among:
+        1. ``self.display_duration`` (a common pattern in scoreboard plugins)
+        2. ``self.config["display_duration"]``
+        3. 15.0
 
-        Can be overridden by plugins to provide dynamic durations based
-        on content (e.g., longer duration for more complex displays).
+        Numeric strings count as numbers. Can be overridden by plugins to
+        provide dynamic durations based on content (e.g., longer duration for
+        more complex displays).
 
         Returns:
             Duration in seconds to display this plugin's content
         """
-        # Check for instance variable first (common pattern in scoreboard plugins)
-        if hasattr(self, 'display_duration'):
-            try:
-                duration = getattr(self, 'display_duration')
-                # Handle None case
-                if duration is None:
-                    pass  # Fall through to config
-                # Try to convert to float if it's a number or numeric string.
-                # bool is excluded: it's an int subclass, and True would
-                # otherwise read as a 1-second duration.
-                elif isinstance(duration, (int, float)) and not isinstance(duration, bool):
-                    if duration > 0:
-                        return float(duration)
-                    else:
-                        self.logger.debug(
-                            "display_duration instance variable is non-positive (%s), using config fallback",
-                            duration
-                        )
-                # Try converting string representations of numbers
-                elif isinstance(duration, str):
-                    try:
-                        duration_float = float(duration)
-                        if duration_float > 0:
-                            return duration_float
-                        else:
-                            self.logger.debug(
-                                "display_duration string value is non-positive (%s), using config fallback",
-                                duration
-                            )
-                    except (ValueError, TypeError):
-                        self.logger.warning(
-                            "display_duration instance variable has invalid string value '%s', using config fallback",
-                            duration
-                        )
-                else:
-                    self.logger.warning(
-                        "display_duration instance variable has unexpected type %s (value: %s), using config fallback",
-                        type(duration).__name__, duration
-                    )
-            except (TypeError, ValueError, AttributeError) as e:
-                self.logger.warning(
-                    "Error reading display_duration instance variable: %s, using config fallback",
-                    e
-                )
-
-        # Fall back to config
-        config_duration = self.config.get("display_duration", 15.0)
         try:
-            # Ensure config value is also a valid float (bool excluded — an
-            # int subclass that would otherwise read True as 1 second)
-            if isinstance(config_duration, (int, float)) and not isinstance(config_duration, bool):
-                if config_duration > 0:
-                    return float(config_duration)
-                else:
-                    self.logger.debug(
-                        "Config display_duration is non-positive (%s), using default 15.0",
-                        config_duration
-                    )
-                    return 15.0
-            elif isinstance(config_duration, str):
-                try:
-                    duration_float = float(config_duration)
-                    if duration_float > 0:
-                        return duration_float
-                    else:
-                        self.logger.debug(
-                            "Config display_duration string is non-positive (%s), using default 15.0",
-                            config_duration
-                        )
-                        return 15.0
-                except ValueError:
-                    self.logger.warning(
-                        "Config display_duration has invalid string value '%s', using default 15.0",
-                        config_duration
-                    )
-                    return 15.0
-            else:
-                self.logger.warning(
-                    "Config display_duration has unexpected type %s (value: %s), using default 15.0",
-                    type(config_duration).__name__, config_duration
-                )
-        except (ValueError, TypeError) as e:
+            duration = getattr(self, 'display_duration', None)
+        except (TypeError, ValueError, AttributeError) as e:
+            # A plugin may define display_duration as a property that raises.
             self.logger.warning(
-                "Error processing config display_duration: %s, using default 15.0",
-                e
-            )
+                "Error reading display_duration instance variable: %s, using config fallback", e)
+            duration = None
+        if duration is not None:
+            seconds = self._positive_seconds(duration, "display_duration instance variable")
+            if seconds is not None:
+                return seconds
 
-        return 15.0
+        seconds = self._positive_seconds(
+            self.config.get("display_duration", 15.0), "config display_duration")
+        return seconds if seconds is not None else 15.0
+
+    def _positive_seconds(self, value: Any, source: str) -> Optional[float]:
+        """``value`` as a positive float, or None (with a log line) if it is not one.
+
+        bool is rejected although it is an int subclass: True would otherwise
+        read as a 1-second duration.
+        """
+        if isinstance(value, bool) or not isinstance(value, (int, float, str)):
+            self.logger.warning("%s has unexpected type %s (value: %s), ignoring it",
+                                source, type(value).__name__, value)
+            return None
+        try:
+            seconds = float(value)
+        except ValueError:
+            self.logger.warning("%s has invalid value %r, ignoring it", source, value)
+            return None
+        if seconds > 0:
+            return seconds
+        self.logger.debug("%s is non-positive (%s), ignoring it", source, value)
+        return None
 
     # ---------------------------------------------------------------------
     # Dynamic duration support hooks
@@ -926,25 +874,20 @@ class BasePlugin(ABC):
                     config_mode, self.plugin_id
                 )
 
-        # Fall back to mapping legacy content_type
-        content_type = self.get_vegas_content_type()
-        if content_type == 'multi':
+        # Fall back to mapping legacy content_type. 'none' (excluded from
+        # Vegas) also maps to FIXED_SEGMENT: exclusion is decided by checking
+        # get_vegas_content_type() separately.
+        if self.get_vegas_content_type() == 'multi':
             return VegasDisplayMode.SCROLL
-        elif content_type == 'static':
-            return VegasDisplayMode.FIXED_SEGMENT
-        elif content_type == 'none':
-            # 'none' means excluded - return FIXED_SEGMENT as default
-            # The exclusion is handled by checking get_vegas_content_type() separately
-            return VegasDisplayMode.FIXED_SEGMENT
-
         return VegasDisplayMode.FIXED_SEGMENT
 
     def get_supported_vegas_modes(self) -> List[VegasDisplayMode]:
         """
         Return list of Vegas display modes this plugin supports.
 
-        Used by the web UI to show available mode options for user configuration.
-        Override to customize which modes are available for this plugin.
+        Not currently consulted by core: neither Vegas mode nor the web UI
+        calls it. It is kept, and plugins override it, as the declared set of
+        modes a future mode picker would offer.
 
         By default:
         - 'multi' content type plugins support SCROLL and FIXED_SEGMENT
@@ -972,6 +915,10 @@ class BasePlugin(ABC):
         """
         Get the preferred width for this plugin in Vegas FIXED_SEGMENT mode.
 
+        Not currently consulted by core: Vegas mode sizes a card from the
+        ``vegas_width_pct`` / ``vegas_scroll.render_width_pct`` settings
+        (see get_vegas_render_width()). Kept because plugins override it.
+
         Returns the number of panels this plugin should occupy when displayed
         as a fixed segment. The actual pixel width is calculated as:
             width = panels * single_panel_width
@@ -1025,7 +972,7 @@ class BasePlugin(ABC):
                 required_fields = ['api_key', 'city']
                 for field in required_fields:
                     if field not in self.config:
-                self.logger.error("Missing required field: %s", field)
+                        self.logger.error("Missing required field: %s", field)
                         return False
                 return True
         """

src/plugin_system/operation_queue.py

@@ -9,8 +9,6 @@ import threading
 import queue
 from typing import Dict, Optional, List, Callable, Any
 from datetime import datetime
-from pathlib import Path
-import json
 
 from src.plugin_system.operation_types import (
     PluginOperation, OperationType, OperationStatus
@@ -28,28 +26,22 @@ class PluginOperationQueue:
     - Prevents concurrent operations on same plugin
     - Operation status tracking
     - Operation cancellation
-    - Operation history
+    - In-memory history of finished operations
+
+    The history is not persisted. The web UI's operation history comes from
+    OperationHistory (operation_history.py), which has its own file; a copy
+    written here was never read back by anything.
     """
-    
-    def __init__(
-        self,
-        history_file: Optional[str] = None,
-        max_history: int = 100,
-        lazy_load: bool = False
-    ):
+
+    def __init__(self, max_history: int = 100):
         """
         Initialize operation queue.
-        
+
         Args:
-            history_file: Optional path to file for persisting operation history
             max_history: Maximum number of operations to keep in history
-            lazy_load: If True, defer loading history file until first access
         """
         self.logger = get_logger(__name__)
-        self.history_file = Path(history_file) if history_file else None
         self.max_history = max_history
-        self._lazy_load = lazy_load
-        self._history_loaded = False
         
         # Operation tracking
         self._operations: Dict[str, PluginOperation] = {}
@@ -62,20 +54,8 @@ class PluginOperationQueue:
         self._worker_thread: Optional[threading.Thread] = None
         self._stop_event = threading.Event()
         
-        # Load history from file if it exists (unless lazy loading)
-        if not self._lazy_load and self.history_file and self.history_file.exists():
-            self._load_history()
-            self._history_loaded = True
-        
-        # Start worker thread
         self._start_worker()
-    
-    def _ensure_loaded(self) -> None:
-        """Ensure history is loaded (for lazy loading)."""
-        if not self._history_loaded and self.history_file and self.history_file.exists():
-            self._load_history()
-            self._history_loaded = True
-    
+
     def enqueue_operation(
         self,
         operation_type: OperationType,
@@ -139,7 +119,6 @@ class PluginOperationQueue:
         Returns:
             PluginOperation if found, None otherwise
         """
-        self._ensure_loaded()
         with self._lock:
             return self._operations.get(operation_id)
     
@@ -184,7 +163,6 @@ class PluginOperationQueue:
         Returns:
             List of operations, sorted by creation time (newest first)
         """
-        self._ensure_loaded()
         with self._lock:
             # Sort by creation time (newest first)
             history = sorted(
@@ -314,11 +292,7 @@ class PluginOperationQueue:
                     if self._active_operations[operation.plugin_id].operation_id == operation.operation_id:
                         del self._active_operations[operation.plugin_id]
                 
-                # Add to history
                 self._add_to_history(operation)
-                
-                # Save history to file
-                self._save_history()
     
     def _add_to_history(self, operation: PluginOperation) -> None:
         """Add operation to history, maintaining max_history limit."""
@@ -330,46 +304,6 @@ class PluginOperationQueue:
             self._operation_history.sort(key=lambda op: op.created_at)
             self._operation_history = self._operation_history[-self.max_history:]
     
-    def _save_history(self) -> None:
-        """Save operation history to file."""
-        if not self.history_file:
-            return
-        
-        try:
-            with self._lock:
-                # Convert operations to dicts
-                history_data = [op.to_dict() for op in self._operation_history]
-            
-            # Ensure directory exists
-            self.history_file.parent.mkdir(parents=True, exist_ok=True)
-            
-            # Write to file
-            with open(self.history_file, 'w') as f:
-                json.dump(history_data, f, indent=2)
-            
-        except Exception as e:
-            self.logger.warning(f"Error saving operation history: {e}")
-    
-    def _load_history(self) -> None:
-        """Load operation history from file."""
-        if not self.history_file or not self.history_file.exists():
-            return
-        
-        try:
-            with open(self.history_file, 'r') as f:
-                history_data = json.load(f)
-            
-            with self._lock:
-                self._operation_history = [
-                    PluginOperation.from_dict(op_data)
-                    for op_data in history_data
-                ]
-            
-            self.logger.info(f"Loaded {len(self._operation_history)} operations from history")
-            
-        except Exception as e:
-            self.logger.warning(f"Error loading operation history: {e}")
-    
     def shutdown(self) -> None:
         """Shutdown the operation queue and worker thread."""
         self.logger.info("Shutting down plugin operation queue")
@@ -377,7 +311,4 @@ class PluginOperationQueue:
         
         if self._worker_thread and self._worker_thread.is_alive():
             self._worker_thread.join(timeout=5.0)
-        
-        # Save history one last time
-        self._save_history()
 

src/plugin_system/plugin_executor.py

@@ -93,7 +93,7 @@ class PluginExecutor:
         if result_container['exception']:
             error = result_container['exception']
             error_msg = f"{plugin_context} operation failed: {error}"
-            self.logger.error(error_msg, exc_info=True)
+            self.logger.error(error_msg, exc_info=error)
             record_error(error, plugin_id=plugin_id, operation="execute")
             raise PluginError(error_msg, plugin_id=plugin_id) from error
         

src/plugin_system/plugin_loader.py

@@ -5,6 +5,7 @@ Handles plugin module imports, dependency installation, and class instantiation.
 Extracted from PluginManager to improve separation of concerns.
 """
 
+import errno
 import importlib
 import importlib.metadata
 import importlib.util
@@ -184,6 +185,46 @@ def find_trusted_subdir(trusted_dir: str, name: str) -> Optional[str]:
     return None
 
 
+def contained_plugin_dir(plugin_dir: Path, plugins_dir: Path) -> Optional[str]:
+    """``plugin_dir`` rebuilt from an entry enumerated under ``plugins_dir``.
+
+    Returns None when ``plugin_dir`` is not a subdirectory of ``plugins_dir``.
+    Callers derive ``plugin_dir`` from a manifest-declared id, so the path is
+    rebuilt from :func:`find_trusted_subdir`'s answer rather than trusted: a
+    name that came out of ``os.scandir()`` on the trusted root carries no
+    taint, which is a real containment guarantee (and one CodeQL's
+    path-injection query can follow), not a string sanitiser.
+    """
+    plugin_dir_real = os.path.realpath(str(plugin_dir))
+    plugins_dir_real = os.path.realpath(str(plugins_dir))
+    matched_name = find_trusted_subdir(plugins_dir_real, os.path.basename(plugin_dir_real))
+    if matched_name is None:
+        return None
+    return os.path.join(plugins_dir_real, matched_name)
+
+
+def requirements_to_install(plugin_dir: str, logger: logging.Logger,
+                            label: str) -> Optional[str]:
+    """The plugin's requirements.txt if pip has work to do, else None.
+
+    None when there is no requirements.txt, when it lists nothing (plugins
+    whose dependencies ship with core often keep an all-comments file), or
+    when every requirement is already installed. Shared by the loader and the
+    store so both skip pip for the same reasons; they differ only in how they
+    run it.
+    """
+    requirements_file = os.path.join(plugin_dir, "requirements.txt")
+    if not os.path.isfile(requirements_file):
+        return None
+    if not requirements_has_real_deps(requirements_file):
+        logger.debug("requirements.txt for %s has no real dependencies, skipping pip", label)
+        return None
+    if requirements_are_satisfied(requirements_file):
+        logger.debug("Dependencies for %s already satisfied, skipping pip", label)
+        return None
+    return requirements_file
+
+
 class PluginLoader:
     """Handles plugin module loading and class instantiation."""
 
@@ -273,43 +314,15 @@ class PluginLoader:
         if not plugin_id:
             return False
 
-        # Resolve to a canonical absolute path (normalises .. and symlinks)
-        plugin_dir_real = os.path.realpath(str(plugin_dir))
-        plugins_dir_real = os.path.realpath(str(plugins_dir))
-        requested_name = os.path.basename(plugin_dir_real)
-
-        # Match the requested directory against an entry actually enumerated
-        # from the trusted plugins_dir, and build the path from that entry --
-        # not from requested_name. A name that came out of os.scandir() on a
-        # trusted root carries no taint regardless of what the caller asked
-        # for, so this is a real containment guarantee (an allowlist check
-        # against a trusted source), not a string-sanitisation of untrusted
-        # input that a static analyzer has to trust blindly.
-        matched_name = find_trusted_subdir(plugins_dir_real, requested_name)
-        if matched_name is None:
+        safe_plugin_dir = contained_plugin_dir(plugin_dir, plugins_dir)
+        if safe_plugin_dir is None:
             self.logger.error(
                 "Plugin directory for %s not found inside plugins dir", plugin_id
             )
             return False
 
-        safe_plugin_dir = os.path.join(plugins_dir_real, matched_name)
-        requirements_file = os.path.join(safe_plugin_dir, "requirements.txt")
-
-        if not os.path.isfile(requirements_file):
-            return True  # No dependencies needed
-
-        if not requirements_has_real_deps(requirements_file):
-            self.logger.debug(
-                "requirements.txt for %s has no real dependencies (comments/blank only), skipping pip",
-                plugin_id
-            )
-            return True
-
-        if requirements_are_satisfied(requirements_file):
-            self.logger.debug(
-                "Dependencies for %s already satisfied in current environment, skipping pip",
-                plugin_id
-            )
+        requirements_file = requirements_to_install(safe_plugin_dir, self.logger, plugin_id)
+        if requirements_file is None:
             return True
 
         try:
@@ -348,8 +361,8 @@ class PluginLoader:
                     # below).
                     try:
                         # sys.executable is this process's own interpreter (not
-                        # attacker-influenced), and requirements_file is a path
-                        # built internally by find_plugin_directory, never raw
+                        # attacker-influenced), and requirements_file is rebuilt
+                        # by contained_plugin_dir() from a trusted listing, never raw
                         # external input.
                         retry_result = subprocess.run(  # nosec B603 - no shell invoked (list-form argv)  # nosemgrep
                             [sys.executable, "-m", "pip", "install", "--break-system-packages",
@@ -384,10 +397,10 @@ class PluginLoader:
         except FileNotFoundError:
             self.logger.warning("pip not found. Skipping dependency installation for %s", plugin_id)
             return True
-        except (BrokenPipeError, OSError) as e:
-            # Handle broken pipe errors (errno 32) which can occur during pip downloads
-            # Often caused by network interruptions or output buffer issues
-            if isinstance(e, OSError) and e.errno == 32:
+        except OSError as e:
+            # A broken pipe (EPIPE) happens when pip's output pipe closes
+            # mid-download, usually a network interruption.
+            if e.errno == errno.EPIPE:
                 self.logger.error(
                     "Broken pipe error during dependency installation for %s. "
                     "This usually indicates a network interruption or pip output buffer issue. "
@@ -528,7 +541,7 @@ class PluginLoader:
         plugin_id: str,
         plugin_dir: Path,
         entry_point: str
-    ) -> Optional[Any]:
+    ) -> Any:
         """
         Load a plugin module from file.
 
@@ -547,7 +560,12 @@ class PluginLoader:
             entry_point: Entry point filename (e.g., 'manager.py')
 
         Returns:
-            Loaded module or None on error
+            The loaded module
+
+        Raises:
+            PluginError: If the plugin id, directory or entry point is
+                invalid. Whatever the module raises while executing
+                propagates unchanged.
         """
         plugin_id = os.path.basename(plugin_id or '')
         if not plugin_id:
@@ -782,9 +800,7 @@ class PluginLoader:
         # Load module
         entry_point = manifest.get('entry_point', 'manager.py')
         module = self.load_module(plugin_id, plugin_dir, entry_point)
-        if module is None:
-            raise PluginError(f"Failed to load module for plugin {plugin_id}", plugin_id=plugin_id)
-        
+
         # Get plugin class
         class_name = manifest.get('class_name')
         if not class_name:

src/plugin_system/plugin_manager.py

@@ -2,7 +2,8 @@
 Plugin Manager
 
 Manages plugin discovery, loading, and lifecycle for the LEDMatrix system.
-Handles dynamic plugin loading from the plugins/ directory.
+Loads plugins from the configured plugins directory
+(``plugin_system.plugins_directory``, ``plugin-repos/`` by default).
 
 API Version: 1.0.0
 """
@@ -40,7 +41,7 @@ class PluginManager:
     Manages plugin discovery, loading, and lifecycle.
     
     The PluginManager is responsible for:
-    - Discovering plugins in the plugins/ directory
+    - Discovering plugins in the configured plugins directory
     - Loading plugin modules and instantiating plugin classes
     - Managing plugin lifecycle (load, unload, reload)
     - Providing access to loaded plugins
@@ -99,10 +100,9 @@ class PluginManager:
         self.plugin_directories: Dict[str, Path] = {}
         self.plugin_last_update: Dict[str, float] = {}
 
-        # Cached data-fetch intervals per plugin_id.
-        # _get_plugin_update_interval falls back to config_manager.get_config()
-        # (a full dict copy) when the manifest lacks an interval — caching avoids
-        # that copy on every 30-fps tick.  Cleared on load/unload.
+        # Cached static data-fetch intervals per plugin_id, so the render
+        # loop's scheduling tick does not repeat the manifest/config lookup
+        # for every plugin. Cleared on load/unload.
         self._update_interval_cache: Dict[str, Optional[float]] = {}
 
         # Health tracking (optional, set by display_controller if available)
@@ -110,14 +110,12 @@ class PluginManager:
         self.resource_monitor = None
 
         # --- Asynchronous plugin updates -------------------------------
-        # update() used to run inline in the render loop (execute_update's
-        # internal thread.join(timeout=30) blocked it), so one slow plugin
-        # HTTP fetch froze scrolling for the whole fetch. Scheduling still
-        # happens on the render thread (run_scheduled_updates), but
-        # execution moves to this single background worker. Per-plugin
-        # locks keep a plugin's update() and display() mutually exclusive —
-        # today's implicit guarantee, now explicit (and, unlike today,
-        # also held across the post-timeout window).
+        # Run inline in the render loop, one slow plugin HTTP fetch in
+        # update() freezes scrolling for the whole fetch. Scheduling happens
+        # on the render thread (run_scheduled_updates); execution happens on
+        # this single background worker. Per-plugin locks keep a plugin's
+        # update() and display() mutually exclusive, including across the
+        # post-timeout window.
         # Kill switch: plugin_system.synchronous_updates: true restores the
         # inline path.
         self._update_queue: "queue.Queue[Optional[Tuple[str, float]]]" = queue.Queue()
@@ -396,7 +394,8 @@ class PluginManager:
                 self.font_manager, 'register_plugin_fonts'
             ):
                 try:
-                    self.font_manager.register_plugin_fonts(plugin_id, font_manifest)
+                    self.font_manager.register_plugin_fonts(
+                        plugin_id, font_manifest, plugin_dir=plugin_dir)
                 except Exception as e:
                     self.logger.warning(
                         "Failed to register fonts for plugin %s: %s", plugin_id, e
@@ -661,9 +660,15 @@ class PluginManager:
             if not self.unload_plugin(plugin_id):
                 return False
         
-        # Re-discover to get updated manifest
-        manifest_path = self.plugins_dir / plugin_id / "manifest.json"
-        if manifest_path.exists():
+        # Re-read the manifest so an edit to it takes effect, from the
+        # directory discovery found the plugin in: a directory's name need not
+        # be the id its manifest declares.
+        with self._discovery_lock:
+            directories = dict(self.plugin_directories)
+        plugin_dir = self.plugin_loader.find_plugin_directory(
+            plugin_id, self.plugins_dir, directories)
+        manifest_path = plugin_dir / "manifest.json" if plugin_dir is not None else None
+        if manifest_path is not None and manifest_path.exists():
             try:
                 with open(manifest_path, 'r', encoding='utf-8') as f:
                     manifest = json.load(f)
@@ -881,11 +886,12 @@ class PluginManager:
         updating, since a scheduler that propagates a plugin bug stops every
         other plugin too.
 
-        The static result is cached per plugin_id after the first lookup to
-        avoid calling config_manager.get_config() — which returns a full dict
-        copy — on every tick of the 30-fps display loop. The cache is
-        invalidated when a plugin is loaded or unloaded. The dynamic hook is
-        deliberately *not* cached: caching it would defeat its only purpose.
+        The static result is cached per plugin_id after the first lookup, so
+        the manifest/config resolution is not repeated on every scheduling
+        tick of the display loop. A change to ``update_interval`` in
+        config.json therefore takes effect when the plugin is next loaded or
+        unloaded, which clears the cache. The dynamic hook is deliberately
+        *not* cached: caching it would defeat its only purpose.
         """
         dynamic = self._dynamic_update_interval(plugin_id, plugin_instance)
         if dynamic is not None:

src/plugin_system/plugin_state.py

@@ -41,7 +41,6 @@ class PluginStateManager:
         self._state_transition_counts: Dict[str, int] = {}
         self._error_info: Dict[str, Dict[str, Any]] = {}
         self._last_update: Dict[str, datetime] = {}
-        self._last_display: Dict[str, datetime] = {}
     
     def _record_transition(self, plugin_id: str) -> None:
         """Count a state transition. Callers must already hold ``_lock``."""
@@ -181,11 +180,7 @@ class PluginStateManager:
     def get_last_update(self, plugin_id: str) -> Optional[datetime]:
         """Get timestamp of last update() call."""
         return self._last_update.get(plugin_id)
-    
-    def get_last_display(self, plugin_id: str) -> Optional[datetime]:
-        """Get timestamp of last display() call."""
-        return self._last_display.get(plugin_id)
-    
+
     def get_state_info(self, plugin_id: str) -> Dict[str, Any]:
         """
         Get comprehensive state information for a plugin.
@@ -212,7 +207,6 @@ class PluginStateManager:
                 'is_error': self.is_error(plugin_id),
                 'can_execute': self.can_execute(plugin_id),
                 'last_update': self.get_last_update(plugin_id),
-                'last_display': self.get_last_display(plugin_id),
                 'error_info': self.get_error_info(plugin_id),
                 'state_history_count': self._state_transition_counts.get(plugin_id, 0)
             }
@@ -231,5 +225,4 @@ class PluginStateManager:
             self._state_transition_counts.pop(plugin_id, None)
             self._error_info.pop(plugin_id, None)
             self._last_update.pop(plugin_id, None)
-            self._last_display.pop(plugin_id, None)
 

src/plugin_system/repo_urls.py

@@ -0,0 +1,70 @@
+"""
+Repository URL helpers shared by the plugin store and saved repositories.
+
+One definition of "the same repository URL", of how a GitHub URL maps to
+``owner/repo``, and of the headers sent to the GitHub API.
+"""
+
+from typing import Dict, Optional, Tuple
+from urllib.parse import urlparse
+
+#: Hosts whose URLs name a GitHub repository. Matched against
+#: ``urlparse(url).hostname``, never by substring: a substring test accepts
+#: ``https://github.com.example.org/...`` as GitHub.
+GITHUB_HOSTS = frozenset({'github.com', 'www.github.com'})
+
+#: Sent on every request the store makes to GitHub.
+USER_AGENT = 'LEDMatrix-Plugin-Manager/1.0'
+
+
+def normalize_repo_url(url: str) -> str:
+    """``url`` without surrounding whitespace, trailing slashes or a trailing ``.git``.
+
+    Only a *trailing* ``.git`` is removed. The unanchored
+    ``url.replace('.git', '')`` this replaces turned
+    ``https://github.com/user/my.github.io`` into ``.../myhub.io``.
+    Case is preserved; compare with :func:`same_repo`.
+    """
+    url = url.strip().rstrip('/')
+    if url.endswith('.git'):
+        url = url[:-4]
+    return url
+
+
+def same_repo(url_a: str, url_b: str) -> bool:
+    """Whether two URLs name the same repository.
+
+    GitHub owner and repository names are case-insensitive, so
+    ``ChuckBuilds/LEDMatrix-Plugins`` and ``chuckbuilds/ledmatrix-plugins``
+    are one repository.
+    """
+    return normalize_repo_url(url_a).lower() == normalize_repo_url(url_b).lower()
+
+
+def github_owner_repo(url: str) -> Optional[Tuple[str, str]]:
+    """``(owner, repo)`` for a github.com repository URL, else None.
+
+    The first two path segments, so a URL that points inside the repository
+    (``.../owner/repo/tree/main/plugins/x``) still names ``owner/repo``.
+    """
+    parsed = urlparse(normalize_repo_url(url))
+    if parsed.hostname not in GITHUB_HOSTS:
+        return None
+    parts = [part for part in parsed.path.split('/') if part]
+    if len(parts) < 2:
+        return None
+    return parts[0], normalize_repo_url(parts[1])
+
+
+def github_api_headers(token: Optional[str] = None) -> Dict[str, str]:
+    """Headers for a GitHub REST API request, authenticated when ``token`` is set.
+
+    An authenticated request gets 5000 requests an hour instead of 60.
+    """
+    headers = {
+        'Accept': 'application/vnd.github.v3+json',
+        'User-Agent': USER_AGENT,
+    }
+    if token:
+        headers['Authorization'] = f'token {token}'
+    return headers

src/plugin_system/resource_monitor.py

@@ -33,7 +33,14 @@ class ResourceLimits:
 
 @dataclass
 class ResourceMetrics:
-    """Resource usage metrics for a plugin."""
+    """Resource usage metrics for a plugin.
+
+    ``memory_mb`` is the largest growth in this *process's* resident memory
+    seen across a single monitored call -- a high-water mark, not current
+    usage, and not the plugin's own footprint (another thread allocating
+    during the call counts too). ``cpu_percent`` is the whole process's CPU
+    use since the previous sample.
+    """
     memory_mb: float = 0.0
     cpu_percent: float = 0.0
     execution_time: float = 0.0
@@ -42,11 +49,6 @@ class ResourceMetrics:
     max_execution_time: float = 0.0
     min_execution_time: float = float('inf')
     last_update_time: float = field(default_factory=time.time)
-    
-    def update_average_execution_time(self):
-        """Update average execution time."""
-        if self.call_count > 0:
-            self.total_execution_time = self.total_execution_time / self.call_count
 
 
 #: How often a plugin's metrics are written to the cache, in seconds.
@@ -287,7 +289,8 @@ class PluginResourceMonitor:
             
             # Calculate execution time
             execution_time = time.time() - start_time
-            
+            memory_growth_mb = 0.0
+
             # Update metrics
             with self._lock:
                 metrics.execution_time = execution_time
@@ -302,17 +305,17 @@ class PluginResourceMonitor:
                 
                 # Update memory and CPU if monitoring enabled
                 if self.enable_monitoring:
-                    end_memory = self._get_process_memory_mb()
-                    metrics.memory_mb = max(metrics.memory_mb, end_memory - start_memory)
+                    memory_growth_mb = self._get_process_memory_mb() - start_memory
+                    metrics.memory_mb = max(metrics.memory_mb, memory_growth_mb)
                     # CPU is harder to measure per-call, so we track it separately
                     metrics.cpu_percent = self._get_process_cpu_percent()
-                
+
                 # Persist metrics, at most once per interval per plugin.
                 self._persist_metrics(plugin_id, metrics)
-            
-            # Check limits
+
             if limits:
-                self._check_limits(plugin_id, metrics, limits, execution_time)
+                self._check_limits(plugin_id, metrics, limits, execution_time,
+                                   memory_growth_mb)
             
             return result
             
@@ -326,9 +329,17 @@ class PluginResourceMonitor:
                 metrics.last_update_time = time.time()
             raise
     
-    def _check_limits(self, plugin_id: str, metrics: ResourceMetrics, 
-                     limits: ResourceLimits, execution_time: float) -> None:
-        """Check if plugin has exceeded resource limits."""
+    def _check_limits(self, plugin_id: str, metrics: ResourceMetrics,
+                      limits: ResourceLimits, execution_time: float,
+                      memory_growth_mb: float) -> None:
+        """Raise ResourceLimitExceeded if this call went over a limit.
+
+        Execution time and memory growth are this call's own; CPU is the
+        latest process sample. Judging memory by the stored high-water mark
+        (``metrics.memory_mb``) instead would fail every call after the first
+        expensive one, so the health tracker's circuit breaker would reopen
+        on every recovery probe and the plugin would never update again.
+        """
         warnings = []
         errors = []
         
@@ -343,13 +354,13 @@ class PluginResourceMonitor:
             )
         
         # Check memory
-        if limits.max_memory_mb and metrics.memory_mb > limits.max_memory_mb:
+        if limits.max_memory_mb and memory_growth_mb > limits.max_memory_mb:
             errors.append(
-                f"Memory usage {metrics.memory_mb:.2f}MB exceeds limit {limits.max_memory_mb:.2f}MB"
+                f"Memory growth {memory_growth_mb:.2f}MB exceeds limit {limits.max_memory_mb:.2f}MB"
             )
-        elif limits.max_memory_mb and metrics.memory_mb > limits.max_memory_mb * limits.warning_threshold:
+        elif limits.max_memory_mb and memory_growth_mb > limits.max_memory_mb * limits.warning_threshold:
             warnings.append(
-                f"Memory usage {metrics.memory_mb:.2f}MB approaching limit {limits.max_memory_mb:.2f}MB"
+                f"Memory growth {memory_growth_mb:.2f}MB approaching limit {limits.max_memory_mb:.2f}MB"
             )
         
         # Check CPU

src/plugin_system/saved_repositories.py

@@ -10,6 +10,8 @@ import os
 from pathlib import Path
 from typing import List, Dict, Optional
 
+from src.plugin_system.repo_urls import normalize_repo_url
+
 
 class SavedRepositoriesManager:
     """Manages saved GitHub repository URLs."""
@@ -71,18 +73,6 @@ class SavedRepositoriesManager:
                 pass
             return False
     
-    @staticmethod
-    def _clean_url(repo_url: str) -> str:
-        """Normalize a repo URL: strip whitespace, trailing slashes, and a
-        trailing ``.git`` suffix ONLY. (The old ``.replace('.git', '')``
-        was an unanchored substring replace that mangled URLs merely
-        containing ``.git``, e.g. ``https://github.com/user/my.github.io``.)
-        """
-        repo_url = repo_url.strip().rstrip('/')
-        if repo_url.endswith('.git'):
-            repo_url = repo_url[:-4]
-        return repo_url
-
     def get_all(self) -> List[Dict[str, str]]:
         """Get all saved repositories."""
         return self.repositories.copy()
@@ -98,7 +88,7 @@ class SavedRepositoriesManager:
         Returns:
             True if added successfully
         """
-        repo_url = self._clean_url(repo_url)
+        repo_url = normalize_repo_url(repo_url)
 
         # Check if already exists
         for repo in self.repositories:
@@ -138,7 +128,7 @@ class SavedRepositoriesManager:
         Returns:
             True if removed successfully
         """
-        repo_url = self._clean_url(repo_url)
+        repo_url = normalize_repo_url(repo_url)
 
         previous = self.repositories
         remaining = [r for r in previous if r.get('url') != repo_url]
@@ -156,7 +146,7 @@ class SavedRepositoriesManager:
     
     def has(self, repo_url: str) -> bool:
         """Check if a repository is already saved."""
-        repo_url = self._clean_url(repo_url)
+        repo_url = normalize_repo_url(repo_url)
         return any(r.get('url') == repo_url for r in self.repositories)
     
     def get_registry_repositories(self) -> List[Dict[str, str]]:

src/plugin_system/schema_manager.py

@@ -14,6 +14,7 @@ import jsonschema
 from jsonschema import Draft7Validator, ValidationError
 
 from src.core_config_keys import CORE_CONFIG_KEYS
+from src.element_style import expand_style_elements
 
 
 def _renders_as_object(prop: Dict[str, Any]) -> bool:
@@ -452,11 +453,7 @@ class SchemaManager:
             # full per-element style blocks (font/size/color + layout
             # offsets) the web-UI config form renders. No-op for schemas
             # without the declaration; never raises.
-            try:
-                from src.element_style import expand_style_elements
-                schema = expand_style_elements(schema)
-            except ImportError:
-                pass
+            schema = expand_style_elements(schema)
 
             # Cache the schema
             self._schema_cache[plugin_id] = schema
@@ -643,20 +640,12 @@ class SchemaManager:
                     [name for name in CORE_PLUGIN_PROPERTIES if name not in declared]
                 )
 
-            # Create validator with enhanced schema
+            # iter_errors reports every violation, including one ``required``
+            # error per missing field at every depth.
             validator = Draft7Validator(enhanced_schema)
-            
-            # Collect all validation errors
             for error in validator.iter_errors(config):
-                error_msg = self._format_validation_error(error, plugin_id)
-                errors.append(error_msg)
-            
-            # Check required fields
-            required_fields = enhanced_schema.get('required', [])
-            for field in required_fields:
-                if field not in config:
-                    errors.append(f"Missing required field: '{field}'")
-            
+                errors.append(self._format_validation_error(error, plugin_id))
+
             if errors:
                 return False, errors
             
@@ -687,7 +676,15 @@ class SchemaManager:
         field_path = f"'{path}'" if path else "root"
         
         if error.validator == 'required':
-            missing = error.validator_value
+            # validator_value is the schema's whole ``required`` list; the
+            # error itself is about one field, which jsonschema names only in
+            # its message ("'api_key' is a required property").
+            missing = next(
+                (name for name in error.validator_value
+                 if error.message.startswith(f"{name!r} ")),
+                None)
+            if missing is None:
+                return f"Field {field_path}: {error.message}"
             return f"Field {field_path}: Missing required property '{missing}'"
         elif error.validator == 'type':
             expected = error.validator_value

src/plugin_system/state_manager.py

@@ -34,7 +34,9 @@ class PluginState:
     version: Optional[str] = None
     installed_at: Optional[datetime] = None
     last_updated: Optional[datetime] = None
-    config_version: int = 1  # For detecting state corruption
+    # Bumped on every update_plugin_state(). Nothing reads it; it stays so
+    # plugin_state.json keeps the shape older releases load with cls(**data).
+    config_version: int = 1
     metadata: Dict[str, Any] = None
     
     def __post_init__(self):
@@ -100,6 +102,8 @@ class PluginStateManager:
         
         # State storage
         self._states: Dict[str, PluginState] = {}
+        # The file's top-level "version", written back as read. Nothing
+        # checks it yet; it is there for a future format change to branch on.
         self._state_version = 1
         
         # Threading
@@ -193,7 +197,6 @@ class PluginStateManager:
                     current_state.metadata = {}
                 current_state.metadata.update(updates['metadata'])
             
-            # Increment config version
             current_state.config_version += 1
             
             # Store updated state

src/startup_validator.py

@@ -1,8 +1,14 @@
 """
 Startup Validator
 
-Validates system configuration, plugins, and dependencies on startup.
-Fails fast with clear error messages to prevent runtime issues.
+Checks configuration, the cache directory, plugins and the installed systemd
+units when the display service starts, and reports what it finds.
+
+validate_all() never raises: it returns (is_valid, errors, warnings) and
+DisplayController logs them. Startup continues either way, so a problem found
+here shows up in the log rather than stopping the display. raise_on_errors()
+turns the errors into exceptions for a caller that does want to stop; the
+display service does not call it.
 """
 
 import os
@@ -180,17 +186,16 @@ class StartupValidator:
         try:
             config = self.config_manager.load_config()
             
-            # Check for required top-level keys
             required_keys = ['display', 'timezone']
             for key in required_keys:
                 if key not in config:
                     self.errors.append(f"Missing required configuration key: {key}")
-            
-            # Validate display configuration
-            display_config = config.get('display', {})
-            if not display_config:
-                self.errors.append("Display configuration is missing or empty")
-            
+
+            # A missing display section is reported once, above, and an empty
+            # one here; _validate_display_config leaves both to this method.
+            if 'display' in config and not config['display']:
+                self.errors.append("Display configuration is empty")
+
         except ConfigError as e:
             self.errors.append(f"Configuration error: {e}")
         except Exception as e:
@@ -247,8 +252,7 @@ class StartupValidator:
             display_config = config.get('display', {})
             
             if not display_config:
-                self.errors.append("Display configuration is missing")
-                return
+                return  # reported by _validate_config
             
             hardware_config = display_config.get('hardware', {})
             if not hardware_config:

src/wifi_manager.py

@@ -9,24 +9,18 @@ Tested and optimized for:
 - Raspberry Pi OS Bookworm (Debian 12) with NetworkManager
 - Raspberry Pi 3B+, 4, 5 with built-in WiFi
 
-Sudoers Requirements:
-    The following sudoers entries are required for passwordless operation.
-    Add to /etc/sudoers.d/ledmatrix_wifi:
-
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/nmcli
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl start hostapd
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl stop hostapd
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl start dnsmasq
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl stop dnsmasq
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl restart NetworkManager
-    ledpi ALL=(ALL) NOPASSWD: /usr/sbin/ip
-    ledpi ALL=(ALL) NOPASSWD: /sbin/ip
-    ledpi ALL=(ALL) NOPASSWD: /usr/sbin/rfkill
-    ledpi ALL=(ALL) NOPASSWD: /usr/sbin/iptables
-    ledpi ALL=(ALL) NOPASSWD: /usr/sbin/sysctl
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/cp /tmp/hostapd.conf /etc/hostapd/hostapd.conf
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/cp /tmp/dnsmasq.conf /etc/dnsmasq.d/ledmatrix-captive.conf
-    ledpi ALL=(ALL) NOPASSWD: /usr/bin/rm -f /etc/dnsmasq.d/ledmatrix-captive.conf
+Privileges:
+    The web interface runs as an unprivileged user and reaches nmcli,
+    systemctl, sysctl, nft and rfkill through exact-command sudo rules.
+    scripts/install/configure_wifi_permissions.sh writes those rules (and a
+    PolicyKit rule for NetworkManager); first_time_install.sh runs it. Use
+    that script rather than granting commands by hand. It deliberately
+    grants neither ``iptables`` nor ``ip``: their rules take a live interface
+    name, so they would need a wildcard, and ``iptables --modprobe=<path>``
+    and ``ip netns exec`` both run an arbitrary program as root. The code
+    paths that call them with sudo therefore only work where the user has
+    broader sudo rights (a stock Raspberry Pi image grants the default user
+    blanket NOPASSWD).
 """
 
 import subprocess
@@ -39,6 +33,8 @@ from pathlib import Path
 from typing import Any, Dict, List, Optional, Tuple
 from dataclasses import dataclass
 
+from src.config_manager_atomic import atomic_write_json
+
 logger = logging.getLogger(__name__)
 
 # Path for storing WiFi configuration (will be set dynamically)
@@ -78,6 +74,22 @@ DNSMASQ_SERVICE = "dnsmasq"
 DEFAULT_AP_SSID = "LEDMatrix-Setup"
 DEFAULT_AP_CHANNEL = 7
 
+#: The access point's own address. Clients get 192.168.4.2-20 from dnsmasq
+#: (hostapd mode) and every DNS name resolves here, which is what makes phones
+#: show the captive-portal page.
+AP_IP = "192.168.4.1"
+
+#: The web interface's port. The captive portal redirects port 80 to it.
+PORTAL_PORT = 5000
+
+#: The NetworkManager profile this module creates for the access point.
+AP_PROFILE_NAME = "LEDMatrix-Setup-AP"
+
+#: AP profiles taken down and deleted before a new one is created and when AP
+#: mode ends: ours, NetworkManager's default hotspot name, and an older name.
+#: Deleted by name only, never by SSID, so a saved home network is never hit.
+AP_PROFILE_NAMES = (AP_PROFILE_NAME, "Hotspot", "TickerSetup-AP")
+
 # LED status message file (for display_controller integration)
 LED_STATUS_FILE = None  # Will be set dynamically
 
@@ -198,30 +210,8 @@ class WiFiManager:
             logger.debug(f"Could not clear LED status message: {e}")
     
     def _check_command(self, command: str) -> bool:
-        """Check if a command is available"""
-        try:
-            # First try 'which' command
-            result = subprocess.run(
-                ["which", command],
-                capture_output=True,
-                timeout=2
-            )
-            if result.returncode == 0:
-                return True
-            
-            # Check common sbin paths (not in standard user PATH)
-            sbin_paths = [
-                f"/usr/sbin/{command}",
-                f"/sbin/{command}",
-                f"/usr/local/sbin/{command}"
-            ]
-            for path in sbin_paths:
-                if os.path.isfile(path) and os.access(path, os.X_OK):
-                    return True
-            
-            return False
-        except (subprocess.TimeoutExpired, subprocess.SubprocessError, OSError):
-            return False
+        """Whether ``command`` is installed (see _find_command_path)."""
+        return self._find_command_path(command) is not None
 
     def _find_command_path(self, command: str) -> Optional[str]:
         """
@@ -321,14 +311,22 @@ class WiFiManager:
             del self.config["saved_networks"]
             self._save_config()
     
-    def _save_config(self):
-        """Save WiFi configuration to file"""
+    def _save_config(self) -> bool:
+        """Write ``self.config`` to ``self.config_path``.
+
+        The write is atomic and keeps the file's owner and shared group (see
+        atomic_write_json), so a save by the root display service does not
+        lock the web user out of the file. Returns False when the file could
+        not be written, for example when an older root-run save left it
+        owned by root; the in-memory config is kept either way.
+        """
         try:
-            with open(self.config_path, 'w') as f:
-                json.dump(self.config, f, indent=2)
-            logger.info(f"Saved WiFi config to {self.config_path}")
-        except Exception as e:
-            logger.error(f"Failed to save WiFi config: {e}")
+            atomic_write_json(self.config_path, self.config)
+        except (OSError, TypeError, ValueError) as e:
+            logger.error(f"Failed to save WiFi config to {self.config_path}: {e}")
+            return False
+        logger.info(f"Saved WiFi config to {self.config_path}")
+        return True
     
     def get_wifi_status(self) -> WiFiStatus:
         """
@@ -402,8 +400,6 @@ class WiFiManager:
                     for line in result.stdout.strip().split('\n'):
                         if '802-11-wireless.ssid:' in line:
                             ssid = line.split(':', 1)[1].strip()
-                            if ssid:
-                                continue
                         elif 'WIFI.SIGNAL:' in line:
                             try:
                                 signal = int(line.split(':', 1)[1].strip())
@@ -425,24 +421,7 @@ class WiFiManager:
                                 ssid = parts[1].strip()
                                 if ssid:
                                     break
-                
-                # Fallback: Get signal strength if not already retrieved
-                if signal == 0 and wlan_device:
-                    result = subprocess.run(
-                        ["nmcli", "-t", "-f", "WIFI.SIGNAL", "device", "show", wlan_device],
-                        capture_output=True,
-                        text=True,
-                        timeout=5
-                    )
-                    if result.returncode == 0:
-                        for line in result.stdout.strip().split('\n'):
-                            if 'WIFI.SIGNAL:' in line:
-                                try:
-                                    signal = int(line.split(':', 1)[1].strip())
-                                    break
-                                except (ValueError, IndexError):
-                                    pass
-            
+
             # Get IP address if connected
             if wifi_connected and wlan_device:
                 result = subprocess.run(
@@ -536,7 +515,7 @@ class WiFiManager:
                 if result.returncode == 0:
                     ips = result.stdout.strip().split()
                     for ip in ips:
-                        if not ip.startswith('192.168.4.1'):  # Exclude AP IP
+                        if ip != AP_IP:
                             ip_address = ip
                             break
             
@@ -778,13 +757,13 @@ class WiFiManager:
         if subprocess.run(
             ["sudo", iptables, "-t", "nat", "-C", "PREROUTING",
              "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
-             "-j", "REDIRECT", "--to-port", "5000"],
+             "-j", "REDIRECT", "--to-port", str(PORTAL_PORT)],
             capture_output=True, timeout=5
         ).returncode != 0:
             r = subprocess.run(
                 ["sudo", iptables, "-t", "nat", "-A", "PREROUTING",
                  "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
-                 "-j", "REDIRECT", "--to-port", "5000"],
+                 "-j", "REDIRECT", "--to-port", str(PORTAL_PORT)],
                 capture_output=True, text=True, timeout=5
             )
             if r.returncode != 0:
@@ -794,12 +773,12 @@ class WiFiManager:
 
         if subprocess.run(
             ["sudo", iptables, "-C", "INPUT",
-             "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000", "-j", "ACCEPT"],
+             "-i", self._wifi_interface, "-p", "tcp", "--dport", str(PORTAL_PORT), "-j", "ACCEPT"],
             capture_output=True, timeout=5
         ).returncode != 0:
             r = subprocess.run(
                 ["sudo", iptables, "-A", "INPUT",
-                 "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000", "-j", "ACCEPT"],
+                 "-i", self._wifi_interface, "-p", "tcp", "--dport", str(PORTAL_PORT), "-j", "ACCEPT"],
                 capture_output=True, text=True, timeout=5
             )
             if r.returncode != 0:
@@ -808,7 +787,7 @@ class WiFiManager:
                 return False
 
         self._redirect_backend = "iptables"
-        logger.info("iptables: port 80→5000 redirect rules added")
+        logger.info(f"iptables: port 80→{PORTAL_PORT} redirect rules added")
         return True
 
     def _setup_iptables_redirect_nftables(self, nft: str) -> bool:
@@ -819,7 +798,7 @@ class WiFiManager:
             ["sudo", nft, "add", "chain", "ip", "ledmatrix", "prerouting",
              "{", "type", "nat", "hook", "prerouting", "priority", "-100", ";", "}"],
             ["sudo", nft, "add", "rule", "ip", "ledmatrix", "prerouting",
-             "iif", self._wifi_interface, "tcp", "dport", "80", "redirect", "to", ":5000"],
+             "iif", self._wifi_interface, "tcp", "dport", "80", "redirect", "to", f":{PORTAL_PORT}"],
         ]
         for cmd in cmds:
             r = subprocess.run(cmd, capture_output=True, text=True, timeout=5)
@@ -832,7 +811,7 @@ class WiFiManager:
                 logger.debug(f"nft cmd non-zero (may already exist): {r.stderr.strip()}")
 
         self._redirect_backend = "nftables"
-        logger.info("nftables: port 80→5000 redirect rule added")
+        logger.info(f"nftables: port 80→{PORTAL_PORT} redirect rule added")
         return True
 
     def _teardown_iptables_redirect(self) -> None:
@@ -847,12 +826,12 @@ class WiFiManager:
                     subprocess.run(
                         ["sudo", iptables, "-t", "nat", "-D", "PREROUTING",
                          "-i", self._wifi_interface, "-p", "tcp", "--dport", "80",
-                         "-j", "REDIRECT", "--to-port", "5000"],
+                         "-j", "REDIRECT", "--to-port", str(PORTAL_PORT)],
                         capture_output=True, timeout=5
                     )
                     subprocess.run(
                         ["sudo", iptables, "-D", "INPUT",
-                         "-i", self._wifi_interface, "-p", "tcp", "--dport", "5000",
+                         "-i", self._wifi_interface, "-p", "tcp", "--dport", str(PORTAL_PORT),
                          "-j", "ACCEPT"],
                         capture_output=True, timeout=5
                     )
@@ -887,7 +866,7 @@ class WiFiManager:
         except Exception as e:
             logger.warning(f"Could not tear down port redirect: {e}")
 
-    def _write_nm_dnsmasq_captive_conf(self, ap_ip: str = "192.168.4.1") -> None:
+    def _write_nm_dnsmasq_captive_conf(self, ap_ip: str = AP_IP) -> None:
         """
         Write the NM dnsmasq-shared.d drop-in that makes NM's built-in dnsmasq
         resolve every hostname to the AP IP.  This triggers the OS captive-portal
@@ -1026,39 +1005,53 @@ class WiFiManager:
             )
             if result.returncode != 0:
                 return []
-
-            seen_ssids = set()
-            for line in result.stdout.strip().split('\n'):
-                if not line or ':' not in line:
-                    continue
-                parts = line.split(':')
-                if len(parts) >= 3:
-                    ssid = parts[0].strip()
-                    if not ssid or ssid in seen_ssids:
-                        continue
-                    seen_ssids.add(ssid)
-                    try:
-                        signal = int(parts[1].strip())
-                        security = parts[2].strip() if len(parts) > 2 else "open"
-                        frequency_str = parts[3].strip() if len(parts) > 3 else "0"
-                        frequency_str = frequency_str.replace(" MHz", "").replace("MHz", "").strip()
-                        frequency = float(frequency_str) if frequency_str else 0.0
-                        if "WPA3" in security:
-                            sec_type = "wpa3"
-                        elif "WPA2" in security:
-                            sec_type = "wpa2"
-                        elif "WPA" in security:
-                            sec_type = "wpa"
-                        else:
-                            sec_type = "open"
-                        networks.append(WiFiNetwork(ssid=ssid, signal=signal, security=sec_type, frequency=frequency))
-                    except (ValueError, IndexError):
-                        continue
-            networks.sort(key=lambda x: x.signal, reverse=True)
+            networks = self._parse_nmcli_wifi_list(result.stdout)
         except Exception as e:
             logger.debug(f"nmcli cached list failed: {e}")
         return networks
 
+    @staticmethod
+    def _parse_nmcli_wifi_list(stdout: str) -> List[WiFiNetwork]:
+        """Parse ``nmcli -t -f SSID,SIGNAL,SECURITY,FREQ device wifi list``.
+
+        One entry per SSID (the first line seen for it; hidden networks with
+        an empty SSID are skipped), security reduced to wpa3/wpa2/wpa/open,
+        sorted strongest first. Unparseable lines are skipped.
+        """
+        networks = []
+        seen_ssids = set()
+        for line in stdout.strip().split('\n'):
+            if not line or ':' not in line:
+                continue
+            parts = line.split(':')
+            if len(parts) < 3:
+                continue
+            ssid = parts[0].strip()
+            if not ssid or ssid in seen_ssids:
+                continue
+            seen_ssids.add(ssid)
+            try:
+                signal = int(parts[1].strip())
+                security = parts[2].strip()
+                frequency_str = parts[3].strip() if len(parts) > 3 else "0"
+                frequency_str = frequency_str.replace(" MHz", "").replace("MHz", "").strip()
+                frequency = float(frequency_str) if frequency_str else 0.0
+            except (ValueError, IndexError) as e:
+                logger.debug(f"Skipping network line due to parsing error: {line[:50]}... Error: {e}")
+                continue
+            if "WPA3" in security:
+                sec_type = "wpa3"
+            elif "WPA2" in security:
+                sec_type = "wpa2"
+            elif "WPA" in security:
+                sec_type = "wpa"
+            else:
+                sec_type = "open"
+            networks.append(WiFiNetwork(ssid=ssid, signal=signal, security=sec_type,
+                                        frequency=frequency))
+        networks.sort(key=lambda x: x.signal, reverse=True)
+        return networks
+
     def _save_cached_scan(self, networks: List[WiFiNetwork]) -> None:
         """Save scan results to a cache file for use during AP mode."""
         try:
@@ -1088,7 +1081,6 @@ class WiFiManager:
 
     def _scan_nmcli(self) -> List[WiFiNetwork]:
         """Scan networks using nmcli"""
-        networks = []
         try:
             # Trigger scan
             subprocess.run(
@@ -1108,52 +1100,7 @@ class WiFiManager:
             
             if result.returncode != 0:
                 return []
-            
-            seen_ssids = set()
-            for line in result.stdout.strip().split('\n'):
-                if not line or ':' not in line:
-                    continue
-                
-                parts = line.split(':')
-                if len(parts) >= 3:
-                    ssid = parts[0].strip()
-                    if not ssid or ssid in seen_ssids:
-                        continue
-                    
-                    seen_ssids.add(ssid)
-                    
-                    try:
-                        signal = int(parts[1].strip())
-                        security = parts[2].strip() if len(parts) > 2 else "open"
-                        
-                        # Parse frequency - strip " MHz" if present
-                        frequency_str = parts[3].strip() if len(parts) > 3 else "0"
-                        frequency_str = frequency_str.replace(" MHz", "").replace("MHz", "").strip()
-                        frequency = float(frequency_str) if frequency_str else 0.0
-                        
-                        # Normalize security type
-                        if "WPA3" in security:
-                            sec_type = "wpa3"
-                        elif "WPA2" in security:
-                            sec_type = "wpa2"
-                        elif "WPA" in security:
-                            sec_type = "wpa"
-                        else:
-                            sec_type = "open"
-                        
-                        networks.append(WiFiNetwork(
-                            ssid=ssid,
-                            signal=signal,
-                            security=sec_type,
-                            frequency=frequency
-                        ))
-                    except (ValueError, IndexError) as e:
-                        logger.debug(f"Skipping network line due to parsing error: {line[:50]}... Error: {e}")
-                        continue
-            
-            # Sort by signal strength
-            networks.sort(key=lambda x: x.signal, reverse=True)
-            return networks
+            return self._parse_nmcli_wifi_list(result.stdout)
         except Exception as e:
             logger.error(f"Error scanning with nmcli: {e}")
             return []
@@ -1357,27 +1304,7 @@ class WiFiManager:
                 disconnect_success, disconnect_msg = self.disconnect_from_network(skip_ap_check=True)
                 if disconnect_success:
                     logger.info(f"Disconnected from {original_ssid}: {disconnect_msg}")
-                    # Wait for device to be ready for new connection
-                    # Check device state before proceeding
-                    max_wait = 5
-                    wait_count = 0
-                    while wait_count < max_wait:
-                        time.sleep(1)
-                        result = subprocess.run(
-                            ["nmcli", "-t", "-f", "STATE", "device", "status", self._wifi_interface],
-                            capture_output=True,
-                            text=True,
-                            timeout=5
-                        )
-                        if result.returncode == 0:
-                            state = result.stdout.strip().split(':')[-1] if ':' in result.stdout else result.stdout.strip()
-                            # Device is ready if it's disconnected or unavailable (not connecting/connected)
-                            if state in ["disconnected", "unavailable", "unmanaged"]:
-                                logger.info(f"Device ready for new connection (state: {state})")
-                                break
-                        wait_count += 1
-                    
-                    if wait_count >= max_wait:
+                    if not self._wait_for_device_idle(5):
                         logger.warning("Device may not be ready, but proceeding with connection attempt")
                 else:
                     logger.warning(f"Failed to disconnect from {original_ssid}: {disconnect_msg}")
@@ -1403,26 +1330,15 @@ class WiFiManager:
                         return False, f"Failed to connect to {ssid}, restored {original_ssid}"
                     else:
                         logger.error(f"Failed to restore original connection: {original_ssid}")
-                        # Trigger AP mode as last resort
-                        self._show_led_message("Enabling AP mode...", duration=5)
-                        ap_success, ap_msg = self.enable_ap_mode(force=True)
-                        if ap_success:
-                            logger.info("AP mode enabled as failsafe")
-                            return False, "Connection failed and restoration failed. AP mode enabled."
-                        else:
-                            logger.error(f"Failed to enable AP mode: {ap_msg}")
-                            return False, f"Connection failed, restoration failed, and AP mode failed: {ap_msg}"
+                        return self._failsafe_ap(
+                            "Connection failed and restoration failed. AP mode enabled.",
+                            "Connection failed, restoration failed, and AP mode failed")
                 
                 # If connection failed and no original connection to restore, enable AP mode
                 elif not success:
                     logger.warning(f"Connection to {ssid} failed and no original connection to restore")
-                    self._show_led_message("Enabling AP mode...", duration=5)
-                    ap_success, ap_msg = self.enable_ap_mode(force=True)
-                    if ap_success:
-                        logger.info("AP mode enabled as failsafe")
-                        return False, "Connection failed. AP mode enabled."
-                    else:
-                        return False, f"Connection failed and AP mode failed: {ap_msg}"
+                    return self._failsafe_ap("Connection failed. AP mode enabled.",
+                                             "Connection failed and AP mode failed")
                 
                 return success, message
             else:
@@ -1443,6 +1359,22 @@ class WiFiManager:
                         logger.error("Last-resort AP mode enable failed in recovery path: %s", ap_error, exc_info=True)
             return False, str(e)
     
+    def _failsafe_ap(self, enabled_msg: str, failed_msg: str) -> Tuple[bool, str]:
+        """Force the setup AP up after a connect that left no working network,
+        so the user can still reach the device.
+
+        Returns the (False, message) result for connect_to_network:
+        ``enabled_msg`` when the AP came up, else ``failed_msg`` plus the
+        reason it did not.
+        """
+        self._show_led_message("Enabling AP mode...", duration=5)
+        ap_success, ap_msg = self.enable_ap_mode(force=True)
+        if ap_success:
+            logger.info("AP mode enabled as failsafe")
+            return False, enabled_msg
+        logger.error(f"Failed to enable AP mode: {ap_msg}")
+        return False, f"{failed_msg}: {ap_msg}"
+
     def _restore_original_connection(self, connection_name: str, ssid: str) -> bool:
         """
         Restore a previously active WiFi connection.
@@ -1496,68 +1428,96 @@ class WiFiManager:
             logger.error(f"Error restoring connection: {e}")
             return False
     
+    def _find_profile_for_ssid(self, ssid: str) -> Optional[str]:
+        """Name of the saved NetworkManager profile for ``ssid``, or None.
+
+        ``802-11-wireless.ssid`` is not a column ``nmcli connection show``
+        can list, so this lists the Wi-Fi profiles and asks each one for its
+        SSID. A profile named after the SSID is the fallback, for when the
+        listing fails.
+        """
+        list_result = subprocess.run(  # nosec B603 B607 - fixed args, no user input
+            ["nmcli", "-t", "-f", "NAME,TYPE", "connection", "show"],
+            capture_output=True, text=True, timeout=5
+        )
+        if list_result.returncode == 0:
+            for line in list_result.stdout.strip().split('\n'):
+                # Terse output escapes a colon inside a field as "\:". TYPE
+                # never contains one, so the last colon ends the name.
+                conn_name, sep, conn_type = line.rpartition(':')
+                if not sep or conn_type.strip() != '802-11-wireless':
+                    continue
+                conn_name = conn_name.replace('\\:', ':').replace('\\\\', '\\')
+                ssid_r = subprocess.run(  # nosec B603 B607 - conn_name from nmcli output, not user input
+                    ["nmcli", "-g", "802-11-wireless.ssid", "connection", "show", conn_name],
+                    capture_output=True, text=True, timeout=5
+                )
+                if ssid_r.returncode == 0 and ssid_r.stdout.strip() == ssid:
+                    return conn_name
+
+        direct_check = subprocess.run(  # nosec B603 B607 - list args, no shell
+            ["nmcli", "connection", "show", ssid],
+            capture_output=True, text=True, timeout=5
+        )
+        if direct_check.returncode == 0:
+            return ssid
+        return None
+
+    def _wait_for_device_idle(self, attempts: int) -> bool:
+        """Poll the Wi-Fi device, once a second for up to ``attempts`` checks,
+        until it is disconnected, unavailable or unmanaged: a profile
+        activated while the device is still connecting or tearing down an
+        old link can fail. True if it went idle, False on timeout."""
+        for attempt in range(attempts):
+            result = subprocess.run(
+                ["nmcli", "-t", "-f", "STATE", "device", "status", self._wifi_interface],
+                capture_output=True,
+                text=True,
+                timeout=5
+            )
+            if result.returncode == 0:
+                state = result.stdout.strip().split(':')[-1]
+                if state in ("disconnected", "unavailable", "unmanaged"):
+                    logger.debug(f"Wi-Fi device idle (state: {state})")
+                    return True
+            if attempt < attempts - 1:
+                time.sleep(1)
+        return False
+
+    def _verify_connected(self, ssid: str, attempts: int = 5, delay: float = 2.0,
+                          stop_on_other_network: bool = False) -> Optional[WiFiStatus]:
+        """Wait for the device to report a connection to ``ssid``.
+
+        nmcli returns before DHCP finishes, so the status is polled every
+        ``delay`` seconds, up to ``attempts`` times. Returns that status, or
+        None if it never showed ``ssid``. With ``stop_on_other_network`` a
+        connection to a different SSID ends the wait at once as a failure.
+        """
+        for _ in range(attempts):
+            time.sleep(delay)
+            status = self.get_wifi_status()
+            if not status.connected:
+                continue
+            if status.ssid == ssid:
+                return status
+            if stop_on_other_network and status.ssid:
+                logger.warning(f"Connected to wrong network: {status.ssid} instead of {ssid}")
+                return None
+        return None
+
     def _connect_nmcli(self, ssid: str, password: str) -> Tuple[bool, str]:
         """Connect using nmcli"""
         try:
             # Show LED message
             self._show_led_message(f"Connecting to {ssid}...", duration=10)
             
-            # Find existing NM connection for this SSID.
-            # 802-11-wireless.ssid is not a valid column in 'nmcli connection show',
-            # so list all wifi connections then query each one's SSID individually.
-            list_result = subprocess.run(  # nosec B603 B607 - fixed args, no user input
-                ["nmcli", "-t", "-f", "NAME,TYPE", "connection", "show"],
-                capture_output=True, text=True, timeout=5
-            )
-            existing_conn_name = None
-            if list_result.returncode == 0:
-                for line in list_result.stdout.strip().split('\n'):
-                    if ':' not in line:
-                        continue
-                    parts = line.split(':')
-                    if len(parts) < 2 or parts[1].strip() != '802-11-wireless':
-                        continue
-                    conn_name = parts[0].strip()
-                    ssid_r = subprocess.run(  # nosec B603 B607 - conn_name from nmcli output, not user input
-                        ["nmcli", "-g", "802-11-wireless.ssid", "connection", "show", conn_name],
-                        capture_output=True, text=True, timeout=5
-                    )
-                    if ssid_r.returncode == 0 and ssid_r.stdout.strip() == ssid:
-                        existing_conn_name = conn_name
-                        break
-            
-            # Also try direct lookup by SSID (in case connection name matches SSID)
-            if not existing_conn_name:
-                direct_check = subprocess.run(
-                    ["nmcli", "connection", "show", ssid],
-                    capture_output=True,
-                    text=True,
-                    timeout=5
-                )
-                if direct_check.returncode == 0:
-                    existing_conn_name = ssid
-            
+            existing_conn_name = self._find_profile_for_ssid(ssid)
             if existing_conn_name:
                 # Connection exists, try to activate it first (faster and more reliable)
                 logger.info(f"Found existing connection for {ssid}, activating...")
                 
-                # Ensure device is ready before activating
-                # Wait for device to be in disconnected/unavailable state
-                max_wait = 3
-                for wait_attempt in range(max_wait):
-                    device_result = subprocess.run(
-                        ["nmcli", "-t", "-f", "STATE", "device", "status", self._wifi_interface],
-                        capture_output=True,
-                        text=True,
-                        timeout=5
-                    )
-                    if device_result.returncode == 0:
-                        state = device_result.stdout.strip().split(':')[-1] if ':' in device_result.stdout else device_result.stdout.strip()
-                        if state in ["disconnected", "unavailable", "unmanaged"]:
-                            break
-                    if wait_attempt < max_wait - 1:
-                        time.sleep(1)
-                
+                self._wait_for_device_idle(3)
+
                 result = subprocess.run(
                     ["nmcli", "connection", "up", existing_conn_name],
                     capture_output=True,
@@ -1566,19 +1526,8 @@ class WiFiManager:
                 )
                 
                 if result.returncode == 0:
-                    # Wait longer for connection to stabilize and verify multiple times
-                    max_verification_attempts = 5
-                    verification_delay = 2
-                    connected = False
-                    
-                    for attempt in range(max_verification_attempts):
-                        time.sleep(verification_delay)
-                        status = self.get_wifi_status()
-                        if status.connected and status.ssid == ssid:
-                            connected = True
-                            break
-                    
-                    if connected:
+                    status = self._verify_connected(ssid)
+                    if status is not None:
                         ip = status.ip_address or "Unknown"
                         self._show_led_message(f"Connected! {ip}", duration=5)
                         logger.info(f"Successfully connected to {ssid} with IP {ip}")
@@ -1605,25 +1554,8 @@ class WiFiManager:
             )
             
             if result.returncode == 0:
-                # Wait longer for connection to stabilize and verify multiple times
-                max_verification_attempts = 5
-                verification_delay = 2
-                connected = False
-                
-                for attempt in range(max_verification_attempts):
-                    time.sleep(verification_delay)
-                    status = self.get_wifi_status()
-                    if status.connected:
-                        # Verify we're connected to the correct SSID
-                        if status.ssid == ssid:
-                            connected = True
-                            break
-                        elif status.ssid:
-                            # Connected to different network - this is a failure
-                            logger.warning(f"Connected to wrong network: {status.ssid} instead of {ssid}")
-                            break
-                
-                if connected:
+                status = self._verify_connected(ssid, stop_on_other_network=True)
+                if status is not None:
                     ip = status.ip_address or "Unknown"
                     self._show_led_message(f"Connected! {ip}", duration=5)
                     logger.info(f"Successfully connected to {ssid} with IP {ip}")
@@ -1717,14 +1649,10 @@ class WiFiManager:
         return any(ind in lower for ind in indicators)
 
     def _connect_wpa_supplicant(self, ssid: str, password: str) -> Tuple[bool, str]:
-        """Connect using wpa_supplicant (fallback)"""
-        try:
-            # This would require modifying /etc/wpa_supplicant/wpa_supplicant.conf
-            # For now, return not implemented
-            return False, "wpa_supplicant connection not yet implemented. Please use NetworkManager (nmcli)."
-        except Exception as e:
-            logger.error(f"Error connecting with wpa_supplicant: {e}")
-            return False, str(e)
+        """Without NetworkManager there is no supported way to connect: doing it
+        through wpa_supplicant would mean editing its config file, which is
+        not implemented. Always returns (False, reason)."""
+        return False, "wpa_supplicant connection not yet implemented. Please use NetworkManager (nmcli)."
     
     def disconnect_from_network(self, skip_ap_check: bool = False) -> Tuple[bool, str]:
         """
@@ -1745,33 +1673,18 @@ class WiFiManager:
             
             # Disconnect using nmcli
             if self.has_nmcli:
-                # Try to disconnect the specific connection first (more reliable)
+                # Take the profile down first, then the device, so the
+                # device ends up disconnected even when no profile is found.
                 if status.ssid:
-                    # Find the connection name for this SSID
-                    conn_result = subprocess.run(
-                        ["nmcli", "-t", "-f", "NAME,802-11-wireless.ssid", "connection", "show"],
-                        capture_output=True,
-                        text=True,
-                        timeout=5
-                    )
-                    if conn_result.returncode == 0:
-                        for line in conn_result.stdout.strip().split('\n'):
-                            if ':' in line:
-                                parts = line.split(':')
-                                if len(parts) >= 2:
-                                    conn_name = parts[0].strip()
-                                    conn_ssid = parts[1].strip() if len(parts) > 1 else ""
-                                    if conn_ssid == status.ssid:
-                                        # Disconnect this specific connection
-                                        subprocess.run(
-                                            ["nmcli", "connection", "down", conn_name],
-                                            capture_output=True,
-                                            timeout=10
-                                        )
-                                        logger.info(f"Disconnected connection {conn_name} for {status.ssid}")
-                                        break
-                
-                # Also disconnect the device to ensure clean state
+                    conn_name = self._find_profile_for_ssid(status.ssid)
+                    if conn_name:
+                        subprocess.run(  # nosec B603 B607 - list args, no shell
+                            ["nmcli", "connection", "down", conn_name],
+                            capture_output=True,
+                            timeout=10
+                        )
+                        logger.info(f"Disconnected connection {conn_name} for {status.ssid}")
+
                 result = subprocess.run(
                     ["nmcli", "device", "disconnect", self._wifi_interface],
                     capture_output=True,
@@ -1814,7 +1727,11 @@ class WiFiManager:
             max_retries: Maximum number of retry attempts to enable WiFi radio
             
         Returns:
-            True if WiFi is enabled or was successfully enabled, False otherwise
+            True if the radio is enabled or was enabled here. Also True when
+            the state could not be checked at all (nmcli or rfkill raised on
+            every attempt): callers go ahead rather than refusing to act on a
+            radio that is probably fine. False only when the radio was seen
+            disabled or blocked and could not be turned on.
         """
         for attempt in range(max_retries):
             try:
@@ -2062,11 +1979,7 @@ class WiFiManager:
                 if result[0]:
                     self._ap_enabled_at = time.time()
                     if force:
-                        try:
-                            self._FORCE_AP_FLAG_PATH.touch()
-                            logger.debug(f"Force-AP flag created: {self._FORCE_AP_FLAG_PATH}")
-                        except OSError as exc:
-                            logger.warning(f"Failed to create force-AP flag {self._FORCE_AP_FLAG_PATH}: {exc}")
+                        self._mark_forced()
                     return result
 
             # Fallback to nmcli hotspot (simpler, no captive portal)
@@ -2077,11 +1990,7 @@ class WiFiManager:
                 if result[0]:
                     self._ap_enabled_at = time.time()
                     if force:
-                        try:
-                            self._FORCE_AP_FLAG_PATH.touch()
-                            logger.debug(f"Force-AP flag created: {self._FORCE_AP_FLAG_PATH}")
-                        except OSError as exc:
-                            logger.warning(f"Failed to create force-AP flag {self._FORCE_AP_FLAG_PATH}: {exc}")
+                        self._mark_forced()
                 return result
 
             return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)"
@@ -2089,6 +1998,15 @@ class WiFiManager:
             logger.error(f"Error in enable_ap_mode: {e}")
             return False, str(e)
     
+    def _mark_forced(self) -> None:
+        """Record that AP mode was forced on, so the periodic check leaves it
+        up even when Ethernet is connected (see _manage_ap_mode)."""
+        try:
+            self._FORCE_AP_FLAG_PATH.touch()
+            logger.debug(f"Force-AP flag created: {self._FORCE_AP_FLAG_PATH}")
+        except OSError as exc:
+            logger.warning(f"Failed to create force-AP flag {self._FORCE_AP_FLAG_PATH}: {exc}")
+
     def _enable_ap_mode_hostapd(self) -> Tuple[bool, str]:
         """Enable AP mode using hostapd and dnsmasq (captive portal)"""
         try:
@@ -2115,7 +2033,7 @@ class WiFiManager:
                     timeout=10
                 )
                 subprocess.run(
-                    ["sudo", "ip", "addr", "add", "192.168.4.1/24", "dev", self._wifi_interface],
+                    ["sudo", "ip", "addr", "add", f"{AP_IP}/24", "dev", self._wifi_interface],
                     capture_output=True,
                     timeout=10
                 )
@@ -2124,7 +2042,7 @@ class WiFiManager:
                     capture_output=True,
                     timeout=10
                 )
-                logger.info(f"Configured {self._wifi_interface} with IP 192.168.4.1 for AP mode")
+                logger.info(f"Configured {self._wifi_interface} with IP {AP_IP} for AP mode")
             except (subprocess.TimeoutExpired, subprocess.SubprocessError, OSError) as e:
                 logger.warning(f"Error setting up {self._wifi_interface} IP: {e}")
             
@@ -2168,7 +2086,7 @@ class WiFiManager:
                 # Use the validated SSID so the displayed name matches what hostapd broadcast
                 ap_ssid, _ = self._validate_ap_config()
                 self._show_led_message(
-                    f"WiFi Setup\n{ap_ssid}\nNo password\n192.168.4.1:5000", duration=10
+                    f"WiFi Setup\n{ap_ssid}\nNo password\n{AP_IP}:{PORTAL_PORT}", duration=10
                 )
                 return True, "AP mode enabled"
             except Exception as e:
@@ -2198,7 +2116,7 @@ class WiFiManager:
 
             # Delete only the specific application-managed AP profiles by name.
             # Never delete by SSID — that would destroy a user's saved home network.
-            for conn_name in ["Hotspot", "LEDMatrix-Setup-AP", "TickerSetup-AP"]:
+            for conn_name in AP_PROFILE_NAMES:
                 subprocess.run(["nmcli", "connection", "down", conn_name],
                                 capture_output=True, timeout=5)
                 subprocess.run(["nmcli", "connection", "delete", conn_name],
@@ -2215,14 +2133,14 @@ class WiFiManager:
             cmd = [
                 "nmcli", "connection", "add",
                 "type", "wifi",
-                "con-name", "LEDMatrix-Setup-AP",
+                "con-name", AP_PROFILE_NAME,
                 "ifname", self._wifi_interface,
                 "ssid", ap_ssid,
                 "802-11-wireless.mode", "ap",
                 "802-11-wireless.band", "bg",   # 2.4 GHz for maximum compatibility
                 "802-11-wireless.channel", str(ap_channel),
                 "ipv4.method", "shared",
-                "ipv4.addresses", "192.168.4.1/24",
+                "ipv4.addresses", f"{AP_IP}/24",
                 # No 802-11-wireless-security section → open network
             ]
 
@@ -2247,14 +2165,14 @@ class WiFiManager:
 
             logger.info("AP connection profile created, bringing it up...")
             up_result = subprocess.run(
-                ["nmcli", "connection", "up", "LEDMatrix-Setup-AP"],
+                ["nmcli", "connection", "up", AP_PROFILE_NAME],
                 capture_output=True, text=True, timeout=20
             )
             if up_result.returncode != 0:
                 error_msg = up_result.stderr.strip() or up_result.stdout.strip()
                 logger.error(f"Failed to bring up AP connection: {error_msg}")
                 self._remove_nm_dnsmasq_captive_conf()
-                subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
+                subprocess.run(["nmcli", "connection", "delete", AP_PROFILE_NAME],
                                capture_output=True, timeout=10)
                 self._show_led_message("AP mode failed", duration=5)
                 return False, f"Failed to start AP: {error_msg}"
@@ -2266,9 +2184,9 @@ class WiFiManager:
             if not self._setup_iptables_redirect():
                 logger.error("Captive-portal redirect setup failed; rolling back AP profile")
                 self._remove_nm_dnsmasq_captive_conf()
-                subprocess.run(["nmcli", "connection", "down", "LEDMatrix-Setup-AP"],
+                subprocess.run(["nmcli", "connection", "down", AP_PROFILE_NAME],
                                capture_output=True, timeout=10)
-                subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
+                subprocess.run(["nmcli", "connection", "delete", AP_PROFILE_NAME],
                                capture_output=True, timeout=10)
                 self._clear_led_message()
                 return False, "AP started but captive-portal redirect setup failed"
@@ -2282,17 +2200,17 @@ class WiFiManager:
                 logger.debug(f"AP verification attempt {_attempt + 1}/5 not yet active, waiting 2s")
                 time.sleep(2)
             if status.get('active'):
-                ip = status.get('ip', '192.168.4.1')
+                ip = status.get('ip', AP_IP)
                 logger.info(f"AP mode confirmed active at {ip} (open network, no password)")
-                self._show_led_message(f"WiFi Setup\n{ap_ssid}\nNo password\n{ip}:5000", duration=10)
-                return True, f"AP mode enabled (open network) - Access at {ip}:5000"
+                self._show_led_message(f"WiFi Setup\n{ap_ssid}\nNo password\n{ip}:{PORTAL_PORT}", duration=10)
+                return True, f"AP mode enabled (open network) - Access at {ip}:{PORTAL_PORT}"
             else:
                 logger.error("AP mode started but not verified by status check — rolling back")
                 self._teardown_iptables_redirect()
                 self._remove_nm_dnsmasq_captive_conf()
-                subprocess.run(["nmcli", "connection", "down", "LEDMatrix-Setup-AP"],
+                subprocess.run(["nmcli", "connection", "down", AP_PROFILE_NAME],
                                capture_output=True, timeout=10)
-                subprocess.run(["nmcli", "connection", "delete", "LEDMatrix-Setup-AP"],
+                subprocess.run(["nmcli", "connection", "delete", AP_PROFILE_NAME],
                                capture_output=True, timeout=10)
                 self._clear_led_message()
                 return False, "AP mode started but verification failed"
@@ -2326,9 +2244,9 @@ class WiFiManager:
                 conn_name = parts[0].strip()
                 conn_type = parts[1].strip().lower()
                 # Match our known AP profile name OR the legacy nmcli hotspot type
-                if conn_name == "LEDMatrix-Setup-AP" or 'hotspot' in conn_type:
+                if conn_name == AP_PROFILE_NAME or 'hotspot' in conn_type:
                     # Get actual IP address (may be 192.168.4.1 or 10.42.0.1 depending on config)
-                    ip = '192.168.4.1'
+                    ip = AP_IP
                     interface = parts[2] if len(parts) > 2 else self._wifi_interface
                     try:
                         ip_result = subprocess.run(
@@ -2399,7 +2317,7 @@ class WiFiManager:
                     )
                 else:
                     # Disable nmcli hotspot mode (fallback)
-                    for conn_name in ["LEDMatrix-Setup-AP", "Hotspot", "TickerSetup-AP"]:
+                    for conn_name in AP_PROFILE_NAMES:
                         subprocess.run(
                             ["nmcli", "connection", "down", conn_name],
                             capture_output=True,
@@ -2428,7 +2346,7 @@ class WiFiManager:
 
                     # Clean up WiFi interface IP configuration
                     subprocess.run(
-                        ["sudo", "ip", "addr", "del", "192.168.4.1/24", "dev", self._wifi_interface],
+                        ["sudo", "ip", "addr", "del", f"{AP_IP}/24", "dev", self._wifi_interface],
                         capture_output=True,
                         timeout=10
                     )
@@ -2541,13 +2459,13 @@ ignore_broadcast_ssid=0
 dhcp-range=192.168.4.2,192.168.4.20,255.255.255.0,24h
 
 # Captive portal: Redirect all DNS queries to Pi
-address=/#/192.168.4.1
+address=/#/{AP_IP}
 
 # Captive portal detection endpoints
-address=/captive.apple.com/192.168.4.1
-address=/connectivitycheck.gstatic.com/192.168.4.1
-address=/www.msftconnecttest.com/192.168.4.1
-address=/detectportal.firefox.com/192.168.4.1
+address=/captive.apple.com/{AP_IP}
+address=/connectivitycheck.gstatic.com/{AP_IP}
+address=/www.msftconnecttest.com/{AP_IP}
+address=/detectportal.firefox.com/{AP_IP}
 """
 
             # Write config (requires sudo)
@@ -2651,9 +2569,10 @@ address=/detectportal.firefox.com/192.168.4.1
                 # Pre-cache a WiFi scan so the captive portal can show networks
                 try:
                     logger.info("Running pre-AP WiFi scan for captive portal cache...")
+                    # AP mode is not up yet, so this is a live scan, and
+                    # scan_networks saves its result for the portal.
                     networks, _cached = self.scan_networks(allow_cached=False)
                     if networks:
-                        self._save_cached_scan(networks)
                         logger.info(f"Cached {len(networks)} networks for captive portal")
                 except Exception as scan_err:
                     logger.debug(f"Pre-AP scan failed (non-critical): {scan_err}")