Compare commits

..
Author SHA1 Message Date
ChuckandClaude Fable 5 dd5bc93d87 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
2026-07-13 09:03:52 -04:00
ChuckandClaude Fable 5 512e38647a 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
2026-07-12 12:24:31 -04:00
6 changed files with 55 additions and 254 deletions
-68
View File
@@ -1,68 +0,0 @@
"""Snapshot write policy for the display preview mirror.
The display service mirrors frames to /tmp/led_matrix_preview.png, which
serves two consumers with different needs:
- The web UI's live preview (SSE reader in web_interface/app.py) wants
fresh frames — but only while a browser is actually watching.
- The health check (web_interface/blueprints/api_v3.py, hardware status)
uses the file's AGE as a liveness proxy: age >= 60s reads as degraded.
PNG-encoding every frame at 5 fps forever — identical frames, no viewers —
was one of the biggest fixed CPU costs on the Pi. This module is the pure
decision logic (extracted so it's unit-testable off-Pi; display_manager
imports rgbmatrix unconditionally and can't be):
WRITE — encode + atomically replace the snapshot file
TOUCH — os.utime only: keeps the health-check mtime fresh and lets
the SSE reader (mtime-gated) resend at a low rate, without
paying for a PNG encode of an unchanged frame
SKIP — do nothing
Policy:
- With a fresh viewer marker: changed frames write at up to 1/VIEWER_INTERVAL.
- Without viewers: changed frames still write at 1/IDLE_INTERVAL so the
preview page shows something recent on open.
- Unchanged frames are never re-encoded; the mtime is touched every
TOUCH_INTERVAL so the health check (60s threshold) never degrades.
If any constant here changes, re-check the health threshold in
api_v3.py (get_hardware_status) — TOUCH_INTERVAL must stay well under it.
"""
from enum import Enum
# Snapshot cadence with a browser preview open (seconds).
VIEWER_INTERVAL = 0.2
# Snapshot cadence with no viewers — cheap freshness for page-open (seconds).
IDLE_INTERVAL = 30.0
# Max age of the last write/touch before bumping mtime for the health
# check. MUST stay well under api_v3's 60s degraded threshold.
TOUCH_INTERVAL = 20.0
# A viewer marker older than this no longer counts as a live viewer.
VIEWER_MARKER_FRESH_SEC = 5.0
class SnapshotAction(Enum):
WRITE = "write"
TOUCH = "touch"
SKIP = "skip"
def decide(now: float, last_write_ts: float, last_touch_ts: float,
viewer_fresh: bool, frame_changed: bool) -> SnapshotAction:
"""Decide what to do with the current frame.
Args:
now: current monotonic-ish timestamp (same clock as the ts args)
last_write_ts: when a frame was last actually encoded+written
last_touch_ts: when the file mtime was last bumped (write or touch)
viewer_fresh: a browser preview is currently watching
frame_changed: the frame differs from the last WRITTEN frame
"""
interval = VIEWER_INTERVAL if viewer_fresh else IDLE_INTERVAL
if frame_changed and (now - last_write_ts) >= interval:
return SnapshotAction.WRITE
if (now - max(last_write_ts, last_touch_ts)) >= TOUCH_INTERVAL:
return SnapshotAction.TOUCH
return SnapshotAction.SKIP
+20 -5
View File
@@ -199,6 +199,10 @@ class DisplayController:
self.wifi_status_file = WIFI_STATUS_FILE self.wifi_status_file = WIFI_STATUS_FILE
self.wifi_status_active = False self.wifi_status_active = False
self.wifi_status_expires_at: Optional[float] = None self.wifi_status_expires_at: Optional[float] = None
# _check_wifi_status_message throttle state (checked at frame rate,
# stat'd at most once per second)
self._wifi_status_check_ts = 0.0
self._wifi_status_last_result: Optional[Dict[str, Any]] = None
# Plugin display() signature cache — must be initialised before the plugin # Plugin display() signature cache — must be initialised before the plugin
# loading loop below so the .pop() invalidation at load time is always safe. # loading loop below so the .pop() invalidation at load time is always safe.
@@ -1635,7 +1639,7 @@ class DisplayController:
self._sleep_with_plugin_updates(60) self._sleep_with_plugin_updates(60)
continue continue
logger.info(f"Display active, processing mode: {self.current_display_mode}") logger.debug("Display active, processing mode: %s", self.current_display_mode)
# Plugins update on their own schedules - no forced sync updates needed # Plugins update on their own schedules - no forced sync updates needed
# Each plugin has its own update_interval and background services # Each plugin has its own update_interval and background services
@@ -1803,7 +1807,7 @@ class DisplayController:
if self.plugin_manager and hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker: if self.plugin_manager and hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker:
should_skip = self.plugin_manager.health_tracker.should_skip_plugin(plugin_id) should_skip = self.plugin_manager.health_tracker.should_skip_plugin(plugin_id)
if should_skip: if should_skip:
logger.info(f"Skipping plugin {plugin_id} due to circuit breaker (mode: {active_mode})") logger.info("Skipping plugin %s due to circuit breaker (mode: %s)", plugin_id, active_mode)
display_result = False display_result = False
# Skip to next mode - let existing logic handle it # Skip to next mode - let existing logic handle it
manager_to_display = None manager_to_display = None
@@ -1861,7 +1865,7 @@ class DisplayController:
if isinstance(result, bool): if isinstance(result, bool):
display_result = result display_result = result
if not display_result: if not display_result:
logger.info(f"Plugin {plugin_id} display() returned False for mode {active_mode}") logger.info("Plugin %s display() returned False for mode %s", plugin_id, active_mode)
# Record success if display completed without exception # Record success if display completed without exception
if self.plugin_manager and hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker: if self.plugin_manager and hasattr(self.plugin_manager, 'health_tracker') and self.plugin_manager.health_tracker:
@@ -2354,6 +2358,16 @@ class DisplayController:
Returns None on any error or if message is expired/invalid. Returns None on any error or if message is expired/invalid.
""" """
try: try:
# Throttle the existence stat to ~1 Hz: this runs on every render
# iteration (60+ fps), and the file usually doesn't exist — the
# status message's lifetime is measured in seconds anyway.
# Both attributes are initialised in __init__.
now = time.time()
if (now - self._wifi_status_check_ts) < 1.0:
return self._wifi_status_last_result
self._wifi_status_check_ts = now
self._wifi_status_last_result = None
# Check if file exists # Check if file exists
if not self.wifi_status_file or not self.wifi_status_file.exists(): if not self.wifi_status_file or not self.wifi_status_file.exists():
return None return None
@@ -2404,13 +2418,14 @@ class DisplayController:
pass pass
return None return None
# Message is valid and not expired # Message is valid and not expired — cache for the throttle window
return { self._wifi_status_last_result = {
'message': message, 'message': message,
'timestamp': timestamp, 'timestamp': timestamp,
'duration': duration, 'duration': duration,
'expires_at': expires_at 'expires_at': expires_at
} }
return self._wifi_status_last_result
except Exception as e: except Exception as e:
# Catch-all for any unexpected errors - log but don't break the display # Catch-all for any unexpected errors - log but don't break the display
+18 -68
View File
@@ -31,23 +31,13 @@ if os.getenv("EMULATOR", "false") == "true":
else: else:
from rgbmatrix import RGBMatrix, RGBMatrixOptions from rgbmatrix import RGBMatrix, RGBMatrixOptions
from contextlib import contextmanager from contextlib import contextmanager
from pathlib import Path
from PIL import Image, ImageDraw, ImageFont from PIL import Image, ImageDraw, ImageFont
import time import time
from typing import Dict, Any, List, Optional from typing import Dict, Any, List, Optional
import logging import logging
import math import math
import zlib
import freetype import freetype
from src.common import snapshot_policy
from src.common.permission_utils import (
ensure_directory_permissions,
ensure_file_permissions,
get_assets_dir_mode,
get_assets_file_mode,
)
# Get logger without configuring # Get logger without configuring
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO) # Set to INFO level logger.setLevel(logging.INFO) # Set to INFO level
@@ -194,19 +184,10 @@ class DisplayManager:
# Avoids re-measuring the same string+font on every display() call. # Avoids re-measuring the same string+font on every display() call.
# Cleared on _load_fonts() so stale entries don't survive a font reload. # Cleared on _load_fonts() so stale entries don't survive a font reload.
self._text_width_cache: Dict[tuple, int] = {} self._text_width_cache: Dict[tuple, int] = {}
# Snapshot mirror for web preview + health check (service writes, web # Snapshot settings for web preview integration (service writes, web reads)
# reads). Cadence/skip decisions live in src/common/snapshot_policy.py:
# full rate only while the web SSE broadcaster keeps the viewer marker
# fresh; unchanged frames are never re-encoded, only mtime-touched.
self._snapshot_path = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path intentional; web UI reads same path self._snapshot_path = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path intentional; web UI reads same path
self._viewer_marker_path = "/tmp/led_matrix_preview_viewer" # nosec B108 - touched by web SSE broadcaster self._snapshot_min_interval_sec = 0.2 # max ~5 fps
self._last_snapshot_ts = 0.0 self._last_snapshot_ts = 0.0
self._last_snapshot_touch_ts = 0.0
self._last_snapshot_digest: Optional[int] = None
self._snapshot_dir_prepared = False
self._viewer_check_ts = 0.0
self._viewer_fresh = False
self._viewer_was_fresh = False
# Scrolling state tracking for graceful updates # Scrolling state tracking for graceful updates
self._scrolling_state = { self._scrolling_state = {
@@ -1147,56 +1128,27 @@ class DisplayManager:
'deferred_update_ttl': self._scrolling_state['deferred_update_ttl'] 'deferred_update_ttl': self._scrolling_state['deferred_update_ttl']
} }
def _viewer_is_fresh(self, now: float) -> bool:
"""True when a browser preview is watching (marker file touched by
the web SSE broadcaster). The marker is stat'd at most once per
second — at 125 fps loops a per-call stat would be pure overhead."""
if (now - self._viewer_check_ts) >= 1.0:
self._viewer_check_ts = now
try:
marker_age = now - os.stat(self._viewer_marker_path).st_mtime
self._viewer_fresh = marker_age < snapshot_policy.VIEWER_MARKER_FRESH_SEC
except OSError:
self._viewer_fresh = False
return self._viewer_fresh
def _write_snapshot_if_due(self) -> None: def _write_snapshot_if_due(self) -> None:
"""Mirror the current frame to the preview snapshot when the policy """Write the current image to a PNG snapshot file at a limited frequency."""
says it's worth it — see src/common/snapshot_policy.py. Unchanged
frames are never re-encoded; without viewers the cadence drops to
the idle keepalive."""
try: try:
now = time.time() now = time.time()
viewer_fresh = self._viewer_is_fresh(now) if (now - self._last_snapshot_ts) < self._snapshot_min_interval_sec:
if viewer_fresh and not self._viewer_was_fresh:
# A preview just opened: let the next changed frame through
# immediately instead of waiting out the idle interval.
self._last_snapshot_ts = 0.0
self._viewer_was_fresh = viewer_fresh
digest = zlib.adler32(self.image.tobytes())
action = snapshot_policy.decide(
now, self._last_snapshot_ts, self._last_snapshot_touch_ts,
viewer_fresh, digest != self._last_snapshot_digest)
if action is snapshot_policy.SnapshotAction.SKIP:
return return
if action is snapshot_policy.SnapshotAction.TOUCH: # Ensure directory exists with proper permissions
# mtime bump only: keeps the health check (snapshot age) from pathlib import Path
# green without paying for a PNG encode of an unchanged frame from src.common.permission_utils import (
os.utime(self._snapshot_path, None) ensure_directory_permissions,
self._last_snapshot_touch_ts = now ensure_file_permissions,
return get_assets_dir_mode,
get_assets_file_mode
# WRITE: ensure directory permissions once, not per frame )
snapshot_path_obj = Path(self._snapshot_path) snapshot_path_obj = Path(self._snapshot_path)
if not self._snapshot_dir_prepared: # Only ensure permissions on non-system directories
# Never modify /tmp permissions - it has special system # Never modify /tmp permissions - it has special system permissions (1777)
# permissions (1777) that must not be changed or it breaks # that must not be changed or it breaks apt and other system tools
# apt and other system tools parent_dir = snapshot_path_obj.parent
parent_dir = snapshot_path_obj.parent if parent_dir and str(parent_dir) != '/tmp': # nosec B108 - guard to skip /tmp for permission ops
if parent_dir and str(parent_dir) != '/tmp': # nosec B108 - guard to skip /tmp for permission ops ensure_directory_permissions(parent_dir, get_assets_dir_mode())
ensure_directory_permissions(parent_dir, get_assets_dir_mode())
self._snapshot_dir_prepared = True
# Write atomically: temp then replace # Write atomically: temp then replace
tmp_path = f"{self._snapshot_path}.tmp" tmp_path = f"{self._snapshot_path}.tmp"
self.image.save(tmp_path, format='PNG') self.image.save(tmp_path, format='PNG')
@@ -1211,8 +1163,6 @@ class DisplayManager:
except Exception: except Exception:
pass pass
self._last_snapshot_ts = now self._last_snapshot_ts = now
self._last_snapshot_touch_ts = now
self._last_snapshot_digest = digest
except Exception as e: except Exception as e:
# Snapshot failures should never break display; log at debug to avoid noise # Snapshot failures should never break display; log at debug to avoid noise
logger.debug(f"Snapshot write skipped: {e}") logger.debug(f"Snapshot write skipped: {e}")
+16 -4
View File
@@ -66,6 +66,10 @@ class RenderPipeline:
else display_manager.height else display_manager.height
) )
# Reusable blank frame for cycle-end pushes (allocated lazily,
# re-blacked before each reuse)
self._blank_frame = None
# ScrollHelper for optimized scrolling # ScrollHelper for optimized scrolling
self.scroll_helper = ScrollHelper( self.scroll_helper = ScrollHelper(
self.display_width, self.display_width,
@@ -234,11 +238,19 @@ class RenderPipeline:
) )
# Push blank immediately so the hardware never shows any # Push blank immediately so the hardware never shows any
# post-wrap content while the coordinator recomposes the # post-wrap content while the coordinator recomposes the
# next cycle (~100 ms). # next cycle (~100 ms). The blank is allocated once and
# reused across cycle wraps (fresh paste each time in case
# a consumer drew on the previous one).
try: try:
from PIL import Image as _Image if self._blank_frame is None or self._blank_frame.size != (
blank = _Image.new('RGB', (self.display_width, self.display_height)) self.display_width, self.display_height):
self.display_manager.image = blank self._blank_frame = Image.new(
'RGB', (self.display_width, self.display_height))
else:
self._blank_frame.paste(
(0, 0, 0),
(0, 0, self.display_width, self.display_height))
self.display_manager.image = self._blank_frame
self.display_manager.update_display() self.display_manager.update_display()
except Exception: except Exception:
logger.exception("Failed to write blank frame to display at cycle end") logger.exception("Failed to write blank frame to display at cycle end")
-93
View File
@@ -1,93 +0,0 @@
"""Tests for the snapshot write policy (src/common/snapshot_policy.py).
The invariants that matter:
- unchanged frames are NEVER re-encoded (the old code PNG-encoded identical
frames at 5 fps, 24/7)
- the file mtime never goes stale enough to trip the health check's 60s
degraded threshold (api_v3 get_hardware_status)
- a viewer gets full cadence; no viewer drops to the idle keepalive
"""
import os
import sys
import pytest
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from src.common.snapshot_policy import ( # noqa: E402
IDLE_INTERVAL,
TOUCH_INTERVAL,
VIEWER_INTERVAL,
SnapshotAction,
decide,
)
class TestViewerCadence:
def test_changed_frame_with_viewer_writes_at_full_rate(self):
assert decide(now=100.0, last_write_ts=100.0 - VIEWER_INTERVAL,
last_touch_ts=0, viewer_fresh=True,
frame_changed=True) is SnapshotAction.WRITE
def test_changed_frame_with_viewer_respects_min_interval(self):
assert decide(now=100.0, last_write_ts=100.0 - VIEWER_INTERVAL / 2,
last_touch_ts=100.0, viewer_fresh=True,
frame_changed=True) is SnapshotAction.SKIP
def test_unchanged_frame_with_viewer_never_writes(self):
"""A static screen with a viewer must not burn PNG encodes."""
assert decide(now=100.0, last_write_ts=90.0, last_touch_ts=90.0,
viewer_fresh=True,
frame_changed=False) is SnapshotAction.SKIP
class TestIdleCadence:
def test_changed_frame_without_viewer_waits_for_idle_interval(self):
assert decide(now=100.0, last_write_ts=100.0 - IDLE_INTERVAL / 2,
last_touch_ts=100.0, viewer_fresh=False,
frame_changed=True) is SnapshotAction.SKIP
def test_changed_frame_without_viewer_writes_at_idle_rate(self):
assert decide(now=100.0, last_write_ts=100.0 - IDLE_INTERVAL,
last_touch_ts=0, viewer_fresh=False,
frame_changed=True) is SnapshotAction.WRITE
class TestHealthKeepalive:
def test_stale_mtime_gets_touched(self):
"""Whatever else happens, mtime must be bumped within TOUCH_INTERVAL
so the health check (60s threshold) never reads the display as dead."""
assert decide(now=100.0, last_write_ts=100.0 - TOUCH_INTERVAL,
last_touch_ts=100.0 - TOUCH_INTERVAL, viewer_fresh=False,
frame_changed=False) is SnapshotAction.TOUCH
def test_touch_applies_with_viewer_too(self):
"""Viewer watching a static screen: no writes, but health stays green."""
assert decide(now=100.0, last_write_ts=100.0 - TOUCH_INTERVAL - 1,
last_touch_ts=100.0 - TOUCH_INTERVAL - 1, viewer_fresh=True,
frame_changed=False) is SnapshotAction.TOUCH
def test_recent_touch_suppresses_another(self):
assert decide(now=100.0, last_write_ts=0.0,
last_touch_ts=100.0 - TOUCH_INTERVAL / 2, viewer_fresh=False,
frame_changed=False) is SnapshotAction.SKIP
def test_touch_interval_stays_under_health_threshold(self):
"""api_v3's hardware status treats snapshot age >= 60s as degraded.
Keep a 2x margin so scheduling jitter can't trip it."""
assert TOUCH_INTERVAL <= 30
def test_worst_case_mtime_age_is_bounded(self):
"""Simulate any interleaving: from any state, within one policy call
after TOUCH_INTERVAL elapses, mtime gets refreshed (WRITE or TOUCH)."""
for viewer in (True, False):
for changed in (True, False):
action = decide(now=1000.0, last_write_ts=900.0,
last_touch_ts=900.0, viewer_fresh=viewer,
frame_changed=changed)
assert action in (SnapshotAction.WRITE, SnapshotAction.TOUCH)
if __name__ == "__main__":
sys.exit(pytest.main([__file__, "-v"]))
-15
View File
@@ -610,22 +610,8 @@ def display_preview_generator():
import io import io
snapshot_path = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path matches display_manager; only read here snapshot_path = "/tmp/led_matrix_preview.png" # nosec B108 - fixed path matches display_manager; only read here
# Viewer marker: this generator only runs while the broadcaster has
# subscribers (it exits with no clients), so touching the marker each
# loop tells the DISPLAY service a browser is actually watching — it
# only pays for full-rate PNG snapshot encodes while this stays fresh
# (see src/common/snapshot_policy.py).
viewer_marker_path = "/tmp/led_matrix_preview_viewer" # nosec B108 - fixed path matches display_manager
last_modified = None last_modified = None
def _touch_viewer_marker():
try:
with open(viewer_marker_path, 'a'):
pass
os.utime(viewer_marker_path, None)
except OSError:
pass # display side treats a missing marker as "no viewer"
# Get display dimensions from config # Get display dimensions from config
try: try:
main_config = config_manager.load_config() main_config = config_manager.load_config()
@@ -641,7 +627,6 @@ def display_preview_generator():
while True: while True:
try: try:
_touch_viewer_marker()
# Check if snapshot file exists and has been modified # Check if snapshot file exists and has been modified
if os.path.exists(snapshot_path): if os.path.exists(snapshot_path):
current_modified = os.path.getmtime(snapshot_path) current_modified = os.path.getmtime(snapshot_path)