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).

src/display_controller.py

@@ -2084,8 +2084,10 @@ class DisplayController:
                         declared = getattr(manager_to_display, 'needs_high_fps', None)
                         if declared is not None:
                             needs_high_fps = bool(declared)
-                            logger.debug("FPS check for %s - plugin declares needs_high_fps=%s",
-                                         active_mode, needs_high_fps)
+                            logger.debug(
+                                "[DisplayController] FPS check for %s (plugin=%s) - "
+                                "plugin declares needs_high_fps=%s",
+                                active_mode, plugin_id, needs_high_fps)
                         elif plugin_id == 'static-image':
                             needs_high_fps = True
                             logger.debug("FPS check - static-image plugin: forcing high-FPS mode for GIF support")

src/display_manager.py

@@ -32,6 +32,7 @@ else:
     from rgbmatrix import RGBMatrix, RGBMatrixOptions
 from contextlib import contextmanager
 from PIL import Image, ImageDraw, ImageFont
+import threading
 import time
 from typing import Dict, Any, List, Optional
 import logging
@@ -195,6 +196,11 @@ class DisplayManager:
         self._dirty_tracking_enabled = bool(
             self.config.get('display', {}).get('dirty_tracking', True))
         self._last_pushed_digest = None
+        # Serializes update_display(): plugins can call it directly from
+        # background threads (see docstring on update_display), not just the
+        # render loop. RLock in case a caller within the critical section
+        # ever re-enters (e.g. via a nested draw callback).
+        self._update_lock = threading.RLock()
         
         # Scrolling state tracking for graceful updates
         self._scrolling_state = {
@@ -533,48 +539,57 @@ class DisplayManager:
         writes to the matrix directly), and brightness is PART of the digest
         so a dim-schedule change is never skipped. Disable via config
         ``display.dirty_tracking: false`` if a redraw issue is ever suspected.
+
+        Serialized via ``_update_lock``: plugins can call this directly from
+        background threads (e.g. sports base classes push an immediate
+        "live" refresh from inside update()), so without a lock two callers
+        could both pass the digest check before either writes it back,
+        double-pushing a frame, or interleave the offscreen/current canvas
+        swap below. The lock is scoped to this method, so callers never
+        need to know about it.
         """
         try:
-            if self.matrix is None:
-                # Fallback mode - no actual hardware to update
-                logger.debug("Update display called in fallback mode (no hardware)")
-                # Still write a snapshot so the web UI can preview
-                self._write_snapshot_if_due()
-                return
-
-            if self._capture_mode_active:
-                return  # Skip hardware write — content is being captured off-screen
-
-            digest = None
-            if self._dirty_tracking_enabled:
-                try:
-                    brightness = getattr(self.matrix, 'brightness', None)
-                except Exception:
-                    brightness = None
-                digest = (zlib.adler32(self.image.tobytes()), brightness)
-                if digest == self._last_pushed_digest:
-                    # Nothing changed since the last push — the panel is
-                    # already showing exactly this frame.
+            with self._update_lock:
+                if self.matrix is None:
+                    # Fallback mode - no actual hardware to update
+                    logger.debug("Update display called in fallback mode (no hardware)")
+                    # Still write a snapshot so the web UI can preview
                     self._write_snapshot_if_due()
                     return
 
-            # Copy the current image to the offscreen canvas. In double-sided
-            # mode the logical screen is first tiled across the full chain.
-            if self._double_sided is not None:
-                self.offscreen_canvas.SetImage(self._composite_double_sided())
-            else:
-                self.offscreen_canvas.SetImage(self.image)
+                if self._capture_mode_active:
+                    return  # Skip hardware write — content is being captured off-screen
 
-            # Swap buffers immediately
-            self.matrix.SwapOnVSync(self.offscreen_canvas)
+                digest = None
+                if self._dirty_tracking_enabled:
+                    try:
+                        brightness = getattr(self.matrix, 'brightness', None)
+                    except AttributeError:
+                        brightness = None
+                    digest = (zlib.adler32(self.image.tobytes()), brightness)
+                    if digest == self._last_pushed_digest:
+                        # Nothing changed since the last push — the panel is
+                        # already showing exactly this frame.
+                        self._write_snapshot_if_due()
+                        return
 
-            # Swap our canvas references
-            self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas
+                # Copy the current image to the offscreen canvas. In double-sided
+                # mode the logical screen is first tiled across the full chain.
+                if self._double_sided is not None:
+                    self.offscreen_canvas.SetImage(self._composite_double_sided())
+                else:
+                    self.offscreen_canvas.SetImage(self.image)
 
-            self._last_pushed_digest = digest
+                # Swap buffers immediately
+                self.matrix.SwapOnVSync(self.offscreen_canvas)
 
-            # Write a snapshot for the web preview (throttled)
-            self._write_snapshot_if_due()
+                # Swap our canvas references
+                self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas
+
+                self._last_pushed_digest = digest
+
+                # Write a snapshot for the web preview (throttled)
+                self._write_snapshot_if_due()
         except Exception as e:
             logger.error(f"Error updating display: {e}")
 

test/test_display_dirty_tracking.py

@@ -36,6 +36,8 @@ def dm():
         },
     }, suppress_test_pattern=True)
     yield manager
+    DisplayManager._instance = None
+    DisplayManager._initialized = False
 
 
 class _SwapSpy:
@@ -101,6 +103,16 @@ class TestDirtyTracking:
         dm.update_display()   # push + snapshot
         assert os.path.exists(dm._snapshot_path)
 
+        # Re-open the snapshot's own throttle and remove the file, then push
+        # the identical frame again: dirty tracking must skip the panel
+        # write but the snapshot must still be (re-)written on that path.
+        os.remove(dm._snapshot_path)
+        dm._last_snapshot_ts = 0.0
+        with _SwapSpy(dm.matrix) as spy:
+            dm.update_display()  # identical frame -> panel push skipped
+        assert spy.count == 0
+        assert os.path.exists(dm._snapshot_path)
+
 
 class TestKillSwitch:
     def test_dirty_tracking_can_be_disabled(self, dm):
@@ -120,15 +132,19 @@ class TestKillSwitch:
         from src.display_manager import DisplayManager
         DisplayManager._instance = None
         DisplayManager._initialized = False
-        manager = DisplayManager({
-            "display": {
-                "hardware": {"rows": 32, "cols": 64, "chain_length": 1,
-                             "parallel": 1},
-                "runtime": {"gpio_slowdown": 0},
-                "dirty_tracking": False,
-            },
-        }, suppress_test_pattern=True)
-        assert manager._dirty_tracking_enabled is False
+        try:
+            manager = DisplayManager({
+                "display": {
+                    "hardware": {"rows": 32, "cols": 64, "chain_length": 1,
+                                 "parallel": 1},
+                    "runtime": {"gpio_slowdown": 0},
+                    "dirty_tracking": False,
+                },
+            }, suppress_test_pattern=True)
+            assert manager._dirty_tracking_enabled is False
+        finally:
+            DisplayManager._instance = None
+            DisplayManager._initialized = False
 
 
 if __name__ == "__main__":