mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-01 16:58:06 +00:00
8a0854472e7e5d090a2f4fd562efda745f7c0bd0
1427
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Claude
|
8a0854472e |
Tag plugin logs structurally and surface the active plugin in System Logs
- get_logger() now returns a PluginLoggerAdapter when given a plugin_id,
so every plugin log call is stamped with plugin_id automatically instead
of only calls that explicitly passed extra={'plugin_id': ...}. This makes
the "[Plugin: x]" prefix reliable in the journalctl-backed log stream.
- display_controller publishes the currently active mode/plugin to the
shared cache whenever it changes, exposed via a new
GET /api/v3/display/current-status endpoint.
- System Logs page: adds a "Now showing" banner backed by that endpoint, a
plugin filter dropdown (populated from parsed log lines), a plugin badge
per log entry, and fixes log parsing to handle the short-iso timestamp
format journalctl actually returns (the old regex only matched syslog
timestamps, so level/plugin extraction silently never ran).
|
||
|
|
4abcd0e4f9 |
Fix PermissionError reading config_secrets.json in web interface (#416)
ledmatrix.service (main display) runs as root while ledmatrix-web.service runs as the non-root install user (install_web_service.sh). Both config_manager.py and config_manager_atomic.py only chmod'd config_secrets.json to 0o640 without ever fixing its group, so a file written by the root service ended up group-owned by root and unreadable by the web user, crashing the settings page with a raw PermissionError. Add ensure_shared_group_ownership() to chgrp secrets/config files (best effort, root-only) to the project directory's owning group whenever they are created or saved, and self-heal existing files on load. Also make get_raw_file_content() tolerate an unreadable secrets file the same way load_config() already does, degrading to empty secrets instead of a 500. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
2a1c47fa76 |
chore: remove dead modules and unused dependencies (~1,180 LOC) (#412)
* chore: remove dead modules and unused dependencies (~1,180 LOC) Deletions, each re-verified with a fresh repo-wide grep (core, web, scripts, docs, plugin monorepo) immediately before removal: Modules with zero live importers: - src/background_cache_mixin.py + src/generic_cache_mixin.py (134+150 LOC — referenced only by each other) - src/font_test_manager.py (134 LOC) - src/image_utils.py (22 LOC, self-documented deprecated) - src/layout_manager.py (408 LOC — only its own test imported it) + test/test_layout_manager.py - src/common/basketball_plugin_example.py (328 LOC sample) requirements.txt entries with zero importers in core (pre-plugin-era manager deps): icalevents, geopy, timezonefinder, unidecode. Plus the google-auth trio (google-auth-oauthlib, google-auth-httplib2, google-api-python-client): their only importer is the calendar PLUGIN, which declares all three in its own requirements.txt (verified in the monorepo and on an installed copy) — the plugin dependency installer owns them. Existing venvs are unaffected (removal doesn't uninstall); fresh installs get them when calendar is installed. Two stale references cleaned (a comment in test_pillow_compat.py, a directory listing in HOW_TO_RUN_TESTS.md). Full suite green except the two documented pre-existing failures (circuit_breaker mock drift, fixed in #400; clock-simple 64x32 overflow, pre-dates this series); all core entry modules verified importing cleanly under the emulator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix: remove content accidentally bundled into the dead-code-removal commit The previous commit's git add/commit swept in a lot of unrelated, unreviewed work alongside the intended dead-code deletions: a new Plugin Composer web UI, new security-audit/CI tooling, a new march-madness plugin, and 23 local-development-only symlinks under plugin-repos/ (per scripts/setup_plugin_repos.py's own docstring, these are meant to be generated locally, never committed -- .gitignore has no entry for them, which is how they slipped in). Removed here, split into their own PRs instead (except plugin-repos/* symlinks and march-madness/ncaa_logos, which are dropped rather than carried forward -- see PR discussion): - web_interface/blueprints/composer.py + composer-app.js + composer-canvas.js + composer.html + manager.py.j2 - scripts/prove_security.py, audit_plugins.py, generate_report.py - .github/workflows/security-audit.yml, .github/workflows/tests.yml, bandit.yaml - All plugin-repos/* symlinks (local dev artifacts, not meant to be committed at all) - plugin-repos/march-madness/* and the 4 new assets/sports/ncaa_logos/* PNGs it needed (left out of every split PR pending a decision on whether march-madness belongs in the core repo or the plugin monorepo) This PR now contains only what its title describes: the dead-code removal from the previous commit, untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
14a59c863c |
perf(wifi): one status fetch per monitor tick instead of three (#411)
* perf(wifi): one status fetch per monitor tick instead of three The wifi monitor daemon fetched WiFi status + ethernet state before its AP-mode check, the check internally fetched the same state again (with retry), and the daemon fetched a third time afterwards — each fetch is several nmcli subprocess forks, every 30s, forever, even on a perfectly healthy link. check_and_manage_ap_mode's decision logic is extracted to _manage_ap_mode(status, ethernet, ap_active); the new check_and_manage_ap_mode_with_state() runs the single (retrying) fetch battery and returns (changed, status, ethernet, ap_active_after) — the post-state is derivable because state only ever flips via one enable or one disable. The original bool-returning method delegates, so existing callers are untouched. The daemon's dead pre-fetch is removed and its post-check reads use the returned state; the AP-enable retry semantics (_get_wifi_status_with_retry) are preserved exactly. ~10-15 forks/30s drops to ~4-6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(wifi): add missing type hints on AP-mode state helpers, fix unused-var lint in tests - check_and_manage_ap_mode_with_state now declares its Tuple[bool, WiFiStatus, bool, bool] return type instead of being untyped. - _manage_ap_mode's status/ethernet_connected/ap_active parameters are now typed (WiFiStatus, bool, bool), matching its existing -> bool return hint. - test_wifi_check_state.py: rename unused unpacked variables (status/ ethernet/ap_after) to underscore-prefixed names at the three call sites that don't use them, leaving the used ones untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
bff13129c4 |
perf(config): mtime-signature fast path for load_config (#410)
load_config re-read and re-parsed config.json, the secrets file, AND the template (running the recursive migration diff) on every call — with ~30 call sites in web request handlers, some hit 2-3x per request. Fast path: stat all three files (mtime_ns + size); when unchanged since the last successful load, return the already-parsed self.config (same aliasing semantics as before). The signature is taken AFTER load + migration so a migration write-back doesn't retrigger, and both save paths refresh it. Cross-process freshness is preserved by construction: a save from the other process bumps the file mtime, so the next load here re-reads — verified by a dedicated test. Same-second edits are caught by mtime_ns plus a size check. Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
6499794c12 |
fix(testing): add MockCacheManager.get_cached_data_with_strategy/save_cache (#409)
* fix(testing): add MockCacheManager.get_cached_data_with_strategy/save_cache ledmatrix-leaderboard's data_fetcher.py calls these two real-CacheManager methods (src/cache_manager.py:313,817), but MockCacheManager had neither -- update() always hit an AttributeError, caught by a broad except and logged, so the harness rendered an empty-but-green leaderboard on every test run without ever exercising real standings data. Both mocks delegate to the existing get()/set() -- a mock doesn't need the real strategy's per-data-type max_age/market-hours timing, plugins under test just need the methods to exist and round-trip whatever was cached. * fix(testing): clear get_cached_data_with_strategy_calls in MockCacheManager.reset() reset() cleared get_calls/set_calls/delete_calls but not the newer get_cached_data_with_strategy_calls tracker, so a reused mock (e.g. across test cases sharing a fixture) retained stale strategy-call records after reset(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ChuckandGitHub
|
3d347a368a |
fix(testing): stop plugin enabled:false schema defaults from silently disabling harness tests (#408)
check_plugin.py, render_plugin.py, and the pytest plugin matrix each built
config as {"enabled": True} then merged in config_schema.json's defaults on
top, letting a plugin's own enabled:false default (a reasonable choice for
a seasonal/opt-in plugin -- 15 of 23 real plugins ship one) silently win.
Every harness/CI render of those plugins was testing "disabled, do
nothing" rather than real behavior.
Extract build_full_config() into testing/loading.py (already the shared
home for plugin-discovery/config-default logic) and use it from all three
call sites: schema defaults, then a forced enabled=True, then harness.json's
config, then the caller's explicit config -- so a test can still
deliberately disable a plugin on purpose, it just can't happen by accident
via the plugin's own shipped schema default anymore.
|
||
|
|
0aca40cf3a |
perf(plugins): run scheduled updates off the render thread (#407)
* perf(plugins): run scheduled updates off the render thread plugin update() executed inline in the render loop — execute_update's internal thread.join(timeout=30) blocked it, so one slow plugin HTTP fetch froze scrolling for the whole fetch (up to 30s; DNS-retry storms made this a regular occurrence on flaky networks). Scheduling stays on the render thread and keeps every existing gate (enabled, circuit breaker, can_execute, interval); due updates are now enqueued to a single background worker (serialized — same one-at-a-time execution as before, no thundering herd). RUNNING is set at enqueue so can_execute blocks re-entry alongside the pending-set dedup. Per-plugin locks make the old implicit update/display no-overlap guarantee explicit: the worker holds the plugin's lock through its update; the display side try-locks and, when the plugin is mid-update, holds the last frame for that iteration — reported as success so a mid-update skip never advances the rotation. Unlike before, the guarantee now also holds across the post-timeout window (previously the lingering update thread overlapped display()). Deadlock-free by construction: the worker takes one lock; display never blocks. Timeout semantics unchanged (lingering daemon thread documented). Kill switch: plugin_system.synchronous_updates: true restores the inline path. 8 new concurrency tests (non-blocking scheduler, overlap assertion under a hammering display loop, lock release on failure/timeout paths, dedup, kill switch); 4-min devpi soak clean (updates completing, rotation advancing, no stuck RUNNING states). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(plugins): fix skipped-frame health/force_change tracking, config validation, and lock-lifetime gaps in async updates Addresses PR #407 review findings: - display_controller: only clear force_change / record health success when display() actually ran this frame, not when the frame was skipped because the plugin's lock was busy (a skip must preserve a pending mode-switch force_clear). - plugin_manager: replace bool() coercion of synchronous_updates with explicit isinstance validation of plugin_system/synchronous_updates, failing safe to synchronous mode (with a logged reason) on malformed config instead of silently defaulting to async. - plugin_manager: _update_worker_loop now acquires the plugin lock before looking up its instance and re-checks under the lock, so an unloaded plugin's lifecycle state is never resurrected to ENABLED. - plugin_manager + display_controller: move lock ownership (and, for updates, RUNNING/pending lifecycle bookkeeping) into the actual update()/ display() call itself rather than the timeout-wrapped caller, so the lock stays held for the real operation's duration even after PluginExecutor's own join(timeout) elapses and a lingering daemon thread keeps running in the background. - DisplayController.cleanup() now stops the update worker before tearing down display/cache resources; stop_update_worker() logs when the join times out instead of failing silently. - test_async_plugin_updates: rewrite test_unloaded_while_queued_is_harmless to exercise the public unload_plugin() lifecycle (via a deterministic blocker) instead of deleting pm.plugins directly, and add a regression test proving the plugin lock stays held through PluginExecutor's own timeout while the real update() call is still running. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
9837315308 |
perf(display): dirty tracking in update_display + plugin FPS declaration (#406)
* perf(display): dirty tracking in update_display + plugin FPS declaration update_display now skips SetImage+SwapOnVSync when the frame is byte-identical to the last pushed one (adler32 digest) AND brightness is unchanged — brightness is part of the digest, and set_brightness additionally resets it, so a dim-schedule change can never be skipped. clear() resets the digest (it writes to the matrix directly). Skipping a swap is hardware-safe: the panel refreshes the current frame from the driver's own thread; swaps only change content. Kill switch: display.dirty_tracking: false restores always-push. display_controller's high-FPS decision gains a precedence step: a plugin exposing needs_high_fps is honored first (so static-image can declare False for still PNGs and stop burning a 125fps loop on them); static-image without the attribute keeps its historical forced high-FPS (GIF back-compat); scrolling logic is otherwise unchanged. Verified with 7 tests against the real DisplayManager on RGBMatrixEmulator (identical-frame skip, pixel-change push, clear and brightness invalidation, snapshot-through-skip, kill switch) plus the 202-test display/controller/vegas suites, and a clean devpi deploy. Audit: every SetImage/SwapOnVSync/Clear/brightness call site is inside display_manager — no external writer can bypass the digest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(display): serialize update_display, narrow brightness exception, log fixes CodeRabbit review on #406, verified against current code: - update_display() can genuinely be called from background threads (some sports base classes call it directly from inside update() for an immediate "live" refresh), not just the render loop — confirmed via the existing follower-mode gating wrapper in display_controller.py, which exists specifically because "background plugin threads" can reach it. Without a lock, two callers could both pass the digest check before either writes _last_pushed_digest back, causing a redundant push, or interleave the offscreen/current canvas swap. Added self._update_lock (RLock, in case of re-entrant callers) around the full method body so every call site is automatically covered — no caller changes needed. (No prior lock existed to reuse on DisplayManager; this adds one.) - Narrowed the brightness-read exception handler to AttributeError, matching the established pattern in get_brightness()/set_brightness() — a getattr() with a default already swallows AttributeError, so the only case this guards is the property getter itself raising, and the established pattern treats that as an expected, specific failure mode rather than something to blanket-catch. - FPS-check debug log now includes the plugin_id already in scope (previously only active_mode) and a "[DisplayController]" prefix for grep-ability, matching the sibling log two lines below it. - test_display_dirty_tracking.py: dm fixture and test_config_flag_wires_through now reset the DisplayManager singleton on teardown, matching the pattern test_display_manager.py already uses elsewhere in the same file family. - test_snapshot_still_written_on_skip previously only exercised the non-skip (push) path despite its name; now performs a second update that meets the skip conditions (identical frame) and asserts the snapshot is still written even though the panel push itself is skipped. All 7 dirty-tracking tests pass, plus the full display_manager/ display_controller/vegas suite (140 passed). Full repo suite has only the 5 known pre-existing failures (double-sided config x2, state_reconciliation x2, and test_circuit_breaker's conftest.py mock signature drift — the latter fixed in #400, which this branch's base predates). --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
c1fa5094be |
fix(store): plugin updates keep the old install until the new one succeeds (#405)
* fix(store): plugin updates keep the old install until the new one succeeds Both reinstall paths in update_plugin — the monorepo-migration remote switch AND the routine archive update every store user hits — deleted the installed plugin directory BEFORE downloading its replacement. A mid-update failure (bad network, registry error) permanently destroyed the plugin. Seen in the field: a Pi with broken DNS lost 12 plugins in one update pass during the monorepo migration. New _reinstall_with_rollback: rename the old install aside (using the '.standalone-backup-' name pattern plugin discovery already excludes), run install_plugin, remove the aside on success — restore it on ANY failure, clearing partial-download debris first. A stale aside from a previous crash is cleared before starting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(store): serialize concurrent updates per plugin, check cleanup results CodeRabbit review on #405 flagged two things in _reinstall_with_rollback, both verified against current code: - Real race: the web UI runs Flask with threaded=True and there's a single update route, so two overlapping requests for the same plugin_id (double-click, two tabs) can interleave. The loser could rename the winner's in-progress install aside mid-download, deleting its own rollback safety net — worse than the bug this function exists to fix. Added a lazy per-plugin_id lock dict (mirrors the plugin_manager per-plugin lock pattern) held for the whole function. - _safe_remove_directory's return value was ignored at both call sites. Stale-aside cleanup failure now aborts cleanly instead of falling through to a rename that would fail anyway with a less useful error; post-success backup-removal failure now logs instead of failing silently (still returns True — the update itself succeeded, and the next update self-heals the leftover aside). Left the third nitpick (test_stale_aside_from_previous_crash_is_cleared) addressed by asserting the stale dir is actually gone and that install_plugin was reached, rather than just the end-to-end result. Added a concurrency regression test asserting install_plugin never runs for the same plugin_id while another call is in flight. --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
4d49b0f892 |
perf(display): snapshot mirror — viewer gating, digest skip, keepalive (#404)
The display service PNG-encoded its frame to /tmp/led_matrix_preview.png at 5 fps, 24/7 — identical frames, no viewers, per-call imports and a chmod every write. On the devpi baseline the display service idles at ~92% CPU; this was one of its biggest fixed costs. - New pure policy (src/common/snapshot_policy.py, unit-tested off-Pi): WRITE changed frames at full rate only while a viewer is watching, at a 30s idle cadence otherwise; NEVER re-encode unchanged frames — bump mtime (os.utime) every 20s instead, keeping the health check's snapshot-age liveness proxy (60s threshold in api_v3) green. Cross- referencing comments guard the two constants. - Viewer detection: the web SSE display broadcaster (which only runs while browsers are subscribed) touches /tmp/led_matrix_preview_viewer each loop; the display service stats it at most 1/s. On viewer arrival the write clock resets so the first frame lands within ~1s. - Hoisted the per-call pathlib/permission_utils imports; directory permissions ensured once instead of every frame. Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
efe76d3add |
perf: hot-path micro fixes in the render loop (#403)
* perf: hot-path micro fixes in the render loop - _check_wifi_status_message stat'd the status file on every render iteration (60+ fps) for a message whose lifetime is seconds; throttle the check to 1 Hz with a cached result. - Demote the per-iteration "Display active, processing mode" INFO to DEBUG and convert the remaining eager f-string logs to lazy % args — the devpi baseline showed ~9 journald lines/sec, which is both noise and SD-card wear. - Vegas cycle-end blank frame: hoist the inline PIL import and reuse a preallocated buffer instead of allocating per cycle wrap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix: initialise wifi-status throttle state in __init__ Codacy (pylint access-member-before-definition) on #403: the throttled early-return read _wifi_status_last_result relying on the non-local invariant that the first call always passes the throttle window and assigns it. Correct at runtime, but fragile — initialise both throttle fields in the constructor and drop the getattr fallback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
273d9962d1 |
fix(cache): stop fsync-hammering the SD card on unchanged data (#402)
DiskCache.set wrote every key as mkstemp -> json.dump(indent=4) -> flush+fsync -> replace -> chmod, on the persistent cache dir — dozens of force-flushed SD writes per minute on an API-heavy install, mostly rewriting identical data every plugin update cycle. - Serialize once, compact (no indent): cache files are machine-read only; indenting multiplied the bytes written. - Skip the disk when the payload for a key is unchanged (adler32 map, per-process); refresh the file mtime instead so records relying on mtime for TTL don't expire early. Self-heals if the file was removed externally (expiry cleanup). - Drop the per-write fsync: os.replace already guarantees readers never see a torn file, and cache data is re-fetchable — the flush bought nothing but card wear. API unchanged; DateTimeEncoder round-trip covered by tests. Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
9e3b5f366e |
fix(core): harden text-measurement caches; surface snapshot failures (#400)
* fix(core): harden text-measurement caches; surface snapshot failures Deep-dive findings, all three latent on every 24/7 install: - font_manager.metrics_cache and display_manager._text_width_cache were unbounded dicts keyed by (text, id(font)). Two problems: keys embed the measured TEXT, so ever-changing strings (a clock, a live score, a ticker) grow them without limit; and id()-keying without holding a reference means a garbage-collected font's id can be recycled by a DIFFERENT font, silently returning wrong widths/metrics (classic plugins create fonts per render, so this is reachable). Both caches are now LRU-bounded (1024) and pin the font in the entry so its id stays valid. metrics_cache also keyed on the text itself instead of hash(text), removing a collision path. - _write_snapshot_if_due logged failures at DEBUG — invisible at the default level. The snapshot's mtime is the web UI's display mirror AND its hardware-liveness proxy, so a quiet failure freezes the mirror and makes health checks lie (seen in the field: a stale root-owned /tmp file froze it for a day). Failures now WARN, rate- limited to once per 5 minutes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * test: sync mock cache manager signature with CacheManager.get test_circuit_breaker has been failing on main: plugin_health passes memory_ttl= to cache_manager.get(), and the conftest mock's signature was never updated — the same component/double drift class as the monitored_update bug (#392). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
6edd80d9f3 |
fix(schedule): stop stray 'days' data from overriding Global schedule (#399)
save_schedule_config never persisted the schedule's 'mode' field, and _check_schedule inferred per-day vs global purely from whether a 'days' dict was present for the current day. Config migration (_merge_template_defaults) re-adds the template's 'schedule.days' (all days disabled by default) whenever it's missing from the user's saved config - which is exactly the case after saving Global mode, since that save path intentionally pops 'days'. The result: a user on Global mode would get their schedule silently reinterpreted as per-day, with today's day disabled, blanking the display. Persist 'mode' on save and have _check_schedule honor it explicitly (mirroring how _check_dim_schedule already does), so a resurrected 'days' dict can't override an explicit Global selection. Falls back to the old inference behavior only when no 'mode' is recorded. Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
1c7a0cef66 |
fix(vegas): lock plugin_last_update snapshot/diff against concurrent mutation (#398)
* fix(vegas): restore live plugin-update refresh dropped by sync refactor Investigating a user report that Vegas scroll mode doesn't update scores or game status. Root cause: PR #299 (Mar 28) added a mechanism so a live score change reached the ticker within a few seconds instead of waiting for a full scroll cycle -- _tick_plugin_updates_for_vegas() diffed plugin_last_update timestamps to detect which plugins got fresh data and called coordinator.mark_plugin_updated() for each, and should_recompose() checked has_pending_updates_for_visible_segments() to trigger an immediate hot-swap. PR #330 (May 14, multi-display wireless sync) refactored both call sites while adding sync support and silently deleted this entire mechanism -- not just gated it behind the new sync-mode deferral it legitimately needed, but removed it outright. The result: VegasModeCoordinator. mark_plugin_updated() and StreamManager.has_pending_updates_for_visible_ segments() have been fully implemented but never called from anywhere since. Vegas mode's only remaining freshness sources are a 5s content cache TTL (fine) and full recompose at cycle boundaries, which depending on min/max_cycle_duration can be minutes away -- so live scores/status can sit stale far longer than a user would expect from a "live" ticker. Fix: - Restored _tick_plugin_updates_for_vegas() in display_controller.py, wired as the Vegas coordinator's update callback in place of the plain _tick_plugin_updates(). Diffs plugin_last_update before/after the tick and calls vegas_coordinator.mark_plugin_updated(plugin_id) for each plugin that actually got new data (rather than returning the list, since the callback interface no longer consumes a return value). - Restored the has_pending_updates_for_visible_segments() check in render_pipeline.should_recompose(), positioned after (not instead of) the sync-mode early return PR #330 added, so standalone installations regain immediate refresh while synced leader/follower pairs correctly keep deferring hot-swaps to cycle boundaries as PR #330 intended. Test plan: - Added test_display_controller_vegas_tick.py and test_vegas_render_pipeline_recompose.py -- neither area had any prior test coverage, which is very likely why this regression went unnoticed for ~2.5 months. - Verified both new test files fail against the pre-fix code (swapped in the current main versions of both files) with exactly the expected errors -- AttributeError for the deleted method, and the recompose assertion returning False instead of True -- then pass against the fix. - Confirmed the sync-mode deferral this restoration must not break still holds: test_sync_active_defers_pending_updates_to_cycle_boundary. - Full related suite (test_vegas_plugin_adapter, test_vegas_config, test_display_controller_plugin_toggle, test_display_controller_ optimizations, test_plugin_system): 108 passed, 1 pre-existing failure unrelated to this change (test_circuit_breaker, stale mock signature). - Full CI plugin-safety suite (test_harness, test_visual_rendering, test_plugin_matrix): 52 passed, 2 pre-existing skips. * fix(vegas): lock plugin_last_update snapshot/diff against concurrent mutation _tick_plugin_updates_for_vegas() snapshotted and later re-iterated plugin_manager.plugin_last_update from the Vegas background update-tick thread while the main render loop (or other callers) could mutate the same dict concurrently — a real race (unprotected dict iteration/mutation across threads), not just a style nit. Move the snapshot/update/diff into a new locked PluginManager.run_scheduled_updates_with_changes() so all reads and mutations of plugin_last_update happen under one lock, and update DisplayController to use it. The lock is only held around the dict accesses, not the update pass itself, so slow plugin update() calls don't serialize against other callers. Also add a regression test covering that the Vegas coordinator is wired to the Vegas-aware tick callback rather than the plain one. Skipped as not worth the change: - Narrowing the broad `except Exception` around vc.mark_plugin_updated(plugin_id) to specific types: it's a deliberate per-plugin isolation boundary (matches the same pattern used elsewhere in this file for plugin/coordinator calls) and there's no documented, stable set of exceptions that call can raise to narrow to. - Adding an inactive-DisplaySyncManager test to test_vegas_render_pipeline_recompose.py: verified VegasModeCoordinator.set_sync_manager() already normalizes a SyncRole.STANDALONE manager to None before handing it to the render pipeline (src/vegas_mode/coordinator.py:152-156), so should_recompose()'s `is not None` check is correct in practice; the suggested case is already covered by that normalization. --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
ChuckandGitHub
|
6052a60d22 |
fix(vegas): restore live plugin-update refresh dropped by sync refactor (#395)
Investigating a user report that Vegas scroll mode doesn't update scores or game status. Root cause: PR #299 (Mar 28) added a mechanism so a live score change reached the ticker within a few seconds instead of waiting for a full scroll cycle -- _tick_plugin_updates_for_vegas() diffed plugin_last_update timestamps to detect which plugins got fresh data and called coordinator.mark_plugin_updated() for each, and should_recompose() checked has_pending_updates_for_visible_segments() to trigger an immediate hot-swap. PR #330 (May 14, multi-display wireless sync) refactored both call sites while adding sync support and silently deleted this entire mechanism -- not just gated it behind the new sync-mode deferral it legitimately needed, but removed it outright. The result: VegasModeCoordinator. mark_plugin_updated() and StreamManager.has_pending_updates_for_visible_ segments() have been fully implemented but never called from anywhere since. Vegas mode's only remaining freshness sources are a 5s content cache TTL (fine) and full recompose at cycle boundaries, which depending on min/max_cycle_duration can be minutes away -- so live scores/status can sit stale far longer than a user would expect from a "live" ticker. Fix: - Restored _tick_plugin_updates_for_vegas() in display_controller.py, wired as the Vegas coordinator's update callback in place of the plain _tick_plugin_updates(). Diffs plugin_last_update before/after the tick and calls vegas_coordinator.mark_plugin_updated(plugin_id) for each plugin that actually got new data (rather than returning the list, since the callback interface no longer consumes a return value). - Restored the has_pending_updates_for_visible_segments() check in render_pipeline.should_recompose(), positioned after (not instead of) the sync-mode early return PR #330 added, so standalone installations regain immediate refresh while synced leader/follower pairs correctly keep deferring hot-swaps to cycle boundaries as PR #330 intended. Test plan: - Added test_display_controller_vegas_tick.py and test_vegas_render_pipeline_recompose.py -- neither area had any prior test coverage, which is very likely why this regression went unnoticed for ~2.5 months. - Verified both new test files fail against the pre-fix code (swapped in the current main versions of both files) with exactly the expected errors -- AttributeError for the deleted method, and the recompose assertion returning False instead of True -- then pass against the fix. - Confirmed the sync-mode deferral this restoration must not break still holds: test_sync_active_defers_pending_updates_to_cycle_boundary. - Full related suite (test_vegas_plugin_adapter, test_vegas_config, test_display_controller_plugin_toggle, test_display_controller_ optimizations, test_plugin_system): 108 passed, 1 pre-existing failure unrelated to this change (test_circuit_breaker, stale mock signature). - Full CI plugin-safety suite (test_harness, test_visual_rendering, test_plugin_matrix): 52 passed, 2 pre-existing skips. |
||
|
|
7f7f0d6464 |
feat: adaptive layout system — size-aware regions, crisp font ladders, image fitting (#393)
* feat(layout): adaptive layout & font scaling system for plugins Add src/adaptive_layout.py — opt-in core helpers so plugins render legibly on any panel size without hand-tuned per-display layouts: - Region: integer rect algebra (bands/columns/weighted splits/centering) that partitions space so text bands can't overlap by construction - Font ladders: ordered (family, size) steps known to render crisply (LADDER_GRID: X11 BDFs at native sizes; LADDER_ARCADE: PressStart2P at 8px multiples) — fitting walks the ladder instead of scaling pixel fonts fractionally - LayoutContext: breakpoint tiers, geometry scale vs. a declared design size, and cached fit_text/fit_lines/font_for_rows queries Generalizes the three patterns proven in the field: f1-scoreboard's scale factor, masters-tournament's tiers, baseball-scoreboard's font fallback ladder. Wiring: BasePlugin gains a lazy .layout property and draw_fit(); FontManager gains get_native_bdf_size() and a cache_generation counter; manifest schema gains display.design_size and requires.display_size max_width/max_height; 96x48 joins DEFAULT_TEST_SIZES; the bounds-check harness records negative-coordinate draws; TextHelper's broken measurement helpers are fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): adaptive image fitting + composite region helpers Add src/adaptive_images.py — the image counterpart to fit_text: - fit_image(img, box, mode=contain|cover|fill_height|stretch, crop_to_ink, anchor, resample, upscale) promoting the proven plugin patterns (football's crop-to-ink fill-height logos, masters' cover crop + NEAREST flags, static-image's letterbox). Upscales by default — thumbnail()'s downscale-only behavior is why imagery stays tiny on big panels. - draw_fitted_image() pastes aligned within a Region with alpha mask. - One central Pillow>=9.1 RESAMPLE shim replacing ~15 plugin copies. LayoutContext.fit_image() caches results per (identity, box size, options) with a 64-entry LRU; id()-keyed entries pin the source image. BasePlugin.draw_image() is the one-liner adoption path beside draw_fit. Composites in adaptive_layout.py: Region.offset() (user x/y-offset passthrough), scoreboard_regions() (the two-logos-plus-score card math duplicated across six sports plugins, logo_slot = min(H, W//2)), and media_row() (art-left/text-right). Fix LogoHelper's size-blind cache key (stale sizes on panel change); deprecation note on dead image_utils.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(harness): scale-up fill check, config variants, multi-size dev gallery Quality gates for adaptive layout: - fill_metrics()/check_scale_up() in the safety harness: overflow catches content too big for a panel, but nothing caught content that stays tiny on panels >= 2x the plugin's declared design size. The check measures lit-content extents and warns (or fails, when a plugin opts into "fill_check": "strict" in test/harness.json) below 50% coverage on the doubled axis. Warn-only by default so no existing plugin breaks. - harness.json "variants": extra runs with config overlays and their own golden dirs, so an opt-in mode (e.g. layout_mode: adaptive) is golden- tested beside the classic default. check_plugin.py loops base + variants and labels variant results mode@name. - Dev preview server: GET /api/sizes (harness size sample), POST /api/render-matrix (render at up to 12 sizes in one call), size-preset dropdown, and an "All Sizes" side-by-side gallery in the preview UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(plugins): adaptive-lib discoverability + advisory version compat warning Discoverability: re-export the adaptive layout/image API from src.common (the blessed-helpers package plugin authors already know) — canonical paths stay src.adaptive_layout / src.adaptive_images so nothing breaks. Document it in src/common/README.md and cross-link ADAPTIVE_LAYOUT.md from the developer docs authors actually read (quick reference, API reference, advanced dev, font manager, dev preview, plugin dev guide); ADAPTIVE_LAYOUT.md gains adaptive-images, composite-layouts and preserving-user-customization sections. Compat: PluginLoader now logs one advisory warning (never raises) when a plugin's manifest declares a min LEDMatrix version newer than the running core, checking the min_ledmatrix_version / requires.* / versions[] spellings found in the wild. Guarded against stale core version numbers. src/__init__.py __version__ bumped 1.0.0 -> 3.1.0 to match the latest release tag (v3.1.0) — it had never been updated and the compat check needs a truthful number. NOTE: verify this matches the intended release numbering before the next tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): add measure_font_crispness — verify a ladder rung isn't blurry PIL antialiases TTF outlines by default; a 'pixel-style' font only rasterizes without antialiasing at specific sizes (for PressStart2P: exact multiples of its 8px design grid). A ladder rung at an unverified size silently renders blurry on an LED panel — this exact bug shipped in both text-display's and football-scoreboard's custom TTF ladders (non-8-multiple PressStart2P sizes, and '5by7.regular'/'4x6-font' at sizes that were never actually crisp). measure_font_crispness(font, sample_text) renders the sample and reports the fraction of ink-bbox pixels that are neither pure black nor pure white. BDF fonts (real bitmaps) always score 0.0; TTF ladders should be verified against this before shipping — see the new TestFontFitting::test_ladder_arcade_is_crisp pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): add fit_text_proportional — proportional sizing vs. always-maximize fit_text always picks the largest ladder rung that fits its box. That's right when an element owns dedicated space, but wrong when several independently-fitted elements need to stay visually harmonious as the panel grows: a score's box might have generous room while a neighboring logo scales by a fixed geometry factor via px() — fit_text lets the score balloon out of proportion (even overlapping the logo) even though its individual pick is technically correct. fit_text_proportional(text, box, base_size_px, ladder) instead targets base_size_px * self.scale (the same scale factor px() already uses), picking the nearest ladder rung at or below that target, still capped to what fits the box, floored at the smallest rung when the target is below every rung. Refactored the shared largest-that-fits/ellipsize walk into _walk_ladder() so fit_text and fit_text_proportional don't duplicate it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): fit_text_proportional gains an axis-specific scale override self.scale (min(width_ratio, height_ratio)) is the right conservative default for anything whose aspect ratio matters, but a caller whose surrounding composition already scales along a single axis — e.g. football-scoreboard's logo_slot = min(height, width // 2), which tracks height alone — needs text sized the same way, or it reads as under-scaled next to logos that grew on a panel that only got taller (128x32 -> 128x64: self.scale stays 1.0 since width didn't grow, but logos still double). fit_text_proportional(..., scale=None) now accepts an explicit override; None keeps the existing self.scale default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(layout): scoreboard_regions reserves real center space at 2:1 aspect ratios logo_slot = min(height, width // 2) has a blind spot: at exactly 2:1 aspect ratio (width == 2 * height -- a very common shape: two, four, or more square modules stacked into a taller panel) width // 2 and height are equal, so the two logo slots claim the ENTIRE width and leave zero pixels for a center column, no matter how large the panel gets. Not a 'small panel' problem -- 96x48, 128x64, and 256x128 (all exactly 2:1) hit it identically, while the 128x32 design baseline and panels like 192x48 or 256x32 never do, because height is already the tighter constraint there. Two new parameters fix it in the one shared helper every scoreboard-style plugin composes through: - min_center_fraction / min_center_design_px reserve at least max(width * fraction, design_px * ctx.scale) for the center column, capping logo_slot further when needed. The scaled design-px term matters on small panels where a flat fraction alone reserves too little absolute space. - score_bleed_fraction extends the score's own fit box (not the logo slots themselves) a controlled amount into each side -- the same way real broadcast scoreboards let a big score number's edges cross into the team marks flanking it. Without this the reserve alone can still be too narrow for a short score to render without truncating. score_area is now genuinely narrower than the full card width (previously identical to status_band/detail_band, which still span the full width and overlay the logos -- short text there was never the problem). Verified against the full harness size spread: a real game score like '17-21' never needs ellipsis at any tested 2:1-or-tighter aspect ratio (test_score_never_needs_ellipsis_for_a_short_score), and wide panels (128x32/192x48/256x32-style) are provably unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: document scoreboard_regions' center-reserve and score-bleed params Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address CodeRabbit review on PR #393 - docs: scope the self.layout note to BasePlugin subclasses (others build a LayoutContext directly) and make explicit that adaptive layout is opt-in — classic rendering stays unless a plugin adopts the APIs. - dev_server: broaden the render-request catch (a bad manifest.json now returns a clean 400 instead of an unhandled 500) and stop echoing raw exception text in the loader-failure responses — full tracebacks go to the dev server's console log instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): allowlist plugin_id before any path lookup CodeQL (py/path-injection): plugin_id arrives in request input and flows into filesystem paths via find_plugin_dir. Gate it with the same ^[a-zA-Z0-9_-]{1,64}$ allowlist the web UI's pages_v3 uses, at the single choke point every route resolves through. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): lexical containment check on resolved plugin dirs CodeQL doesn't recognize the interprocedural allowlist as a path-injection barrier; add the canonical one — normalize (without following symlinks, since dev plugins are commonly symlinked into plugins/) and require the result to stay inside the search dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): inline normpath containment barrier before render CodeQL doesn't credit the sanitization inside find_plugin_dir along this flow; apply its documented barrier (normpath + startswith against the allowed roots) inline in _parse_render_request, on the exact path that reaches the render/load sinks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): derive plugin dir from trusted directory listings CodeQL's barrier-guard recognition doesn't see a startswith check inside an any() comprehension, so the normalize-and-prefix approach still flagged. Break the taint outright instead: after lookup, re-derive the directory by enumerating the search dirs (iterdir) and matching by path equality — the Path used for all downstream file access is built solely from trusted listings, never from request input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): use os.scandir for path-injection barrier, redact stack traces from render responses CodeQL doesn't model Path.iterdir() as a taint-clearing enumeration the way it does os.scandir() -- _trusted_plugin_dir's iterdir-based rebuild still traced plugin_id through to the manifest.json open(). Switched to scandir, matching the pattern already verified clean on PR #396. Also stops surfacing raw exception text (update()/display() failures) in the JSON render response -- logs full detail server-side via exc_info instead, returning only the exception class name to the client. And drops path values from three plugin_loader debug/error logs that CodeQL flags as clear-text-logging of externally-influenced data, keeping plugin_id (not flagged) for context. * fix(dev-server): remove conditional-reassignment ambiguity in plugin_dir resolution CodeQL's path-injection flow still traced through _parse_render_request after the scandir fix -- the tainted find_plugin_dir() result and the scandir-derived _trusted_plugin_dir() result shared the same variable name (plugin_dir), reassigned only on the truthy branch. That merge point apparently isn't treated as a barrier by the flow analysis, so it kept tracing the pre-reassignment value through to the manifest open(). Split into two distinct names -- candidate_dir (tainted, used only to call _trusted_plugin_dir) and trusted_dir (the only name used for any downstream file access) -- so there's no reassigned variable for the flow to walk through. * fix: remove unused imports flagged by Codacy Union in adaptive_images.py and field in adaptive_layout.py are both imported but never used -- the last two Codacy findings on this PR, matching the same fix already applied on PR #396. * fix(layout): bound the fit cache; never alias the source image in fits Two latent issues found in a self-review pass: - LayoutContext._fit_cache was an unbounded dict (the image cache got an LRU cap, the text-fit cache didn't). Cache keys embed the fitted TEXT, so a plugin fitting changing strings — a live game clock, a ticker — on a 24/7 service grows it forever. Now LRU-bounded at 512 entries via the same pattern as the image cache. - fit_image returned the caller's ORIGINAL image object when the source was already RGBA at target size (contain/fill_height, no ink crop). ImageFitResult is documented as an independent copy, and LayoutContext caches results — an aliased image lets later mutations of the source corrupt cached fits (or vice versa). Copy in that branch. Both covered by new regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
05e7c43b27 |
fix(plugins): replace dependency marker files with a real satisfaction check (#390)
* fix(plugins): replace dependency marker files with a real satisfaction check The .dependencies_installed hash-marker system only tracked "was this exact requirements.txt hashed before" — not whether the packages it names are actually present. That made it fragile (a wiped venv, a manually removed package, or a lost/corrupted marker forces a needless full pip reinstall or, worse, a false skip) and produced dead weight for the ~10 plugins whose requirements.txt is comment-only (they still paid a pip subprocess on first boot before a marker existed). Replace it with requirements_are_satisfied() in plugin_loader.py, which checks each real requirement line against importlib.metadata directly, so install_dependencies() only shells out to pip when something is actually missing or version-mismatched. Drops the marker file entirely: removed all marker read/write sites in plugin_loader.py and store_manager.py, the now-pointless marker-cleanup step in the git-update path, the unused legacy marker implementation in plugin_manager.py, and the already-stale clear_dependency_markers.sh script. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(security): close path-injection gap in dependency-satisfaction checks CodeQL flagged 2 new high-severity "uncontrolled data used in path expression" alerts at the open() calls inside this PR's new requirements_has_real_deps()/requirements_are_satisfied() -- both are reachable from paths that were never run through the basename+trusted-base sanitiser this codebase already uses elsewhere: - PluginLoader.install_dependencies() only applied that sanitiser when its optional plugins_dir argument was actually passed; the "no plugins_dir" branch trusted plugin_dir_real directly. Made plugins_dir required (not Optional) so that branch can't exist, and added an explicit guard in load_plugin() so install_deps=True without a plugins_dir fails loudly instead of silently. Production's only real caller (PluginManager) always passes plugins_dir already; the harness/dev-server/render-plugin callers all use install_deps=False and are unaffected. - StoreManager._install_dependencies() never sanitised plugin_path at all, and its call sites ultimately derive that path from a plugin's own manifest.json "id" field (install_plugin_from_url) -- a malicious plugin could otherwise point requirements_file outside plugins_dir. Applied the same os.path.basename()-based containment pattern PluginLoader already uses (and that CodeQL recognises as a real sanitiser). Added test_install_dependencies_requires_plugins_dir and test_install_dependencies_rejects_path_outside_plugins_dir to lock in the actual security property, not just quiet the scanner. Verified: all 20 tests in test_plugin_loader.py pass, plus the PR's existing test plan (test_plugin_system.py, test_store_manager_caches.py: 53 passed) and the full CI plugin-safety suite (test_harness.py, test_visual_rendering.py, test_plugin_matrix.py: 52 passed, 2 pre-existing skips) all still pass. * fix(security): replace basename-only sanitiser with a trusted-enumeration check The previous commit's os.path.basename() + os.path.join() pattern (which a pre-existing code comment claimed CodeQL recognises as a sanitiser) did not actually clear the alert -- the next CodeQL run still flagged the same 2 sink lines, plus a new one at the os.path.join() call itself. Taking a substring of tainted data apparently isn't treated as a barrier by this query, whatever the comment assumed. Replaced it with find_trusted_subdir(): enumerate the trusted plugins_dir via os.scandir() and only use a name that scandir itself produced, matched by equality against the caller's requested name. The path is then built from that enumerated entry, not from the caller's string -- a value sourced from iterating a trusted, non-tainted directory carries no taint regardless of what it happens to equal, which is a stronger and more conventional allowlist-style barrier than string-stripping. Applied identically in both PluginLoader.install_dependencies() and StoreManager._install_dependencies(), sharing one implementation. Re-verified: all 65 tests across test_plugin_loader.py (20, including the 2 new security regression tests), test_store_manager_caches.py (35), test_plugin_system.py (10) pass, plus the full CI plugin-safety suite (test_harness.py/test_visual_rendering.py/test_plugin_matrix.py: 52 passed, 2 pre-existing skips). * fix(security): redact URL credentials from pip subprocess output before logging CodeQL flagged 3 clear-text-logging-of-secrets alerts in install_requirements_file() (src/common/permission_utils.py:353,360,371). Pre-existing on main, unrelated to this PR's own diff, but now visible since the path-injection alerts that previously took priority in the annotation list are fixed. The underlying risk is real: pip can echo a private index URL's embedded basic-auth credentials (from a requirements.txt --index-url line or PIP_INDEX_URL) back verbatim in its own stderr/stdout on failure, and this function both logs that output directly and returns it to callers -- store_manager.py's _install_dependencies() logs result.stderr from this same function too. Added _redact_url_credentials(), applied immediately after each of the two subprocess.run() calls (mutating result.stderr/stdout in place) rather than patching each log call site individually. This closes the leak at the source: every downstream use -- the three flagged log lines, the "note" string embedded in the returned stdout, and store_manager.py's own logging of the returned result -- gets the redacted text for free. Verified the fixed-phrase "denied" check (`"a password is required" in result.stderr`) is unaffected, since URL syntax and those phrases don't overlap -- covered explicitly by test_does_not_touch_denied_check_phrases. Added test/test_permission_utils.py (6 tests) covering the redaction helper directly and both subprocess.run() call sites (the sudo-wrapper branch, which this repo's scripts/fix_perms/safe_pip_install.sh makes live, and the no-wrapper fallback branch). All pass. * fix(security): stop interpolating req_file/pip-output into log calls The previous commit's redaction (mutating result.stderr/stdout right after each subprocess.run()) didn't clear CodeQL's clear-text-logging alerts -- same lesson as the path-injection fix earlier in this PR: a static analyzer can't tell "this value was already sanitised two lines up" from "this is still the raw tainted value" just by looking at a single log call in isolation, so it conservatively keeps flagging it regardless of what the redaction function actually does. Removed all dynamic interpolation (req_file, result.stderr) from the 3 flagged logger.warning() calls entirely, replacing them with fixed messages plus (for the one that had it) result.returncode, which is a plain int with no possible taint. The full redacted detail is still available where it actually matters -- in the returned CompletedProcess.stderr/stdout and the "note" text -- just not duplicated into a log line a scanner has to reason about in isolation. Re-verified: all 6 test_permission_utils.py tests still pass (they assert on the returned result, not log call arguments), plus the full test_plugin_loader.py/test_store_manager_caches.py/test_plugin_system.py suite (71 passed, 1 pre-existing deselect, 4 subtests). --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
ChuckandGitHub
|
2ffc57cf40 |
fix(plugin-harness): add no-op process_deferred_updates to test double (#391)
The safety harness's VisualTestDisplayManager (base of BoundsCheckingDisplayManager) doesn't implement process_deferred_updates(), which 5 first-party ledmatrix-plugins call unconditionally between set_scrolling_state() and their scroll-position update: news, odds-ticker, ledmatrix-leaderboard, stock-news, and ledmatrix-stocks. Any of them fails the harness with AttributeError the moment it's touched (surfaced when ledmatrix-plugins#177 had to add a local hasattr guard in ledmatrix-stocks just to pass CI). Add the method as a no-op, mirroring the existing "no-op for testing" pattern already used for set_scrolling_state, so these plugins render under the harness without every touching PR needing its own guard. |
||
|
ChuckandGitHub
|
aab0e9ade0 |
fix(plugin-manager): fix TypeError breaking every plugin's scheduled update (#392)
run_scheduled_updates()'s resource-monitor branch wrapped the update call
in a closure stored as a *class* attribute on a dynamically-built type
(type('obj', (object,), {'update': monitored_update})()). The descriptor
protocol turns a function found via class-attribute lookup into a bound
method on instance access, silently prepending the synthetic instance as
an implicit first argument -- but monitored_update() takes none, so every
call raised "monitored_update() takes 0 positional arguments but 1 was
given", was caught by run_scheduled_updates' try/except, and recorded as
an update failure.
self.resource_monitor is None by default and was dormant until PR #388
("activate dormant plugin health/metrics subsystem") wired it up in both
display_controller.py and web_interface/app.py -- meaning this bug went
live in every real deployment as of that merge (2026-07-09) despite the
buggy line itself dating back to 2025-12-27. In practice this means no
plugin's update() has succeeded since upgrading past #388: circuit
breakers cycle through half-open -> immediate failure -> reopened every
health-check interval forever, and all plugin data (scores, odds, prices,
etc.) goes stale from whatever was last fetched before the upgrade.
Confirmed live on a running instance: odds-ticker (and stock-news,
ledmatrix-stocks, baseball-scoreboard, ledmatrix-leaderboard, of-the-day)
failing this exact way every 5-minute circuit-breaker retry.
Fixed by using types.SimpleNamespace(update=monitored_update) instead of
a dynamic class: SimpleNamespace stores attributes on the instance
itself, so attribute lookup returns the plain function unchanged --
never routed through the class-attribute descriptor protocol that
injects an implicit self.
Added test_run_scheduled_updates_calls_update_with_resource_monitor to
test/test_plugin_system.py using a real PluginResourceMonitor (not a
mock of it), so the test exercises the actual descriptor-binding
behavior that caused this. Verified the test fails with the exact
reported error against the pre-fix code and passes against the fix.
|
||
|
|
bd9f461f70 |
Add system diagnostics, power controls, and WiFi radio toggle to Tools tab (#389)
* Add system diagnostics, power controls, and WiFi radio toggle to Tools tab Expands the web UI Tools tab with safe, purpose-built controls so users can manage the Pi without SSHing in, instead of an arbitrary-command terminal (the web UI has no auth and CSRF is disabled, so a shell would be unsafe). - System Diagnostics card: renders the existing but previously-unused GET /api/v3/system/status endpoint (CPU, memory, temp, disk, uptime), with a manual refresh and a 10s poll. - System Power section: reboot/shutdown buttons wired to the existing reboot_system / shutdown_system actions, behind a confirm step, with a dedicated powerAction() helper that treats the dropped connection as the expected "going offline" outcome rather than an error. - Network Radio section: WiFi on/off toggle backed by new GET/POST /api/v3/wifi/radio endpoints and WiFiManager.set_wifi_radio() / get_wifi_radio_state(). Disabling WiFi is refused unless a wired connection is present (reusing the existing lockout guards), with an explicit force-off confirmation for advanced users. No new privileged commands: uses nmcli radio wifi on|off (already sudo-allowlisted) and the existing reboot/poweroff grants, so the sudoers-alignment guard test stays green. Bluetooth toggle intentionally omitted since the installer removes the BlueZ stack for LED timing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE * Harden WiFi radio endpoint and diagnostics poll (review feedback) Addresses code review feedback on the Tools tab additions: - api_v3.py: parse the `enabled` POST field with the same string-aware coercion as `force`. A plain bool() cast turned {"enabled":"false"} into True (enabling instead of disabling) for any non-UI API caller. - wifi_manager.set_wifi_radio() now returns a reason code alongside (success, message); the /wifi/radio error response includes it. The Tools UI only shows the force-off confirmation when reason == 'no_ethernet', so a genuine nmcli failure surfaces its real error instead of a misleading "no wired connection" prompt that would just retry into the same failure. - tools.html: gate the 10s diagnostics poll on panel visibility (document.hidden / offsetParent), so switching to another tab stops the recurring /api/v3/system/status calls instead of churning the Pi off-screen. The initial load and manual Refresh remain unconditional. Verified: {"enabled":"false"} now disables (refused w/ reason:no_ethernet), {"enabled":"true"} enables; inline JS passes node --check; py_compile clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE * Narrow WiFi-disable fallback except and add stack trace (review) Addresses a CodeRabbit nitpick: the fallback handler in set_wifi_radio()'s disable path caught bare Exception and logged without a traceback. Narrow it to (OSError, subprocess.SubprocessError) — the errors subprocess.run realistically raises — and log with exc_info=True for full context on the Pi. Anything genuinely unexpected now propagates to the endpoint's outer handler (500), matching the codebase's specific-exception convention. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019X3he87Ggr1qt8y7mnFeuE --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
3b93024993 |
feat: activate dormant plugin health/metrics subsystem and surface it in the web UI (#388)
* feat(plugin-system): activate dormant plugin health & metrics subsystem
PluginManager shipped a fully-built health tracker, resource monitor and
circuit breaker that were never instantiated (health_tracker/resource_monitor
were left as None), so the circuit breaker never engaged and the existing
health/metrics API routes always returned "not available".
- DisplayController now wires a PluginHealthTracker and PluginResourceMonitor
onto the plugin manager, enabling the circuit breaker (a repeatedly-failing
plugin's update() is skipped after consecutive failures, then retried after
a cooldown) and per-plugin execution-time metrics. Both persist to the
shared cache.
- load_plugin() now validates each plugin's config against its JSON schema in
a strictly warn/degrade-only way: a violation logs a warning and flags the
plugin degraded in the health tracker, but never changes whether the plugin
loads or its pass/fail behaviour. Adds PluginHealthTracker.set_degraded(),
which never touches the circuit breaker.
- ResourceMonitor CPU/memory sampling now reuses a cached psutil.Process and
reads cpu_percent(interval=None), so monitoring no longer blocks ~100ms per
call on the display loop's update path.
- Fix DiskCache.get() raising TypeError for max_age=None ("never expires"),
which silently discarded persisted plugin health/metrics on read and thus
broke cross-process and post-restart surfacing.
- Fix two dead PluginManager helpers that called non-existent tracker methods.
Tests: new test_resource_monitor, test_plugin_health,
test_plugin_manager_schema_soft; extended test_cache_manager and
test_display_controller.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq
* feat(web-ui): surface plugin health, metrics and load state
With the health/metrics subsystem now active in the display service, expose it
in the web UI (which runs as a separate process from the display loop):
- Wire a health tracker / resource monitor backed by the shared on-disk cache
into the web process so /api/v3/plugins/health and /plugins/metrics read the
data the display service persists.
- Build those route responses per installed plugin id (the tracker's in-memory
view is empty in a fresh web process) so cross-process data is included.
- Add state + error_info to /plugins/installed entries so the UI can show why a
plugin isn't running instead of just loaded:false.
- Add a "Plugin Health" panel to the Tools page (circuit status, avg/max update
time, update count, last error) plus PluginAPI.getPluginMetrics().
Tests: route-level tests for the health/metrics endpoints in test_web_api.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq
* fix(plugin-metrics): refresh cross-process health/metrics reads; type hints
Addresses CodeRabbit review on #388:
- Major: the web process's health/resource trackers cached the first persisted
read in an in-memory dict (and the CacheManager memory tier held max_age=None
entries indefinitely), so a long-lived web process showed the first snapshot
and never reflected the display service's later updates. Add an opt-in
force_reload path (get_health_summary/get_health_state/_load_health_state and
get_metrics_summary/get_metrics) that bypasses the in-memory copy and, via a
new memory_ttl passthrough on CacheManager.get, the cache manager's memory
tier — so each /plugins/health and /plugins/metrics poll reads fresh persisted
state. Default behaviour (force_reload=False) is unchanged for the display
process and existing callers.
- Minor: DiskCache.get type hint is now Optional[int] with the None ("never
expires") semantics documented, matching MemoryCache.get.
Tests: new force_reload staleness cases in test_plugin_health and
test_resource_monitor.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq
---------
Co-authored-by: Claude <noreply@anthropic.com>
|
||
|
|
85d321cf33 |
fix: plugin_loader retries with --ignore-installed on apt/pip RECORD conflicts (#386)
* fix: plugin_loader retries with --ignore-installed before assuming apt package satisfies pin install_dependencies treated any "uninstall-no-record-file" pip failure as "dependency satisfied" and wrote the success marker without ever attempting --ignore-installed, unlike install_dependencies_apt.py and safe_pip_install.sh (added in #385 for the Plugin Store/first-time-install paths). A plugin pinning a newer version of a system-managed package (e.g. requests) would silently keep running against whatever version apt shipped, while the marker file claimed the pinned requirement was met. Now retries the same install with --ignore-installed on that specific failure so pip actually lays the pinned version down (shadowing the system-managed copy) before falling back to the prior tolerant behavior if the retry itself fails too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: suppress Codacy finding on new retry subprocess.run call Same generic Bandit/semgrep pattern-match on non-literal subprocess.run argv flagged in #385's install_requirements_file, now on the new --ignore-installed retry call added here: list-form argv (no shell=True), sys.executable is this process's own interpreter, and requirements_file is built internally by find_plugin_directory, never raw external input. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: tolerate a timed-out --ignore-installed retry, dedupe marker-write logic CodeRabbit review caught a real inconsistency: if the --ignore-installed retry itself timed out, subprocess.TimeoutExpired propagated to the outer handler and returned False, failing plugin load — contradicting the intended "tolerate this specific apt/pip conflict" behavior, where a mere non-zero retry return code already returns True. Wraps the retry in its own try/except so a timeout is logged and tolerated the same way as any other retry failure. Also extracts the marker-writing logic (open/write/chmod, ignoring OSError) into _write_dependency_marker, since it was duplicated identically between the direct-success path and the apt-conflict-retry-fallback path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: revert marker-write dedup helper, resolves CodeQL path-injection alert CodeQL flagged _write_dependency_marker's open(marker_file, ...) as "uncontrolled data used in path expression" (high severity) once the marker-write logic was extracted into its own method. marker_file is actually safe — it's built from safe_plugin_dir, which install_dependencies sanitizes via os.path.basename() (CodeQL's own recognized py/path-injection sanitizer, per the existing comment a few lines above) — but CodeQL's interprocedural analysis doesn't carry that sanitized status across the new method boundary, since the sanitizer call and the open() sink were no longer in the same function. This exact code produced zero CodeQL findings before the extraction (in two duplicated inline blocks) and is unchanged in what data reaches it — only its location moved. Reverting the extraction (keeping CodeRabbit's other, independent timeout-handling fix) restores the previously-clean shape rather than trying to convince the analyzer's cross-function taint tracking that a refactor changed nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
63a233f3ed |
fix: dependency installation gaps in Plugin Store and first-time install (#385)
* fix: install plugin dependencies through root-visible installer in Plugin Store install_plugin/update_plugin (store_manager.py) installed requirements.txt with a bare `pip3` off PATH, bypassing the root-visible installer added in #380 for the "Reinstall Plugin Deps" button. Two bugs stacked: (1) `pip3` can resolve to a different Python install than the one that actually runs ledmatrix.service, and (2) even when it resolves correctly, ledmatrix-web runs as a non-root user so the package lands in that user's local site-packages, invisible to root-run ledmatrix.service. Either way the install reports success and writes the .dependencies_installed hash marker, so plugin_loader's own (correct) install-on-load path skips reinstalling — leaving the dependency permanently missing until a user finds and clicks the separate "Reinstall Plugin Deps" tool. This is why users kept hitting "No module named 'astral'" for the weather plugin even after installing it from the Store. Extracts the sudo-wrapper-then-fallback install logic from api_v3.py's _pip_install_requirements into src/common/permission_utils.py as install_requirements_file, and routes store_manager.py's dependency installation through it so the automatic Store install/update path now matches the manual "Reinstall Plugin Deps" path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: suppress Codacy false-positive on subprocess.run in install_requirements_file Codacy's generic subprocess-security rule (Bandit B603 equivalent) flagged the pip/sudo subprocess.run calls in install_requirements_file for lacking a "static string argument" — the standard pattern-based flag for any subprocess.run() call with a variable in its argv list. Both calls use list-form argv (no shell=True, so no shell-injection surface), and the only dynamic value is req_file, a Path built internally by callers rather than raw external input; safe_pip_install.sh independently re-validates it before installing anything as root. Suppresses with inline `# nosec B603` comments matching this codebase's existing convention (see permission_utils.py's own PROTECTED_SYSTEM_DIRECTORIES, display_manager.py, sync_manager.py, etc.). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: first-time install script fails on apt-managed requests package web_interface/requirements.txt and requirements.txt both pin requests>=2.33.0,<3.0.0, but Raspberry Pi OS ships an apt-managed python3-requests with no pip RECORD file. Upgrading it via plain `pip install` aborts with "uninstall-no-record-file" because pip refuses to uninstall a package it has no record of, in place — which is exactly the "Some web interface dependencies failed to install" warning first-time install hits. scripts/install_dependencies_apt.py and scripts/fix_perms/safe_pip_install.sh already work around this with --ignore-installed (lets pip lay the new version down in /usr/local, shadowing the apt copy, instead of trying to remove it first). first_time_install.sh's own direct pip invocations — the per-package requirements.txt loop, the web_interface/requirements.txt install, and the requirements_web_v2.txt fallback — didn't have it. Adds --ignore-installed to all three so first-time install no longer fails on this well-known apt/pip conflict. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: add nosemgrep to subprocess.run calls Codacy still flagged The prior # nosec B603 comments suppressed Bandit's check but Codacy's semgrep-based rule ("subprocess function 'run' without a static string") kept flagging the same two lines as a critical security issue even after that fix landed. install_dependencies_apt.py's _run() already needed both tags together (# nosec B603 B607 ... # nosemgrep) for the identical subprocess.run pattern, so apply the same double suppression here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: add --ignore-installed to install_requirements_file fallback path CodeRabbit review caught this (confirming a gap already flagged in conversation): the non-sudo fallback pip install in install_requirements_file was missing --ignore-installed, unlike the sudo-wrapper branch and safe_pip_install.sh. Without it, the same apt/pip RECORD-file conflict this PR fixes elsewhere (first_time_install.sh, install_dependencies_apt.py) could still hit installs that fall back to this path (e.g. a plugin's requirements.txt on a host where safe_pip_install.sh isn't set up yet). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
7a9d01342a |
Fix inconsistent Vegas scroll transition gaps caused by plugin-baked padding (#384)
* Strip plugin-baked scroll padding when capturing content for Vegas mode Plugins that build their own ticker image via ScrollHelper.create_scrolling_image() (or that manually pad both ends for a clean standalone loop) carry a solid-black margin up to display_width wide on one or both edges. Vegas mode already adds its own configurable gap around every item, so leaving that margin in place stacked an extra, uncontrolled blank stretch on top of separator_width for whichever plugin took the ScrollHelper-capture path — producing inconsistent transition gaps between modules compared to plugins that provide content natively via get_vegas_content(). _get_scroll_helper_content() now detects and crops any such margin before handing the image to the Vegas render pipeline, so every plugin's gap is governed solely by vegas_scroll.separator_width regardless of which capture path produced its content. * Address CodeRabbit nitpick: warn on double-edge padding crop, add unit tests Logging a double-edge match at warning level (vs. info for a single edge) makes it easy to spot an unexpected crop in the field, since two edges matching at once is a much stronger signal of genuine baked-in padding than one edge coinciding with real all-black content. Also adds test/test_vegas_plugin_adapter.py covering _strip_scroll_padding's branch logic: leading-only, trailing-only, both-edges, no-match, degenerate all-black, missing/undersized display_width, and the info-vs-warning log level. * Add type hints and docstring to test _solid helper (CodeRabbit nitpick) --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|
|
7a6bad29fe |
feat(display-controller): hot-reload plugin enable/disable without a restart (#374)
* feat(display-controller): hot-reload plugin enable/disable without a restart Enabling or disabling a plugin in config previously required restarting the display service: the plugin list and available_modes were built once at init and the run loop never revisited them. (Per-plugin config *values* already hot-reloaded; only the enabled set was restart-only.) Now the controller reconciles its running plugins against the config's enabled set whenever that set changes: - The ConfigService watcher thread only sets a `_pending_plugin_reconcile` flag (via a cheap enabled-set diff). It never mutates loop state. - The run loop applies the reconcile on its own thread (top of each iteration, deferred while on-demand is active), so loading/unloading and rebuilding available_modes can't race with rendering. - `_reconcile_enabled_plugins` diffs desired vs running plugins, unloads the removed ones (cleanup + on_disable + config-unsubscribe via the new `_unregister_plugin`) and loads the added ones, then clamps the rotation index so the current mode stays valid. The per-plugin registration done at startup is extracted into `_register_loaded_plugin` and reused by the live-enable path so both build identical state. Extracting it also fixes a latent late-binding bug: the per-plugin config-change callbacks were closures over the loop variable, so every plugin's callback targeted the last-loaded instance; each now binds its own id/instance. Adds test/test_display_controller_plugin_toggle.py covering live enable, live disable, index clamping, no-op when unchanged, and the enabled-set diff. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(display-controller): don't exit on empty available_modes, guard rotation modulo Hot-reload means available_modes can legitimately be empty at startup (no plugins enabled yet) and become non-empty later via the web UI, or vice versa mid-run. Fix four issues found reviewing this PR: - run() exited the process entirely when available_modes was empty at startup instead of idling, permanently defeating the point of live enable/disable for anyone who starts with zero plugins enabled. - The mode-rotation step divided by len(available_modes) unconditionally, raising ZeroDivisionError if the last enabled plugin is disabled between frames. - _reconcile_enabled_plugins() called .get('enabled', False) on a config section without checking it was a dict first, raising AttributeError on a malformed config value. - Minor: pop the config-change callback only after attempting to unsubscribe it, and log the exception in the config-read fallback instead of swallowing it silently. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(display-controller): address review findings on the hot-reload PR - Idle-wait tick was a fixed 30s sleep, delaying pickup of a plugin enabled via the web UI while no modes were active. Shortened to ~1s so it's roughly as responsive as the per-frame check once modes exist. - _unregister_plugin popped the config-change callback from _plugin_config_callbacks even when config_service.unsubscribe() raised, losing the only reference to it. Now only pops on a successful unsubscribe. - _pending_plugin_reconcile was cleared before _reconcile_enabled_plugins() ran, so a retryable failure (e.g. plugin discovery erroring) silently dropped the enable/disable request. _reconcile_enabled_plugins() now returns True/False and the caller only clears the flag on True. - Added a warning log for the malformed-config case (a plugin's config section present but not a dict) so it's actually visible, and updated the existing test to assert it via caplog. Left the broad `except Exception` around config_service.unsubscribe() as Exception -- the current implementation is a simple lock+dict/list op that doesn't document or realistically raise a narrower type, so this is a defensive catch-all, not user error handling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: ChuckBuilds <charlesmynard@gmail.com> |
||
|
|
fefc2d44a2 |
feat(display): double-sided mode — mirror one screen across the panel chain (#375)
* feat(display): add double-sided mode to mirror one screen across the panel chain
Renders a plugin once at a logical (per-screen) size, then tiles the
rendered frame across the full physical chain so two (or more) panels show
identical content. A 128x32 chain configured with 2 copies drives two 64x32
screens; vertical axis splits parallel outputs instead of the chain.
Plugins size themselves from matrix.width/height, so a thin _LogicalMatrix
proxy reports the logical size while delegating every real operation
(CreateFrameCanvas, SwapOnVSync, brightness, Clear) to the physical matrix —
no plugin changes required. Duplication is a single PIL paste per copy in
update_display(), so render cost is unchanged.
Config: display.double_sided { enabled, copies, axis }. Invalid config
(non-divisible dimension, bad axis/copies) logs a warning and falls back to
single-screen rather than failing to light up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(web): expose double-sided display config in the settings UI
Adds a Double-Sided Display section to the Display settings page (enabled
checkbox, copies, horizontal/vertical axis) and wires the save handler to
persist it under display.double_sided. Validates copies (2-8) and axis,
returning 400 on bad input; an omitted checkbox is saved as disabled.
Like the other hardware fields, changes take effect after a display restart.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(display): add type hints and docstrings to double-sided proxy
Addresses CodeRabbit nits: sort _LogicalMatrix.__slots__ (Ruff RUF023),
annotate the proxy's __init__/properties/dunders and _resolve_double_sided's
return type, and add docstrings to the property/dunder methods.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
||
|
|
d297dd6217 |
feat(display-controller): round-robin between simultaneous live-priority games (#372)
_check_live_priority() was stateless first-match-wins: it returned the first plugin in registration order with live content, and the post-dwell hold pinned the carousel to it, so when two games were live at once (e.g. a baseball game and a soccer match) the second never showed until the first ended. Add _collect_live_modes() (all currently-live modes, deduped, in registration order) and give _check_live_priority an 'advance' flag. The main rotation calls it with advance=True, which returns the live mode after the one currently shown -- using current_display_mode as the cursor -- so each dwell advances to the next live game and they take turns. The Vegas coordinator and the vegas-active check keep the default non-advancing peek (advance=False), so they only report whether any game is live without spinning the cursor. should_rotate and _apply_live_priority are unchanged; a single live game still holds as before. Adds regression tests to TestDisplayControllerLivePriority. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
d22d0a3754 |
fix(plugins): stop core updates from resurrecting uninstalled built-in plugins (#368)
* fix(plugins): stop core updates from resurrecting uninstalled built-in plugins Built-in plugins (e.g. web-ui-info, starlark-apps) are committed into the repo under plugin-repos/. When a user uninstalls one, a subsequent core `git pull` update restores the committed files, so the plugin reappears on every update. The update endpoint stashes the deletion and never pops it, and `git pull` faithfully restores any committed file whose deletion was never committed — so excluding plugin-repos/ from the stash can't fix this (it would only make `git pull --rebase` fail on a dirty tree). Add a persistent uninstall registry (config/uninstalled_plugins.json, gitignored) that survives restarts, unlike the existing in-memory tombstone: - Uninstall records the plugin id; install clears it. - purge_uninstalled_plugins() re-removes any recorded plugin whose directory reappears on disk; called after a successful git-pull update and at web startup (covers manual `git pull` on the Pi too). - The state reconciler also refuses to auto-repair a persistently uninstalled plugin. Wires up mark_recently_uninstalled in the uninstall flow (previously only referenced by tests) via the new persistent record. Adds regression tests covering record/forget/purge lifecycle, persistence across manager instances, and corrupt-registry tolerance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(plugins): validate uninstall-registry ids and lock registry writes Address review feedback on the persistent uninstall registry: - Critical: validate plugin ids on read/record and add a containment guard in purge_uninstalled_plugins. A corrupt or hand-edited registry entry of "" resolves to the plugins root, so purge could have deleted every plugin; traversal ids ("..", "../x") could target paths outside the root. Invalid ids are now dropped on read, refused on record, and never removed unless the path is a direct child of the plugins directory. - Major: guard record/forget read-modify-write with a lock so concurrent install/uninstall requests can't lose updates. - Minor: narrow the startup and post-update purge exception handlers from bare Exception to (OSError, RuntimeError). Adds regression tests for empty-id, traversal-id, and invalid-record cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
cf28a8c0d5 |
fix(display): restore early-continue guard for mid-loop mode changes (#367)
When the display loop breaks early because current_display_mode changed (on-demand activation, live priority, etc.), it would fall through to the "honour minimum duration" sleep for the *previous* mode — blocking for up to that mode's full display_duration (default 30s) without polling on-demand requests or re-checking the mode. New modes could sit unrendered for up to 30s, or get clobbered by a queued stop request before ever displaying. This guard was added in #298 to fix #196 (live priority not interrupting long display durations) and was accidentally dropped in #330 as collateral damage of an unrelated time.monotonic() -> time.time() cleanup in the same diff hunk. Restoring it fixes both the original #196 regression and a new symptom found via the on-air MQTT plugin, where ON/OFF toggles could be delayed by up to 30s or missed entirely depending on timing within the previous mode's display cycle. Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
313e35a98f |
Add cross-size/cross-screen plugin safety harness (#361)
* feat(testing): add cross-size/cross-screen plugin safety harness Render every plugin across all supported matrix sizes (64x32, 128x32, 128x64, 256x32) and every declared screen, failing on crashes, content drawn past the panel edge, or visual drift vs committed golden images. - BoundsCheckingDisplayManager: oversized-canvas overflow detection - harness.py: multi-size/multi-screen render engine + golden compare - scripts/check_plugin.py: CLI (functional+bounds, --out-dir, --update-golden, --freeze-time); render_plugin.py refactored onto shared loading helpers - test/plugins/test_harness.py + test_plugin_matrix.py (parametrized, honors per-plugin test/harness.json; skips when no plugins present) - MockCacheManager.cache_dir so cache-dir-using plugins load headlessly - .github/workflows/test.yml + docs/plugin-safety-harness.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(testing): address PR review feedback on plugin safety harness - check_plugin: friendly error for non-numeric --sizes; reject non-object --config / --mock-data JSON; sanitize plugin mode before using as a filename; stop --update-golden from masking crash/overflow failures - bounds_display_manager: pad the canvas out to the largest supported panel (not a fixed 16px) so far-overshoot coordinates are caught, not clipped - harness: merge config_schema defaults inside render_plugin_matrix; surface update() failures as a non-fatal warning + result field instead of a debug log; sanitize mode in golden_path - loading: fail fast when harness.json references a missing mock_data fixture - mocks: clean up the per-instance temp cache dir via weakref.finalize - test_plugin_matrix: add a discovery guard that fails when LEDMATRIX_REQUIRE_PLUGINS=1 but none found (still skips locally); type hints - bound test deps with upper version pins for deterministic CI Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(testing): render plugins across arbitrary panel sizes, not a fixed list Addresses maintainer feedback that there is no canonical set of supported panel sizes — a build can be any size/configuration (square, 2x2, 4x4, 8x2, long strips, tall stacks). - sizes.py: SUPPORTED_SIZES -> DEFAULT_TEST_SIZES (back-compat alias kept), reframed as a representative SAMPLE of real panel-grid arrangements rather than an authoritative list; add parse_size_token / coerce_sizes / resolve_test_sizes helpers - sizes are now fully overridable: LEDMATRIX_TEST_SIZES env (global, e.g. test on your exact hardware) > per-plugin harness.json "sizes" > default sample; CLI --sizes unchanged - bounds_display_manager: pad the canvas to the largest panel IN THE CURRENT RUN (via overflow_extent) instead of a hardcoded max, so cross-size overflow detection scales to whatever sizes a run uses - harness: compute per-run extent and thread it into the bounds manager - tests: arbitrary-shape + size-parsing/precedence coverage - docs: rewrite "Supported sizes" -> "Sizes: a sample, not a fixed list" Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(testing): fail the harness on non-connectivity update() errors Addresses the remaining review thread: recording every update() exception as a non-fatal warning still let a real update() regression pass green as long as display() survived. Now update() failures are classified — a tolerated set of connectivity errors (ConnectionError/TimeoutError/socket/ssl/urllib/http/ requests) is recorded non-fatally (expected with no network in CI), while any other exception is treated as a genuine bug and fails that render. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(security): pin actions to SHAs and disable checkout credential persistence Addresses the CodeRabbit/zizmor workflow-hardening finding: pin actions/checkout and actions/setup-python to full commit SHAs and set persist-credentials: false on checkout to reduce supply-chain and token-exposure risk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(testing): validate positive sizes; narrow requests import except Two review findings: - sizes.py: parse_size_token / coerce_sizes now reject non-positive dimensions (0x32, -64x32) with a clear message instead of passing invalid sizes downstream (CodeRabbit). - harness.py: the optional `requests` import now catches ImportError specifically and logs instead of `except Exception: pass`, clearing the Codacy medium "Try, Except, Pass" (harness.py L52) and Ruff S110/BLE001. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
b9dcbb5152 |
fix(display): resume rotation where it left off after live priority ends (#362)
When a live-priority plugin (e.g. live sports, flights overhead) preempted the rotation, the controller overwrote current_mode_index with the live plugin's index. Once live priority ended, rotation continued from after the live plugin's mode, skipping every mode between the interrupted position and the live plugin. With a live plugin late in the order, modes just before it were starved indefinitely. Save the rotation position on the initial live-priority switch and restore it when live priority ends, in a new _apply_live_priority() helper. Add regression tests covering resume, no-double-save during the hold, and the idle no-op. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
eedf680a8c |
perf: display pipeline optimizations — caching, logging, scroll, text width (#358)
* docs(core): add module and class docstrings to the 5 undocumented core files
Fills the only significant documentation gaps found during a codebase
audit. All other core files (plugin_system/, logging_config.py, etc.)
already have complete module, class, and function docstrings.
Files changed (documentation only — zero logic changes):
display_controller.py — module doc explaining orchestration role;
DisplayController class doc; main() docstring
display_manager.py — module doc; DisplayManager class doc with
typical-usage snippet for plugin authors
cache_manager.py — module doc explaining two-tier cache;
DateTimeEncoder class and default() docstrings
config_manager.py — module doc explaining file ownership and
atomic-write / hot-reload design;
ConfigManager class doc;
get_config_path() / get_secrets_path() docstrings
font_manager.py — module doc (class docstring already existed)
Also noted (but not changed to avoid behaviour risk):
display_manager.py and font_manager.py use logging.getLogger() directly
instead of the project's get_logger() wrapper. display_manager.py also
calls setLevel(logging.INFO) immediately after, which would be lost if
switched to get_logger().
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* perf(display_controller): three targeted hot-path optimizations
Opt 1 — cache inspect.signature() per plugin_id
inspect.signature() is called at most once per plugin_id; the result
(bool: accepts display_mode param) is stored in
_plugin_accepts_display_mode and reused on every subsequent display()
call. Eliminates all reflection from the display path at runtime.
Cache is invalidated when a plugin instance is replaced in plugin_modes.
Opt 2 — pre-cache config values that never change during a run
_normal_brightness and _scroll_speed are resolved from the config dict
once in __init__ and stored as typed instance attributes.
- Removes 2+ chained dict.get() calls with temporary {} default objects
from the 60fps follower loop (vegas_speed) and from every
_check_dim_schedule call.
- current_brightness init now uses _normal_brightness directly.
Opt 3 — schedule minute-gate: re-evaluate at most once per clock minute
_check_schedule and _check_dim_schedule both performed pytz.timezone(),
datetime.now(), strftime(), and datetime.strptime() on every outer loop
call. Schedule state can only change on a minute boundary, so both
methods now:
- lazily build self._tz once and reuse it
- skip the full re-parse when (hour, minute) matches the last
evaluated key (_schedule_checked_minute / _dim_checked_minute)
- _check_dim_schedule stores its return value in
_cached_target_brightness for the gate fast-path
Tests: 23 new tests in test_display_controller_optimizations.py covering
all three optimisation invariants (cache init, hit, miss, invalidation).
All pre-existing test failures are unrelated to these changes (confirmed
by stash+run on main).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: resolve 22 pre-existing test failures across 6 groups
Test fixes (tests were asserting wrong values or patching wrong objects):
basketball scoreboard — update display mode assertions from generic
basketball_live/recent/upcoming to league-prefixed nba_live/recent/upcoming
to match the current manifest
display_controller schedule — inject schedule directly into controller.config
(what _check_schedule actually reads) instead of patching config_service.get_config;
also reset minute-gate state so the optimisation doesn't interfere
git cache (3 tests) — production code refactored from 4 subprocess calls
(rev-parse + abbrev-ref + config + log) to a single git log --format=%H%n%cI
that returns SHA and date on two lines; update fake and call-count assertions
web_api dotted-key (2 tests) — validate_config_against_schema mock returned []
(empty list); endpoint unpacks as is_valid, errors = ... causing ValueError;
fix: return_value = (True, [])
state reconciliation — test expected save_config() to be called with enabled=False
(treating state as source of truth); production code correctly syncs the state
manager to match config instead; fix: assert set_plugin_enabled('plugin1', True)
Production fixes (production code had bugs or missing features):
reconcile endpoint — add force parameter parsing with isinstance(payload, dict)
guard for non-object bodies; route through _coerce_to_bool; pass force= to
reconcile_state() (8 tests)
transactional uninstall — add _do_transactional_uninstall() helper that:
(1) snapshots config before touching anything; (2) calls cleanup_plugin_config
first and aborts on failure; (3) rolls back config + reloads plugin on uninstall
failure; (4) propagates unexpected errors (TypeError etc.) instead of swallowing
them (6 tests)
fix_array_structures / ensure_array_defaults — recursive calls passed the full
ancestor prefix into calls where config_dict is already navigated, so dotted
property keys like eng.1 caused parent_parts.split('.') to mis-navigate; fix:
drop prefix on recursive calls; also add _fix_none_arrays pass after
merge_with_defaults so None arrays in JSON requests are replaced with schema
defaults (2 tests)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* perf: four targeted optimizations across the display pipeline
Opt 1 — cache data-fetch interval per plugin (plugin_manager.py)
_get_plugin_update_interval fell back to config_manager.get_config()
(a full dict copy) when the manifest lacked an interval. Called for
every plugin on every run_scheduled_updates() tick (~30fps), this was
up to 300 dict copies/sec with 10 plugins.
Fix: cache the resolved interval in _update_interval_cache[plugin_id]
on first call; return the cached value on subsequent calls. Cache is
cleared on load_plugin and unload_plugin.
Opt 2 — demote noisy per-cycle INFO logs to DEBUG (display_controller.py)
Four logger.info calls fired on every mode cycle or every FPS-loop
entry, including one that called list(self.plugin_modes.keys())
unconditionally (allocating a list every outer loop iteration).
- "Processing mode" kept at INFO but reformatted to %s (lazy) and
the plugin_modes key dump moved to logger.debug
- "Attempting/Got cycle duration" → logger.debug
- "Entering high/normal FPS loop" → logger.debug
Mode name at INFO is preserved for black-screen troubleshooting.
Opt 3 — use Image.frombytes instead of Image.fromarray in scroll hot path
(scroll_helper.py)
Image.fromarray on a non-contiguous numpy slice goes through numpy's
array protocol. Image.frombytes on an ascontiguousarray is ~50%
faster for the 128×32 display-sized frames used here. Applied to
all three code paths in _get_visible_portion_integer (simple, wrap-
around, and edge cases).
Opt 5 — cache get_text_width per (text, font) pair (display_manager.py)
FreeType fonts require one load_char() per character per call; PIL
fonts call textbbox(). Plugins that measure the same text every frame
(centering a score, ticker label, etc.) were re-measuring from scratch
on every display() call.
Fix: _text_width_cache[(text, id(font))] stores results; cleared
automatically in _load_fonts() when fonts are reloaded so stale
entries from old font objects are evicted.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(scroll_helper): fix edge-case bug exposed by frombytes switch
The previous commit replaced Image.fromarray with Image.frombytes in
_get_visible_portion_integer. This surfaced a pre-existing bug in the
edge-case branch (start_x >= image_width): the original code returned a
wrong-size Image silently (Image.fromarray accepts a too-short array);
Image.frombytes raises ValueError instead.
Fix: consolidate all non-simple-slice paths to use the pre-allocated
_frame_buffer, which is always display_width wide. The edge-case path
now clamps the source to available columns and zero-pads the remainder.
Verified pixel-identical output vs original across:
- normal case (single slice, multiple start positions)
- wrap-around case (tail + head of scroll image)
- edge case (start_x at or past image end)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address CodeRabbit review comments on PR #358
1. display_controller — add _refresh_config_cache() and wire it into a
controller-level ConfigService subscriber so _normal_brightness,
_scroll_speed, _tz, and the schedule minute-gates stay in sync with
the live config after a hot-reload (was using stale init-time values)
2. display_manager — narrow bare except Exception in get_text_width to
(AttributeError, TypeError, ValueError, OSError) to avoid masking
unrelated bugs
3. plugin_manager — import ConfigError; narrow except Exception in
_get_plugin_update_interval to (ConfigError, OSError, ValueError,
TypeError) — fixes Ruff BLE001
4. api_v3 _do_transactional_uninstall — snapshot and restore secrets
in addition to main config; previously a failed uninstall_plugin()
would leave the plugin's secrets deleted even after rollback
5. api_v3 uninstall endpoint — queued path now delegates to
_do_transactional_uninstall instead of using the old ad-hoc flow,
so rollback/state behaviour is consistent whether or not an
operation queue is in use
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(display_controller): move _plugin_accepts_display_mode init before plugin loop
Codacy HIGH: 'access to member before its definition' — the dict was
initialised at line 441 but accessed at line 364 inside the plugin-
loading loop, both within __init__.
Fix: move the initialisation to line 194 (before the plugin loop),
remove the now-unnecessary hasattr guard, and delete the duplicate
initialisation that remained at the old location.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Chuck <chuck@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
|
cac9644b6d |
fix(plugin-loader): auto-detect new dependencies via requirements.txt hash (#355)
* fix(plugin-loader): detect new deps via requirements.txt hash instead of empty marker The .dependencies_installed marker was an empty file, so adding a new package to requirements.txt (e.g. astral in ledmatrix-weather v2.3.0) never triggered a pip re-install on existing installs — the file existed so the check returned early. The marker now stores a SHA-256 hash of requirements.txt. On every plugin load, the loader compares the current hash to the stored one; a mismatch (or missing marker) triggers pip install and writes the new hash. store_manager._install_dependencies() also writes the hash marker after a store install/update so the loader skips a redundant pip run on next boot. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugin-loader): address CodeQL path expression and I/O error handling - Add explicit relative_to() containment check after path resolution so CodeQL recognizes the plugin directory boundary (fixes 4 CodeQL alerts: Uncontrolled data used in path expression, lines 168/172/189/205) - Wrap requirements_file.read_bytes() in try/except OSError — on Raspberry Pi with flaky SD card storage this can fail; returns False with a clear log - Wrap marker_path.read_text() in try/except OSError — a corrupted marker falls through to a clean reinstall instead of crashing - Wrap both marker_path.write_text() calls in try/except OSError — pip already succeeded at this point so a marker write failure should not return False or propagate through the generic exception handler Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugin-loader): use realpath+startswith containment check for CodeQL path-injection Replace relative_to() (not recognised by CodeQL as a path sanitiser) with the os.path.realpath() + startswith() pattern that CodeQL explicitly models as sanitising py/path-injection. - Add plugins_dir optional param to install_dependencies() and load_plugin() - PluginManager.load_plugin() passes self.plugins_dir as the trusted anchor; install_dependencies() validates that the resolved plugin_dir starts with the resolved plugins_dir before any file I/O - Replace all Path.read_bytes/read_text/write_text/exists with open() and os.path.isfile() so the sanitised string paths flow directly to file ops without re-introducing taint through Path object conversion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugin-loader): fail-fast when install_dependencies returns False Previously the boolean result was silently discarded, so a failed pip install would log a warning but continue attempting to import the plugin module — resulting in a confusing ModuleNotFoundError instead of a clear dependency failure message. Now raises PluginError with plugin_id and plugin_dir if dependency installation fails, stopping the load before the import is attempted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugin-loader): use basename+reconstruct to satisfy CodeQL py/path-injection startswith() is a validation check in CodeQL's model, not a sanitiser — taint still flows through plugin_dir_real to the file operations. os.path.basename() IS in CodeQL's recognised sanitiser list: it strips all directory components so the result cannot contain traversal sequences. Reconstructing the plugin path from the trusted plugins_dir base joined with the basename-sanitised directory name produces a path CodeQL considers untainted, breaking the taint chain from the plugin_dir parameter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugin-loader): guard against empty basename when plugin_dir resolves to fs root If plugin_dir somehow resolves to '/' or a bare drive root, os.path.basename() returns '', causing safe_plugin_dir to equal plugins_dir_real and the isdir() check to pass incorrectly. Reject early with a clear error in that case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
35c540d0e0 |
fix(reconciler): prefer config.json over state manager for enabled mismatch (#353)
* fix(reconciler): prefer config.json over state manager for enabled mismatch When the enabled state in config.json and plugin_state.json diverged, the reconciler was syncing config.json to match plugin_state.json (state manager wins). This silently disabled plugins on every restart whenever the state file had an outdated enabled=false entry — most commonly after an uninstall+reinstall cycle, where the reinstall left the plugin in the installed-but-not-enabled state while config.json still had enabled=true. Flip the sync direction: update plugin_state.json via set_plugin_enabled() to match config.json instead. config.json is the user-editable source of truth; the state file is an internal tracker that should follow it when they disagree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(reconciler): return set_plugin_enabled result instead of always True Capture the boolean returned by set_plugin_enabled() and propagate it so reconciliation accurately reflects failure to update the state manager. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
ea95f37d73 |
fix(reconciler): add sync, github, youtube to _SYSTEM_CONFIG_KEYS (#351)
config_manager.load_config() deep-merges config_secrets.json into the main config before returning it. This means secrets top-level keys (github, youtube) appear alongside structural config keys (sync) in the dict that _get_config_state() iterates. _SYSTEM_CONFIG_KEYS was missing all three, so the reconciler treated them as plugin IDs and flagged them as PLUGIN_MISSING_ON_DISK on every startup, showing the "Stale plugin config entries found" warning banner to users on a fresh install where those plugins have never existed. Add the three keys with brief comments explaining which file each comes from so the distinction is clear when the list grows. Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
321a87f734 |
fix(wifi): fix AP mode, captive portal, and WiFi connect flow (#348)
* fix(wifi): fix AP mode, captive portal, and WiFi connect flow - Fix scan API returning 500: scan_networks() returns a tuple but the endpoint was iterating it directly; unpack with _was_cached - Fix IP address display showing 'IP4.ADDRESS[1]:x.x.x.x': nmcli -t output includes the field label; split on ':' before '/' - Add force parameter to enable_ap_mode() to bypass WiFi/Ethernet guards; expose via force JSON body field in the AP enable endpoint - Fix daemon auto-disabling forced AP: add _FORCE_AP_FLAG_PATH flag file written on force-enable and checked in check_and_manage_ap_mode before auto-disabling; disable_ap_mode() clears it - Fix wifi_connected false positive in AP mode: _get_status_nmcli() was reporting wlan0 as 'connected' when it was running as AP; override wifi_connected=False when _is_ap_mode_active() is True - Fix AP verification failure on async NM activation: retry _get_ap_status_nmcli() up to 5 times with 2s delay instead of single immediate check - Fix WiFi connect ignoring existing NM connections: nmcli does not support 802-11-wireless.ssid as a column in 'connection show'; replace with NAME,TYPE list then per-connection SSID query via -g (fixes 'netplan generate failed' error on Trixie / netplan systems) - Fix failsafe AP re-enable blocked by Ethernet: all recovery-path enable_ap_mode() calls in connect_to_network() now pass force=True Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): strict bool parsing for force; nosec annotation parity - api_v3.py: replace bool(...) coercion for force with strict check — only actual boolean True or strings "true"/"1" (case-insensitive) pass; "false", integers, and other strings are treated as False so the Ethernet/WiFi guards and _FORCE_AP_FLAG_PATH cannot be bypassed by accident - wifi_manager.py: add nosec B108 annotation to _IP_FORWARD_SAVE_PATH to match the identical annotation already on _FORCE_AP_FLAG_PATH Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): suppress false-positive Bandit B603/B607 on new nmcli calls Both subprocess.run calls in the SSID connection lookup use fixed arguments (no user input) or values derived from nmcli's own output — not from user-controlled data. Add nosec B603 B607 annotations to silence the Codacy/Bandit warnings, consistent with existing nosec usage in the file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wifi): address four review findings in wifi_manager.py IP parsing (line 476): use partition(':') so bare "ip/mask" lines (no field-label prefix) are handled without IndexError; falls back to the full string when no ':' is present before splitting on '/'. AP-mode override comment (line 503): add one-line explanation above the wifi_connected/ssid/ip_address clear so maintainers know why the fields are reset while wlan0 reports as "connected". Stale force-flag cleanup (__init__): remove a left-over _FORCE_AP_FLAG_PATH from a prior crash on first instantiation per process (guarded by class-level _startup_cleanup_done so the nmcli AP-state check only runs once, not on every per-request instantiation). Force-flag logging (enable_ap_mode): log at debug when force=True is applied, log success at debug and failure with OSError details at warning for both the hostapd and nmcli hotspot paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Chuck <chuck@example.com> |
||
|
|
713539e491 |
fix(web-ui): fix quick actions not firing, add toast feedback, suppress install handler warning (#346)
* fix(web-ui): fix quick actions not firing, add toast feedback, suppress install handler warning - base.html: add htmx:afterSettle listener to set data-loaded on tab containers after HTMX swaps their content, preventing the overview partial from being re-fetched (and handlers lost) on every tab switch - base.html: call htmx.process() in loadOverviewDirect/loadPluginsDirect fallbacks so buttons get HTMX handlers even if HTMX finished its initial body scan before the fallback fetch completed - overview.html + index.html (11 buttons): replace event.detail.xhr.responseJSON (undefined in HTMX 1.9.x) with JSON.parse(event.detail.xhr.responseText) so quick action toast notifications actually fire - plugins_manager.js: add guarded htmx:afterSettle listener that only calls attachInstallButtonHandler when #install-plugin-from-url is in the DOM, eliminating the spurious console warning on non-plugin tab loads Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): ensure quick-action toasts always fire even on xhr/parse failure Replace silent catch(e){} in all 11 hx-on:htmx:after-request handlers with a pattern that sets default message/status before the try block and calls showNotification(m,s) unconditionally after it, so a fallback toast is shown whenever xhr is absent or responseText is not valid JSON. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): show error toast on non-JSON 4xx/5xx quick-action responses In the catch block of all 11 hx-on:htmx:after-request handlers, check xhr.status >= 400 and downgrade s to 'error' so a failed action that returns an HTML error page (or other non-JSON body) surfaces as an error toast instead of the optimistic 'success'/'info' default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(web-ui): guard setTimeout fallback for attachInstallButtonHandler The 500ms fallback setTimeout was calling attachInstallButtonHandler() unconditionally even when the plugins partial wasn't in the DOM, causing a spurious console.warn on every page load. Add the same element-existence check already present on the htmx:afterSettle listener. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix backup API 404s, hardware status 500, and HTMX loading race - Add all backup API routes to api_v3.py: preview, list, export, validate, restore (with plugin reinstall), download, delete - Fix PermissionError on /hardware/status: return graceful 200 instead of 500 when the status file is owned by a different user; also fix root cause by writing the file world-readable (0o644) in display_manager - Fix HTMX race: dispatch htmx:ready window event from HTMX onload callback; loadTabContent now waits for that event instead of immediately falling back to direct fetch (eliminating the "HTMX not available" console warning on initial load) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Cancel HTMX fallback timers when htmx:ready fires The 5-second setTimeout fallbacks for plugins and overview were firing before the htmx:ready event arrived, logging spurious warnings. Each timer now self-cancels via htmx:ready so the fallback only triggers when HTMX genuinely fails to load. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review feedback: error leaks, ok:false, htmx:ready coverage - Backup endpoints: replace raw str(e) in user-facing responses with a generic message; full exception still logged via exc_info=True - hardware/status: change ok:null to ok:false for PermissionError and json.JSONDecodeError so the UI's hw.ok===false check triggers correctly - base.html: dispatch htmx:ready from the fallback load path so any deferred listeners fire on CDN-fallback loads too - loadTabContent: also listen for htmx-load-failed so overview/wifi/plugins fall back to direct fetch when HTMX is completely unavailable Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Treat system-managed pip packages as satisfied for dependency marker When a plugin's requirements.txt includes a package installed via the system package manager (dnf/apt), pip fails with 'uninstall-no-record-file' because it can't replace the system-tracked copy. The package is present and functional, but the missing marker caused the install to be retried on every service restart. Detect this specific error pattern: if the only pip failure is uninstall-no-record-file, write the .dependencies_installed marker and log a warning instead of returning False, suppressing the repeated warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix uninstall-no-record-file detection condition The previous check used a string replacement that left 'error:' in the remaining text, causing the condition to always evaluate false. Simplify to a direct substring check: if 'uninstall-no-record-file' appears in pip stderr the affected package is installed at the system level and we write the marker, suppressing the repeated warning on every restart. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Resolve CodeQL security findings in backup API Path traversal (CWE-22): - backup_download: switch from send_file(user-tainted-path) to send_from_directory(_BACKUP_EXPORT_DIR, filename); Flask uses werkzeug safe_join internally which CodeQL recognises as a sanitizer - backup_delete: enumerate the export directory and match by name so entry.unlink() operates on a filesystem-derived Path rather than one constructed from user input; _safe_backup_path still guards first Information exposure through exceptions (CWE-209): - backup_validate: err_msg from validate_backup() can embed exception strings containing temp-file paths; log the detail, return a generic 'Invalid or corrupted backup file' to the client - Other backup endpoints: already fixed (str(e) -> generic message); CodeQL alerts will clear on next scan plugin_loader.py:185 (path traversal): false positive — requirements_file is constructed from plugin_dir returned by find_plugin_directory() (a filesystem scan), not from raw HTTP request input; no change needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix pre-existing information exposure in version and action endpoints - get_system_version (alert #218): replaced str(e) with generic message; exception still logged via logger.error(exc_info=True) - execute_system_action (alert #216): removed str(e) and full traceback.format_exc() from the HTTP response — the full stack trace was being sent directly to clients; replaced with generic message and proper logger.error call Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix remaining GitHub CodeQL security alerts - py/stack-trace-exposure: Remove str(e) and traceback.format_exc() from all HTTP responses across api_v3.py, pages_v3.py, and app.py; replace with generic messages and logger.error(exc_info=True) - py/reflective-xss: Escape partial_name via markupsafe.escape in the load_partial 404 response - py/path-injection: Add regex validation of plugin_id before filesystem use in _load_plugin_config_partial - py/incomplete-url-substring-sanitization: Replace 'github.com' in substring checks with urlparse hostname comparison in store_manager.py - py/clear-text-logging-sensitive-data: Remove football-scoreboard debug prints and sensitive request-body prints from update endpoint - js/bad-tag-filter: Replace script-only regex in BaseWidget.sanitizeValue with DOM-based textContent stripping that removes all HTML - js/incomplete-sanitization: Fix escapeAttr to properly encode &, ", ', <, > using HTML entities instead of backslash escaping - js/prototype-pollution-utility: Add __proto__/constructor/prototype key guards to deepMerge function in plugins_manager.js - app.py error handlers: Always return generic messages; remove debug-mode branches that could expose tracebacks in production Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix three remaining CodeQL path-injection and info-exposure alerts - plugin_loader.py: resolve plugin_dir with strict=True and validate marker_path with relative_to() before any filesystem writes, giving CodeQL the positive sanitization pattern it requires (py/path-injection) - api_v3.py _safe_backup_path: replace substring negative checks with a strict positive regex (^[a-zA-Z0-9][a-zA-Z0-9._-]{0,200}\.zip$) that CodeQL recognises as sanitising the user-supplied filename (py/path-injection) - api_v3.py backup_validate: whitelist known-safe manifest fields before returning JSON, preventing any exception strings captured inside validate_backup() from reaching the HTTP response (py/stack-trace-exposure) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Resolve 29 open CodeQL security alerts across 5 files py/flask-debug (#214): - debug_web_manual.py: read debug mode from LEDMATRIX_FLASK_DEBUG env var instead of hardcoded True py/stack-trace-exposure (#216, #218): - api_v3.py execute_system_action: remove subprocess stdout/stderr from HTTP responses; log via logger instead - api_v3.py get_git_version: validate output matches safe ref format (^[a-zA-Z0-9._-]+$) before including in response - api_v3.py: remove all remaining traceback.format_exc() dead variables and print() debug calls (replaced with logger.debug/warning) py/reflective-xss (#207, #208, #209, #210, #211, #212): - api_v3.py: remove plugin_id from all error/success response messages (uninstall, install, update, health, not-found responses) - pages_v3.py load_partial: return static "Partial not found" message instead of echoing partial_name - pages_v3.py _load_starlark_config_partial: add app_id regex validation, use static error messages instead of f-strings with app_id py/path-injection (#187–#206): - pages_v3.py _load_plugin_config_partial: resolve plugins_base and validate _plugin_dir with relative_to() before all file operations; same for assets metadata directory - pages_v3.py _load_starlark_config_partial: resolve starlark_base and validate schema_file/config_file paths with relative_to() - plugin_loader.py _find_plugin_directory: resolve plugins_dir and validate strategy-2 candidates with relative_to() - plugin_loader.py install_dependencies: resolve plugin_dir first, then construct requirements_file and marker_path from resolved base - plugin_loader.py load_module: resolve plugin_dir with strict=True and validate entry_file with relative_to() before exec_module Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix 15 remaining CodeQL path-injection and stack-trace-exposure alerts Switch from resolve()+relative_to() to os.path.basename() reassignment, which CodeQL recognizes as a path sanitizer that breaks the taint chain. Also remove exception objects from backup_manager validate_backup return strings to eliminate the stack-trace-exposure taint source. Fixes alerts #227, #233, #234, #235, #237, #238, #239, #240, #241, #242, #243, #244, #245, #246, #247. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix broken logger format string and leaked exception in config save error - pages_v3.py: plain string was used instead of %-style substitution, so every manifest-read failure logged the literal "{plugin_id}" - api_v3.py save_main_config: exception message was still leaking through the error response; replace with generic message (consistent with the rest of the CodeQL sweep in this PR) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
327e87f735 |
fix(pi5): auto-detect Pi 5 and force rgbmatrix rebuild when rp1_rio missing (#341)
* fix(pi5): auto-detect Pi 5 and force rgbmatrix rebuild when rp1_rio missing first_time_install.sh: - Detect Pi 5 from /proc/device-tree/model at startup - Step 6 skip logic now also checks hasattr(RGBMatrixOptions(), 'rp1_rio'): if the installed library lacks rp1_rio (built before Pi 5 support was added) the build is forced even when the module is already importable. This is the root cause of mmap errors to 0x3f000000 (Pi 3 bus) on Pi 5 hardware. - After a successful Pi 5 build, verify rp1_rio is present and print a diagnostic with the submodule update command if it's still missing. src/display_manager.py: - rp1_rio warning now names the symptom (mmap to 0x3f000000) and gives the exact fix command so users can act immediately from the log. README.md: - Remove "Pi 5 is unsupported" — Pi 5 is fully supported since the library submodule includes rp1_pio and rp1_rio backends. - Document the forced-rebuild command for users migrating from Pi 4. - Fix gpio_slowdown guidance: Pi 5 PIO mode uses 1–2, not 5. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(install): only append Pi 5 suffix in skip-build message when IS_PI5=1 ${IS_PI5:+...} expands whenever IS_PI5 is set, including when it's "0". Replace with an explicit equality check so the suffix only appears on actual Pi 5 installs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
302ab1da4f |
fix(plugin-config): handle missing type key in oneOf/anyOf schema fields (#344)
* fix(web-ui): dedup registry fetches, surface reconciliation warnings, add check-update endpoint
Story 1 — src/plugin_system/store_manager.py:
Add threading.Lock (_registry_fetch_lock) to fetch_registry(). The outer cache
check remains the hot path (no lock). When the cache is cold, only one thread
hits the network; concurrent callers block on the lock then get the result from
the warm cache (double-checked locking). Eliminates duplicate GitHub requests
on every page load when the 15-minute cache expires.
Story 2 — web_interface/app.py + api_v3.py + overview.html:
_run_startup_reconciliation() now writes /tmp/ledmatrix_reconciliation.json
(atomic tempfile+replace, mirrors hw_status pattern) so the result survives
the background thread. New GET /api/v3/plugins/reconciliation-status reads
that file. Overview page gains a dismissible yellow banner that shows stale
plugin_id values (e.g. sync, github, youtube) and tells the user to remove
them or reinstall from the Plugin Store. Banner is suppressed for the session
after dismiss using sessionStorage keyed on the plugin_id list.
Story 3 — web_interface/blueprints/api_v3.py:
Add GET /api/v3/system/check-update. Does git fetch origin main then compares
local HEAD vs origin/main to compute update_available, remote_sha, and
commits_behind. Result is cached for 5 minutes so it doesn't run git on every
page load. Falls back to {update_available: false} on any error. Eliminates
the 404 logged on every page load.
Story 4 (Pi 5 rgbmatrix rebuild) was already fixed in PR #341.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(plugin-config): handle missing `type` key in schema fields using oneOf/anyOf
Jinja2's `prop.type` on a dict without a `type` key returns an Undefined
object. Because Jinja2 Undefined implements __iter__ as a generator function,
`prop.type is iterable` evaluates True, then `prop.type[0]` calls
Undefined.__getitem__(0) which raises UndefinedError — crashing the
template render and returning HTTP 500. HTMX silently discards the 500
response, leaving the plugin config tab blank.
Fix: use `prop.get('type')` which returns None for missing keys instead of
Undefined. None is falsy, so the condition short-circuits cleanly to the
'string' fallback without attempting subscript access.
Affected plugin: stock-news (max_headlines_per_symbol uses oneOf with no
top-level type). Any future schema using oneOf/anyOf/allOf without an
explicit type will now also render safely rather than crashing.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(security): harden check-update, reconciliation status endpoint, and temp-file write
api_v3.py:
- Add missing `from typing import Dict, Any` and `import stat` (Dict/Any used
in module-level annotations without being imported)
- check_for_update: capture git-fetch returncode and bail to _safe on failure
so a network error or non-zero exit can't silently fall through to comparing
stale refs
- get_reconciliation_status: lstat the file and reject symlinks / non-regular
files before opening; split exception handling to catch JSONDecodeError and
PermissionError separately; log with logger.exception; return a generic
'Status file unavailable' message instead of str(e) to avoid leaking
internal details
overview.html:
- Replace one-shot reconciliation fetch with a polling loop (2 s interval via
setTimeout) so the banner still appears when reconciliation finishes after
the page first loads
- dismissReconciliationBanner: write sessionStorage immediately using the key
stored on the banner element (set at show time) so dismissal persists even
if the background sync fetch fails; clear the polling timer on dismiss to
avoid leaks
app.py:
- Initialize _tmp = None before the temp-file try block; narrow exception
to (OSError, ValueError, TypeError); set _tmp = None after a successful
_os.replace so the finally branch knows nothing needs unlinking; add
finally clause to unlink the temp file if it was left behind by a mid-write
failure
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: reconciliation status errors return graceful not-done instead of HTTP 500; log fetch stderr
get_reconciliation_status: symlink/non-regular-file, JSONDecodeError, and
PermissionError all now return {'done': False, 'unresolved': []} so the
polling loop in overview.html keeps retrying rather than stopping on a
transient error.
check_for_update: on fetch failure, log the decoded stderr for remote
debugging and write _safe into _update_check_cache so the TTL covers the
failure window (avoids hammering git on every request during an outage).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(bandit): replace hardcoded /tmp paths with tempfile.gettempdir() (B108)
Codacy/Bandit B108 flagged two hardcoded '/tmp/' string literals in app.py
(lines 737, 741). Replaced with _tempfile.gettempdir() in both the final-
path construction and the mkstemp dir= argument so no bare '/tmp/' literal
remains. Also updated the matching reader path in api_v3.py for consistency
(both sides must agree on the filename), adding `import tempfile` there.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Chuck <chuck@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
|
33f76b4895 |
feat(pi5): RP1 backend UI, gpio slowdown guidance, and hardware init error banner (#337)
* feat(pi5): expose RP1 backend selector, fix gpio defaults, surface init failures in web UI - Add rp1_rio select (PIO/RIO) to Display Settings hardware config section; saved via /api/v3/config/main with 0-or-1 validation — previously the key existed in config.json but was not editable from the UI - Update gpio_slowdown help text with per-model guidance (Pi 3: 3, Pi 4: 4, Pi 5: 4–5) and raise max from 5 → 10 to match full library range - Fix gpio_slowdown Python fallback default from 2 → 3 (only affects edge case where the runtime config section is absent; explicit config values are unchanged) - display_manager writes /tmp/led_matrix_hw_status.json at startup: ok/error; Display Settings page fetches it and shows a yellow warning banner when the matrix failed to initialize, including Pi 5 remediation steps - Add GET /api/v3/hardware/status endpoint that reads the status file - Improve fallback error log to include Pi 5 rebuild hint Pi 3/4 users: rp1_rio=0 is set in config but silently ignored by the library on non-RP1 hardware; all other changes are additive or tighten defaults only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pi5): correct gpio_slowdown guidance — Pi 5 PIO default is 1, not 4-5 The upstream library defaults gpio_slowdown to 1 for Pi 5 (IsPi4() ? 2 : 1). In PIO mode the value is a pixel-clock divisor, so 4-5 was unnecessarily conservative advice. Updated help text and error log to reflect the actual range (1-3 typical for Pi 5 PIO; inverted effect in RIO mode). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): atomic hw-status write, narrow bare excepts, urllib3 CVE floor - display_manager: replace open()+bare-except with tempfile.mkstemp→fsync→ chmod(0o600)→os.replace; adds symlink guard and logs errors via logger instead of swallowing them silently; pull json/tempfile to module imports - display_manager cleanup(): narrow broad `except Exception: pass` to (OSError, RuntimeError, ValueError, MemoryError) with debug log - api_v3 get_hardware_status(): catch json.JSONDecodeError and PermissionError explicitly; log full traceback server-side; return generic "Unable to read hardware status" to client instead of leaking str(e) - march-madness/requirements.txt: bump urllib3 floor 2.2.2→2.6.3 (CVE fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(template): apply |int filter to rp1_rio comparisons in display.html Without |int, a string-typed value (e.g. from a hand-edited config.json) causes both selected tests to fail and the select renders with no option pre-selected. Matches the existing pattern used for multiplexing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
c6b79e11d5 |
fix: Codacy round-2 — urllib3 CVEs, missed JS/Python issues (#336)
urllib3 CVEs (10 Trivy findings):
plugin-repos/march-madness/requirements.txt: bump urllib3>=1.26.0 to
>=2.2.2 to address CVE-2021-33503, CVE-2023-43804, CVE-2023-45803,
CVE-2024-37891, and 2025-2026 decompression/redirect CVEs.
Missed code fixes from round-1:
display_helper.py: remove unused draw=ImageDraw.Draw(img) — the method
delegates to _draw_centered_text which creates its own draw context.
custom-feeds.js:334: one bare removeCustomFeedRow(this) was missed by
the earlier replace_all; changed to window.removeCustomFeedRow(this).
app.js: add htmx to /* global */ declaration — htmx.ajax() is called
at lines 146 and 172 but htmx was only declared in the extension files.
timezone-selector.js:215: second unused catch (e) → catch {} missed
when we fixed line 361 in round-1.
Bandit B110 annotations (3 new except/pass blocks from newer PRs):
start.py: hostname -I IP parsing — non-critical startup info.
display_controller.py: scroll_helper.get_portion_at — optional method.
display_manager.py: canvas reset during cleanup — best-effort.
41 confirmed false positives suppressed via Codacy API:
35x pyflakes in test/, plugin-repos/, scripts/ — not production code
Flask 0.0.0.0, os.execvp, Bandit B603, vendor ESLint, already-fixed
Biome noPrototypeBuiltins.
Co-authored-by: Chuck <chuck@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
|
054ad78d7b |
chore(deps): update rpi-rgb-led-matrix to latest upstream for Pi 5 support (#334)
* chore(deps): update rpi-rgb-led-matrix to latest upstream for Pi 5 support Configure submodule to track upstream master branch (branch = master in .gitmodules) so future updates are a single 'git submodule update --remote' rather than manual SHA management. Update first_time_install.sh to use --remote flag so fresh installs always pull the current upstream master, not the commit recorded at clone time. Current upstream HEAD (8907235) brings: - PR #1886: Raspberry Pi 5 support — new RP1 PIO and RIO backends. The library auto-detects Pi 5 hardware at runtime; no config change required for basic operation. adafruit-hat-pwm is confirmed supported on Pi 5. - PR #1833: setup.py migrated from distutils → setuptools, fixing Python 3.12+ build failure (Pi runs Python 3.13). Previous version could not build the bindings at all on current Pi OS. Expose new rp1_rio option in display_manager.py and config.template.json: 0 (default) = PIO mode — uses Pi 5 RP1 coprocessor, minimal CPU usage 1 = RIO mode — Registered IO, faster throughput, higher CPU; note that gpio_slowdown has inverted effect in this mode No API changes to RGBMatrix, RGBMatrixOptions, or FrameCanvas. Pi 4 and earlier hardware is unaffected — rp1_rio is silently ignored on non-Pi-5. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(deps): update rpi-rgb-led-matrix install for new scikit-build-core system The library migrated from 'make build-python' + 'pip install bindings/python' to a scikit-build-core + cmake build where the entire repo root is pip- installable via 'pip install .'. Update first_time_install.sh accordingly: - Remove the 'make build-python' step (target no longer exists) - Install directly from the repo root instead of bindings/python - Replace build deps: remove cython3/scons/python3-dev, add python-dev-is-python3 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: deterministic submodule install + guard rp1_rio for older rgbmatrix first_time_install.sh: remove --remote from both git submodule update calls so first-time installs check out the pinned commit recorded in the repo rather than whatever upstream master happens to be at install time. The branch = master config in .gitmodules reserves --remote for an explicit maintainer upgrade (git submodule update --remote). display_manager.py: guard rp1_rio assignment with hasattr() so setting the option in config does not cause an AttributeError and silently fall through to emulator mode when running against RGBMatrixEmulator or an older rgbmatrix build that predates the Pi 5 property. Emit a warning instead so the operator knows the value was ignored. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
05b3fa56cb |
fix: Codacy security fixes, CVE dependency bumps, and code quality cleanup (#331)
* fix(deps): bump minimum versions to address CVEs Pillow 10.4.0 → 12.2.0: CVE-2026-40192 (DoS via FITS decompression bomb), CVE-2026-25990 (OOB write via PSD image), CVE-2026-42311/42308/42310 requests 2.32.0 → 2.33.0: CVE-2026-25645 (temp file security bypass), CVE-2024-47081 (.netrc credentials leak) werkzeug 3.0.0 → 3.1.6: CVE-2023-46136, CVE-2024-49766/49767, CVE-2025-66221, CVE-2026-21860/27199 (DoS, path traversal, safe_join bypass) Flask 3.0.0 → 3.1.3: CVE-2026-27205 (session data caching info disclosure) spotipy 2.24.0 → 2.25.2: CVE-2025-27154, CVE-2025-66040 python-socketio 5.11.0 → 5.14.0: CVE-2025-61765 pytest 7.4.0 → 9.0.3: CVE-2025-71176 (insecure temp dir handling) Updated in requirements.txt, web_interface/requirements.txt, plugin-repos/starlark-apps/requirements.txt, and plugin-repos/march-madness/requirements.txt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve Pylint errors in executor, data service, and odds call Rename TimeoutError to PluginTimeoutError in plugin_executor.py to avoid shadowing the built-in; no external callers affected. Remove dead try/except in BackgroundDataService.shutdown: executor.shutdown() never accepted a timeout kwarg so the try branch always raised TypeError. Simplify to a direct shutdown(wait=wait) call. Remove is_live kwarg from odds_manager.get_odds() call in sports.py; BaseOddsManager.get_odds() has no such parameter. The live update interval is already encoded in the update_interval_seconds argument passed alongside. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: MD5→SHA-256, shellcheck warnings, and broken doc links config_service.py: replace MD5 with SHA-256 for config change detection; same semantics (equality comparison), no stored hashes affected. Shell scripts — shellcheck warnings: - diagnose_web_interface.sh: remove useless cat (SC2002) - dev_plugin_setup.sh: restructure A&&B||C into if/then (SC2015) - fix_assets_permissions.sh: remove unused REAL_HOME block (SC2034) - install_web_service.sh: remove unused USER_HOME assignment (SC2034) - diagnose_web_ui.sh: remove unused SUDO assignments (SC2034) - diagnose_plugin_permissions.sh: remove unused BLUE color var (SC2034) - first_time_install.sh: remove unused CLEAR var, PACKAGE_NAME assignment, and replace loop variable with _ (SC2034) docs/PLUGIN_ARCHITECTURE_SPEC.md: fix 10 broken TOC anchor links to include section numbers matching the actual headings (MD051). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unused imports and bare exception aliases (pyflakes F401/F841) Remove unused imports across 86 files in src/, web_interface/, test/, and scripts/ using autoflake. No logic changes — only dead import statements and unused names in from-imports are removed. Also remove bare exception aliases where the variable is never referenced in the handler body: - src/cache/disk_cache.py: except (IOError, OSError, PermissionError) as e - src/cache_manager.py: except (OSError, IOError, PermissionError) as perm_error - src/plugin_system/resource_monitor.py: except Exception as e - web_interface/app.py: except Exception as read_err 86 files changed, 205 lines removed, 18 pre-existing test failures unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unused local variable assignments (pyflakes F841) Dead assignments removed across src/ and web_interface/: - background_data_service: drop future= on fire-and-forget executor.submit - base_classes/baseball: drop font= (all rendering uses self.fonts['time']) - base_classes/hockey: drop status_short= (never referenced after assignment) - common/cli: drop game_helper=/config_helper= bindings in import-test block; constructors called for instantiation-only validation - common/display_helper: drop text_width= (x_position uses display_width directly); drop draw= in create_error_image (uses _draw_centered_text) - config_manager: remove dead secrets_content loading block in migration path (comment already noted save_config_atomic handles secrets internally) - display_manager: drop setup_start= (timing was never completed or read) - font_manager: drop target_path= (catalog uses font_file_path directly); drop face=/font= bindings in validate_font (validation by construction — TypeError on failure is the signal, not the return value) - font_test_manager: drop width=/height= (draw_text uses display_manager directly) - plugin_system/state_reconciliation: drop manager= (only config/disk/state_mgr used) - plugin_system/store_manager: drop result= on pip install subprocess.run (check=True raises on failure; stdout unused) - web_interface/blueprints/pages_v3: drop main_config_path=""/secrets_config_path="" (render_template uses config_manager.get_*_path() inline) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(js): resolve ESLint no-undef warnings across 6 JS files Three distinct patterns: 1. Vendor library globals — htmx is injected by <script> before these extension files load; ESLint lints files in isolation and doesn't know. Fix: add /* global htmx */ to htmx-sse.js and htmx-json-enc.js. 2. Cross-file globals — showNotification is defined as window.showNotification in app.js/notification.js but called bare in app.js and error_handler.js. ESLint doesn't connect window.X = Y with a bare call to X. Fix: add /* global showNotification */ to app.js and error_handler.js. 3. Forward-reference window.* functions — in array-table.js, checkbox-group.js, and custom-feeds.js, functions like removeArrayTableRow are called early inside event-handler closures but assigned to window.* later in the file. At runtime this works (the handler fires after the assignment), but ESLint sees the bare name at the call site. Fix: change bare calls to window.removeArrayTableRow(this) etc. so the reference is explicit and ESLint-safe. Also guard the updateSystemStats call in app.js reconnectSSE: the function is called but defined nowhere in the codebase. Guard with typeof check so it won't throw ReferenceError if the reconnect path is hit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(js): resolve Biome lint warnings across 9 JS files noUnusedVariables (catch bindings → optional catch syntax): - app.js, file-upload.js, timezone-selector.js: } catch (e) { → } catch { ES2019 optional catch binding; e was unused in all three handlers noUnusedVariables (dead assignments): - app.js: remove const data= in display SSE stub (handler does nothing yet) - api_client.js: remove const timeoutId= (setTimeout ID never used to cancel) - custom-feeds.js: remove const oldIndex= (getAttribute result never read) - schedule-picker.js: remove const compactMode= (never used in HTML build) - select-dropdown.js: remove const icons= (icons not yet rendered in options) noPrototypeBuiltins: - day-selector.js: DAY_LABELS.hasOwnProperty(x) → Object.prototype.hasOwnProperty.call(DAY_LABELS, x) Safe form that works even on null-prototype objects useIterableCallbackReturn: - file-upload.js, notification.js: forEach(x => expr) → forEach(x => { expr; }) — forEach ignores return values; implicit return from arrow body was misleading htmx-sse.js is a vendor extension file with old-style var/== patterns that are correct for it; 18 Biome issues suppressed via Codacy API rather than modifying the vendor source. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): escape user input in raw HTML responses in pages_v3.py plugin_id comes directly from the URL path (/partials/plugin-config/<plugin_id>) and was interpolated into an HTML fragment without escaping. A crafted URL like /partials/plugin-config/<script>alert(1)</script> would inject that tag into the DOM via the HTMX partial response. Fix: wrap all user-controlled values in markupsafe.escape() before embedding in raw HTML strings. Affects the plugin-not-found 404 response and both error 500 responses in the plugin config partial. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Bandit B108/B110 across production code B110 (try/except/pass): - display_controller.py: narrow 'except Exception' to 'except AttributeError' for get_offset_frame() — plugins not having this optional method is the expected case, not all exceptions - config_manager.py: B110 already resolved by the earlier removal of the dead secrets-loading block (the except/pass was inside it) - All other except/pass blocks in src/ and web_interface/ are intentional (last-resort recovery, best-effort fallbacks, non-critical startup probes). Annotated each with # nosec B110 and a brief inline reason so the decision is explicit for future reviewers. - Test files and plugin-repos B110 suppressed via Codacy API (not prod code). B108 (/tmp usage): - permission_utils.py: /tmp listed to PREVENT permission changes on it — not used as a temp path. Annotated # nosec B108. - display_manager.py: fixed snapshot path is intentional (web UI reads same path); path-check guard also annotated. - wifi_manager.py: named /tmp files match the sudoers allowlist installed with the system (the paths are hard-coded in both places by design). Annotated all six open/cp references # nosec B108. - scripts/render_plugin.py: dev script default overridable by user. Annotated. - web_interface/app.py: reads the same fixed path written by display_manager. Annotated # nosec B108. - Test files suppressed via Codacy API. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address remaining Codacy security findings Flask debug=True (real fix): - web_interface/app.py: debug=True in __main__ block exposes the Werkzeug interactive debugger (arbitrary code execution). Changed to os.environ.get('FLASK_DEBUG', '0') == '1' — off by default, opt-in via environment variable for local development. nosec annotations (accepted risk with documented rationale): - disk_cache.py: os.chmod(0o660) is intentional — web UI and LED matrix service share a group, 660 gives group write while denying world access (B103 + Semgrep insecure-file-permissions suppressed in Codacy) - wifi_manager.py: urlopen to hardcoded connectivity-check.ubuntu.com URL (B310 — no user input involved) - font_manager.py: urlretrieve URL comes from user's own config file on their local device (B310) - start_web_conditionally.py: os.execvp with both sys.executable and a fixed PROJECT_DIR-relative constant (B606) Confirmed false positives suppressed via Codacy API (15 issues): - SSRF (3x): client-side JS fetch — SSRF is server-side; browser fetch is CORS-restricted to same origin - B105 (3x): test fixtures use dummy secrets by design; store_manager checks for the placeholder string, it is not itself a secret - PMD numeric literal (2x): 10000000 is within Number.MAX_SAFE_INTEGER - Prototype pollution (1x): read-only schema traversal, no writes - no-unsanitized_method (1x): dynamic import() is CORS-restricted - detect-unsafe-regex (1x): operates on server-controlled config values - plugin-repos B103 (1x): vendor code chmod on executable - Semgrep insecure-file-permissions (3x): same disk_cache 0o660 as above Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove unnecessary f prefix from f-strings without placeholders (F541) Pyflakes F541 flags f-strings that contain no {} interpolation — they are identical to plain strings but trigger unnecessary string formatting overhead. Fixed in production code: - src/base_classes/data_sources.py (2 debug log calls) - src/logo_downloader.py (1 error log) - src/plugin_system/store_manager.py (5 strings across 3 log calls) - src/web_interface/validators.py (1 return value) - src/wifi_manager.py (4 log/message strings) - web_interface/start.py (1 print) F541 issues in test/, scripts/, and plugin-repos/ suppressed via Codacy API as non-production code. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(dev): add Pillow compatibility smoke test script Covers all Pillow APIs used in LEDMatrix — image creation, drawing, font metrics, LANCZOS resampling, paste/alpha_composite, and PNG I/O. Run after any Pillow version bump to catch regressions before deploy. python3 scripts/dev/test_pillow_compat.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: resolve 8 new Codacy issues introduced by PR changes shellcheck SC2034: - first_time_install.sh: 'type' loop variable also unused in the wifi status loop (we previously fixed 'device' → '_' but left 'type'). Changed to '_ _ state' since neither device nor type is referenced. ESLint no-undef: - app.js: typeof guards don't satisfy no-undef; added updateSystemStats to the /* global */ declaration alongside showNotification. nosec annotation: - web_interface/app.py: app.run(host='0.0.0.0') line changed when we fixed debug=True, giving it a new issue ID. Re-added # nosec B104. pyflakes F401: - scripts/dev/test_pillow_compat.py: ImageFilter was imported but never used in the smoke test. Removed from the import. Codacy API suppressions (false positives on changed lines): - disk_cache.py 0o660 chmod (2x): lines changed when # nosec B103 was added, producing new Semgrep issue IDs. Re-suppressed. - pages_v3.py raw-html-concat: Semgrep does not recognise escape() as a sanitizer; the escape() call IS the correct fix. - app.py flask 0.0.0.0: same line as B104 above; Semgrep rule also re-suppressed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address PR review findings Fix (10 of 15 findings): plugin-repos/march-madness/requirements.txt: Add urllib3>=1.26.0 — manager.py directly imports from urllib3; it was an undeclared transitive dependency via requests. scripts/dev/dev_plugin_setup.sh: Restore subshell form (cd "$target_dir" && git pull --rebase) || true so the shell's working directory is not permanently changed after the if-cd block. Previous fix for SC2015 leaked cwd into the remainder of the script. src/base_classes/sports.py: Narrow 'except Exception' to 'except RuntimeError as e' and log via self.logger.debug — Path.home() raises only RuntimeError for service users; other exceptions should not be silently swallowed. src/config_service.py: Fix stale "MD5 checksum" in ConfigVersion.__init__ docstring (line 40); the implementation uses SHA-256 since the Codacy fix. src/wifi_manager.py: Log the last-resort AP enable failure with exc_info=True instead of silently passing — failure here means the device may be unreachable. web_interface/blueprints/pages_v3.py: Log the outer metadata pre-load exception at debug level instead of swallowing it silently; schema still loads fully below. src/background_data_service.py: Remove unused 'timeout' parameter from shutdown() — executor.shutdown() does not accept timeout; update __del__ caller accordingly. src/font_manager.py: Validate URL scheme before urlretrieve — reject non-http/https schemes (e.g. file://) to prevent reading local files from config-supplied URLs. src/plugin_system/plugin_executor.py: Simplify redundant except tuple: (PluginTimeoutError, PluginError, Exception) → Exception, which already covers the others. test/test_display_controller.py: Mark empty test_plugin_discovery_and_loading as @pytest.mark.skip with reason. Move duplicate 'from datetime import datetime' to module header and remove the stray mid-module copy. Skip (5 of 15 findings, with reasons): - pytest 9.0.3 concerns: full suite already verified (467 pass, 18 pre-existing) - Pillow 12.2.0 API concerns: no deprecated APIs in codebase; tests + Pi smoke test pass - diagnose_web_ui.sh sudo validation: set -e already ensures fail-fast on any sudo failure - app.py request-logging except: must stay silent (recursive logging risk); annotated - app.py SSE file-read except: genuinely transient I/O; annotated Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
44d1a08db4 |
perf(plugins): dramatically speed up plugin manager tab load time (#333)
* fix(cache): check odds keys before generic live check in get_data_type_from_key Cache keys like odds_espn_basketball_nba_<id>_live contain both 'odds' and 'live'. The previous ordering matched the generic 'live' check first, returning 'sports_live' (30 s TTL) instead of the correct 'odds_live' (120 s TTL). This caused the ESPN odds API to be hit every 30 s per live game, frequently triggering the 3-second per-request timeout and returning no odds data. Moving the 'odds' check above the generic 'live' block restores the correct 120-second cache TTL for in-progress game odds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(display): use single-quoted HTML attributes for JSON hidden inputs Placing |tojson output (which contains double quotes) inside a double-quoted HTML attribute broke the attribute — browsers closed the attribute at the first inner quote, leaving JS with an empty or truncated value. JSON.parse then failed silently, leaving excluded=[] so all Vegas scroll plugins appeared checked (included) regardless of the actual excluded_plugins config. Switch to single-quoted HTML attributes so the JSON double quotes are valid inside the attribute value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * perf(plugins): dramatically speed up plugin manager tab load time ## Problem The Plugins tab loaded slowly and inconsistently (5–30s depending on cache state), with a blank spinner for the entire wait. Three root causes: 1. **N+1 subprocess per installed plugin** — `_get_local_git_info` ran 4 separate git subprocesses per plugin (rev-parse HEAD, abbrev-ref, config --get remote.origin.url, log --format=%cI). With 15 plugins that's 60 blocking subprocess spawns before the endpoint returned. 2. **Serial per-plugin loop** — the `/plugins/installed` endpoint processed each plugin sequentially: manifest read → git info → instance lookup → Vegas mode query, one plugin at a time. 3. **Serial JS loading** — the store search only started after installed plugins fully completed, so users waited for both round-trips back to back. No UI feedback during the wait. ## Changes ### Backend — src/plugin_system/store_manager.py - Consolidate 4 git subprocesses → 1: branch read from `.git/HEAD` (file I/O, no subprocess), remote URL parsed from `.git/config` (file I/O, no subprocess), SHA + commit date fetched together in a single `git log -1 --format=%H%n%cI` call - Existing signature-based cache already eliminates all subprocesses on warm hits; this change cuts cold-cache cost from 4 → 1 per plugin ### Backend — web_interface/blueprints/api_v3.py - Wrap per-plugin work in a `_build_plugin_entry()` helper and execute it across a `ThreadPoolExecutor(max_workers=8)` so all plugins are processed in parallel instead of sequentially - Fix double `get_plugin()` call per plugin (was called once for the enabled fallback and again for Vegas mode — now one shared call) ### Frontend — web_interface/static/v3/plugins_manager.js - Fire `searchPluginStore()` and `loadInstalledPlugins()` simultaneously instead of waiting for installed to complete before starting the store - After installed data arrives, call `applyStoreFiltersAndSort(true)` to refresh install/update/reinstall badges from already-cached store data (instant, no extra network call) ### Frontend — web_interface/templates/v3/partials/plugins.html - Add responsive skeleton cards to the installed plugins section that match real card proportions (removed automatically when data renders) - Replace the 5 featureless gray boxes in the store skeleton with 10 structured skeleton cards matching the real card layout ## Measured improvement on Pi 4 (11 installed plugins, ledpi-ticker) | Scenario | Before | After | |---|---|---| | Cold cache (first open) | ~8–15s | **0.9s** | | Warm cache (git cache hit) | ~1–2s | **55ms** | | UI feedback during load | blank spinner | skeleton cards | | Store waits for installed | yes (serial) | no (parallel) | Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(plugins): harden git metadata parsing and plugin entry building store_manager.py: - Detect worktree/submodule .git files (gitdir: <path>) and resolve to the actual git directory before reading HEAD or config - Wrap HEAD read_text in try/except OSError/NotADirectoryError so atypical repos return None instead of propagating exceptions - Guard config url line split with '=' presence check to avoid IndexError on malformed lines api_v3.py: - Wrap _build_plugin_entry body in a try/except via a thin outer wrapper so a single plugin's failure doesn't 500 the whole endpoint; failed entries return None and are filtered by the existing [r for r in results if r is not None] step - Narrow manifest except clause to FileNotFoundError, PermissionError, json.JSONDecodeError instead of bare Exception - Validate manifest is a dict before calling plugin_info.update() and log a debug message when it isn't Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
6a4644007d |
fix(display): Vegas excluded plugins always showing as checked (#332)
* fix(cache): check odds keys before generic live check in get_data_type_from_key Cache keys like odds_espn_basketball_nba_<id>_live contain both 'odds' and 'live'. The previous ordering matched the generic 'live' check first, returning 'sports_live' (30 s TTL) instead of the correct 'odds_live' (120 s TTL). This caused the ESPN odds API to be hit every 30 s per live game, frequently triggering the 3-second per-request timeout and returning no odds data. Moving the 'odds' check above the generic 'live' block restores the correct 120-second cache TTL for in-progress game odds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(display): use single-quoted HTML attributes for JSON hidden inputs Placing |tojson output (which contains double quotes) inside a double-quoted HTML attribute broke the attribute — browsers closed the attribute at the first inner quote, leaving JS with an empty or truncated value. JSON.parse then failed silently, leaving excluded=[] so all Vegas scroll plugins appeared checked (included) regardless of the actual excluded_plugins config. Switch to single-quoted HTML attributes so the JSON double quotes are valid inside the attribute value. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
1c4d5c5271 |
feat(sync): multi-display wireless sync — extend scrolling across two LED matrices (#330)
* feat(sync): multi-display wireless sync — extend scrolling across two LED matrices Adds a leader/follower sync system that extends Vegas scroll mode content continuously across two physically adjacent LED matrix units over WiFi. Architecture: - Leader broadcasts scroll position via UDP at ~90fps; follower renders the offset slice of the same image at 60fps using dead reckoning to absorb UDP jitter (smooth, stutter-free motion) - At each cycle transition the leader sends the composed scroll image via TCP (PNG-compressed ~15–40KB) so both displays render pixel-identical content regardless of plugin data timing differences - Auto-discovery via UDP subnet broadcast — no IP configuration required - Heartbeat watchdog (6s timeout) falls back to standalone if peer goes offline Key files: - src/common/sync_manager.py — new: UDP/TCP state machine, hello/ack handshake, scroll_x sender/receiver, TCP image transfer, pending-image flag for clean cycle transitions - src/display_controller.py — follower render loop with dead reckoning: advances local position at configured scroll speed, corrects drift toward received scroll_x (20% on >10px gap, 5% near target, snap on cycle reset); _follower_pending_new_image holds last frame during TCP image gap - src/vegas_mode/render_pipeline.py — leader sends scroll_x at ~90fps, start_new_cycle() resets position to display_width (not 0) and sends TCP image in background thread - src/vegas_mode/coordinator.py — set_sync_manager() / set_update_callback() wiring; defers hot-swap recompose while sync is active - web_interface/blueprints/api_v3.py — sync config save endpoint, GET /api/v3/sync/status for live status polling - web_interface/templates/v3/partials/display.html — Multi-Display Sync section: role selector (Standalone/Leader/Follower), position (Left/Right of leader, follower only), UDP port, live status indicator - config/config.template.json — sync block: role, port, follower_position Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address PR review findings - sync_manager: replace Optional[callable] with proper Callable types from typing; tighten set_on_new_cycle/set_on_scroll_image/set_on_follower_connected signatures to match their actual callback signatures - sync_manager: log a one-shot warning when send_frame produces a packet exceeding the 65000-byte UDP cap instead of silently dropping it - display_controller: correct stale comment in _send_follower_frame (was "30fps / PNG encode/decode"; actual behavior is ~90fps raw RGB) - display.html: guard setInterval with window.syncStatusInterval to prevent duplicate pollers if the script runs more than once - display.html: replace innerHTML with DOM node creation + textContent for status icon/text to avoid inserting API-derived values via innerHTML Skip: time.time() → monotonic and self.config staleness are pre-existing issues not introduced by this PR. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address second round of PR review findings - sync_manager: guard TCP image receive against OOM — validate length against 10 MB cap before allocating; log and close on invalid length - display_controller: _follower_gated_update now allows update_display() through when the leader is offline (is_follower_active() == False) so the display recovers normally when falling back to standalone mode - coordinator: normalize a standalone SyncManager to None in set_sync_manager() so the render pipeline never treats a no-op manager as an active one - coordinator: derive _UPDATE_TICK_FRAMES from target_fps * 4 instead of the hardcoded 500 so the ~4s cadence holds at any configured FPS - render_pipeline: replace bare except/pass on blank-frame push with logger.exception() so failures are visible in logs Skip: config.template.json comments — JSON does not support inline comments. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address third round of PR review findings - sync_manager: use 'with socket.socket(...)' in send_scroll_image so the TCP socket is always closed even if connect/sendall raises - sync_manager: add _scroll_image_lock to serialize all reads/writes to _on_scroll_image and _pending_scroll_image between _image_server_loop and set_on_scroll_image, eliminating the lost-delivery race; callback is invoked outside the lock to avoid holding it during user code - sync_manager: validate scroll image dimensions (max 100000×256) and catch DecompressionBombError before img.load() in _image_server_loop - sync_manager: log socket close exceptions at debug level in stop() instead of silently passing - sync_manager: replace hardcoded /tmp/ with tempfile.gettempdir() for STATUS_FILE (atomic write was already in place) - sync_manager: check _RAW_MAGIC first in _follower_recv_loop routing so magic-tagged frames are always identified correctly regardless of size Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): address fourth round of PR review findings - sync_manager: log INCOMPATIBLE error only on state transition (guard with prev_state != LeaderState.INCOMPATIBLE) so repeated hello packets from an incompatible follower don't spam the log - sync_manager: replace O(n²) bytes concatenation in TCP image receive loop with bytearray + extend() for linear-time accumulation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): suppress Codacy false positives - display_controller: rename local var 'sh' to 'scroll_h' so Codacy's pattern matcher doesn't confuse it with the 'sh' shell library - sync_manager: add '# nosec B104' to all socket.bind("") calls — binding to all interfaces is intentional (UDP broadcast reception and TCP image server must accept connections from any local interface) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(sync): add nosec B104 to socket creation lines for Codacy Codacy attributes the bind-to-all-interfaces finding to the socket.socket() creation lines (140, 439) rather than the .bind() calls. Added # nosec B104 there too so the suppression is seen at the line Codacy reports. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
dbb53da31d |
fix(vegas): eliminate plugin re-appearance at scroll cycle boundaries (#327)
* fix(vegas): eliminate plugin re-appearance at scroll cycle boundaries The Vegas scroll image is wider than the display. scroll_helper marks a cycle complete only after total_distance_scrolled >= total_scroll_width + display_width, meaning it keeps scrolling for an extra display_width of pixels after all content has exited left. During that extra travel the scroll_position wraps back to ~0 and the first plugin re-enters from the right - visible for ~2-3 seconds as a plugin partially displaying before the next one starts. render_pipeline.render_frame(): end the cycle the moment total_distance_scrolled >= total_scroll_width (the natural wrap point), before any second-pass content becomes visible. Push a blank frame immediately on detection so hardware never shows a frozen content snapshot while start_new_cycle() recomposes (~100 ms). display_manager.py: add capture_mode() context manager. When active, update_display() and the canvas clear in clear() skip the hardware write, preventing plugins that call update_display() internally from flashing on the matrix during off-screen content capture inside start_new_cycle(). plugin_adapter.py: wrap all plugin.display() calls in _capture_display_content() and _trigger_scroll_content_generation() with capture_mode() so the fallback capture path never produces hardware output. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(vegas): tighten exception handling in clear() and blank-frame push display_manager.clear(): replace bare except/pass on the three hardware Clear() calls with (RuntimeError, OSError) and a logger.error() so failures are visible in logs rather than silently swallowed. Still non-fatal — the PIL image buffer is already black before these calls, so the next update_display() will push clean content regardless. render_pipeline.render_frame(): replace broad except/pass in the blank-frame push with (ImportError, ValueError, TypeError, MemoryError) and a logger.error() that includes display dimensions for context. update_display() already handles its own hardware errors internally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(vegas): catch OSError and RuntimeError in blank-frame push Image.new() can raise OSError in some PIL environments and hardware libraries may surface RuntimeError on I/O failures. Add both to the exception tuple alongside the existing ImportError/ValueError/TypeError/ MemoryError so no boundary failure escapes the local handler. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
|
1a0f1c8015 |
fix: service control buttons and AP-mode SSH lockout post-install (#326)
* fix: service control buttons and AP-mode SSH lockout post-install
Two user-reported issues after fresh install:
1. All service buttons (Start/Stop/Restart Display, Restart Web Service)
failed silently — only Reboot worked.
Root cause: sudoers rules use `ledmatrix.service` (with suffix) but
api_v3.py called `sudo systemctl start ledmatrix` (no suffix). sudo
does exact string matching, so every service action was rejected with
returncode=1. Also missing from sudoers: ledmatrix-web, journalctl,
and is-active entries.
Fix:
- Add `.service` suffix to all 8 sudo systemctl call sites in
api_v3.py (_ensure_display_service_running, _stop_display_service,
and all execute_system_action branches).
- Add timeout=15 to all subprocess.run calls in execute_system_action
(previously could hang indefinitely).
- Add missing sudoers rules to first_time_install.sh and
configure_web_sudo.sh: ledmatrix-web.service start/stop/restart,
is-active for both name forms, and journalctl -u/-t ledmatrix rules.
2. SSH and web UI became inaccessible after ~1 hour even though the
display kept running.
Root cause: wifi_monitor_daemon restarts NetworkManager after 5
consecutive internet failures (~2.5 min). Each NM restart drops WiFi
briefly. During that window check_and_manage_ap_mode() increments
_disconnected_checks but the daemon never reset it after the restart.
After 3 such NM-restart cycles, _disconnected_checks reached 3 and
AP mode activated — changing the Pi from WiFi client to hotspot
(192.168.4.1) and killing SSH on the old IP.
Fix:
- Reset wifi_manager._disconnected_checks = 0 in the daemon
immediately after a successful NM restart so the brief drop it
causes doesn't count toward AP-mode activation.
- Increase _disconnected_checks_required from 3 to 6 (90s → 3min)
as an additional buffer against transient network flaps.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* revert: restore AP-mode grace period to 90s (3 checks)
The counter reset after NM restart already fully prevents the SSH-lockout
cascade: _disconnected_checks can never accumulate across NM restarts
because it is reset to 0 before the next daemon iteration runs.
The 3→6 increase provided no additional fix for the described problem and
caused a UX regression: fresh Pi devices with no WiFi configured would
wait 3 minutes instead of 90 seconds for the LEDMatrix-Setup hotspot to
appear.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address five valid review findings; skip two
Fixed:
- march-madness/requirements.txt: Pillow>=10.3.0 (patches CVE-2024-28219;
10.3.0 is the actual fix version — reviewer cited 12.2.0 but that risks
breaking API changes without test coverage)
- wifi_monitor_daemon.py: add missing `import subprocess`; subprocess.run
and CalledProcessError would NameError at runtime on the NM restart path
- wifi_manager.py: validate ap_idle_timeout_minutes before arithmetic —
coerce to int, clamp 1–1440, fall back to 15 on bad config values
- wifi_manager.py: call _remove_nm_dnsmasq_captive_conf() on all three
rollback paths in _enable_ap_mode_nmcli_hotspot() and in the top-level
except block so stale dnsmasq drop-ins are never left behind
- api_v3.py: fix wrong_password prefix strip — removeprefix("wrong_password:")
then lstrip() handles both "wrong_password: msg" and "wrong_password:msg"
- plugins_manager.js: add .catch() to loadInstalledPlugins().then() to
surface failures instead of silently dropping unhandled rejections
Skipped:
- WiFiManager AP state persistence: architectural overhaul; _is_ap_mode_active()
already derives from live system state, not in-memory variables
- Absolute subprocess paths in api_v3.py: paths vary by distro (/usr/bin vs
/bin); web service has a normal PATH; sudoers already use resolved paths
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address five review findings (NM retry loop, start_display message, code quality)
- wifi_monitor_daemon: reset _consecutive_internet_failures = 0 in both
NM-restart exception handlers; previously both left the counter at threshold,
causing an immediate retry on the next iteration instead of waiting another
full backoff period
- api_v3: fix start_display failure message — when mode is set and systemctl
returns non-zero, message now includes the failure reason and a hint rather
than always reporting success phrasing
- wifi_manager: move _redirect_backend from class variable to instance variable
in __init__ alongside _ap_enabled_at; class-level default shadowed correctly
in practice (single instance) but was misleading
- wifi_manager: narrow broad except Exception in _check_internet_connectivity
to (subprocess.SubprocessError, OSError) for ping and OSError for HTTP
(urllib.error.URLError is an OSError subclass in Python 3)
- wifi_manager: remove redundant local 'import re as _re' in _validate_ap_config;
re is already imported at module level (line 37)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix: address five review findings (Pillow CVEs, daemon exception narrowing, timeout handling, plugin store)
- march-madness/requirements.txt: Pillow>=12.2.0 (patches CVE-2026-42308
and CVE-2026-42310; previous floor of 10.3.0 was insufficient)
- wifi_monitor_daemon: narrow final except Exception to
(subprocess.SubprocessError, OSError) so programming errors in the NM
restart block are no longer silently swallowed
- api_v3/execute_system_action: add explicit subprocess.TimeoutExpired
handler before the generic Exception catch; returns action-specific
message with 'status','message','returncode','stdout','stderr' fields
so the UI receives a precise, actionable payload instead of the generic
'Failed to execute system action' string
- plugins_manager.js: move searchPluginStore into .finally() so the
plugin store renders regardless of whether loadInstalledPlugins succeeds
or fails; .catch() still logs the error
- first_time_install.sh: add safe_plugin_rm.sh NOPASSWD rule to the
/tmp/ledmatrix_web_sudoers block; configure_web_sudo.sh had this rule
but the standalone installer never granted it, leaving plugin removal
broken after first-time install
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(api): resolve sudo/systemctl/reboot/poweroff paths at startup
Use shutil.which() with safe fallbacks for the four privileged binaries
instead of relying on bare names being resolved by the subprocess shell
search. Resolves paths once at module load rather than per-call.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Chuck <chuck@example.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|