fix: /errors stack traces, Wi-Fi disconnect and save, plugin fonts, API cache TTL (#636)

* fix(errors): record the exception's own stack trace

record_error() called traceback.format_exc(), which only sees an
exception while its except block is running. plugin_executor records
exceptions caught on a worker thread after that block has ended, so
every trace on /errors read "NoneType: None". The trace is now built
from the exception's __traceback__. The executor's log call had the
same problem with exc_info=True and now passes the exception.

record_error() also merged LEDMatrixError context into the caller's
dict in place; it now works on a copy.

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

* docs(wifi): point at configure_wifi_permissions.sh instead of a sudoers list

The module docstring told users to grant NOPASSWD sudo on iptables and
ip. configure_wifi_permissions.sh refuses those grants on purpose: a
wildcard rule for either runs an arbitrary program as root. Point at
the script and say why it leaves them out.

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

* fix(wifi): disconnect finds the saved profile by SSID

disconnect_from_network() asked `nmcli -f NAME,802-11-wireless.ssid
connection show` for the profile to take down, but nmcli rejects that
column for `connection show`, so the lookup always failed and only the
device was disconnected. The per-profile lookup _connect_nmcli() already
used is now _find_profile_for_ssid(), and both callers share it. It
also splits terse output on the last colon and unescapes "\:", so a
profile name containing a colon is found.

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

* fix(wifi): write wifi_config.json atomically and report a failed save

_save_config() opened the file for writing in place and swallowed any
error, so a wifi_config.json left owned by root made the web toggle for
auto-enabling AP mode report success while nothing was saved, and a
crash mid-write could truncate the file. It now uses atomic_write_json,
which also keeps the file's owner and shared group when root saves it,
and returns False on failure. POST /wifi/ap/auto-enable answers 500 in
that case.

The file is now written with indent=4, like the other config files.

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

* fix(fonts): resolve plugin:// fonts in the plugin's own directory

FontManager looked for a plugin's bundled fonts under Path("plugins") /
plugin_id: relative to the process cwd, and not the default install
directory (plugin-repos/), so a manifest's plugin:// fonts never loaded.

register_plugin_fonts() takes an optional plugin_dir, and PluginManager
passes the directory it loaded the plugin from. Callers that omit it get
a lookup in the configured plugin_system.plugins_directory, then plugins/,
resolved against the install root.

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

* fix(api-helper): cache responses for the requested cache_ttl

APIHelper.get(cache_ttl=...) and set_cache(ttl=...) dropped the ttl on
the claim that CacheManager does not support one, but CacheManager.set()
takes a ttl, stores it with the entry, and both cache tiers honour it
over a reader's max_age. Without it every response expired after the
300-second default read age, whatever the plugin asked for. The ttl is
now passed through, and the cache read passes cache_ttl as max_age for
entries written without one. The class docstring describes what the
helper actually does.

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

* fix(style): one scale range for the schema, element_scale and LogoHelper

The generated Scale field allowed 0.1 to 10, element_style's reader
capped at 10 with no floor, and LogoHelper accepted 0.05 to 8 and reset
anything else to 1.0. A logo scale of 9, which the form accepts, drew at
the shipped size.

MIN_ELEMENT_SCALE / MAX_ELEMENT_SCALE (0.1, 10.0) in src.element_style
are now the schema bounds and the clamp every reader applies through
coerce_scale(): a positive number outside the range is clamped, and
anything that is not a finite positive number means the default. That
also stops element_scale() passing NaN through, since min(nan, 10.0)
is nan.

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

* fix(logos): placeholder lands at the requested path; empty logos list

download_missing_logo() wrote its fallback placeholder to
<normalize_abbreviation(abbr)>.png in the logo directory rather than to
the logo_path the caller passed, so it could return True while nothing
existed where the plugin looks (e.g. "TA&M.png" vs "TAANDM.png").
create_placeholder_logo() takes an optional filepath, and
download_missing_logo passes the requested one.

download_missing_logo_for_team() only caught KeyError, so a team whose
"logos" list is empty raised IndexError; it now treats KeyError,
IndexError and TypeError as "no logo URL".

The placeholder is drawn with PLACEHOLDER_SIZE / PLACEHOLDER_BG, the
constants is_placeholder_logo() recognises it by, instead of repeated
literals.

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

* fix(fonts): resolve bundled font paths against the install root

TextHelper's default font_dir, the logo placeholder's font and
FontManager's font_overrides.json were all relative to the process cwd,
so a process started anywhere but the install root (the plugin safety
harness, a manual run, a unit without WorkingDirectory) drew with PIL's
default face and read no overrides. They now go through
font_layout.resolve_asset_path; the overrides file sits in the install
root's config/.

The resolver docstrings described an order the code does not follow:
resolve_asset_path never consults the cwd, and sports_shared's
_resolve_font_path tries the cwd first. Both docstrings now say what
the code does, and _resolve_font_path calls resolve_asset_path instead
of probing FontManager for it.

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

* fix(sync): the web UI reads the sync status file the display writes

sync_manager writes its status to tempfile.gettempdir(), but
GET /api/v3/sync/status read a hardcoded /tmp/led_matrix_sync_status.json
and defaulted the port to a literal 5765. Wherever TMPDIR is set (or on
any non-/tmp host) the page only ever showed "starting". The endpoint now
uses sync_manager.STATUS_FILE and SYNC_PORT.

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

* fix(http): the rankings resolver sends the project's User-Agent

DynamicTeamResolver fetched ESPN rankings with a bare requests.get, so
it sent python-requests' default User-Agent, which ESPN rejects; the
AP_TOP_N favourites then resolved to nothing. It now sends
DEFAULT_HTTP_HEADERS. BaseOddsManager carried its own copy of the
User-Agent string and now uses the same shared headers (which also adds
Accept-Language).

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

* fix(backup): record the core release and read the configured plugin dir

The manifest's ledmatrix_version came from a VERSION file that does not
exist, then from .git/HEAD: a 12-character sha, or "ref: refs/he" when
the branch's ref was packed. It is now src.__version__.

list_installed_plugins() scanned a hardcoded plugin-repos/, so on an
install whose plugin_system.plugins_directory points elsewhere, plugins
missing from plugin_state.json were left out of the backup. It now reads
the configured directory from config/config.json, defaulting to
plugin-repos.

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

* fix(startup): report a missing display section once

A config without a display section produced three errors for the one
problem ("Missing required configuration key: display", "Display
configuration is missing or empty" and "Display configuration is
missing"), and an empty one produced two. _validate_config now reports
it once, as a missing key or an empty section, and
_validate_display_config leaves it to that.

The module docstring said the validator fails fast; nothing in the
display service calls raise_on_errors(), so it now says the errors are
reported and startup continues.

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

* refactor(wifi): share the copied blocks and name the AP constants

- _parse_nmcli_wifi_list() is the one parser behind _scan_nmcli and
  _scan_nmcli_cached.
- _verify_connected(), _wait_for_device_idle(), _failsafe_ap() and
  _mark_forced() replace blocks that were pasted two or three times in
  the connect and enable-AP paths. The device-idle wait now checks
  before its first one-second sleep instead of after it.
- _check_command() calls _find_command_path() instead of repeating it.
- AP_IP, PORTAL_PORT, AP_PROFILE_NAME and AP_PROFILE_NAMES name values
  that were spelled out 14, 12, 8 and 2 times; the two deletion loops
  now walk the same tuple. The iwconfig status path compares the AP
  address exactly: startswith() also skipped 192.168.4.10-19.
- Dropped a second WIFI.SIGNAL query that repeated the first, a no-op
  "if ssid: continue", the try/except around _connect_wpa_supplicant's
  constant return, and a second save of a scan scan_networks already
  saves.
- _ensure_wifi_radio_enabled's docstring says it returns True when the
  radio state cannot be read at all.

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

* refactor(config): drop dead branches and history comments in ConfigManager

- The module docstring pointed plugin authors at update_plugin_config(),
  which does not exist; it now names save_config_atomic() and
  save_raw_file_content().
- load_config's FileNotFoundError handler tested the message for
  "config_secrets.json", but a missing secrets file is handled where it
  is read, so only config.json reaches it; the check is gone.
- save_raw_file_content's `file_type == "main" or "secrets"` guard was
  always true (anything else raised earlier).
- get_raw_file_content('secrets') already returns {} for a missing file,
  so the os.path.exists() in front of two calls to it is gone.
- Comments that narrated earlier behaviour are rewritten as what the
  code does now.

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

* refactor(background-data): present-tense comments, drop unused API

- Comments that told the history of each fix (what "used to" happen,
  "the old per-delivery release") now state the invariant the code keeps.
- get_statistics() no longer reports a constant 'queue_size': 0, and the
  uncalled clear_completed_requests() is gone (_cleanup_completed_requests
  does that job on every completion). Neither is referenced in core, the
  web UI or the plugin monorepo.

shutdown_background_service() has no production caller either, but it
is the only way to tear down the get_background_service() singleton,
which the tests rely on, so it stays.

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

* refactor(odds): drop the unread cache_ttl and merge the odds_data branches

BaseOddsManager loaded base_odds_manager.cache_ttl from config and never
used it: cached odds live for the update interval (get_odds' ttl=interval).
No core or monorepo code reads the attribute, so it is gone along with
its log line. The two consecutive `if odds_data:` blocks are one.

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

* refactor(backup): one table for the single-file sections

config, secrets, wifi and ytm_auth were each spelled out in create,
preview, validate and restore. _SINGLE_FILE_SECTIONS lists them once,
with the RestoreOptions flag that restores each, and all four walk it.
Restore error messages keep their wording ("Failed to restore
<file name>").

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

* refactor(fonts): drop FontManager's write-only state and duplicate logs

- fonts_config, font_metadata and font_dependencies were written and
  never read; the performance_stats keys font_load_times, render_times,
  total_renders and the per-call "resolve" timings
  (_record_performance_metric) likewise. get_performance_stats() reads
  only the counters that remain. Nothing in core or the plugin monorepo
  references any of them.
- A failed BDF load was logged twice, by _load_bdf_font and again by
  get_font; get_font's line is the one kept.
- Removed "NEW:" and commented-out cozette entries, the "Copy font to
  assets/fonts" comment on code that copies nothing, and local imports
  of names the module already imports. The deprecated add_font() now
  resolves assets/fonts against the install root.

The @deprecated methods stay.

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

* refactor(text-helper): cache loaded fonts; drop the pre-textlength fallback

TextHelper declared _font_cache, cleared it and reported its size, but
never stored anything in it. load_fonts() now keeps each (file, size)
it loads there, so clear_font_cache() and get_font_cache_stats() mean
what they say and repeated load_fonts() calls reuse the fonts.

get_text_width() no longer catches AttributeError for Pillow releases
without ImageDraw.textlength; requirements.txt pins Pillow>=12.2.
The class docstring describes what the helper does.

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

* docs(common): fix wrong docstrings in api_helper, permission_utils, snapshot_policy

- permission_utils called 0o2775 "sticky bit"; the 2 is setgid, which is
  what makes new files take the directory's group.
- snapshot_policy pointed at web_interface/blueprints/api_v3.py, which
  is a package now; the health check is in api_v3/misc.py.
- APIHelper.clear_cache() lost a history note and a fallback to a
  clear() method that neither CacheManager nor the testing
  MockCacheManager has. The session headers are built from
  DEFAULT_HTTP_HEADERS instead of a copy of them, and the module
  docstring says what the module offers.

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

* docs(sports): present-tense comments in the shared scoreboard renderers

- sports_scroll and sports_game_renderer comments that referred to "this
  PR", "the old flat 128px card" or what the renderer "previously" did
  now describe the current behaviour and its reason.
- The block explaining why non-finite settings are rejected sat above
  _score_reserve_width; it describes _center_gap_width and now lives in
  it.
- unshare_element_fonts wrapped its import of font_layout.load_truetype
  in an `except ImportError` that cannot fire inside core; the import
  stays at call time so tests can spy on the pinned loader.
- sports_card docstrings that told the history of a fix say what the
  code does.

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

* refactor(sports-shared): drop dead code, name the ESPN limit

- _get_weeks_data asked for limit=1000, which fetch_espn_scoreboard
  clamps to ESPN_MAX_LIMIT anyway; it now names that constant. Its
  unused `immediate_events = []` is gone.
- _get_season_schedule_dates() returned ("", "") and has no caller in
  core or the plugin monorepo.
- _should_log keeps its warning_type parameter (part of the inherited
  signature, though nothing in core or the monorepo calls it) and its
  docstring says the cooldown is shared across types.
- An unused ImageFont import is gone.

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

* refactor(sync): one follower-mode switch, shared panel defaults

- The class docstring said the leader sends PNG frames. Frames go over
  UDP as raw RGB; PNG is only the Vegas scroll image sent over TCP. It
  now describes both paths.
- _enter_follower_mode() replaces the two copies of "note the leader,
  switch from standalone to follower, log, write status" in the frame
  and scroll-position handlers.
- The rows/cols fallbacks use DEFAULT_ROWS / DEFAULT_COLS from
  src.display_geometry, as chain_length already did.

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

* refactor(style): drop _layout_axis, name the layout group title

- ElementStyleResolver._layout_axis() had no caller in core or the
  plugin monorepo.
- _element_block_from_spec checked spec['size'] was a dict again after
  size_spec already had; it reads size_spec.
- The "Layout Offsets" title written into three generated schema blocks
  is _LAYOUT_TITLE.

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

* docs(logo-helper): say what the placeholder draws; name the 1.5 box factor

- _create_placeholder_logo's docstring said it draws the team
  abbreviation; it draws an outlined grey box and nothing else. The
  docstring says so, and the "in a real implementation you'd want text"
  comments are gone.
- The 1.5 x panel default logo box, written out six times, is
  DEFAULT_LOGO_BOX_FACTOR.
- ImageDraw is imported with Image at the top of the module.

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

* refactor(logos): drop dead code and a duplicate regex in logo_downloader

- _SAFE_LEAGUE_CODE_RE was the same pattern as _SAFE_LEAGUE_RE; both
  checks use the one.
- get_logo_filename_variations reassigned the TA&M case to the list it
  already had; the function returns the two names directly.
- _get_team_name_variations() had no caller in core or the plugin
  monorepo.
- fetch_single_team's docstring was copied from fetch_teams_data; a log
  message read "for{team_id}".

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

* refactor: drop the Pillow<9.1 resample shim and a catch-and-reraise

- adaptive_images fell back to Image.LANCZOS/NEAREST for Pillow < 9.1;
  requirements.txt pins Pillow>=12.2. RESAMPLE_LANCZOS and
  RESAMPLE_NEAREST keep their names (src.common re-exports them).
- CacheManager.save_cache caught CacheError only to re-raise it; the
  disk write is now called directly, with the same result.

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

* test(api-helper): stop the real CacheManager's cleanup thread

The cache-lifetime tests built a CacheManager and left its cleanup
thread's class-wide claim on the directory in place, which broke
test_cache_cleanup_thread_ownership when it ran later in the session.
The fixture now stops the thread on teardown.

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

* docs(changelog): core-common

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-24 17:32:29 -04:00
committed by GitHub
co-authored by Claude Opus 5.5
parent b11bcfa204
commit 7b90759252
45 changed files with 1197 additions and 1031 deletions
+12
View File
@@ -42,6 +42,18 @@ accepts both, but the store flags the old spelling as deprecated
- `reload_plugin` reads the manifest from the plugin's discovered directory. - `reload_plugin` reads the manifest from the plugin's discovered directory.
- Removed: `last_display` from plugin state info and `get_last_display()` (nothing recorded them); `PluginOperationQueue`'s `history_file` and `lazy_load` arguments; and `data/plugin_operations.json`, which nothing read. - Removed: `last_display` from plugin state info and `get_last_display()` (nothing recorded them); `PluginOperationQueue`'s `history_file` and `lazy_load` arguments; and `data/plugin_operations.json`, which nothing read.
- Core service fixes:
- `/api/v3/errors` shows each exception's real stack trace instead of `NoneType: None`.
- Wi-Fi disconnect takes the saved connection profile down.
- `wifi_config.json` is written atomically, and a save that fails now gets a 500.
- `plugin://` fonts load from the plugin's own install directory. `FontManager.register_plugin_fonts()` takes an optional `plugin_dir`.
- `APIHelper` keeps cached responses for the `cache_ttl` it was given, instead of always 300 s.
- Logo scales from 0.1 to 10 are honoured everywhere; values outside that range are clamped.
- `LogoHelper` and `logo_downloader`: an empty ESPN logo list counts as a failed download, and the placeholder is written at the requested path.
- Bundled font paths no longer depend on the directory the process was started from.
- Backups record `src.__version__`.
- Removed: `BackgroundDataService`'s `queue_size` stat and `clear_completed_requests()`.
- The web service (`ledmatrix-web`) logs through `src.logging_config` like the - The web service (`ledmatrix-web`) logs through `src.logging_config` like the
display service, so `journalctl -p err -u ledmatrix-web` works. Successful display service, so `journalctl -p err -u ledmatrix-web` works. Successful
GET/HEAD/OPTIONS requests (the UI's polling) are logged at DEBUG instead of GET/HEAD/OPTIONS requests (the UI's polling) are logged at DEBUG instead of
+3 -7
View File
@@ -29,13 +29,9 @@ from typing import Any, Optional, Tuple
from PIL import Image from PIL import Image
# The one Pillow >= 9.1 compat shim (replaces the per-plugin copies). # Re-exported by src.common for plugins, which import them from there.
try: RESAMPLE_LANCZOS = Image.Resampling.LANCZOS
RESAMPLE_LANCZOS = Image.Resampling.LANCZOS RESAMPLE_NEAREST = Image.Resampling.NEAREST
RESAMPLE_NEAREST = Image.Resampling.NEAREST
except AttributeError: # Pillow < 9.1
RESAMPLE_LANCZOS = Image.LANCZOS
RESAMPLE_NEAREST = Image.NEAREST
FIT_MODES = ("contain", "cover", "fill_height", "stretch") FIT_MODES = ("contain", "cover", "fill_height", "stretch")
+21 -60
View File
@@ -131,19 +131,14 @@ class BackgroundDataService:
# Thread management # Thread management
self.executor = ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix="BackgroundData") self.executor = ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix="BackgroundData")
# cache_key -> request_id for fetches currently in flight. Submitting # cache_key -> request_id for fetches currently in flight, so a second
# the same key twice used to start two identical fetches: request_id # submit for the same key joins the running fetch instead of starting
# carries a millisecond timestamp, so every submit looked new, and # another. It is the normal case: a sport's Recent and Upcoming
# active_requests is keyed by it rather than by what is being fetched. # managers miss the cache for the same season schedule together.
# 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.
self._inflight_by_cache_key: Dict[str, str] = {} self._inflight_by_cache_key: Dict[str, str] = {}
# request_id was sport_year_milliseconds, which is not unique: two # Makes every request_id unique. The id also carries a millisecond
# submits inside the same millisecond produced the SAME id, so one # timestamp, but two submits can share a millisecond, and a joiner
# silently replaced the other in active_requests and completed_requests. # uses the id as its handle for get_result().
# 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.
self._request_seq = itertools.count() self._request_seq = itertools.count()
self.active_requests: Dict[str, FetchRequest] = {} self.active_requests: Dict[str, FetchRequest] = {}
self.completed_requests: Dict[str, FetchResult] = {} self.completed_requests: Dict[str, FetchResult] = {}
@@ -186,9 +181,9 @@ class BackgroundDataService:
This ensures Recent/Upcoming managers and background service This ensures Recent/Upcoming managers and background service
use the same cache keys. use the same cache keys.
""" """
# Same format as CacheManager.generate_sport_cache_key(). This used to # Same format as CacheManager.generate_sport_cache_key(), built here
# build a whole CacheManager to call it -- config load, cache-dir # rather than by constructing a CacheManager (config load, cache-dir
# probing with test writes -- on every submit without a cache_key. # probing) on every submit without a cache_key.
if date_str is None: if date_str is None:
date_str = datetime.now(pytz.utc).strftime('%Y%m%d') date_str = datetime.now(pytz.utc).strftime('%Y%m%d')
return f"{sport}_{date_str}" return f"{sport}_{date_str}"
@@ -331,10 +326,8 @@ class BackgroundDataService:
try: try:
with self._lock: with self._lock:
# A request cancelled while it sat in the executor queue must # A request cancelled while it sat in the executor queue stays
# stay cancelled. Overwriting the status here undid the cancel # cancelled: no download, no cache write, no callback.
# outright: the worker went on to download, cache and call back
# for work the caller had already withdrawn.
if request.status == FetchStatus.CANCELLED: if request.status == FetchStatus.CANCELLED:
cancelled_before_start = True cancelled_before_start = True
else: else:
@@ -463,10 +456,9 @@ class BackgroundDataService:
logger.error(f"Failed to fetch {request.sport} {request.year} data: {error_msg}") logger.error(f"Failed to fetch {request.sport} {request.year} data: {error_msg}")
with self._lock: with self._lock:
# Don't relabel a cancelled request. The callback gate in the # A cancelled request stays CANCELLED even when its fetch
# finally block only suppresses CANCELLED, so promoting it to # failed: the finally block skips callbacks only for
# FAILED here delivered an error callback for a fetch nobody # CANCELLED, and nobody is waiting on this fetch any more.
# was waiting on any more.
if request.status != FetchStatus.CANCELLED: if request.status != FetchStatus.CANCELLED:
request.status = FetchStatus.FAILED request.status = FetchStatus.FAILED
request.error = error_msg request.error = error_msg
@@ -526,20 +518,13 @@ class BackgroundDataService:
except Exception as e: except Exception as e:
logger.error(f"Error in callback for request {request.id}: {e}") logger.error(f"Error in callback for request {request.id}: {e}")
# Released AFTER the loop, not inside it. Every callback here holds # Released after the loop, never inside it: every callback holds
# the same FetchResult, so releasing per-delivery handed the first # the same FetchResult (a sport's recent, upcoming and live
# one the data and every joiner `result.data is None` -- which is # managers usually share one fetch), so a release between
# not a quiet degradation: they read `result.data.get('events')` and # deliveries would hand the later ones `result.data is None`.
# 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.
# #
# Guarded on `callbacks`, because a request submitted without one # Only when there were callbacks: a request submitted without one
# has no other way to collect its payload than polling get_result(). # collects its payload by polling get_result().
# The old per-delivery release got that right by accident: an empty
# list never entered the loop body.
if callbacks: if callbacks:
self._release_payload(result) self._release_payload(result)
request.result = None request.result = None
@@ -721,9 +706,6 @@ class BackgroundDataService:
'completed_requests_count': len(self.completed_requests), 'completed_requests_count': len(self.completed_requests),
'max_completed_requests': self._max_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, '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, 'last_cleanup': self._last_completed_requests_cleanup,
'cleanup_interval': self._completed_requests_cleanup_interval 'cleanup_interval': self._completed_requests_cleanup_interval
} }
@@ -793,27 +775,6 @@ class BackgroundDataService:
return removed_count 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): def shutdown(self, wait: bool = True):
""" """
Shutdown the background data service. Shutdown the background data service.
+71 -106
View File
@@ -83,14 +83,25 @@ BUNDLED_FONTS: frozenset[str] = frozenset({
_CONFIG_REL = Path("config/config.json") _CONFIG_REL = Path("config/config.json")
_SECRETS_REL = Path("config/config_secrets.json") _SECRETS_REL = Path("config/config_secrets.json")
_WIFI_REL = Path("config/wifi_config.json") _WIFI_REL = Path("config/wifi_config.json")
# Sits in config/ next to the three above and is pure user state — a # A YouTube Music session: pure user state that has to be re-authenticated by
# YouTube Music session that has to be re-authenticated by hand if lost. # hand if lost, so a restore must bring it back.
# It was omitted from backups, so a restore silently signed the user out.
_YTM_REL = Path("config/ytm_auth.json") _YTM_REL = Path("config/ytm_auth.json")
_FONTS_REL = Path("assets/fonts") _FONTS_REL = Path("assets/fonts")
_PLUGIN_UPLOADS_REL = Path("assets/plugins") _PLUGIN_UPLOADS_REL = Path("assets/plugins")
_STATE_REL = Path("data/plugin_state.json") _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" MANIFEST_NAME = "manifest.json"
PLUGINS_MANIFEST_NAME = "plugins.json" PLUGINS_MANIFEST_NAME = "plugins.json"
@@ -140,34 +151,18 @@ class RestoreResult:
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
def _ledmatrix_version(project_root: Path) -> str: def _ledmatrix_version() -> str:
"""Best-effort version string for the current install.""" """The release of the running core (``src.__version__``), recorded in the
version_file = project_root / "VERSION" manifest so a restore can tell which release wrote the backup."""
if version_file.exists(): from src import __version__
try: return __version__
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 _build_manifest(contents: List[str], project_root: Path) -> Dict[str, Any]: def _build_manifest(contents: List[str]) -> Dict[str, Any]:
return { return {
"schema_version": SCHEMA_VERSION, "schema_version": SCHEMA_VERSION,
"created_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"), "created_at": datetime.now(timezone.utc).isoformat().replace("+00:00", "Z"),
"ledmatrix_version": _ledmatrix_version(project_root), "ledmatrix_version": _ledmatrix_version(),
"hostname": socket.gethostname(), "hostname": socket.gethostname(),
"contents": contents, "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]]: def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]:
""" """
Return a list of currently-installed plugins suitable for the backup Return a list of currently-installed plugins suitable for the backup
manifest. Each entry has ``plugin_id`` and ``version``. manifest. Each entry has ``plugin_id`` and ``version``.
Reads ``data/plugin_state.json`` if present; otherwise walks the plugin Reads ``data/plugin_state.json`` if present, then adds any plugin it
directory and reads each ``manifest.json``. does not list from the ``manifest.json`` files in the configured plugin
directory (see :func:`_plugins_directory`).
""" """
plugins: Dict[str, Dict[str, Any]] = {} 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: except (OSError, json.JSONDecodeError) as e:
logger.warning("Could not read plugin_state.json: %s", e) logger.warning("Could not read plugin_state.json: %s", e)
# Fall back to scanning plugin-repos/ for manifests. plugins_root = _plugins_directory(project_root)
plugins_root = project_root / "plugin-repos"
if plugins_root.exists(): if plugins_root.exists():
for entry in sorted(plugins_root.iterdir()): for entry in sorted(plugins_root.iterdir()):
if not entry.is_dir(): if not entry.is_dir():
@@ -298,19 +313,10 @@ def create_backup(
tmp_path = zip_path.with_suffix(".zip.tmp") tmp_path = zip_path.with_suffix(".zip.tmp")
try: try:
with zipfile.ZipFile(tmp_path, "w", compression=zipfile.ZIP_DEFLATED) as zf: with zipfile.ZipFile(tmp_path, "w", compression=zipfile.ZIP_DEFLATED) as zf:
# Config files. for section, rel, _flag in _SINGLE_FILE_SECTIONS:
if (project_root / _CONFIG_REL).exists(): if (project_root / rel).exists():
zf.write(project_root / _CONFIG_REL, _CONFIG_REL.as_posix()) zf.write(project_root / rel, rel.as_posix())
contents.append("config") contents.append(section)
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")
# User-uploaded fonts. # User-uploaded fonts.
user_fonts = iter_user_fonts(project_root) user_fonts = iter_user_fonts(project_root)
@@ -338,7 +344,7 @@ def create_backup(
contents.append("plugins") contents.append("plugins")
# Manifest goes last so that `contents` reflects what we actually wrote. # 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)) zf.writestr(MANIFEST_NAME, json.dumps(manifest, indent=2))
os.replace(tmp_path, zip_path) os.replace(tmp_path, zip_path)
@@ -352,15 +358,16 @@ def create_backup(
def preview_backup_contents(project_root: Path) -> Dict[str, Any]: def preview_backup_contents(project_root: Path) -> Dict[str, Any]:
"""Return a summary of what ``create_backup`` would include.""" """Return a summary of what ``create_backup`` would include."""
project_root = Path(project_root).resolve() project_root = Path(project_root).resolve()
return { preview: Dict[str, Any] = {
"has_config": (project_root / _CONFIG_REL).exists(), f"has_{section}": (project_root / rel).exists()
"has_secrets": (project_root / _SECRETS_REL).exists(), for section, rel, _flag in _SINGLE_FILE_SECTIONS
"has_wifi": (project_root / _WIFI_REL).exists(), }
"has_ytm_auth": (project_root / _YTM_REL).exists(), preview.update({
"user_fonts": [p.name for p in iter_user_fonts(project_root)], "user_fonts": [p.name for p in iter_user_fonts(project_root)],
"plugin_uploads": len(iter_plugin_uploads(project_root)), "plugin_uploads": len(iter_plugin_uploads(project_root)),
"plugins": list_installed_plugins(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] = [] detected: List[str] = [
if _CONFIG_REL.as_posix() in names: section for section, rel, _flag in _SINGLE_FILE_SECTIONS
detected.append("config") if rel.as_posix() in names
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")
if any(n.startswith(_FONTS_REL.as_posix() + "/") for n in names): if any(n.startswith(_FONTS_REL.as_posix() + "/") for n in names):
detected.append("fonts") detected.append("fonts")
if any( if any(
@@ -584,55 +586,18 @@ def restore_backup(
result.errors.append("Failed to extract backup") result.errors.append("Failed to extract backup")
return result return result
# Main config. for section, rel, flag in _SINGLE_FILE_SECTIONS:
if options.restore_config and (tmp_dir / _CONFIG_REL).exists(): if not (tmp_dir / rel).exists():
continue
if not getattr(options, flag):
result.skipped.append(section)
continue
try: try:
_copy_file(tmp_dir / _CONFIG_REL, project_root / _CONFIG_REL) _copy_file(tmp_dir / rel, project_root / rel)
result.restored.append("config") result.restored.append(section)
except OSError as e: except OSError as e:
logger.error("[Backup] Failed to restore config.json: %s", e, exc_info=True) logger.error("[Backup] Failed to restore %s: %s", rel.name, e, exc_info=True)
result.errors.append("Failed to restore config.json") result.errors.append(f"Failed to restore {rel.name}")
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")
# User fonts — skip anything that collides with a bundled font. # User fonts — skip anything that collides with a bundled font.
tmp_fonts = tmp_dir / _FONTS_REL tmp_fonts = tmp_dir / _FONTS_REL
+10 -21
View File
@@ -18,6 +18,8 @@ import requests
import json import json
from typing import Dict, Any, Optional, List from typing import Dict, Any, Optional, List
from src.common.api_helper import DEFAULT_HTTP_HEADERS
class BaseOddsManager: class BaseOddsManager:
""" """
@@ -45,22 +47,15 @@ class BaseOddsManager:
self.logger = logging.getLogger(__name__) self.logger = logging.getLogger(__name__)
self.base_url = "https://sports.core.api.espn.com/v2/sports" self.base_url = "https://sports.core.api.espn.com/v2/sports"
# This path used a bare requests.get, so it identified itself as # Core's shared headers: ESPN rejects requests' default User-Agent
# python-requests/x.y -- the one thing ESPN is known to reject. Around # (see api_helper.USER_AGENT), and a rejected odds request costs the
# 2026-08-04 it began 403ing browser strings and bare custom tokens # calling plugin its update budget.
# 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.
# #
# Deliberately no retry adapter, unlike api_helper: retries multiply # Deliberately no retry adapter, unlike api_helper: retries multiply
# request_timeout, which is set to 5s precisely to stay inside that # request_timeout, which is set to 5s precisely to stay inside that
# budget. One try, then the cooldown below. # budget. One try, then the cooldown below.
self.session = requests.Session() self.session = requests.Session()
self.session.headers.update({ self.session.headers.update(DEFAULT_HTTP_HEADERS)
'User-Agent': 'LEDMatrix/1.0 (+https://github.com/ChuckBuilds/LEDMatrix)',
'Accept': 'application/json',
})
# Configuration with defaults # Configuration with defaults
self.update_interval = 3600 # 1 hour default self.update_interval = 3600 # 1 hour default
@@ -72,7 +67,6 @@ class BaseOddsManager:
self.request_timeout = 5 self.request_timeout = 5
# Set when a request fails; until then, skip the network entirely. # Set when a request fails; until then, skip the network entirely.
self._skip_network_until = 0.0 self._skip_network_until = 0.0
self.cache_ttl = 1800 # 30 minutes default
# Load configuration if available # Load configuration if available
if config_manager: if config_manager:
@@ -89,12 +83,10 @@ class BaseOddsManager:
self.update_interval = odds_config.get('update_interval', self.update_interval) self.update_interval = odds_config.get('update_interval', self.update_interval)
self.request_timeout = odds_config.get('timeout', self.request_timeout) 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: " self.logger.debug(f"BaseOddsManager configuration loaded: "
f"update_interval={self.update_interval}s, " f"update_interval={self.update_interval}s, "
f"timeout={self.request_timeout}s, " f"timeout={self.request_timeout}s")
f"cache_ttl={self.cache_ttl}s")
except Exception as e: except Exception as e:
self.logger.warning(f"Failed to load BaseOddsManager configuration: {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) odds_data = self._extract_espn_data(raw_data)
if odds_data: if odds_data:
self.logger.info(f"Successfully extracted odds data: {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.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") self.logger.info(f"Saved odds data to cache for {cache_key} with TTL {interval}s")
else: else:
self.logger.debug(f"No odds data available for {cache_key}") 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) self.cache_manager.set(cache_key, {"no_odds": True}, ttl=interval)
return odds_data return odds_data
+3 -7
View File
@@ -32,7 +32,6 @@ from typing import Any, Dict, List, Optional
import logging import logging
import threading import threading
import tempfile import tempfile
from src.exceptions import CacheError
from src.cache.memory_cache import MemoryCache, default_max_size from src.cache.memory_cache import MemoryCache, default_max_size
from src.cache.disk_cache import DiskCache from src.cache.disk_cache import DiskCache
from src.cache.cache_strategy import CacheStrategy from src.cache.cache_strategy import CacheStrategy
@@ -272,12 +271,9 @@ class CacheManager:
# Update memory cache first # Update memory cache first
self._memory_cache_component.set(key, data) self._memory_cache_component.set(key, data)
# Save to disk cache # DiskCache logs a failed write and raises CacheError, which the
try: # caller gets as is.
self._disk_cache_component.set(key, data) self._disk_cache_component.set(key, data)
except CacheError:
# Disk cache errors are already logged and raised by DiskCache
raise
def load_cache(self, key: str) -> Optional[Dict[str, Any]]: def load_cache(self, key: str) -> Optional[Dict[str, Any]]:
"""Load data from cache with memory caching.""" """Load data from cache with memory caching."""
+41 -41
View File
@@ -1,8 +1,9 @@
""" """
API Helper API Helper
Handles HTTP requests, caching, and ESPN API integration for LED matrix plugins. HTTP requests, response caching and ESPN fetch helpers for plugins
Extracted from LEDMatrix core to provide reusable functionality for plugins. (``from src.common import APIHelper``), plus the headers every core request
sends (:data:`USER_AGENT`, :data:`DEFAULT_HTTP_HEADERS`).
""" """
import logging import logging
@@ -36,13 +37,20 @@ DEFAULT_HTTP_HEADERS: Mapping[str, str] = MappingProxyType({
class APIHelper: class APIHelper:
""" """
Helper class for HTTP requests, caching, and ESPN API integration. HTTP requests with retries, response caching and ESPN helpers.
Provides functionality for: - Requests go through one ``requests.Session`` that retries GET, HEAD
- HTTP requests with retry logic and timeouts and OPTIONS on 429 and 5xx with exponential backoff, and sends
- Response caching with TTL support :data:`DEFAULT_HTTP_HEADERS`.
- ESPN API integration for sports data - Consecutive requests from one helper are spaced at least
- Request rate limiting and throttling ``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, def __init__(self, cache_manager=None, default_timeout: int = 30,
@@ -73,13 +81,7 @@ class APIHelper:
self.session.mount("https://", adapter) self.session.mount("https://", adapter)
self.session.mount("http://", adapter) self.session.mount("http://", adapter)
# Default headers self.session.headers.update({**DEFAULT_HTTP_HEADERS, 'Connection': 'keep-alive'})
self.session.headers.update({
'User-Agent': USER_AGENT,
'Accept': 'application/json',
'Accept-Language': 'en-US,en;q=0.9',
'Connection': 'keep-alive'
})
# Rate limiting # Rate limiting
self._last_request_time = 0 self._last_request_time = 0
@@ -102,9 +104,8 @@ class APIHelper:
Returns: Returns:
Response data as dictionary or None if request fails Response data as dictionary or None if request fails
""" """
# Check cache first
if cache_key and self.cache_manager: 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: if cached is not None:
self.logger.debug(f"Using cached response for {cache_key}") self.logger.debug(f"Using cached response for {cache_key}")
return cached return cached
@@ -268,33 +269,31 @@ class APIHelper:
Args: Args:
key: Cache key key: Cache key
data: Data to cache 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._set_cache(key, data, ttl)
self.cache_manager.set(key, data)
def get_cache(self, key: str) -> Optional[Any]: def get_cache(self, key: str) -> Optional[Any]:
""" """
Get cached data. Get cached data.
Args: Args:
key: Cache key key: Cache key
Returns: 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._get_from_cache(key)
return self.cache_manager.get(key)
return None
def clear_cache(self, pattern: Optional[str] = None) -> None: def clear_cache(self, pattern: Optional[str] = None) -> None:
""" """
Clear cache data. Clear cache data.
Uses CacheManager's real surface (clear_cache / delete / Uses CacheManager's clear_cache(), or list_cache_files() and delete()
list_cache_files); safely no-ops on managers without it. The old for a pattern. A cache manager without those methods is left alone.
implementation guarded on a nonexistent ``clear`` method, so it
silently never cleared anything.
Args: Args:
pattern: Optional substring to match cache keys; only matching pattern: Optional substring to match cache keys; only matching
@@ -315,21 +314,22 @@ class APIHelper:
"cannot clear by pattern") "cannot clear by pattern")
elif hasattr(self.cache_manager, 'clear_cache'): elif hasattr(self.cache_manager, 'clear_cache'):
self.cache_manager.clear_cache() self.cache_manager.clear_cache()
elif hasattr(self.cache_manager, 'clear'):
self.cache_manager.clear()
else: else:
self.logger.debug("Cache manager exposes no clear method; no-op") self.logger.debug("Cache manager exposes no clear method; no-op")
def _get_from_cache(self, key: str) -> Optional[Any]: def _get_from_cache(self, key: str, max_age: Optional[int] = None) -> Optional[Any]:
"""Get data from cache.""" """Cached data for ``key``, or None. ``max_age`` only matters for an
if self.cache_manager: 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 self.cache_manager.get(key)
return None return self.cache_manager.get(key, max_age=max_age)
def _set_cache(self, key: str, data: Any, ttl: int) -> None: def _set_cache(self, key: str, data: Any, ttl: Optional[int]) -> None:
"""Set data in cache.""" """Store ``data`` under ``key`` for ``ttl`` seconds."""
if self.cache_manager: if self.cache_manager:
self.cache_manager.set(key, data) self.cache_manager.set(key, data, ttl=ttl)
def _enforce_rate_limit(self) -> None: def _enforce_rate_limit(self) -> None:
"""Enforce rate limiting between requests.""" """Enforce rate limiting between requests."""
+5 -4
View File
@@ -67,10 +67,11 @@ _INSTALL_ROOT = Path(__file__).resolve().parents[2]
def resolve_asset_path(relative_path: str) -> str: def resolve_asset_path(relative_path: str) -> str:
"""Resolve a repo-relative asset path independently of the process cwd. """Resolve a repo-relative asset path independently of the process cwd.
Prefers the path as given — so an absolute path is returned untouched and In order: an absolute path that exists is returned untouched; otherwise
behaviour is unchanged wherever the cwd already happened to be the install ``relative_path`` under the install root derived above, if that exists;
root — then the install root derived above, then the original string so a otherwise ``relative_path`` unchanged, so a caller that wants to raise
caller that wants to raise and fall back still can. 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 Without the fallback, any process started outside the install root (the
plugin safety harness, a manual ``python run.py`` from ``$HOME``, a unit plugin safety harness, a manual ``python run.py`` from ``$HOME``, a unit
+33 -52
View File
@@ -11,7 +11,7 @@ from pathlib import Path
from typing import Dict, List, Optional, Union from typing import Dict, List, Optional, Union
import requests import requests
from PIL import Image from PIL import Image, ImageDraw
from src.common.api_helper import USER_AGENT from src.common.api_helper import USER_AGENT
from src.common.permission_utils import ( from src.common.permission_utils import (
ensure_directory_permissions, 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. # trade for not re-warning about a file nobody is going to add.
MISSING_LOGO_RECHECK_SECONDS = 3600.0 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. # 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. # The cap for every logo download: src.logo_downloader.fetch_logo uses it too.
MAX_LOGO_BYTES = 10 * 1024 * 1024 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: class LogoHelper:
""" """
@@ -119,12 +98,16 @@ class LogoHelper:
Args: Args:
team_abbr: Team abbreviation for caching team_abbr: Team abbreviation for caching
logo_path: Path to the logo file logo_path: Path to the logo file
max_width: Maximum width (defaults to display_width * 1.5) max_width: Maximum width (default display_width *
max_height: Maximum height (defaults to display_height * 1.5) DEFAULT_LOGO_BOX_FACTOR)
max_height: Maximum height (default display_height *
DEFAULT_LOGO_BOX_FACTOR)
scale: User's size multiplier for this image, from scale: User's size multiplier for this image, from
``customization.layout.<element>.scale``. 1.0 is untouched and ``customization.layout.<element>.scale``; 1.0 leaves the box
takes exactly the path it always did. Callers hold the config, as is. Callers hold the config, so they resolve the element
so they resolve the element name; this only applies the number. 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: Returns:
PIL Image object or None if loading fails 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 # key is size-qualified — a panel-size change must not return a
# logo resized for the old dimensions. # logo resized for the old dimensions.
if max_width is None: 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: if max_height is None:
max_height = int(self.display_height * 1.5) max_height = int(self.display_height * DEFAULT_LOGO_BOX_FACTOR)
scale = _usable_scale(scale) # 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: if scale != 1.0:
max_width = max(1, int(round(max_width * scale))) max_width = max(1, int(round(max_width * scale)))
max_height = max(1, int(round(max_height * 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. nobody asked to grow would change every existing render.
""" """
if max_width is None: 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: 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 # Only resize if necessary
if logo.width <= max_width and logo.height <= max_height: if logo.width <= max_width and logo.height <= max_height:
@@ -429,31 +415,26 @@ class LogoHelper:
max_width: Optional[int] = None, max_width: Optional[int] = None,
max_height: Optional[int] = None) -> Optional[Image.Image]: 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: Args:
team_abbr: Team abbreviation to display team_abbr: Team the placeholder stands in for
max_width: Maximum width max_width: Width (default display_width * DEFAULT_LOGO_BOX_FACTOR)
max_height: Maximum height max_height: Height (default display_height * DEFAULT_LOGO_BOX_FACTOR)
Returns: Returns:
PIL Image with placeholder logo The RGBA placeholder, or None if it could not be created
""" """
try: try:
if max_width is None: 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: 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)) 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 = ImageDraw.Draw(placeholder)
# Draw a simple rectangle with team abbreviation
draw.rectangle([0, 0, max_width-1, max_height-1], draw.rectangle([0, 0, max_width-1, max_height-1],
fill=(100, 100, 100, 200), outline=(200, 200, 200, 255)) fill=(100, 100, 100, 200), outline=(200, 200, 200, 255))
+8 -4
View File
@@ -241,7 +241,8 @@ def get_assets_dir_mode() -> int:
Return permission mode for asset directories. Return permission mode for asset directories.
Returns: 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) return 0o2775 # rwxrwsr-x (setgid + group writable)
@@ -251,7 +252,8 @@ def get_config_dir_mode() -> int:
Return permission mode for config directory. Return permission mode for config directory.
Returns: 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) return 0o2775 # rwxrwsr-x (setgid + group writable)
@@ -271,7 +273,8 @@ def get_plugin_dir_mode() -> int:
Return permission mode for plugin directories. Return permission mode for plugin directories.
Returns: 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) return 0o2775 # rwxrwsr-x (setgid + group writable)
@@ -281,7 +284,8 @@ def get_cache_dir_mode() -> int:
Return permission mode for cache directories. Return permission mode for cache directories.
Returns: 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) return 0o2775 # rwxrwsr-x (setgid + group writable)
+3 -3
View File
@@ -5,7 +5,7 @@ serves two consumers with different needs:
- The web UI's live preview (SSE reader in web_interface/app.py) wants - The web UI's live preview (SSE reader in web_interface/app.py) wants
fresh frames — but only while a browser is actually watching. 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. 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 — 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. TOUCH_INTERVAL so the health check (60s threshold) never degrades.
If any constant here changes, re-check the health threshold in 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 from enum import Enum
@@ -37,7 +37,7 @@ VIEWER_INTERVAL = 0.2
# Snapshot cadence with no viewers — cheap freshness for page-open (seconds). # Snapshot cadence with no viewers — cheap freshness for page-open (seconds).
IDLE_INTERVAL = 30.0 IDLE_INTERVAL = 30.0
# Max age of the last write/touch before bumping mtime for the health # 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 TOUCH_INTERVAL = 20.0
# A viewer marker older than this no longer counts as a live viewer. # A viewer marker older than this no longer counts as a live viewer.
VIEWER_MARKER_FRESH_SEC = 5.0 VIEWER_MARKER_FRESH_SEC = 5.0
+11 -15
View File
@@ -101,11 +101,10 @@ def element_color(config: Optional[Dict[str, Any]], element: str,
mode: Optional[str] = None): mode: Optional[str] = None):
"""Per-element text colour from customization.<element>.text_color. """Per-element text colour from customization.<element>.text_color.
Delegated rather than reimplemented: there were two copies of this Delegates to src.element_style.element_color, which also resolves the
read and three of the offset read, and the shared one also resolves element under the names plugins actually use (the layout block says
the element under the names plugins actually use (the layout block `score` where the style block says `score_text`) and honours a per-mode
says `score` where the style block says `score_text`) and honours a override. Hex strings are accepted.
per-mode override. Hex strings are still accepted.
""" """
from src.element_style import element_color as _shared from src.element_style import element_color as _shared
return _shared(config, element, default, mode) 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 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 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. 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 Ambiguity is therefore narrowed before it is given up on: among the
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
elements sharing a face, a single configured colour is the only thing 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 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 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 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 with identical metrics, so nothing about the rendering changes; only
the ability to tell two elements apart does. Faces that cannot be the ability to tell two elements apart does. Faces that cannot be
rebuilt (a BDF loaded through freetype.Face, anything without a usable 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 *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 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 which names different keys -- see ``resolve_font_color`` for why the two
vocabularies are kept apart. vocabularies are kept apart.
""" """
try: # Looked up at call time so tests can spy on the pinned loader.
from src.common.font_layout import load_truetype as _load from src.common.font_layout import load_truetype
except ImportError: # pragma: no cover
return fonts
if element_for_font is None: if element_for_font is None:
element_for_font = ELEMENT_FOR_FONT element_for_font = ELEMENT_FOR_FONT
seen = {} seen = {}
@@ -509,7 +505,7 @@ def unshare_element_fonts(logger, fonts, element_for_font=None):
if not path or not size: if not path or not size:
continue continue
try: try:
fonts[key] = _load(path, size) fonts[key] = load_truetype(path, size)
except (OSError, ValueError, TypeError): except (OSError, ValueError, TypeError):
logger.debug( logger.debug(
"Could not un-share the %s face; it keeps the default colour", key) "Could not un-share the %s face; it keeps the default colour", key)
+14 -20
View File
@@ -56,21 +56,13 @@ class SportsGameRendererMixin:
# ---- geometry ------------------------------------------------------ # ---- 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: 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 Measured rather than derived from the card width, because the score's
CENTER_GAP_RATIO, clamped to CENTER_GAP_MAX_PX) while the score's size size comes from config and the element-style resolver: a strip sized
comes from config and the element-style resolver. Nothing compared the from the width alone lets a large score run over the logos.
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.
""" """
try: try:
probe = ImageDraw.Draw(Image.new("RGB", (4, 4))) 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 the card width between the configurable min and max. 0 restores
edge-to-edge logos. 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") configured = self._scroll_card_option("center_gap")
if (isinstance(configured, (int, float)) if (isinstance(configured, (int, float))
and math.isfinite(configured) and configured >= 0): and math.isfinite(configured) and configured >= 0):
@@ -110,10 +106,9 @@ class SportsGameRendererMixin:
def _logo_slot_width(self) -> int: def _logo_slot_width(self) -> int:
"""Per-side logo slot, leaving the center gap clear. """Per-side logo slot, leaving the center gap clear.
No longer capped at display_height: the card is sized as two Not capped at display_height: the card is sized as two full-height
full-height logos plus the measured gap, so what is left after the gap logos plus the measured gap, so what is left after the gap is exactly
is exactly the logo's share. The cap was what froze the logos at 46px the logo's share. At least 8 px.
on the old flat 128px card.
""" """
available = (self.display_width - self._center_gap_width()) // 2 available = (self.display_width - self._center_gap_width()) // 2
return max(8, available) return max(8, available)
@@ -130,9 +125,8 @@ class SportsGameRendererMixin:
"""X/Y nudge for one element, from customization.layout. """X/Y nudge for one element, from customization.layout.
Same block the full-screen scorebug reads (sports.py Same block the full-screen scorebug reads (sports.py
_get_layout_offset), so a nudge configured in the web UI now moves _get_layout_offset), so a nudge configured in the web UI moves the
the element on the scroll/Vegas card too -- previously the schema element on the scroll/Vegas card as well as on the scorebug.
advertised these offsets but this renderer ignored them.
""" """
from src.element_style import layout_offset from src.element_style import layout_offset
return layout_offset(self.config, element, axis, default, return layout_offset(self.config, element, axis, default,
+3 -3
View File
@@ -487,9 +487,9 @@ class SportsScrollDisplayManager:
) )
except Exception: except Exception:
# prepare_scroll_content is subclass-implemented and builds cards # prepare_scroll_content is subclass-implemented and builds cards
# straight from feed data, which is exactly where this PR's other # straight from feed data, so it can raise on a malformed payload.
# crashes came from. One sport's bad payload must not take down the # One sport's bad payload must not take down the shared
# shared orchestration for the others. # orchestration for the others.
self.logger.exception( self.logger.exception(
"Error preparing scroll content for game_type=%s", game_type "Error preparing scroll content for game_type=%s", game_type
) )
+15 -37
View File
@@ -97,11 +97,11 @@ from datetime import datetime, timedelta, timezone
from typing import Any, ClassVar, Dict, List, Optional, Tuple from typing import Any, ClassVar, Dict, List, Optional, Tuple
import pytz 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 import requests
from PIL import Image, ImageDraw, ImageFont from PIL import Image, ImageDraw
from src.common import sports_card as _card 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__) 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 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. default face instead of the pixel font it was laid out for.
Resolution order matches the core's own resolver: the path as given Resolution order: the path as given, relative to the cwd, when it
first, so behaviour is unchanged wherever it already worked and a exists -- the order the scoreboards' own sports.py copies used, so a
configured absolute path is returned untouched, then the core install process running from another checkout keeps that checkout's fonts --
root, then the original string so callers still raise and fall back then :func:`src.common.font_layout.resolve_asset_path` (the install
exactly as they do today. root), which returns the original string when neither exists so callers
still raise and fall back.
""" """
if os.path.exists(path): if os.path.exists(path):
return path return path
try: return resolve_asset_path(path)
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
class SportsCoreSharedMixin: class SportsCoreSharedMixin:
@@ -201,9 +181,6 @@ class SportsCoreSharedMixin:
#: How long to stay quiet between ranking-coverage warnings. #: How long to stay quiet between ranking-coverage warnings.
_RANKING_COVERAGE_SECONDS: ClassVar[int] = 60 * 60 _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: def _draw_scorebug_layout(self, game: Dict, force_clear: bool = False) -> None:
"""Placeholder draw method - subclasses should override.""" """Placeholder draw method - subclasses should override."""
# This base method will be simple, subclasses provide specifics # This base method will be simple, subclasses provide specifics
@@ -889,7 +866,10 @@ class SportsCoreSharedMixin:
draw.text((x, y), text, font=font, fill=fill) draw.text((x, y), text, font=font, fill=fill)
def _should_log(self, warning_type: str, cooldown: int = 60) -> bool: 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() current_time = time.time()
if current_time - self._last_warning_time > cooldown: if current_time - self._last_warning_time > cooldown:
self._last_warning_time = current_time self._last_warning_time = current_time
@@ -904,8 +884,6 @@ class SportsCoreSharedMixin:
try: try:
# Fetch current week and next few days for immediate display # Fetch current week and next few days for immediate display
now = datetime.now(pytz.utc) now = datetime.now(pytz.utc)
immediate_events = []
start_date = now - timedelta(days=self.schedule_lookback_days) start_date = now - timedelta(days=self.schedule_lookback_days)
end_date = now + timedelta(days=self.schedule_lookahead_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')}" 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( data = fetch_espn_scoreboard(
self.session, self.session,
url, url,
params={"dates": date_str, "limit": 1000}, params={"dates": date_str, "limit": ESPN_MAX_LIMIT},
headers=self.headers, headers=self.headers,
timeout=10, timeout=10,
logger=self.logger, logger=self.logger,
+29 -27
View File
@@ -32,7 +32,7 @@ from typing import Callable, Optional
import numpy as np import numpy as np
from PIL import Image 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 # 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 # Much faster than PNG: no encode/decode, negligible CPU, same UDP packet size
@@ -75,9 +75,12 @@ class FollowerState(Enum):
class DisplaySyncManager: class DisplaySyncManager:
""" """
Core sync manager. Instantiated by DisplayController based on config['sync']. 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 The leader sends each rendered frame to the follower over UDP as raw RGB
goes offline. 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__( def __init__(
@@ -192,8 +195,8 @@ class DisplaySyncManager:
def _handle_hello(self, msg: dict, sender_ip: str) -> None: def _handle_hello(self, msg: dict, sender_ip: str) -> None:
hw = self._hw_config hw = self._hw_config
local_rows = hw.get("rows", 32) local_rows = hw.get("rows", DEFAULT_ROWS)
local_cols = hw.get("cols", 64) local_cols = hw.get("cols", DEFAULT_COLS)
peer_rows = int(msg.get("rows", 0)) peer_rows = int(msg.get("rows", 0))
peer_cols = int(msg.get("cols", 0)) peer_cols = int(msg.get("cols", 0))
peer_chain = int(msg.get("chain", DEFAULT_CHAIN_LENGTH)) 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.""" """Record a decoded leader frame and enter follower mode if needed."""
with self._frame_lock: with self._frame_lock:
self._latest_frame = img 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._last_leader_frame_time = time.time()
self._leader_ip = sender_ip self._leader_ip = sender_ip
if self._follower_state != FollowerState.STANDALONE:
if self._follower_state == FollowerState.STANDALONE: return False
self._follower_state = FollowerState.FOLLOWER self._follower_state = FollowerState.FOLLOWER
self.logger.info( self.logger.info(
"Sync: leader active at %s — switching to follower mode", "Sync: leader active at %s — switching to follower mode",
sender_ip, sender_ip,
) )
self.write_status_file() self.write_status_file()
return True
def _follower_recv_loop(self) -> None: def _follower_recv_loop(self) -> None:
while self._running: while self._running:
@@ -559,15 +569,7 @@ class DisplaySyncManager:
# back from. Treat it as malformed. # back from. Treat it as malformed.
raise ValueError(f"non-finite scroll x: {msg['x']!r}") raise ValueError(f"non-finite scroll x: {msg['x']!r}")
self._latest_scroll_x = scroll_x self._latest_scroll_x = scroll_x
self._last_leader_frame_time = time.time() if self._enter_follower_mode(sender_ip):
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()
fire_new_cycle = True # build initial scroll image fire_new_cycle = True # build initial scroll image
elif t == "nc": elif t == "nc":
# Leader started a new scroll cycle — rebuild local image # Leader started a new scroll cycle — rebuild local image
@@ -589,8 +591,8 @@ class DisplaySyncManager:
hw = self._hw_config hw = self._hw_config
hello = json.dumps({ hello = json.dumps({
"t": "hello", "t": "hello",
"rows": hw.get("rows", 32), "rows": hw.get("rows", DEFAULT_ROWS),
"cols": hw.get("cols", 64), "cols": hw.get("cols", DEFAULT_COLS),
"chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH), "chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH),
}).encode("utf-8") }).encode("utf-8")
heartbeat = json.dumps({"t": "hb"}).encode("utf-8") heartbeat = json.dumps({"t": "hb"}).encode("utf-8")
@@ -660,8 +662,8 @@ class DisplaySyncManager:
base = { base = {
"role": self.role.value, "role": self.role.value,
"port": self.port, "port": self.port,
"local_rows": hw.get("rows", 32), "local_rows": hw.get("rows", DEFAULT_ROWS),
"local_cols": hw.get("cols", 64), "local_cols": hw.get("cols", DEFAULT_COLS),
"local_chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH), "local_chain": hw.get("chain_length", DEFAULT_CHAIN_LENGTH),
} }
+26 -22
View File
@@ -10,7 +10,7 @@ from pathlib import Path
from typing import Dict, List, Optional, Tuple, Union from typing import Dict, List, Optional, Tuple, Union
from PIL import Image, ImageDraw, ImageFont 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. # Shared throwaway draw surface for measuring text without a target canvas.
_measure_draw = ImageDraw.Draw(Image.new("RGB", (1, 1))) _measure_draw = ImageDraw.Draw(Image.new("RGB", (1, 1)))
@@ -18,13 +18,14 @@ _measure_draw = ImageDraw.Draw(Image.new("RGB", (1, 1)))
class TextHelper: class TextHelper:
""" """
Helper class for text rendering with outlines and font management. Font loading, outlined text and text measurement for plugins.
Provides functionality for: - :meth:`load_fonts` loads TrueType fonts from ``font_dir`` (the install's
- Loading and managing fonts assets/fonts by default) with the layout engine pinned
- Drawing text with outlines for better readability (font_layout.load_truetype). Each (file, size) is loaded once per helper
- Calculating text dimensions and positioning and reused; a missing or unloadable file becomes PIL's default font.
- Managing font resources - :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, def __init__(self, font_dir: Optional[Union[str, Path]] = None,
@@ -33,22 +34,26 @@ class TextHelper:
Initialize the TextHelper. Initialize the TextHelper.
Args: 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 logger: Optional logger instance
""" """
self.logger = logger or logging.getLogger(__name__) 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] = {} self._font_cache: Dict[str, ImageFont.ImageFont] = {}
def load_fonts(self, font_config: Optional[Dict[str, Dict]] = None) -> 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. Load fonts for different text elements.
Args: 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: 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: if font_config is None:
font_config = self._get_default_font_config() font_config = self._get_default_font_config()
@@ -61,9 +66,13 @@ class TextHelper:
size = config['size'] size = config['size']
if font_path.exists(): 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 fonts[font_name] = font
self.logger.debug(f"Loaded font: {font_name} ({font_path}, size {size})")
else: else:
# Fallback to default font # Fallback to default font
font = ImageFont.load_default() font = ImageFont.load_default()
@@ -115,12 +124,7 @@ class TextHelper:
Returns: Returns:
Width in pixels Width in pixels
""" """
try: return int(_measure_draw.textlength(text, font=font))
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]
def get_text_height(self, text: str, font: ImageFont.ImageFont) -> int: def get_text_height(self, text: str, font: ImageFont.ImageFont) -> int:
""" """
+30 -37
View File
@@ -16,8 +16,10 @@ additionally keeps rotating backups in ``config/backups/``.
Plugin configuration Plugin configuration
-------------------- --------------------
Plugin configs are stored inside ``config.json`` under the plugin's ID key Plugin configs are stored inside ``config.json`` under the plugin's ID key
and survive plugin reinstalls. Use :meth:`ConfigManager.update_plugin_config` and survive plugin reinstalls. Write them by saving the whole config with
to write plugin settings; never write directly to the plugin directory. :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 Hot-reload
---------- ----------
@@ -127,13 +129,10 @@ class ConfigManager:
# Update in-memory config if save was successful # Update in-memory config if save was successful
if result.status == SaveResultStatus.SUCCESS: if result.status == SaveResultStatus.SUCCESS:
self.config = new_config_data self.config = new_config_data
# In-memory config now matches what was just written; refresh # In-memory config now matches what was just written, so the
# the load signature so the fast path stays valid. NOTE: the # load_config fast path may return it. It still carries the
# in-memory copy includes merged secrets; the on-disk file has # merged secrets that were stripped on disk; that matches a full
# them stripped — the fast path returning self.config preserves # reload, because the secrets file was not changed by the save.
# 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).
self._loaded_sig = self._files_signature() self._loaded_sig = self._files_signature()
self.logger.info(f"Configuration successfully saved atomically to {os.path.abspath(self.config_path)}") self.logger.info(f"Configuration successfully saved atomically to {os.path.abspath(self.config_path)}")
elif result.status == SaveResultStatus.ROLLED_BACK: elif result.status == SaveResultStatus.ROLLED_BACK:
@@ -253,11 +252,11 @@ class ConfigManager:
return self.config return self.config
except FileNotFoundError as e: except FileNotFoundError as e:
if str(e).find('config_secrets.json') == -1: # Only raise if main config is missing # Only config.json can get here: a missing or unreadable secrets
error_msg = f"Configuration file not found at {os.path.abspath(self.config_path)}" # file is handled where it is read.
self.logger.error(error_msg, exc_info=True) error_msg = f"Configuration file not found at {os.path.abspath(self.config_path)}"
raise ConfigError(error_msg, config_path=self.config_path) from e self.logger.error(error_msg, exc_info=True)
return self.config raise ConfigError(error_msg, config_path=self.config_path) from e
except json.JSONDecodeError as e: except json.JSONDecodeError as e:
error_msg = f"Error parsing configuration file {os.path.abspath(self.config_path)}" error_msg = f"Error parsing configuration file {os.path.abspath(self.config_path)}"
self.logger.error(error_msg, exc_info=True) 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 A missing secrets file is fine (nothing to strip). But a file that
EXISTS and cannot be read or parsed means stripping is impossible — EXISTS and cannot be read or parsed means stripping is impossible —
and the in-memory config being saved has secrets deep-merged into it, and the in-memory config being saved has secrets deep-merged into it,
so proceeding would write them into config.json in plaintext. That so proceeding would write them into config.json in plaintext. The
was the historical behavior; it is now a hard refusal. The save save raises instead, so the caller (and user) fixes the secrets file
raises so the caller (and user) fixes the secrets file instead of rather than leaking its contents into the world-readable main config.
silently leaking its contents into the world-readable main config.
""" """
if not os.path.exists(self.secrets_path): if not os.path.exists(self.secrets_path):
return {} return {}
@@ -465,9 +463,8 @@ class ConfigManager:
# Merge template defaults into current config # Merge template defaults into current config
self._merge_template_defaults(self.config, template_config) self._merge_template_defaults(self.config, template_config)
# Save migrated config using atomic save to preserve permissions # save_config_atomic strips the merged secrets back out and
# Use atomic save to preserve file permissions # keeps the file's owner and mode.
# Note: save_config_atomic handles secrets internally
result = self.save_config_atomic( result = self.save_config_atomic(
new_config_data=self.config, new_config_data=self.config,
create_backup=False, # Already created backup above 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)}") 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. # The merged self.config is now stale; reload it. A reload failure
# Reload it to reflect the new state. # (a migration error, say) is logged, not raised: the file itself
# Note: We wrap this in try-except because reload failures (e.g., migration errors) # was saved.
# should not cause the save operation to fail - the file was saved successfully. try:
if file_type == "main" or file_type == "secrets": self.load_config()
try: except Exception as reload_error:
self.load_config() self.logger.warning(
except Exception as reload_error: f"Configuration file saved successfully, but reload failed: {reload_error}. "
# Log the reload error but don't fail the save operation f"The file on disk is valid, but in-memory config may be stale."
# 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."
)
except PermissionError as e: except PermissionError as e:
# Provide helpful error message with fix instructions # Provide helpful error message with fix instructions
@@ -670,7 +663,7 @@ class ConfigManager:
try: try:
# Load current configs # Load current configs
main_config = self.get_raw_file_content('main') 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 # Remove plugin from main config
if plugin_id in main_config: if plugin_id in main_config:
@@ -703,7 +696,7 @@ class ConfigManager:
try: try:
# Load current configs # Load current configs
main_config = self.get_raw_file_content('main') 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) valid_set = set(valid_plugin_ids)
+5 -1
View File
@@ -21,6 +21,8 @@ import time
import requests import requests
from typing import Dict, List from typing import Dict, List
from src.common.api_helper import DEFAULT_HTTP_HEADERS
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class DynamicTeamResolver: class DynamicTeamResolver:
@@ -141,7 +143,9 @@ class DynamicTeamResolver:
self.logger.info("Fetching fresh NCAA Football rankings from ESPN API") 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" 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() response.raise_for_status()
data = response.json() data = response.json()
+31 -25
View File
@@ -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. # Config keys a style element block carries, in schema/UI order.
_STYLE_KEYS = ('font', 'font_size', 'text_color', 'visible', 'align') _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) @dataclass(frozen=True)
class ElementStyle: class ElementStyle:
@@ -301,7 +312,7 @@ def expand_style_elements(schema: Dict[str, Any]) -> Dict[str, Any]:
if layout_props: if layout_props:
layout = props.setdefault('layout', { layout = props.setdefault('layout', {
'type': 'object', 'type': 'object',
'title': 'Layout Offsets', 'title': _LAYOUT_TITLE,
'description': 'Pixel offsets applied to each element ' 'description': 'Pixel offsets applied to each element '
'(positive x moves right, positive y moves down)', '(positive x moves right, positive y moves down)',
'x-advanced': True, 'x-advanced': True,
@@ -345,16 +356,14 @@ def _element_block_from_spec(element_key: str,
'type': 'string', 'type': 'string',
'title': 'Font Family', 'title': 'Font Family',
'x-advanced': True, 'x-advanced': True,
# The core already ships this widget and the config form already # The core's font picker; without the hint the form renders a
# allowlists it; without the hint the field rendered as a bare # bare text box the user has to type a filename into.
# text box the user had to type a filename into.
'x-widget': 'font-selector', 'x-widget': 'font-selector',
} }
# A bitmap font ignores font_size and renders at its own baked-in # 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 # size, so the size ceiling has to be enforced when picking the
# font, not when setting the size. # font, not when setting the size.
max_size = (size_spec or {}).get('max') if isinstance( max_size = size_spec.get('max') if size_spec else None
spec.get('size'), dict) else None
if isinstance(max_size, (int, float)): if isinstance(max_size, (int, float)):
font_prop['x-options'] = {'maxFixedSize': max_size} font_prop['x-options'] = {'maxFixedSize': max_size}
if 'default' in font_spec: if 'default' in font_spec:
@@ -457,8 +466,8 @@ def _offset_block_from_spec(element_key: str,
'title': 'Scale', 'title': 'Scale',
'description': 'Size multiplier; 1 is the shipped size.', 'description': 'Size multiplier; 1 is the shipped size.',
'default': 1.0, 'default': 1.0,
'minimum': 0.1, 'minimum': MIN_ELEMENT_SCALE,
'maximum': 10.0, 'maximum': MAX_ELEMENT_SCALE,
'x-advanced': True, 'x-advanced': True,
} }
if isinstance(scale_spec, dict): if isinstance(scale_spec, dict):
@@ -539,7 +548,7 @@ def _modes_block(declaration: Dict[str, Any],
if layout_props: if layout_props:
element_props['layout'] = { element_props['layout'] = {
'type': 'object', 'type': 'object',
'title': 'Layout Offsets', 'title': _LAYOUT_TITLE,
'x-advanced': True, 'x-advanced': True,
'additionalProperties': False, 'additionalProperties': False,
'properties': layout_props, 'properties': layout_props,
@@ -680,7 +689,7 @@ def _modes_block_from_properties(props: Dict[str, Any], element_keys: list,
if layout_props: if layout_props:
element_props['layout'] = { element_props['layout'] = {
'type': 'object', 'type': 'object',
'title': 'Layout Offsets', 'title': _LAYOUT_TITLE,
'x-advanced': True, 'x-advanced': True,
'additionalProperties': False, 'additionalProperties': False,
'properties': layout_props, 'properties': layout_props,
@@ -1010,12 +1019,15 @@ def _coerce_align(value: Any) -> Optional[str]:
return None return None
def _coerce_scale(value: Any, default: float) -> float: def coerce_scale(value: Any, default: float = 1.0) -> float:
"""A positive size multiplier, or ``default``. """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 ``default`` is returned for anything that is not a finite positive number
a zero-or-inverted image, and the panel is 32 pixels tall -- a typo (None, a bool, a string, 0, a negative, NaN, infinity): those are typos,
should cost a wrong size, not a crash inside PIL. 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: if isinstance(value, bool) or value is None:
return default return default
@@ -1023,9 +1035,9 @@ def _coerce_scale(value: Any, default: float) -> float:
scale = float(value) scale = float(value)
except (TypeError, ValueError): except (TypeError, ValueError):
return default return default
if scale <= 0: if not math.isfinite(scale) or scale <= 0:
return default 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, 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: try:
value = _element_field(config, element_key, 'scale', mode, value = _element_field(config, element_key, 'scale', mode,
in_layout=True) 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: except Exception as e:
logger.warning("Error reading scale for %s: %s", element_key, e) logger.warning("Error reading scale for %s: %s", element_key, e)
return default return default
@@ -1371,12 +1383,6 @@ class ElementStyleResolver:
return {} return {}
return _lookup_element(block.get('layout'), element_key) 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 ----------------------------------------------- # -- resolution internals -----------------------------------------------
@@ -1497,7 +1503,7 @@ class ElementStyleResolver:
user_forced_color=bool(color_forced), user_forced_color=bool(color_forced),
visible=_coerce_bool(visible, True), visible=_coerce_bool(visible, True),
align=_coerce_align(align), 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, def _classic_style(self, classic_font: str, classic_size: int,
+16 -3
View File
@@ -26,6 +26,19 @@ from src.exceptions import LEDMatrixError
from src.redaction import redact_credentials 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 @dataclass
class ErrorRecord: class ErrorRecord:
"""Record of a single error occurrence.""" """Record of a single error occurrence."""
@@ -145,8 +158,8 @@ class ErrorAggregator:
with self._lock: with self._lock:
error_type = type(error).__name__ error_type = type(error).__name__
# Extract additional context from LEDMatrixError subclasses # A copy, so the caller's dict is not changed behind its back.
error_context = context or {} error_context = dict(context) if context else {}
if isinstance(error, LEDMatrixError) and error.context: if isinstance(error, LEDMatrixError) and error.context:
error_context.update(error.context) error_context.update(error.context)
@@ -157,7 +170,7 @@ class ErrorAggregator:
context=error_context, context=error_context,
plugin_id=plugin_id, plugin_id=plugin_id,
operation=operation, operation=operation,
stack_trace=traceback.format_exc() stack_trace=_format_trace(error)
) )
# Add record (with size limit) # Add record (with size limit)
+53 -74
View File
@@ -40,6 +40,11 @@ from pathlib import Path
from PIL import ImageFont from PIL import ImageFont
from src.common.bdf_font import load_bdf_face, read_bdf_native_size 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.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 typing import Dict, Tuple, Optional, Union, Any, List
from src.deprecation import deprecated from src.deprecation import deprecated
@@ -57,7 +62,6 @@ class FontManager:
def __init__(self, config: Dict[str, Any]): def __init__(self, config: Dict[str, Any]):
self.config = config self.config = config
self.fonts_config = config.get("fonts", {})
# Font discovery and catalog # Font discovery and catalog
self.font_catalog: Dict[str, str] = {} # family_name -> file_path self.font_catalog: Dict[str, str] = {} # family_name -> file_path
@@ -73,10 +77,8 @@ class FontManager:
# Plugin font management # Plugin font management
self.plugin_fonts: Dict[str, Dict[str, Any]] = {} # plugin_id -> font_manifest 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.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.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} 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 # 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 = Path(tempfile.gettempdir()) / "ledmatrix_fonts"
self.temp_font_dir.mkdir(exist_ok=True) self.temp_font_dir.mkdir(exist_ok=True)
# Performance monitoring # Counters behind get_performance_stats().
self.performance_stats = { self.performance_stats = {
"font_load_times": {},
"cache_hits": 0, "cache_hits": 0,
"cache_misses": 0, "cache_misses": 0,
"render_times": {},
"total_renders": 0,
"failed_loads": 0, "failed_loads": 0,
"start_time": time.time() "start_time": time.time()
} }
@@ -105,9 +104,6 @@ class FontManager:
"four_by_six": "assets/fonts/4x6-font.ttf", "four_by_six": "assets/fonts/4x6-font.ttf",
"five_by_seven": "assets/fonts/5x7.bdf", "five_by_seven": "assets/fonts/5x7.bdf",
"tom_thumb": "assets/fonts/tom-thumb.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 # Size tokens for convenience
@@ -116,7 +112,10 @@ class FontManager:
} }
# Font overrides storage (for manual overrides) # 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]] = {} self.font_overrides: Dict[str, Dict[str, Any]] = {}
# Bumped whenever cached font objects are invalidated, so holders of # 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]): def reload_config(self, new_config: Dict[str, Any]):
"""Reload configuration and refresh font catalog.""" """Reload configuration and refresh font catalog."""
self.config = new_config self.config = new_config
self.fonts_config = new_config.get("fonts", {})
self.font_cache.clear() # Clear cache to force reload self.font_cache.clear() # Clear cache to force reload
self.metrics_cache.clear() # Clear metrics cache self.metrics_cache.clear() # Clear metrics cache
self.cache_generation += 1 self.cache_generation += 1
@@ -136,7 +134,6 @@ class FontManager:
logger.info("FontManager configuration reloaded successfully") logger.info("FontManager configuration reloaded successfully")
# ==================== Manager Font Registration ==================== # ==================== Manager Font Registration ====================
# NEW: Support for managers to register their font choices dynamically
def register_manager_font(self, manager_id: str, element_key: str, def register_manager_font(self, manager_id: str, element_key: str,
family: str, size_px: int, color: Optional[Tuple[int, int, int]] = None): family: str, size_px: int, color: Optional[Tuple[int, int, int]] = None):
@@ -209,16 +206,23 @@ class FontManager:
# ==================== Plugin Font Management ==================== # ==================== 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. Register fonts for a specific plugin.
Args: Args:
plugin_id: Unique identifier for the plugin 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: 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: try:
# Validate font manifest structure # Validate font manifest structure
@@ -235,7 +239,7 @@ class FontManager:
# Process font definitions # Process font definitions
fonts = font_manifest.get("fonts", []) fonts = font_manifest.get("fonts", [])
for font_def in 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"Successfully registered font {font_def.get('family')} for plugin {plugin_id}")
logger.info(f"Registered {len(fonts)} fonts for plugin {plugin_id}") logger.info(f"Registered {len(fonts)} fonts for plugin {plugin_id}")
@@ -270,7 +274,8 @@ class FontManager:
return True 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.""" """Register a single font from a plugin."""
try: try:
family = font_def["family"] family = font_def["family"]
@@ -284,7 +289,7 @@ class FontManager:
elif source.startswith("plugin://"): elif source.startswith("plugin://"):
# Relative to plugin directory # Relative to plugin directory
relative_path = source.replace("plugin://", "") 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: else:
# Absolute or relative path # Absolute or relative path
font_path = source font_path = source
@@ -298,14 +303,6 @@ class FontManager:
self.plugin_font_catalogs[plugin_id][family] = font_path self.plugin_font_catalogs[plugin_id][family] = font_path
self.font_catalog[namespaced_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}") logger.info(f"Registered plugin font: {namespaced_family} -> {font_path}")
return True return True
@@ -367,11 +364,15 @@ class FontManager:
return '.zip' return '.zip'
return '.ttf' # default return '.ttf' # default
def _resolve_plugin_font_path(self, plugin_id: str, relative_path: str) -> Optional[str]: def _resolve_plugin_font_path(self, plugin_id: str, relative_path: str,
"""Resolve a plugin-relative font path.""" plugin_dir: Optional[Union[str, Path]] = None) -> Optional[str]:
# Assume plugins are in a 'plugins' directory """Resolve a ``plugin://`` font path against the plugin's directory."""
plugin_dir = Path("plugins") / plugin_id if plugin_dir is None:
font_path = plugin_dir / relative_path 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(): if font_path.exists():
return str(font_path) return str(font_path)
@@ -379,6 +380,18 @@ class FontManager:
logger.error(f"Plugin font not found: {font_path}") logger.error(f"Plugin font not found: {font_path}")
return None 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") @deprecated("3.7.0")
def unregister_plugin_fonts(self, plugin_id: str) -> bool: def unregister_plugin_fonts(self, plugin_id: str) -> bool:
"""Unregister all fonts for a plugin.""" """Unregister all fonts for a plugin."""
@@ -390,8 +403,6 @@ class FontManager:
namespaced_family = f"{plugin_id}::{family}" namespaced_family = f"{plugin_id}::{family}"
if namespaced_family in self.font_catalog: if namespaced_family in self.font_catalog:
del self.font_catalog[namespaced_family] 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] del self.plugin_font_catalogs[plugin_id]
@@ -442,8 +453,6 @@ class FontManager:
Returns: Returns:
Resolved font object Resolved font object
""" """
start_time = time.time()
try: try:
# Check for manual overrides first # Check for manual overrides first
if element_key in self.font_overrides: 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]: if plugin_id in self.plugin_font_catalogs and family in self.plugin_font_catalogs[plugin_id]:
family = f"{plugin_id}::{family}" family = f"{plugin_id}::{family}"
# Get the font return self.get_font(family, size_px)
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
except Exception as e: except Exception as e:
logger.error(f"Error resolving font for {element_key}: {e}", exc_info=True) 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] return self.font_cache[cache_key]
self.performance_stats["cache_misses"] += 1 self.performance_stats["cache_misses"] += 1
start_time = time.time()
# Load font # Load font
font_path = self.font_catalog.get(family) font_path = self.font_catalog.get(family)
@@ -506,15 +507,13 @@ class FontManager:
else: else:
font = load_truetype(font_path, size_px) font = load_truetype(font_path, size_px)
except Exception as e: 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}") logger.error(f"Error loading font {font_path}: {e}")
self.performance_stats["failed_loads"] += 1 self.performance_stats["failed_loads"] += 1
font = ImageFont.load_default() font = ImageFont.load_default()
# Cache and record performance
self.font_cache[cache_key] = font self.font_cache[cache_key] = font
duration = time.time() - start_time
self.performance_stats["font_load_times"][cache_key] = duration
return font return font
def _load_bdf_font(self, font_path: str, size_px: int) -> freetype.Face: 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 rather than failing over to PIL's default font, a different typeface
(see :func:`src.common.bdf_font.load_bdf_face`). (see :func:`src.common.bdf_font.load_bdf_face`).
""" """
try: return load_bdf_face(font_path, size_px)[0]
return load_bdf_face(font_path, size_px)[0]
except Exception as e:
logger.error(f"Error loading BDF font {font_path}: {e}")
raise
def get_native_bdf_size(self, family: str) -> Optional[int]: def get_native_bdf_size(self, family: str) -> Optional[int]:
"""The one true pixel size of a BDF family in the catalog, or None """The one true pixel size of a BDF family in the catalog, or None
@@ -723,11 +718,6 @@ class FontManager:
def _save_overrides(self): def _save_overrides(self):
"""Save current font overrides to file.""" """Save current font overrides to file."""
try: 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) font_overrides_path = Path(self.font_overrides_file)
ensure_directory_permissions(font_overrides_path.parent, get_config_dir_mode()) ensure_directory_permissions(font_overrides_path.parent, get_config_dir_mode())
with open(self.font_overrides_file, 'w') as f: with open(self.font_overrides_file, 'w') as f:
@@ -754,12 +744,6 @@ class FontManager:
"""Get available size tokens.""" """Get available size tokens."""
return self.size_tokens.copy() 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") @deprecated("3.7.0")
def get_performance_stats(self) -> Dict[str, Any]: def get_performance_stats(self) -> Dict[str, Any]:
"""Get performance statistics.""" """Get performance statistics."""
@@ -789,7 +773,8 @@ class FontManager:
@deprecated("3.7.0") @deprecated("3.7.0")
def add_font(self, font_file_path: str, family_name: str) -> bool: 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: try:
# Validate font file # Validate font file
if not os.path.exists(font_file_path): if not os.path.exists(font_file_path):
@@ -801,13 +786,7 @@ class FontManager:
logger.warning(f"Font family '{family_name}' already exists") logger.warning(f"Font family '{family_name}' already exists")
return False return False
# Copy font to assets/fonts directory fonts_dir = Path(resolve_asset_path("assets/fonts"))
from pathlib import Path
from src.common.permission_utils import (
ensure_directory_permissions,
get_assets_dir_mode
)
fonts_dir = Path("assets/fonts")
ensure_directory_permissions(fonts_dir, get_assets_dir_mode()) ensure_directory_permissions(fonts_dir, get_assets_dir_mode())
# Add to catalog # Add to catalog
+50 -73
View File
@@ -16,7 +16,7 @@ import json
from typing import Dict, List, Optional, Tuple from typing import Dict, List, Optional, Tuple
from pathlib import Path from pathlib import Path
from PIL import Image, ImageDraw, ImageFont, UnidentifiedImageError 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 PIL.PngImagePlugin import PngInfo
from requests.adapters import HTTPAdapter from requests.adapters import HTTPAdapter
from urllib3.util.retry import Retry from urllib3.util.retry import Retry
@@ -370,22 +370,15 @@ class LogoDownloader:
@staticmethod @staticmethod
def get_logo_filename_variations(abbreviation: str) -> list: def get_logo_filename_variations(abbreviation: str) -> list:
"""Get possible filename variations for a team abbreviation.""" """Filenames a logo for ``abbreviation`` may be stored under: the
variations = [] upper-cased abbreviation as given, then its normalize_abbreviation()
form (``TA&M.png``, then ``TAANDM.png``)."""
original = abbreviation.upper() original = abbreviation.upper()
normalized = LogoDownloader.normalize_abbreviation(abbreviation) normalized = LogoDownloader.normalize_abbreviation(abbreviation)
return [f"{original}.png", f"{normalized}.png"]
# 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
# 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_-]+$') _SAFE_LEAGUE_RE = re.compile(r'^[a-z0-9_-]+$')
def get_logo_directory(self, league: str) -> str: 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}") logger.error(f"Unexpected error downloading logo for {team_abbreviation}: {e}")
return False 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]: 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.""" """Resolve the ESPN API teams URL for a league, with dynamic fallback for custom soccer leagues."""
api_url = self.API_ENDPOINTS.get(league) api_url = self.API_ENDPOINTS.get(league)
if not api_url and league.startswith('soccer_'): if not api_url and league.startswith('soccer_'):
league_code = league[len('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}") logger.warning(f"Rejecting unsafe league_code for ESPN URL construction: {league_code!r}")
return None return None
api_url = f'https://site.api.espn.com/apis/site/v2/sports/soccer/{league_code}/teams' api_url = f'https://site.api.espn.com/apis/site/v2/sports/soccer/{league_code}/teams'
@@ -501,7 +491,8 @@ class LogoDownloader:
return None return None
def fetch_single_team(self, league: str, team_id: str) -> Optional[Dict]: 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) api_url = self._resolve_api_url(league)
if not api_url: if not api_url:
logger.error(f"No API endpoint configured for league: {league}") 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}") logger.error(f"Error fetching team data for {team_id} in {league}: {e}")
return None return None
except json.JSONDecodeError as e: 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 return None
def extract_teams_from_data(self, data: Dict, league: str) -> List[Dict[str, str]]: 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 # Default to FBS for unknown conferences
return 'FBS' 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]: def download_missing_logos_for_league(self, league: str, force_download: bool = False) -> Tuple[int, int]:
"""Download missing logos for a specific league.""" """Download missing logos for a specific league."""
logger.info(f"Starting logo download for league: {league}") logger.info(f"Starting logo download for league: {league}")
@@ -794,7 +749,9 @@ class LogoDownloader:
return False return False
try: try:
logo_url = data["team"]["logos"][0]["href"] 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 return False
# Download the logo # Download the logo
success = self.download_logo(logo_url, logo_path, team_abbreviation) 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") logger.info(f"Overall logo download results: {total_downloaded} downloaded, {total_failed} failed")
return results return results
def create_placeholder_logo(self, team_abbreviation: str, logo_dir: str) -> bool: def create_placeholder_logo(self, team_abbreviation: str, logo_dir: str,
"""Create a placeholder logo when real logo cannot be downloaded.""" 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: try:
# Ensure the logo directory exists
if not self.ensure_logo_directory(logo_dir): if not self.ensure_logo_directory(logo_dir):
logger.error(f"Failed to create logo directory: {logo_dir}") logger.error(f"Failed to create logo directory: {logo_dir}")
return False return False
filename = f"{self.normalize_abbreviation(team_abbreviation)}.png"
filepath = Path(logo_dir) / filename
# Create a simple placeholder logo if filepath is None:
logo = Image.new('RGBA', (64, 64), (100, 100, 100, 255)) # Gray background 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) draw = ImageDraw.Draw(logo)
# Try to load a font, fallback to default # Try to load a font, fallback to default
try: 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): except (OSError, IOError):
try: try:
font = ImageFont.load_default() font = ImageFont.load_default()
@@ -855,8 +827,8 @@ class LogoDownloader:
bbox = draw.textbbox((0, 0), text, font=font) bbox = draw.textbbox((0, 0), text, font=font)
text_width = bbox[2] - bbox[0] text_width = bbox[2] - bbox[0]
text_height = bbox[3] - bbox[1] text_height = bbox[3] - bbox[1]
x = (64 - text_width) // 2 x = (PLACEHOLDER_SIZE[0] - text_width) // 2
y = (64 - text_height) // 2 y = (PLACEHOLDER_SIZE[1] - text_height) // 2
draw.text((x, y), text, font=font, fill=(255, 255, 255, 255)) draw.text((x, y), text, font=font, fill=(255, 255, 255, 255))
else: else:
# Fallback without font # 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. Convenience function to download a missing team logo.
Args: Args:
team_abbreviation: Team abbreviation (e.g., 'UGA', 'BAMA', 'TA&M')
league: League identifier (e.g., 'ncaa_fb', 'nfl') 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 logo_url: Optional direct URL to the logo
create_placeholder: Whether to create a placeholder if download fails create_placeholder: Whether to create a placeholder if download fails
Returns: 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() 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 time.sleep(0.1) # Small delay
if not success and create_placeholder: if not success and create_placeholder:
logger.info(f"Creating placeholder logo for {team_abbreviation}") 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 return success
success = downloader.download_missing_logo_for_team(league, team_id, team_abbreviation, logo_path) 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: if not success and create_placeholder:
logger.info(f"Creating placeholder logo for {team_abbreviation}") logger.info(f"Creating placeholder logo for {team_abbreviation}")
# Create placeholder as fallback # 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: if success:
logger.info(f"Successfully handled logo for {team_abbreviation}") logger.info(f"Successfully handled logo for {team_abbreviation}")
+1 -1
View File
@@ -93,7 +93,7 @@ class PluginExecutor:
if result_container['exception']: if result_container['exception']:
error = result_container['exception'] error = result_container['exception']
error_msg = f"{plugin_context} operation failed: {error}" 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") record_error(error, plugin_id=plugin_id, operation="execute")
raise PluginError(error_msg, plugin_id=plugin_id) from error raise PluginError(error_msg, plugin_id=plugin_id) from error
+2 -1
View File
@@ -394,7 +394,8 @@ class PluginManager:
self.font_manager, 'register_plugin_fonts' self.font_manager, 'register_plugin_fonts'
): ):
try: 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: except Exception as e:
self.logger.warning( self.logger.warning(
"Failed to register fonts for plugin %s: %s", plugin_id, e "Failed to register fonts for plugin %s: %s", plugin_id, e
+15 -11
View File
@@ -1,8 +1,14 @@
""" """
Startup Validator Startup Validator
Validates system configuration, plugins, and dependencies on startup. Checks configuration, the cache directory, plugins and the installed systemd
Fails fast with clear error messages to prevent runtime issues. 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 import os
@@ -180,17 +186,16 @@ class StartupValidator:
try: try:
config = self.config_manager.load_config() config = self.config_manager.load_config()
# Check for required top-level keys
required_keys = ['display', 'timezone'] required_keys = ['display', 'timezone']
for key in required_keys: for key in required_keys:
if key not in config: if key not in config:
self.errors.append(f"Missing required configuration key: {key}") self.errors.append(f"Missing required configuration key: {key}")
# Validate display configuration # A missing display section is reported once, above, and an empty
display_config = config.get('display', {}) # one here; _validate_display_config leaves both to this method.
if not display_config: if 'display' in config and not config['display']:
self.errors.append("Display configuration is missing or empty") self.errors.append("Display configuration is empty")
except ConfigError as e: except ConfigError as e:
self.errors.append(f"Configuration error: {e}") self.errors.append(f"Configuration error: {e}")
except Exception as e: except Exception as e:
@@ -247,8 +252,7 @@ class StartupValidator:
display_config = config.get('display', {}) display_config = config.get('display', {})
if not display_config: if not display_config:
self.errors.append("Display configuration is missing") return # reported by _validate_config
return
hardware_config = display_config.get('hardware', {}) hardware_config = display_config.get('hardware', {})
if not hardware_config: if not hardware_config:
+266 -347
View File
@@ -9,24 +9,18 @@ Tested and optimized for:
- Raspberry Pi OS Bookworm (Debian 12) with NetworkManager - Raspberry Pi OS Bookworm (Debian 12) with NetworkManager
- Raspberry Pi 3B+, 4, 5 with built-in WiFi - Raspberry Pi 3B+, 4, 5 with built-in WiFi
Sudoers Requirements: Privileges:
The following sudoers entries are required for passwordless operation. The web interface runs as an unprivileged user and reaches nmcli,
Add to /etc/sudoers.d/ledmatrix_wifi: systemctl, sysctl, nft and rfkill through exact-command sudo rules.
scripts/install/configure_wifi_permissions.sh writes those rules (and a
ledpi ALL=(ALL) NOPASSWD: /usr/bin/nmcli PolicyKit rule for NetworkManager); first_time_install.sh runs it. Use
ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl start hostapd that script rather than granting commands by hand. It deliberately
ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl stop hostapd grants neither ``iptables`` nor ``ip``: their rules take a live interface
ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl start dnsmasq name, so they would need a wildcard, and ``iptables --modprobe=<path>``
ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl stop dnsmasq and ``ip netns exec`` both run an arbitrary program as root. The code
ledpi ALL=(ALL) NOPASSWD: /usr/bin/systemctl restart NetworkManager paths that call them with sudo therefore only work where the user has
ledpi ALL=(ALL) NOPASSWD: /usr/sbin/ip broader sudo rights (a stock Raspberry Pi image grants the default user
ledpi ALL=(ALL) NOPASSWD: /sbin/ip blanket NOPASSWD).
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
""" """
import subprocess import subprocess
@@ -39,6 +33,8 @@ from pathlib import Path
from typing import Any, Dict, List, Optional, Tuple from typing import Any, Dict, List, Optional, Tuple
from dataclasses import dataclass from dataclasses import dataclass
from src.config_manager_atomic import atomic_write_json
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
# Path for storing WiFi configuration (will be set dynamically) # Path for storing WiFi configuration (will be set dynamically)
@@ -78,6 +74,22 @@ DNSMASQ_SERVICE = "dnsmasq"
DEFAULT_AP_SSID = "LEDMatrix-Setup" DEFAULT_AP_SSID = "LEDMatrix-Setup"
DEFAULT_AP_CHANNEL = 7 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 message file (for display_controller integration)
LED_STATUS_FILE = None # Will be set dynamically LED_STATUS_FILE = None # Will be set dynamically
@@ -198,30 +210,8 @@ class WiFiManager:
logger.debug(f"Could not clear LED status message: {e}") logger.debug(f"Could not clear LED status message: {e}")
def _check_command(self, command: str) -> bool: def _check_command(self, command: str) -> bool:
"""Check if a command is available""" """Whether ``command`` is installed (see _find_command_path)."""
try: return self._find_command_path(command) is not None
# 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
def _find_command_path(self, command: str) -> Optional[str]: def _find_command_path(self, command: str) -> Optional[str]:
""" """
@@ -321,14 +311,22 @@ class WiFiManager:
del self.config["saved_networks"] del self.config["saved_networks"]
self._save_config() self._save_config()
def _save_config(self): def _save_config(self) -> bool:
"""Save WiFi configuration to file""" """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: try:
with open(self.config_path, 'w') as f: atomic_write_json(self.config_path, self.config)
json.dump(self.config, f, indent=2) except (OSError, TypeError, ValueError) as e:
logger.info(f"Saved WiFi config to {self.config_path}") logger.error(f"Failed to save WiFi config to {self.config_path}: {e}")
except Exception as e: return False
logger.error(f"Failed to save WiFi config: {e}") logger.info(f"Saved WiFi config to {self.config_path}")
return True
def get_wifi_status(self) -> WiFiStatus: def get_wifi_status(self) -> WiFiStatus:
""" """
@@ -402,8 +400,6 @@ class WiFiManager:
for line in result.stdout.strip().split('\n'): for line in result.stdout.strip().split('\n'):
if '802-11-wireless.ssid:' in line: if '802-11-wireless.ssid:' in line:
ssid = line.split(':', 1)[1].strip() ssid = line.split(':', 1)[1].strip()
if ssid:
continue
elif 'WIFI.SIGNAL:' in line: elif 'WIFI.SIGNAL:' in line:
try: try:
signal = int(line.split(':', 1)[1].strip()) signal = int(line.split(':', 1)[1].strip())
@@ -425,24 +421,7 @@ class WiFiManager:
ssid = parts[1].strip() ssid = parts[1].strip()
if ssid: if ssid:
break 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 # Get IP address if connected
if wifi_connected and wlan_device: if wifi_connected and wlan_device:
result = subprocess.run( result = subprocess.run(
@@ -536,7 +515,7 @@ class WiFiManager:
if result.returncode == 0: if result.returncode == 0:
ips = result.stdout.strip().split() ips = result.stdout.strip().split()
for ip in ips: for ip in ips:
if not ip.startswith('192.168.4.1'): # Exclude AP IP if ip != AP_IP:
ip_address = ip ip_address = ip
break break
@@ -778,13 +757,13 @@ class WiFiManager:
if subprocess.run( if subprocess.run(
["sudo", iptables, "-t", "nat", "-C", "PREROUTING", ["sudo", iptables, "-t", "nat", "-C", "PREROUTING",
"-i", self._wifi_interface, "-p", "tcp", "--dport", "80", "-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 capture_output=True, timeout=5
).returncode != 0: ).returncode != 0:
r = subprocess.run( r = subprocess.run(
["sudo", iptables, "-t", "nat", "-A", "PREROUTING", ["sudo", iptables, "-t", "nat", "-A", "PREROUTING",
"-i", self._wifi_interface, "-p", "tcp", "--dport", "80", "-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 capture_output=True, text=True, timeout=5
) )
if r.returncode != 0: if r.returncode != 0:
@@ -794,12 +773,12 @@ class WiFiManager:
if subprocess.run( if subprocess.run(
["sudo", iptables, "-C", "INPUT", ["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 capture_output=True, timeout=5
).returncode != 0: ).returncode != 0:
r = subprocess.run( r = subprocess.run(
["sudo", iptables, "-A", "INPUT", ["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 capture_output=True, text=True, timeout=5
) )
if r.returncode != 0: if r.returncode != 0:
@@ -808,7 +787,7 @@ class WiFiManager:
return False return False
self._redirect_backend = "iptables" 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 return True
def _setup_iptables_redirect_nftables(self, nft: str) -> bool: def _setup_iptables_redirect_nftables(self, nft: str) -> bool:
@@ -819,7 +798,7 @@ class WiFiManager:
["sudo", nft, "add", "chain", "ip", "ledmatrix", "prerouting", ["sudo", nft, "add", "chain", "ip", "ledmatrix", "prerouting",
"{", "type", "nat", "hook", "prerouting", "priority", "-100", ";", "}"], "{", "type", "nat", "hook", "prerouting", "priority", "-100", ";", "}"],
["sudo", nft, "add", "rule", "ip", "ledmatrix", "prerouting", ["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: for cmd in cmds:
r = subprocess.run(cmd, capture_output=True, text=True, timeout=5) 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()}") logger.debug(f"nft cmd non-zero (may already exist): {r.stderr.strip()}")
self._redirect_backend = "nftables" 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 return True
def _teardown_iptables_redirect(self) -> None: def _teardown_iptables_redirect(self) -> None:
@@ -847,12 +826,12 @@ class WiFiManager:
subprocess.run( subprocess.run(
["sudo", iptables, "-t", "nat", "-D", "PREROUTING", ["sudo", iptables, "-t", "nat", "-D", "PREROUTING",
"-i", self._wifi_interface, "-p", "tcp", "--dport", "80", "-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 capture_output=True, timeout=5
) )
subprocess.run( subprocess.run(
["sudo", iptables, "-D", "INPUT", ["sudo", iptables, "-D", "INPUT",
"-i", self._wifi_interface, "-p", "tcp", "--dport", "5000", "-i", self._wifi_interface, "-p", "tcp", "--dport", str(PORTAL_PORT),
"-j", "ACCEPT"], "-j", "ACCEPT"],
capture_output=True, timeout=5 capture_output=True, timeout=5
) )
@@ -887,7 +866,7 @@ class WiFiManager:
except Exception as e: except Exception as e:
logger.warning(f"Could not tear down port redirect: {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 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 resolve every hostname to the AP IP. This triggers the OS captive-portal
@@ -1026,39 +1005,53 @@ class WiFiManager:
) )
if result.returncode != 0: if result.returncode != 0:
return [] return []
networks = self._parse_nmcli_wifi_list(result.stdout)
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)
except Exception as e: except Exception as e:
logger.debug(f"nmcli cached list failed: {e}") logger.debug(f"nmcli cached list failed: {e}")
return networks 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: def _save_cached_scan(self, networks: List[WiFiNetwork]) -> None:
"""Save scan results to a cache file for use during AP mode.""" """Save scan results to a cache file for use during AP mode."""
try: try:
@@ -1088,7 +1081,6 @@ class WiFiManager:
def _scan_nmcli(self) -> List[WiFiNetwork]: def _scan_nmcli(self) -> List[WiFiNetwork]:
"""Scan networks using nmcli""" """Scan networks using nmcli"""
networks = []
try: try:
# Trigger scan # Trigger scan
subprocess.run( subprocess.run(
@@ -1108,52 +1100,7 @@ class WiFiManager:
if result.returncode != 0: if result.returncode != 0:
return [] return []
return self._parse_nmcli_wifi_list(result.stdout)
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
except Exception as e: except Exception as e:
logger.error(f"Error scanning with nmcli: {e}") logger.error(f"Error scanning with nmcli: {e}")
return [] return []
@@ -1357,27 +1304,7 @@ class WiFiManager:
disconnect_success, disconnect_msg = self.disconnect_from_network(skip_ap_check=True) disconnect_success, disconnect_msg = self.disconnect_from_network(skip_ap_check=True)
if disconnect_success: if disconnect_success:
logger.info(f"Disconnected from {original_ssid}: {disconnect_msg}") logger.info(f"Disconnected from {original_ssid}: {disconnect_msg}")
# Wait for device to be ready for new connection if not self._wait_for_device_idle(5):
# 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:
logger.warning("Device may not be ready, but proceeding with connection attempt") logger.warning("Device may not be ready, but proceeding with connection attempt")
else: else:
logger.warning(f"Failed to disconnect from {original_ssid}: {disconnect_msg}") 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}" return False, f"Failed to connect to {ssid}, restored {original_ssid}"
else: else:
logger.error(f"Failed to restore original connection: {original_ssid}") logger.error(f"Failed to restore original connection: {original_ssid}")
# Trigger AP mode as last resort return self._failsafe_ap(
self._show_led_message("Enabling AP mode...", duration=5) "Connection failed and restoration failed. AP mode enabled.",
ap_success, ap_msg = self.enable_ap_mode(force=True) "Connection failed, restoration failed, and AP mode failed")
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}"
# If connection failed and no original connection to restore, enable AP mode # If connection failed and no original connection to restore, enable AP mode
elif not success: elif not success:
logger.warning(f"Connection to {ssid} failed and no original connection to restore") logger.warning(f"Connection to {ssid} failed and no original connection to restore")
self._show_led_message("Enabling AP mode...", duration=5) return self._failsafe_ap("Connection failed. AP mode enabled.",
ap_success, ap_msg = self.enable_ap_mode(force=True) "Connection failed and AP mode failed")
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 success, message return success, message
else: else:
@@ -1443,6 +1359,22 @@ class WiFiManager:
logger.error("Last-resort AP mode enable failed in recovery path: %s", ap_error, exc_info=True) logger.error("Last-resort AP mode enable failed in recovery path: %s", ap_error, exc_info=True)
return False, str(e) 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: def _restore_original_connection(self, connection_name: str, ssid: str) -> bool:
""" """
Restore a previously active WiFi connection. Restore a previously active WiFi connection.
@@ -1496,68 +1428,96 @@ class WiFiManager:
logger.error(f"Error restoring connection: {e}") logger.error(f"Error restoring connection: {e}")
return False 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]: def _connect_nmcli(self, ssid: str, password: str) -> Tuple[bool, str]:
"""Connect using nmcli""" """Connect using nmcli"""
try: try:
# Show LED message # Show LED message
self._show_led_message(f"Connecting to {ssid}...", duration=10) self._show_led_message(f"Connecting to {ssid}...", duration=10)
# Find existing NM connection for this SSID. existing_conn_name = self._find_profile_for_ssid(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
if existing_conn_name: if existing_conn_name:
# Connection exists, try to activate it first (faster and more reliable) # Connection exists, try to activate it first (faster and more reliable)
logger.info(f"Found existing connection for {ssid}, activating...") logger.info(f"Found existing connection for {ssid}, activating...")
# Ensure device is ready before activating self._wait_for_device_idle(3)
# 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)
result = subprocess.run( result = subprocess.run(
["nmcli", "connection", "up", existing_conn_name], ["nmcli", "connection", "up", existing_conn_name],
capture_output=True, capture_output=True,
@@ -1566,19 +1526,8 @@ class WiFiManager:
) )
if result.returncode == 0: if result.returncode == 0:
# Wait longer for connection to stabilize and verify multiple times status = self._verify_connected(ssid)
max_verification_attempts = 5 if status is not None:
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:
ip = status.ip_address or "Unknown" ip = status.ip_address or "Unknown"
self._show_led_message(f"Connected! {ip}", duration=5) self._show_led_message(f"Connected! {ip}", duration=5)
logger.info(f"Successfully connected to {ssid} with IP {ip}") logger.info(f"Successfully connected to {ssid} with IP {ip}")
@@ -1605,25 +1554,8 @@ class WiFiManager:
) )
if result.returncode == 0: if result.returncode == 0:
# Wait longer for connection to stabilize and verify multiple times status = self._verify_connected(ssid, stop_on_other_network=True)
max_verification_attempts = 5 if status is not None:
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:
ip = status.ip_address or "Unknown" ip = status.ip_address or "Unknown"
self._show_led_message(f"Connected! {ip}", duration=5) self._show_led_message(f"Connected! {ip}", duration=5)
logger.info(f"Successfully connected to {ssid} with IP {ip}") 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) return any(ind in lower for ind in indicators)
def _connect_wpa_supplicant(self, ssid: str, password: str) -> Tuple[bool, str]: def _connect_wpa_supplicant(self, ssid: str, password: str) -> Tuple[bool, str]:
"""Connect using wpa_supplicant (fallback)""" """Without NetworkManager there is no supported way to connect: doing it
try: through wpa_supplicant would mean editing its config file, which is
# This would require modifying /etc/wpa_supplicant/wpa_supplicant.conf not implemented. Always returns (False, reason)."""
# For now, return not implemented return False, "wpa_supplicant connection not yet implemented. Please use NetworkManager (nmcli)."
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)
def disconnect_from_network(self, skip_ap_check: bool = False) -> Tuple[bool, str]: def disconnect_from_network(self, skip_ap_check: bool = False) -> Tuple[bool, str]:
""" """
@@ -1745,33 +1673,18 @@ class WiFiManager:
# Disconnect using nmcli # Disconnect using nmcli
if self.has_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: if status.ssid:
# Find the connection name for this SSID conn_name = self._find_profile_for_ssid(status.ssid)
conn_result = subprocess.run( if conn_name:
["nmcli", "-t", "-f", "NAME,802-11-wireless.ssid", "connection", "show"], subprocess.run( # nosec B603 B607 - list args, no shell
capture_output=True, ["nmcli", "connection", "down", conn_name],
text=True, capture_output=True,
timeout=5 timeout=10
) )
if conn_result.returncode == 0: logger.info(f"Disconnected connection {conn_name} for {status.ssid}")
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
result = subprocess.run( result = subprocess.run(
["nmcli", "device", "disconnect", self._wifi_interface], ["nmcli", "device", "disconnect", self._wifi_interface],
capture_output=True, capture_output=True,
@@ -1814,7 +1727,11 @@ class WiFiManager:
max_retries: Maximum number of retry attempts to enable WiFi radio max_retries: Maximum number of retry attempts to enable WiFi radio
Returns: 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): for attempt in range(max_retries):
try: try:
@@ -2062,11 +1979,7 @@ class WiFiManager:
if result[0]: if result[0]:
self._ap_enabled_at = time.time() self._ap_enabled_at = time.time()
if force: if force:
try: self._mark_forced()
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}")
return result return result
# Fallback to nmcli hotspot (simpler, no captive portal) # Fallback to nmcli hotspot (simpler, no captive portal)
@@ -2077,11 +1990,7 @@ class WiFiManager:
if result[0]: if result[0]:
self._ap_enabled_at = time.time() self._ap_enabled_at = time.time()
if force: if force:
try: self._mark_forced()
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}")
return result return result
return False, "No WiFi tools available (nmcli, hostapd, or dnsmasq required)" 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}") logger.error(f"Error in enable_ap_mode: {e}")
return False, str(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]: def _enable_ap_mode_hostapd(self) -> Tuple[bool, str]:
"""Enable AP mode using hostapd and dnsmasq (captive portal)""" """Enable AP mode using hostapd and dnsmasq (captive portal)"""
try: try:
@@ -2115,7 +2033,7 @@ class WiFiManager:
timeout=10 timeout=10
) )
subprocess.run( 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, capture_output=True,
timeout=10 timeout=10
) )
@@ -2124,7 +2042,7 @@ class WiFiManager:
capture_output=True, capture_output=True,
timeout=10 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: except (subprocess.TimeoutExpired, subprocess.SubprocessError, OSError) as e:
logger.warning(f"Error setting up {self._wifi_interface} IP: {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 # Use the validated SSID so the displayed name matches what hostapd broadcast
ap_ssid, _ = self._validate_ap_config() ap_ssid, _ = self._validate_ap_config()
self._show_led_message( 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" return True, "AP mode enabled"
except Exception as e: except Exception as e:
@@ -2198,7 +2116,7 @@ class WiFiManager:
# Delete only the specific application-managed AP profiles by name. # Delete only the specific application-managed AP profiles by name.
# Never delete by SSID — that would destroy a user's saved home network. # 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], subprocess.run(["nmcli", "connection", "down", conn_name],
capture_output=True, timeout=5) capture_output=True, timeout=5)
subprocess.run(["nmcli", "connection", "delete", conn_name], subprocess.run(["nmcli", "connection", "delete", conn_name],
@@ -2215,14 +2133,14 @@ class WiFiManager:
cmd = [ cmd = [
"nmcli", "connection", "add", "nmcli", "connection", "add",
"type", "wifi", "type", "wifi",
"con-name", "LEDMatrix-Setup-AP", "con-name", AP_PROFILE_NAME,
"ifname", self._wifi_interface, "ifname", self._wifi_interface,
"ssid", ap_ssid, "ssid", ap_ssid,
"802-11-wireless.mode", "ap", "802-11-wireless.mode", "ap",
"802-11-wireless.band", "bg", # 2.4 GHz for maximum compatibility "802-11-wireless.band", "bg", # 2.4 GHz for maximum compatibility
"802-11-wireless.channel", str(ap_channel), "802-11-wireless.channel", str(ap_channel),
"ipv4.method", "shared", "ipv4.method", "shared",
"ipv4.addresses", "192.168.4.1/24", "ipv4.addresses", f"{AP_IP}/24",
# No 802-11-wireless-security section → open network # No 802-11-wireless-security section → open network
] ]
@@ -2247,14 +2165,14 @@ class WiFiManager:
logger.info("AP connection profile created, bringing it up...") logger.info("AP connection profile created, bringing it up...")
up_result = subprocess.run( up_result = subprocess.run(
["nmcli", "connection", "up", "LEDMatrix-Setup-AP"], ["nmcli", "connection", "up", AP_PROFILE_NAME],
capture_output=True, text=True, timeout=20 capture_output=True, text=True, timeout=20
) )
if up_result.returncode != 0: if up_result.returncode != 0:
error_msg = up_result.stderr.strip() or up_result.stdout.strip() error_msg = up_result.stderr.strip() or up_result.stdout.strip()
logger.error(f"Failed to bring up AP connection: {error_msg}") logger.error(f"Failed to bring up AP connection: {error_msg}")
self._remove_nm_dnsmasq_captive_conf() 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) capture_output=True, timeout=10)
self._show_led_message("AP mode failed", duration=5) self._show_led_message("AP mode failed", duration=5)
return False, f"Failed to start AP: {error_msg}" return False, f"Failed to start AP: {error_msg}"
@@ -2266,9 +2184,9 @@ class WiFiManager:
if not self._setup_iptables_redirect(): if not self._setup_iptables_redirect():
logger.error("Captive-portal redirect setup failed; rolling back AP profile") logger.error("Captive-portal redirect setup failed; rolling back AP profile")
self._remove_nm_dnsmasq_captive_conf() 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) 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) capture_output=True, timeout=10)
self._clear_led_message() self._clear_led_message()
return False, "AP started but captive-portal redirect setup failed" 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") logger.debug(f"AP verification attempt {_attempt + 1}/5 not yet active, waiting 2s")
time.sleep(2) time.sleep(2)
if status.get('active'): 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)") 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) 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}:5000" return True, f"AP mode enabled (open network) - Access at {ip}:{PORTAL_PORT}"
else: else:
logger.error("AP mode started but not verified by status check — rolling back") logger.error("AP mode started but not verified by status check — rolling back")
self._teardown_iptables_redirect() self._teardown_iptables_redirect()
self._remove_nm_dnsmasq_captive_conf() 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) 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) capture_output=True, timeout=10)
self._clear_led_message() self._clear_led_message()
return False, "AP mode started but verification failed" return False, "AP mode started but verification failed"
@@ -2326,9 +2244,9 @@ class WiFiManager:
conn_name = parts[0].strip() conn_name = parts[0].strip()
conn_type = parts[1].strip().lower() conn_type = parts[1].strip().lower()
# Match our known AP profile name OR the legacy nmcli hotspot type # 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) # 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 interface = parts[2] if len(parts) > 2 else self._wifi_interface
try: try:
ip_result = subprocess.run( ip_result = subprocess.run(
@@ -2399,7 +2317,7 @@ class WiFiManager:
) )
else: else:
# Disable nmcli hotspot mode (fallback) # Disable nmcli hotspot mode (fallback)
for conn_name in ["LEDMatrix-Setup-AP", "Hotspot", "TickerSetup-AP"]: for conn_name in AP_PROFILE_NAMES:
subprocess.run( subprocess.run(
["nmcli", "connection", "down", conn_name], ["nmcli", "connection", "down", conn_name],
capture_output=True, capture_output=True,
@@ -2428,7 +2346,7 @@ class WiFiManager:
# Clean up WiFi interface IP configuration # Clean up WiFi interface IP configuration
subprocess.run( 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, capture_output=True,
timeout=10 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 dhcp-range=192.168.4.2,192.168.4.20,255.255.255.0,24h
# Captive portal: Redirect all DNS queries to Pi # Captive portal: Redirect all DNS queries to Pi
address=/#/192.168.4.1 address=/#/{AP_IP}
# Captive portal detection endpoints # Captive portal detection endpoints
address=/captive.apple.com/192.168.4.1 address=/captive.apple.com/{AP_IP}
address=/connectivitycheck.gstatic.com/192.168.4.1 address=/connectivitycheck.gstatic.com/{AP_IP}
address=/www.msftconnecttest.com/192.168.4.1 address=/www.msftconnecttest.com/{AP_IP}
address=/detectportal.firefox.com/192.168.4.1 address=/detectportal.firefox.com/{AP_IP}
""" """
# Write config (requires sudo) # 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 # Pre-cache a WiFi scan so the captive portal can show networks
try: try:
logger.info("Running pre-AP WiFi scan for captive portal cache...") 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) networks, _cached = self.scan_networks(allow_cached=False)
if networks: if networks:
self._save_cached_scan(networks)
logger.info(f"Cached {len(networks)} networks for captive portal") logger.info(f"Cached {len(networks)} networks for captive portal")
except Exception as scan_err: except Exception as scan_err:
logger.debug(f"Pre-AP scan failed (non-critical): {scan_err}") logger.debug(f"Pre-AP scan failed (non-critical): {scan_err}")
+42 -13
View File
@@ -92,7 +92,7 @@ class TestGet:
helper.session.get.assert_not_called() helper.session.get.assert_not_called()
rate_spy.assert_not_called() rate_spy.assert_not_called()
def test_cache_miss_fetches_and_caches_without_ttl(self, helper, cache): def test_cache_miss_fetches_and_caches_with_ttl(self, helper, cache):
cache.get.return_value = None cache.get.return_value = None
helper.session.get = Mock(return_value=_make_response({'a': 1})) helper.session.get = Mock(return_value=_make_response({'a': 1}))
@@ -100,9 +100,47 @@ class TestGet:
cache_ttl=999) cache_ttl=999)
assert result == {'a': 1} assert result == {'a': 1}
# Pin the ttl-dropped contract: CacheManager.set is called with cache.set.assert_called_once_with('k', {'a': 1}, ttl=999)
# (key, data) only — the cache_ttl argument is discarded.
cache.set.assert_called_once_with('k', {'a': 1}) def test_set_cache_passes_ttl(self, helper, cache):
helper.set_cache('k', {'a': 1}, ttl=42)
cache.set.assert_called_once_with('k', {'a': 1}, ttl=42)
class TestCacheLifetimeWithRealCacheManager:
"""cache_ttl decides how long a response is reused, in both directions:
past CacheManager's 300-second default read age, and not beyond it."""
@pytest.fixture
def real_cache(self, tmp_path):
from unittest.mock import patch
from src.cache_manager import CacheManager
with patch('src.cache_manager.CacheManager._get_writable_cache_dir',
return_value=str(tmp_path)):
cache = CacheManager()
yield cache
# Releases the class-wide cleanup-thread claim on this directory,
# which would otherwise leak into test_cache_cleanup_thread_ownership.
cache.stop_cleanup_thread()
def _fetch_twice(self, real_cache, monkeypatch, ttl, elapsed):
helper = APIHelper(cache_manager=real_cache)
helper.set_rate_limit(0)
helper.session.get = Mock(side_effect=[_make_response({'n': 1}),
_make_response({'n': 2})])
now = [1_000_000.0]
monkeypatch.setattr('src.cache.memory_cache.time.time', lambda: now[0])
monkeypatch.setattr('src.cache.disk_cache.time.time', lambda: now[0])
monkeypatch.setattr('src.cache_manager.time.time', lambda: now[0])
helper.get('https://example.com/api', cache_key='lifetime_test', cache_ttl=ttl)
now[0] += elapsed
return helper.get('https://example.com/api', cache_key='lifetime_test', cache_ttl=ttl)
def test_long_ttl_outlives_the_default_read_age(self, real_cache, monkeypatch):
assert self._fetch_twice(real_cache, monkeypatch, ttl=3600, elapsed=1000) == {'n': 1}
def test_short_ttl_expires(self, real_cache, monkeypatch):
assert self._fetch_twice(real_cache, monkeypatch, ttl=60, elapsed=120) == {'n': 2}
def test_request_exception_returns_none_and_caches_nothing( def test_request_exception_returns_none_and_caches_nothing(
self, helper, cache): self, helper, cache):
@@ -223,15 +261,6 @@ class TestClearCache:
manager.clear_cache.assert_called_once_with() manager.clear_cache.assert_called_once_with()
def test_no_pattern_falls_back_to_clear(self):
manager = types.SimpleNamespace(clear=Mock())
helper = APIHelper(cache_manager=manager)
helper.set_rate_limit(0)
helper.clear_cache()
manager.clear.assert_called_once_with()
def test_no_pattern_manager_without_any_clear_is_noop(self): def test_no_pattern_manager_without_any_clear_is_noop(self):
helper = APIHelper(cache_manager=object()) helper = APIHelper(cache_manager=object())
helper.set_rate_limit(0) helper.set_rate_limit(0)
+8
View File
@@ -408,6 +408,14 @@ class TestAutoEnableApMode:
assert response.status_code == 400 assert response.status_code == 400
assert "auto_enable_ap_mode" not in wifi_manager.config assert "auto_enable_ap_mode" not in wifi_manager.config
def test_a_failed_save_is_reported(self, api_v3_client, wifi_manager):
# wifi_config.json left owned by root is the usual cause.
wifi_manager.config = {}
wifi_manager._save_config.return_value = False
response = api_v3_client.post(self.URL, json={"auto_enable_ap_mode": False})
assert response.status_code == 500
assert response.get_json()["status"] == "error"
class TestRadioEnabledAndForceAcceptIntegers: class TestRadioEnabledAndForceAcceptIntegers:
"""`{"enabled": 1}` / `{"enabled": 0}` used to be mishandled: the old """`{"enabled": 1}` / `{"enabled": 0}` used to be mishandled: the old
-3
View File
@@ -362,6 +362,3 @@ class TestPriorityIsAcceptedAndIgnored:
rid = service.submit_fetch_request( rid = service.submit_fetch_request(
"nfl", 2026, "http://example.invalid/x", cache_key="k", priority=5) "nfl", 2026, "http://example.invalid/x", cache_key="k", priority=5)
assert service.get_result(rid).cached is True assert service.get_result(rid).cached is True
def test_statistics_still_report_an_empty_queue(self, service):
assert service.get_statistics()["queue_size"] == 0
+25
View File
@@ -180,6 +180,31 @@ def test_create_backup_manifest(project: Path, tmp_path: Path) -> None:
assert set(manifest["contents"]) >= {"config", "secrets", "wifi", "fonts", "plugin_uploads", "plugins"} assert set(manifest["contents"]) >= {"config", "secrets", "wifi", "fonts", "plugin_uploads", "plugins"}
def test_manifest_version_is_the_core_release(project: Path, tmp_path: Path) -> None:
"""Not a git sha or a truncated "ref: refs/he..." read from .git/HEAD."""
from src import __version__
git = project / ".git"
git.mkdir()
(git / "HEAD").write_text("ref: refs/heads/some-branch-that-is-not-there\n", encoding="utf-8")
zip_path = create_backup(project, output_dir=tmp_path / "exports")
with zipfile.ZipFile(zip_path) as zf:
manifest = json.loads(zf.read("manifest.json"))
assert manifest["ledmatrix_version"] == __version__
def test_installed_plugins_come_from_the_configured_directory(tmp_path: Path) -> None:
root = tmp_path / "proj"
(root / "config").mkdir(parents=True)
(root / "config" / "config.json").write_text(
json.dumps({"plugin_system": {"plugins_directory": "plugins"}}), encoding="utf-8")
plugin_dir = root / "plugins" / "dev-plugin"
plugin_dir.mkdir(parents=True)
(plugin_dir / "manifest.json").write_text(
json.dumps({"id": "dev-plugin", "version": "0.3.0"}), encoding="utf-8")
assert [p["plugin_id"] for p in list_installed_plugins(root)] == ["dev-plugin"]
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# Validate # Validate
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
-3
View File
@@ -342,7 +342,6 @@ class TestLoadConfiguration:
'base_odds_manager': { 'base_odds_manager': {
'update_interval': 100, 'update_interval': 100,
'timeout': 5, 'timeout': 5,
'cache_ttl': 42,
} }
} }
@@ -352,7 +351,6 @@ class TestLoadConfiguration:
# Key/attr mismatch pin: the config key is 'timeout' but the # Key/attr mismatch pin: the config key is 'timeout' but the
# attribute is request_timeout. # attribute is request_timeout.
assert manager.request_timeout == 5 assert manager.request_timeout == 5
assert manager.cache_ttl == 42
def test_get_config_raising_keeps_defaults(self, cache_manager): def test_get_config_raising_keeps_defaults(self, cache_manager):
config_manager = MagicMock() config_manager = MagicMock()
@@ -362,4 +360,3 @@ class TestLoadConfiguration:
assert manager.update_interval == 3600 assert manager.update_interval == 3600
assert manager.request_timeout == 5 assert manager.request_timeout == 5
assert manager.cache_ttl == 1800
@@ -185,3 +185,35 @@ class TestLogoScale:
def test_an_unusable_scale_is_ignored(self, logo, bad): def test_an_unusable_scale_is_ignored(self, logo, bad):
helper = LogoHelper(display_width=64, display_height=32) helper = LogoHelper(display_width=64, display_height=32)
assert helper.load_logo("AAA", logo, 32, 32, scale=bad).size == (32, 32) assert helper.load_logo("AAA", logo, 32, 32, scale=bad).size == (32, 32)
def test_a_scale_the_schema_allows_is_applied(self, logo):
"""The Scale field's maximum is honoured, not reset to 1.0."""
from src.element_style import MAX_ELEMENT_SCALE
helper = LogoHelper(display_width=64, display_height=32)
big = helper.load_logo("AAA", logo, 4, 4, scale=MAX_ELEMENT_SCALE)
assert big.size == (40, 40)
def test_a_scale_beyond_the_range_is_clamped(self, logo):
from src.element_style import MAX_ELEMENT_SCALE, MIN_ELEMENT_SCALE
helper = LogoHelper(display_width=64, display_height=32)
assert helper.load_logo("AAA", logo, 4, 4, scale=MAX_ELEMENT_SCALE * 3).size == (40, 40)
assert helper.load_logo("AAA", logo, 40, 40, scale=MIN_ELEMENT_SCALE / 2).size == (4, 4)
class TestScaleCoercion:
"""One range for the schema, element_scale and LogoHelper."""
def test_schema_bounds_are_the_clamp_bounds(self):
from src.element_style import (MAX_ELEMENT_SCALE, MIN_ELEMENT_SCALE,
_offset_block_from_spec)
prop = _offset_block_from_spec("home_logo", {"scale": True})["properties"]["scale"]
assert (prop["minimum"], prop["maximum"]) == (MIN_ELEMENT_SCALE, MAX_ELEMENT_SCALE)
@pytest.mark.parametrize("raw,expected", [
(0.5, 0.5), (25, 10.0), (0.01, 0.1),
(0, 1.0), (-2, 1.0), ("x", 1.0), (True, 1.0),
(float("nan"), 1.0), (float("inf"), 1.0),
])
def test_element_scale_clamps_and_rejects(self, raw, expected):
cfg = {"customization": {"layout": {"home_logo": {"scale": raw}}}}
assert element_scale(cfg, "home_logo") == expected
+39
View File
@@ -124,6 +124,45 @@ class TestErrorRecording:
assert aggregator._plugin_error_counts["plugin-a"]["ValueError"] == 2 assert aggregator._plugin_error_counts["plugin-a"]["ValueError"] == 2
assert aggregator._plugin_error_counts["plugin-b"]["ValueError"] == 1 assert aggregator._plugin_error_counts["plugin-b"]["ValueError"] == 1
def test_stack_trace_recorded_outside_except_block(self):
"""The trace comes from the exception, not from the handler in progress.
plugin_executor records exceptions caught on a worker thread after
its except block has ended, where format_exc() only says
"NoneType: None".
"""
def failing_plugin_update():
raise ValueError("boom")
caught = []
def worker():
try:
failing_plugin_update()
except ValueError as e:
caught.append(e)
thread = threading.Thread(target=worker)
thread.start()
thread.join()
record = ErrorAggregator().record_error(caught[0], plugin_id="p")
assert "NoneType: None" not in record.stack_trace
assert "failing_plugin_update" in record.stack_trace
assert "ValueError: boom" in record.stack_trace
def test_record_error_leaves_caller_context_unchanged(self):
"""LEDMatrixError context is merged into a copy of the caller's dict."""
context = {"caller": "value"}
error = PluginError("failed", plugin_id="p", context={"extra": 1})
record = ErrorAggregator().record_error(error, context=context)
assert context == {"caller": "value"}
assert record.context["caller"] == "value"
assert record.context["extra"] == 1
class TestPatternDetection: class TestPatternDetection:
"""Test error pattern detection.""" """Test error pattern detection."""
+38
View File
@@ -8,10 +8,14 @@ test here asserts observable behavior: returned font types, cache identity,
fallback selection, and BDF native-size reading. fallback selection, and BDF native-size reading.
""" """
import json
import shutil
import freetype import freetype
import pytest import pytest
from PIL import ImageFont from PIL import ImageFont
from src.common.font_layout import resolve_asset_path
from src.font_manager import FontManager from src.font_manager import FontManager
@@ -132,3 +136,37 @@ class TestCacheLifecycle:
fm.reload_config({}) fm.reload_config({})
assert fm.cache_generation == gen_before + 1 assert fm.cache_generation == gen_before + 1
assert not fm.font_cache assert not fm.font_cache
class TestPluginFonts:
"""plugin:// sources resolve against the plugin's own directory, which
by default lives under plugin-repos/, not a cwd-relative plugins/."""
MANIFEST = {"fonts": [{"family": "bundled", "source": "plugin://fonts/Bundled.ttf"}]}
@staticmethod
def _plugin_with_font(root, name="my-plugin"):
plugin_dir = root / name
(plugin_dir / "fonts").mkdir(parents=True)
(plugin_dir / "manifest.json").write_text(json.dumps({"id": "my-plugin"}))
shutil.copy(resolve_asset_path("assets/fonts/PressStart2P-Regular.ttf"),
plugin_dir / "fonts" / "Bundled.ttf")
return plugin_dir
def test_font_resolves_under_the_given_plugin_dir(self, fm, tmp_path):
plugin_dir = self._plugin_with_font(tmp_path / "plugin-repos")
assert fm.register_plugin_fonts("my-plugin", self.MANIFEST, plugin_dir=plugin_dir)
assert fm.font_catalog["my-plugin::bundled"] == str(plugin_dir / "fonts" / "Bundled.ttf")
font = fm.resolve_font("x.y", "bundled", 8, plugin_id="my-plugin")
assert isinstance(font, ImageFont.FreeTypeFont)
def test_without_a_plugin_dir_the_configured_directory_is_searched(self, tmp_path):
plugins_root = tmp_path / "installed"
plugin_dir = self._plugin_with_font(plugins_root, name="ledmatrix-my-plugin")
fm = FontManager({"plugin_system": {"plugins_directory": str(plugins_root)}})
assert fm.register_plugin_fonts("my-plugin", self.MANIFEST)
assert fm.font_catalog["my-plugin::bundled"] == str(plugin_dir / "fonts" / "Bundled.ttf")
+20
View File
@@ -133,6 +133,26 @@ class TestAssetPathsIgnoreTheWorkingDirectory:
rel = f"assets/fonts/{FOUR_BY_SIX}" rel = f"assets/fonts/{FOUR_BY_SIX}"
assert FontManager._resolve_asset_path(rel) == resolve_asset_path(rel) assert FontManager._resolve_asset_path(rel) == resolve_asset_path(rel)
def test_font_overrides_file_lives_in_the_install_config(self, tmp_path, monkeypatch):
from src.font_manager import FontManager
monkeypatch.chdir(tmp_path)
fm = FontManager({})
assert fm.font_overrides_file == str(PROJECT_ROOT / "config" / "font_overrides.json")
def test_logo_placeholder_draws_with_the_bundled_font(self, tmp_path, monkeypatch):
import src.logo_downloader as logo_downloader
from src.logo_downloader import LogoDownloader
loaded = []
def spy(font, size, **kwargs):
loaded.append(font)
return load_truetype(font, size, **kwargs)
monkeypatch.chdir(tmp_path)
monkeypatch.setattr(logo_downloader, "load_truetype", spy)
assert LogoDownloader().create_placeholder_logo("AB", str(tmp_path))
assert loaded == [str(PROJECT_ROOT / "assets" / "fonts" / PRESS_START)]
class TestTheHarnessForkAgreesWithTheCore: class TestTheHarnessForkAgreesWithTheCore:
"""The divergence that let the wrong rendering be blessed as golden. """The divergence that let the wrong rendering be blessed as golden.
+22
View File
@@ -78,3 +78,25 @@ class TestBackgroundDataServiceHeaders:
assert 'yourusername' not in str(headers) assert 'yourusername' not in str(headers)
finally: finally:
service.shutdown(wait=False) service.shutdown(wait=False)
class TestResolverHeaders:
def test_dynamic_team_resolver_sends_the_user_agent(self):
from unittest.mock import patch
from src.dynamic_team_resolver import DynamicTeamResolver
DynamicTeamResolver._rankings_cache = {}
DynamicTeamResolver._cache_timestamp = 0
try:
with patch('src.dynamic_team_resolver.requests.get',
side_effect=RuntimeError("stop")) as get:
DynamicTeamResolver().resolve_teams(["AP_TOP_5"])
assert get.call_args.kwargs['headers']['User-Agent'] == USER_AGENT
finally:
DynamicTeamResolver._rankings_cache = {}
DynamicTeamResolver._cache_timestamp = 0
def test_odds_manager_uses_the_shared_headers(self):
from src.base_odds_manager import BaseOddsManager
headers = BaseOddsManager(MagicMock()).session.headers
for name, value in DEFAULT_HTTP_HEADERS.items():
assert headers[name] == value
+27
View File
@@ -357,6 +357,33 @@ class TestRefreshPlaceholderTimestamp:
assert refresh_placeholder_timestamp(tmp_path / "nope.png") is False assert refresh_placeholder_timestamp(tmp_path / "nope.png") is False
class TestFailurePaths:
def test_a_team_without_logos_is_a_failed_download(self, tmp_path):
downloader = LogoDownloader()
with patch.object(downloader, "fetch_single_team",
return_value={"team": {"logos": []}}):
assert downloader.download_missing_logo_for_team(
"nfl", "1", "XYZ", tmp_path / "XYZ.png") is False
def test_placeholder_is_written_where_the_caller_looks(self, tmp_path):
"""A path that is not <normalized abbreviation>.png (the plugin's own
file naming, or an abbreviation normalize_abbreviation rewrites)
still ends up holding the placeholder, so True means it exists."""
logo_path = tmp_path / "TA&M.png"
with patch.object(LogoDownloader, "download_logo", return_value=False):
assert download_missing_logo(
"ncaa_fb", "245", "TA&M", logo_path,
logo_url="http://example/tamu.png") is True
assert is_placeholder_logo(logo_path)
assert not (tmp_path / "TAANDM.png").exists()
def test_placeholder_uses_the_placeholder_geometry(self, tmp_path):
assert LogoDownloader().create_placeholder_logo("AB", str(tmp_path))
with Image.open(tmp_path / "AB.png") as img:
assert img.size == PLACEHOLDER_SIZE
assert img.convert("RGBA").getpixel((0, 0)) == PLACEHOLDER_BG
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------
# download_logo: the download the scoreboard plugins actually use # download_logo: the download the scoreboard plugins actually use
# #
+25
View File
@@ -294,6 +294,31 @@ class TestValidateConfigFailure:
assert result is False assert result is False
class TestPluginFontRegistration:
"""A manifest's fonts block is registered against the directory the
plugin was loaded from, which plugin:// font sources are relative to."""
def test_plugin_dir_is_passed_to_the_font_manager(self, temp_plugin_dir, mock_managers):
plugin_dir = temp_plugin_dir / "test-plugin"
plugin_dir.mkdir()
fonts = {"fonts": [{"family": "f", "source": "plugin://f.ttf"}]}
manifest = {"id": "test-plugin", "name": "Test Plugin",
"entry_point": "manager.py", "class_name": "TestPlugin",
"fonts": fonts}
with patch('src.common.permission_utils.ensure_directory_permissions'):
manager = PluginManager(plugins_dir=str(temp_plugin_dir), **mock_managers)
manager.plugin_manifests["test-plugin"] = manifest
with patch.object(manager.plugin_loader, 'load_plugin',
return_value=(MagicMock(), MagicMock())):
with patch.object(manager.plugin_loader, 'find_plugin_directory',
return_value=plugin_dir):
manager.load_plugin("test-plugin")
mock_managers["font_manager"].register_plugin_fonts.assert_called_once_with(
"test-plugin", fonts, plugin_dir=plugin_dir)
class TestPluginStateOnFailure: class TestPluginStateOnFailure:
"""Test that plugin state is correctly set on various failures.""" """Test that plugin state is correctly set on various failures."""
+9
View File
@@ -63,6 +63,15 @@ class TestValidateConfig:
assert "Missing required configuration key: display" in errors assert "Missing required configuration key: display" in errors
assert "Missing required configuration key: timezone" in errors assert "Missing required configuration key: timezone" in errors
@pytest.mark.parametrize("config,expected", [
({'timezone': 'UTC'}, "Missing required configuration key: display"),
({'display': {}, 'timezone': 'UTC'}, "Display configuration is empty"),
])
def test_a_missing_display_section_is_reported_once(self, good_cache, config, expected):
validator = StartupValidator(make_config_manager(config))
_, errors, _ = validator.validate_all()
assert errors == [expected]
def test_config_error_does_not_propagate(self, good_cache): def test_config_error_does_not_propagate(self, good_cache):
mgr = make_config_manager(GOOD_CONFIG) mgr = make_config_manager(GOOD_CONFIG)
mgr.load_config.side_effect = ConfigError("bad json") mgr.load_config.side_effect = ConfigError("bad json")
+9
View File
@@ -38,6 +38,7 @@ import numpy as np
import pytest import pytest
from PIL import Image from PIL import Image
from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401
from src.common import sync_manager from src.common import sync_manager
from src.common.sync_manager import ( from src.common.sync_manager import (
DisplaySyncManager, DisplaySyncManager,
@@ -878,6 +879,14 @@ class TestWriteStatusFile:
mgr.write_status_file() # must not raise mgr.write_status_file() # must not raise
assert mgr.logger.debug.called assert mgr.logger.debug.called
def test_web_status_endpoint_reads_the_file_that_was_written(self, api_v3_client):
"""GET /sync/status reads STATUS_FILE, which lives under
tempfile.gettempdir() -- not always /tmp."""
mgr = make_manager(role=SyncRole.LEADER)
mgr.write_status_file()
response = api_v3_client.get("/api/v3/sync/status")
assert response.get_json()["data"]["role"] == "leader"
class TestStop: class TestStop:
def _stub_with_sockets(self): def _stub_with_sockets(self):
+18 -4
View File
@@ -24,10 +24,13 @@ class TestTextHelper:
assert th.font_dir == tmp_path assert th.font_dir == tmp_path
assert th._font_cache == {} assert th._font_cache == {}
def test_init_default_font_dir(self): def test_init_default_font_dir(self, tmp_path, monkeypatch):
"""Test TextHelper initialization with default font directory.""" """The default is the install's assets/fonts, not a cwd-relative path."""
from pathlib import Path
monkeypatch.chdir(tmp_path)
th = TextHelper() th = TextHelper()
assert th.font_dir == pytest.importorskip("pathlib").Path("assets/fonts") assert th.font_dir == Path(__file__).resolve().parents[1] / "assets" / "fonts"
assert isinstance(th.load_fonts()["score"], ImageFont.FreeTypeFont)
@patch('PIL.ImageFont.truetype') @patch('PIL.ImageFont.truetype')
@patch('PIL.ImageFont.load_default') @patch('PIL.ImageFont.load_default')
@@ -123,6 +126,17 @@ class TestTextHelper:
def test_get_default_font_config(self, text_helper): def test_get_default_font_config(self, text_helper):
"""Test getting default font configuration.""" """Test getting default font configuration."""
config = text_helper._get_default_font_config() config = text_helper._get_default_font_config()
assert isinstance(config, dict) assert isinstance(config, dict)
assert len(config) > 0 assert len(config) > 0
def test_each_font_file_and_size_is_loaded_once(self):
th = TextHelper()
first = th.load_fonts()
second = th.load_fonts()
# Six names, three (file, size) pairs: PressStart2P at 10 and 8, 4x6 at 6.
assert first["score"] is second["score"] is first["rank"]
assert th.get_font_cache_stats()["cached_fonts"] == 3
th.clear_font_cache()
assert th.get_font_cache_stats()["cached_fonts"] == 0
assert th.load_fonts()["score"] is not first["score"]
+94
View File
@@ -369,6 +369,26 @@ def test_default_config_has_no_saved_networks(tmp_path: Path) -> None:
assert "saved_networks" not in json.loads(config_path.read_text()) assert "saved_networks" not in json.loads(config_path.read_text())
@pytest.mark.unit
def test_save_config_reports_a_failed_write(manager: WiFiManager, tmp_path: Path) -> None:
# A directory where the file should be: every write to it fails, as one
# to a root-owned wifi_config.json does for the web user.
blocked = tmp_path / "blocked.json"
blocked.mkdir()
manager.config_path = blocked
assert manager._save_config() is False
assert list(tmp_path.glob(".blocked.json.tmp.*")) == []
@pytest.mark.unit
def test_save_config_round_trips(manager: WiFiManager) -> None:
manager.config["auto_enable_ap_mode"] = False
assert manager._save_config() is True
assert json.loads(manager.config_path.read_text())["auto_enable_ap_mode"] is False
@pytest.mark.unit @pytest.mark.unit
def test_connecting_does_not_store_the_password(manager: WiFiManager) -> None: def test_connecting_does_not_store_the_password(manager: WiFiManager) -> None:
commands = [] commands = []
@@ -389,3 +409,77 @@ def test_connecting_does_not_store_the_password(manager: WiFiManager) -> None:
"the new-connection path was not reached" "the new-connection path was not reached"
assert "hunter22" not in json.dumps(manager.config) assert "hunter22" not in json.dumps(manager.config)
assert "hunter22" not in manager.config_path.read_text() assert "hunter22" not in manager.config_path.read_text()
# ---------------------------------------------------------------------------
# 7. Disconnect takes the saved profile down
# ---------------------------------------------------------------------------
def _profile_nmcli(profiles: dict):
"""A fake subprocess.run for `nmcli connection show` over ``profiles``
(profile name -> SSID), recording every command it is given."""
commands = []
def fake_run(cmd, *args, **kwargs):
commands.append(cmd)
if cmd == ["nmcli", "-t", "-f", "NAME,TYPE", "connection", "show"]:
lines = [name.replace(":", "\\:") + ":802-11-wireless" for name in profiles]
lines.append("Wired connection 1:802-3-ethernet")
return _ok(stdout="\n".join(lines) + "\n")
if cmd[:4] == ["nmcli", "-g", "802-11-wireless.ssid", "connection"]:
return _ok(stdout=profiles.get(cmd[-1], "") + "\n")
if cmd[:3] == ["nmcli", "connection", "show"]:
return _ok() if cmd[3] in profiles else _fail()
# nmcli rejects 802-11-wireless.ssid as a `connection show -f` column.
if "802-11-wireless.ssid" in cmd:
return _fail(stderr="Error: invalid field '802-11-wireless.ssid'")
return _ok()
return fake_run, commands
@pytest.mark.unit
def test_find_profile_for_ssid_matches_by_ssid_not_name(manager: WiFiManager) -> None:
fake_run, _ = _profile_nmcli({"home: upstairs": "HomeNet", "Office": "OfficeNet"})
with patch("src.wifi_manager.subprocess.run", side_effect=fake_run):
assert manager._find_profile_for_ssid("HomeNet") == "home: upstairs"
assert manager._find_profile_for_ssid("OfficeNet") == "Office"
assert manager._find_profile_for_ssid("Elsewhere") is None
@pytest.mark.unit
def test_disconnect_takes_the_profile_down(manager: WiFiManager) -> None:
from src.wifi_manager import WiFiStatus
fake_run, commands = _profile_nmcli({"Home profile": "HomeNet"})
with patch("src.wifi_manager.subprocess.run", side_effect=fake_run), \
patch("src.wifi_manager.time.sleep"), \
patch.object(manager, "get_wifi_status",
return_value=WiFiStatus(connected=True, ssid="HomeNet")):
ok, _ = manager.disconnect_from_network(skip_ap_check=True)
assert ok
assert ["nmcli", "connection", "down", "Home profile"] in commands
assert ["nmcli", "device", "disconnect", "wlan0"] in commands
# ---------------------------------------------------------------------------
# 8. The nmcli Wi-Fi list parser both scan paths share
# ---------------------------------------------------------------------------
@pytest.mark.unit
def test_nmcli_wifi_list_parsing() -> None:
out = (
"HomeNet:40:WPA2:2437 MHz\n"
"Cafe:80::5180 MHz\n"
"HomeNet:90:WPA2:5180 MHz\n" # duplicate SSID: first line wins
":70:WPA2:2412 MHz\n" # hidden network
"Broken:notanumber:WPA2:2412 MHz\n"
"Modern:60:WPA3 SAE:5745 MHz\n"
)
networks = WiFiManager._parse_nmcli_wifi_list(out)
assert [(n.ssid, n.signal, n.security, n.frequency) for n in networks] == [
("Cafe", 80, "open", 5180.0),
("Modern", 60, "wpa3", 5745.0),
("HomeNet", 40, "wpa2", 2437.0),
]
+6 -5
View File
@@ -14,6 +14,7 @@ from web_interface.blueprints.api_v3 import (
subprocess, success_response, tempfile, subprocess, success_response, tempfile,
) )
from src.common.path_safety import safe_path_component from src.common.path_safety import safe_path_component
from src.common import sync_manager as _sync
from src import error_aggregator as _errors from src import error_aggregator as _errors
import web_interface.blueprints.api_v3 as _pkg import web_interface.blueprints.api_v3 as _pkg
# Read through the module rather than bound by value: tests patch these # Read through the module rather than bound by value: tests patch these
@@ -190,21 +191,21 @@ def get_logs():
@api_v3.route('/sync/status', methods=['GET']) @api_v3.route('/sync/status', methods=['GET'])
def get_sync_status(): def get_sync_status():
"""Return live multi-display sync status written by the display process.""" """Return live multi-display sync status written by the display process."""
import os as _os # The display process writes this file; read it where it is written.
status_file = "/tmp/led_matrix_sync_status.json" status_file = _sync.STATUS_FILE
# Also surface config so the UI can show the configured role even before # Also surface config so the UI can show the configured role even before
# the display process has written a status file. # the display process has written a status file.
cfg_role = "standalone" cfg_role = "standalone"
cfg_port = 5765 cfg_port = _sync.SYNC_PORT
if api_v3.config_manager: if api_v3.config_manager:
try: try:
cfg = api_v3.config_manager.load_config().get("sync", {}) cfg = api_v3.config_manager.load_config().get("sync", {})
cfg_role = cfg.get("role", "standalone") cfg_role = cfg.get("role", "standalone")
cfg_port = int(cfg.get("port", 5765)) cfg_port = int(cfg.get("port", _sync.SYNC_PORT))
except Exception: except Exception:
pass pass
if _os.path.exists(status_file): if os.path.exists(status_file):
try: try:
with open(status_file) as f: with open(status_file) as f:
live = json.load(f) live = json.load(f)
+6 -1
View File
@@ -372,7 +372,12 @@ def set_auto_enable_ap_mode():
wifi_manager = WiFiManager() wifi_manager = WiFiManager()
wifi_manager.config["auto_enable_ap_mode"] = auto_enable wifi_manager.config["auto_enable_ap_mode"] = auto_enable
wifi_manager._save_config() if not wifi_manager._save_config():
return jsonify({
'status': 'error',
'message': (f'Could not save the setting to {wifi_manager.config_path}; '
'check that the web interface user can write it.'),
}), 500
return jsonify({ return jsonify({
'status': 'success', 'status': 'success',