From 1b33620412f5817d3d1cd89225906ff588f09fef Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 22:30:01 -0400 Subject: [PATCH 1/2] fix(display): a frame the preview throttle skipped still reaches the snapshot The preview snapshot (/api/v3/display/current, the web UI's live preview) is written only from update_display(), at most once per write interval. A changed frame pushed inside that interval was skipped and left for the next update_display() -- which a screen that draws its card once and then holds it never makes. Soccer's recent/upcoming cards skip redundant redraws. After an on-demand start, the card is pushed a few milliseconds after the start's clear wrote a black frame, so the throttle skipped it and the preview stayed black for the whole 15 s screen while the panel showed the card. The next screen looked fine because its first push came after the interval had passed. DisplayManager now records a skipped changed frame as owed, and the render loop (_display_once) calls write_owed_snapshot() after each frame, which writes it once the interval has passed. The cadence is unchanged, an unchanged frame is never re-encoded, and nothing runs when no frame is owed. Golden run-loop traces are unchanged. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 15 +++ src/display_controller.py | 7 ++ src/display_manager.py | 48 ++++++++- test/test_snapshot_owed_frame.py | 162 +++++++++++++++++++++++++++++++ 4 files changed, 230 insertions(+), 2 deletions(-) create mode 100644 test/test_snapshot_owed_frame.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9257449fc..0397a0910 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,21 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +### Fixed + +- The web preview and `/api/v3/display/current` no longer stay black for a + whole screen that draws its card once and then holds it. The snapshot is + written from `update_display()` at most once per write interval, so a frame + pushed inside that interval was skipped and left for the next + `update_display()` -- which such a screen never makes. Soccer's + recent/upcoming cards skip redundant redraws, and the first one after an + on-demand start lands a few milliseconds after the start's clear wrote a + black frame: on ledpi the preview showed 0 lit pixels for the whole 15 s + while the panel showed the card. `DisplayManager` now remembers a skipped + changed frame, and the render loop writes it (`write_owed_snapshot()`) + once the interval has passed. The cadence is unchanged, and nothing extra + runs when no frame is owed. + ### Cheap per-frame and per-fetch savings - `BaseOddsManager.get_odds()` no longer pretty-prints every odds response diff --git a/src/display_controller.py b/src/display_controller.py index 95234d2dd..4cf981fcf 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -1215,6 +1215,13 @@ def _display_once(self, plugin, mode: str, accepts_display_mode: bool, note = getattr(self.plugin_manager, 'note_display_duration', None) if note is not None and plugin_id: note(plugin_id, time.monotonic() - started) + # A screen that drew once and holds makes no more + # update_display() calls, so a frame the preview throttle + # skipped would otherwise never reach the snapshot. + write_owed = getattr(getattr(self, 'display_manager', None), + 'write_owed_snapshot', None) + if write_owed is not None: + write_owed() def _health_tracker(self): """The plugin circuit breaker, or None when it is not enabled.""" diff --git a/src/display_manager.py b/src/display_manager.py index 3e211e9f2..07471f5b8 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -317,6 +317,11 @@ def __init__(self, config: Dict[str, Any] = None, force_fallback: bool = False, # is handed to the writer; this only once it has been saved, so an # mtime touch never vouches for a frame still waiting to be written. self._saved_snapshot_digest: Optional[int] = None + # A changed frame reached _write_snapshot_if_due() inside the write + # interval and was skipped. Nothing writes it unless update_display() + # runs again, and a screen that draws once and holds never calls it + # again -- see write_owed_snapshot(). + self._snapshot_owed = False self._snapshot_dir_prepared = False # Background writer used mid-scroll; see _write_snapshot_if_due. self._snapshot_cond = threading.Condition() @@ -1788,9 +1793,10 @@ def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None: if frame_checksum is not None: digest = frame_checksum + frame_changed = digest != self._last_snapshot_digest action = snapshot_policy.decide( now, self._last_snapshot_ts, self._last_snapshot_touch_ts, - viewer_fresh, digest != self._last_snapshot_digest) + viewer_fresh, frame_changed) else: # Ask as if the frame had changed before paying to find out. # decide() is monotone in frame_changed -- a SKIP for a @@ -1802,15 +1808,24 @@ def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None: now, self._last_snapshot_ts, self._last_snapshot_touch_ts, viewer_fresh, True) if action is snapshot_policy.SnapshotAction.SKIP: + # Not hashed, so not known to be unchanged: owed until a + # later look finds it written or unchanged. + self._snapshot_owed = True return digest = zlib.adler32(self.image.tobytes()) - if digest == self._last_snapshot_digest: + frame_changed = digest != self._last_snapshot_digest + if not frame_changed: # Unchanged after all: the decision an unchanged frame gets. action = snapshot_policy.decide( now, self._last_snapshot_ts, self._last_snapshot_touch_ts, viewer_fresh, False) if action is snapshot_policy.SnapshotAction.SKIP: + # A changed frame inside the write interval stays owed: the + # next update_display() would write it, but a static screen + # may not make one -- write_owed_snapshot() covers that. + self._snapshot_owed = frame_changed return + self._snapshot_owed = False if (action is snapshot_policy.SnapshotAction.TOUCH and self._saved_snapshot_digest == digest): # mtime bump only: keeps the health check (snapshot age) @@ -1845,6 +1860,35 @@ def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None: except Exception as e: self._log_snapshot_failure(e) + def write_owed_snapshot(self) -> None: + """Write a frame the snapshot throttle skipped, once it is due. + + The preview snapshot is only ever written from update_display(), and + at most once per write interval (snapshot_policy). A frame pushed + inside that interval is skipped, and is written by the next + update_display() that comes after it -- but a screen that draws its + card once and then holds it makes no further call. Its frame was on + the panel and never in the preview: soccer's recent/upcoming cards + skip redundant redraws, and the first one after an on-demand start + (pushed a few milliseconds after the controller's clear) left + /api/v3/display/current and the web preview black for the whole + screen while the panel showed the card. + + The render loop calls this after each frame. Cheap when nothing is + owed (one attribute read); otherwise the usual policy decides, so + the write still waits out the interval and an unchanged frame is + never re-encoded. + """ + if not self._snapshot_owed: + return + try: + if self._writes_suppressed(): + return + with self._update_lock: + self._write_snapshot_if_due() + except Exception as e: # pylint: disable=broad-except + self._log_snapshot_failure(e) + def _log_snapshot_failure(self, error: Exception) -> None: # Snapshot failures must never break display — but they must not # be silent either: the snapshot's mtime is the web UI's display diff --git a/test/test_snapshot_owed_frame.py b/test/test_snapshot_owed_frame.py new file mode 100644 index 000000000..fe027e6e2 --- /dev/null +++ b/test/test_snapshot_owed_frame.py @@ -0,0 +1,162 @@ +"""A frame the preview throttle skipped still reaches the snapshot. + +The preview snapshot (/api/v3/display/current, the web UI's live preview) is +only written from update_display(), at most once per write interval. A screen +that draws its card once and then holds it -- soccer's recent/upcoming cards +skip redundant redraws -- pushes exactly one frame. When that push lands inside +the interval, e.g. a few milliseconds after the on-demand start's clear wrote a +black frame, the throttle skips it and nothing ever writes it: on ledpi the +preview stayed black for soccer's whole 15 s screen while the panel showed the +card, and the next screen "rendered immediately". + +Runs the real DisplayManager on the emulator, like test_display_dirty_tracking. +""" + +import os +import sys +import types + +os.environ["EMULATOR"] = "true" + +import pytest +from PIL import Image + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + + +@pytest.fixture(scope="module") +def dm(tmp_path_factory): + from src.display_manager import DisplayManager + DisplayManager._instance = None + manager = DisplayManager({ + "display": { + "hardware": {"rows": 32, "cols": 64, "chain_length": 2, + "parallel": 1, "brightness": 90}, + "runtime": {"gpio_slowdown": 0}, + }, + }, suppress_test_pattern=True) + manager._snapshot_path = str( + tmp_path_factory.mktemp("owed_snapshot") / "led_matrix_preview.png") + yield manager + DisplayManager._instance = None + + +@pytest.fixture +def viewer(dm, monkeypatch, tmp_path): + """A preview is open (1 s write interval); fresh snapshot bookkeeping.""" + monkeypatch.setattr(dm, "_viewer_is_fresh", lambda now: True) + dm._viewer_was_fresh = True + dm._snapshot_path = str(tmp_path / "snap.png") + dm._last_snapshot_ts = 0.0 + dm._last_snapshot_touch_ts = 0.0 + dm._last_snapshot_digest = None + dm._saved_snapshot_digest = None + dm._snapshot_owed = False + dm.set_scrolling_state(False) + return dm + + +def _lit(path): + with Image.open(path) as img: + return sum(1 for p in img.convert("RGB").getdata() if max(p) > 20) + + +def _age_last_write(dm, seconds=2.0): + """As if `seconds` had passed since the last snapshot write.""" + dm._last_snapshot_ts -= seconds + dm._last_snapshot_touch_ts -= seconds + + +def _clear_then_draw_card(dm): + """The on-demand start's clear, then the card a few ms later.""" + dm.clear() + dm.update_display() # black frame: written + assert _lit(dm._snapshot_path) == 0 + dm.draw.rectangle([4, 4, 40, 20], fill=(255, 255, 0)) + dm.update_display() # the card: inside the interval + + +def _controller(dm): + from src import display_controller as dc_module + controller = dc_module.DisplayController.__new__(dc_module.DisplayController) + controller.plugin_manager = None + controller.display_manager = dm + return controller + + +class _HoldingPlugin: + """Already showing its card: display() returns True and draws nothing.""" + + plugin_id = "holding" + + def __init__(self): + self.calls = 0 + + def display(self, display_mode=None, force_clear=False): + self.calls += 1 + return True + + +def test_a_held_card_reaches_the_preview_on_the_next_frame(viewer): + dm = viewer + _clear_then_draw_card(dm) + assert _lit(dm._snapshot_path) == 0 # the throttle skipped the card + + controller = _controller(dm) + plugin = _HoldingPlugin() + _age_last_write(dm) + # The render loop's next frame: the plugin draws nothing and makes no + # update_display() call, as soccer's switch cards do. + assert controller._display_once(plugin, "soccer_eng.1_recent", True) is True + assert plugin.calls == 1 + assert _lit(dm._snapshot_path) > 0 + + +def test_the_owed_write_still_waits_out_the_interval(viewer, monkeypatch): + dm = viewer + _clear_then_draw_card(dm) + saves = [] + monkeypatch.setattr(dm, "_save_snapshot", lambda image: saves.append(image)) + dm.write_owed_snapshot() # still inside the interval + assert saves == [] + _age_last_write(dm) + dm.write_owed_snapshot() + assert len(saves) == 1 + # Written: nothing is owed, so later frames do no work and the unchanged + # frame is not encoded again. + assert dm._snapshot_owed is False + _age_last_write(dm) + dm.write_owed_snapshot() + assert len(saves) == 1 + + +def test_nothing_owed_after_a_frame_that_was_written(viewer, monkeypatch): + dm = viewer + dm.draw.rectangle([0, 0, 8, 8], fill=(0, 255, 0)) + dm.update_display() # due: written at once + assert dm._snapshot_owed is False + calls = [] + monkeypatch.setattr(dm, "_write_snapshot_if_due", + lambda *a, **k: calls.append(a)) + dm.write_owed_snapshot() + assert calls == [] + + +def test_an_unchanged_frame_inside_the_interval_is_not_owed(viewer): + dm = viewer + dm.draw.rectangle([0, 0, 8, 8], fill=(0, 0, 255)) + dm.update_display() + dm.update_display() # same frame, inside the interval + assert dm._snapshot_owed is False + + +def test_a_controller_without_the_hook_still_draws(): + """Controllers built without a display manager (tests) are unaffected.""" + from src import display_controller as dc_module + controller = dc_module.DisplayController.__new__(dc_module.DisplayController) + controller.plugin_manager = None + plugin = _HoldingPlugin() + assert controller._display_once(plugin, "x", True) is True + controller.display_manager = types.SimpleNamespace() + assert controller._display_once(plugin, "x", True) is True + assert plugin.calls == 2 From 411436dd8c0e42dfd577c3b52af60e10336185c3 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sun, 4 Oct 2026 14:28:07 -0400 Subject: [PATCH 2/2] fix(display): a failed owed snapshot write stays owed and is retried Review finding: the owed flag was cleared before the write, so one failed write left a held screen's preview stale. It now clears only after the write (or a touch of a frame already on disk) succeeds. Co-Authored-By: Claude Opus 5.5 --- src/display_manager.py | 8 +++++++- test/test_snapshot_owed_frame.py | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/display_manager.py b/src/display_manager.py index 07471f5b8..05b04cb79 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -1825,14 +1825,19 @@ def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None: # may not make one -- write_owed_snapshot() covers that. self._snapshot_owed = frame_changed return - self._snapshot_owed = False if (action is snapshot_policy.SnapshotAction.TOUCH and self._saved_snapshot_digest == digest): # mtime bump only: keeps the health check (snapshot age) # green without paying for a PNG encode of an unchanged frame + # (this frame is already on disk, so nothing is owed). + self._snapshot_owed = False os.utime(self._snapshot_path, None) self._last_snapshot_touch_ts = now return + # Owed until the write below succeeds: if it raises, the frame + # stays owed and write_owed_snapshot() retries it, rather than a + # held screen leaving the preview stale after one failed write. + self._snapshot_owed = True # (A TOUCH for a frame that isn't on disk yet -- still queued, or # its write failed -- is written instead: touching would make the # older file on disk look current.) @@ -1857,6 +1862,7 @@ def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None: self._last_snapshot_ts = now self._last_snapshot_touch_ts = now self._last_snapshot_digest = digest + self._snapshot_owed = False except Exception as e: self._log_snapshot_failure(e) diff --git a/test/test_snapshot_owed_frame.py b/test/test_snapshot_owed_frame.py index fe027e6e2..2db890a17 100644 --- a/test/test_snapshot_owed_frame.py +++ b/test/test_snapshot_owed_frame.py @@ -130,6 +130,27 @@ def test_the_owed_write_still_waits_out_the_interval(viewer, monkeypatch): assert len(saves) == 1 +def test_a_failed_owed_write_stays_owed_and_is_retried(viewer, monkeypatch): + dm = viewer + _clear_then_draw_card(dm) + _age_last_write(dm) + attempts = [] + + def failing_save(image): + attempts.append(image) + raise OSError("disk full") + + monkeypatch.setattr(dm, "_save_snapshot", failing_save) + dm.write_owed_snapshot() # the write fails + assert len(attempts) == 1 + assert dm._snapshot_owed is True # still owed: a held screen + saves = [] # makes no update_display() + monkeypatch.setattr(dm, "_save_snapshot", lambda image: saves.append(image)) + dm.write_owed_snapshot() # retried on the next frame + assert len(saves) == 1 + assert dm._snapshot_owed is False + + def test_nothing_owed_after_a_frame_that_was_written(viewer, monkeypatch): dm = viewer dm.draw.rectangle([0, 0, 8, 8], fill=(0, 255, 0))