feat(render_plugin): add --display-mode so multi-mode plugins can be rendered (#522)

* feat(render_plugin): add --display-mode so multi-mode plugins can be rendered

render_plugin.py always called plugin.display(force_clear=True) with no mode.
A plugin that declares one display mode is fine, but the sports scoreboards
declare three or more and keep their per-mode state on sub-managers; their
no-argument path selects nothing and returns False, so the render came out
blank with nothing to say why. Measured on nrl-scoreboard with identical
seeded state:

  live.display() directly                 True,  1892 lit pixels
  plugin.display(display_mode="nrl_live") True,  1892 lit pixels
  plugin.display()                        False,    0 lit pixels

--display-mode passes the requested mode through. It is only passed when
asked for, so the many plugins whose display() takes no display_mode keep
working untouched, and a plugin that declares modes but does not accept the
argument degrades to its default screen with a warning rather than a
TypeError.

This is what lets the plugin READMEs show a scoreboard at all, and it also
unblocks screens like birdnet_stats and the weather plugin's hourly, daily and
almanac modes, which could previously only be described in prose.

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

* Only fall back when plugin display rejects display_mode

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

scripts/render_plugin.py

@@ -52,6 +52,10 @@ def main() -> int:
     parser.add_argument('--height', type=int, default=32, help='Display height (default: 32)')
     parser.add_argument('--skip-update', action='store_true',
                         help='Skip calling update() (render display only)')
+    parser.add_argument('--display-mode', default=None,
+                        help='Display mode to render, for plugins that declare '
+                             'more than one in their manifest (e.g. nrl_live). '
+                             'Omitted, the plugin picks its own default.')
 
     args = parser.parse_args()
 
@@ -141,8 +145,26 @@ def main() -> int:
         except Exception as e:
             logger.warning("update() raised: %s — continuing to display()", e)
 
+    # A plugin that declares several display modes usually renders nothing
+    # useful without being told which one to draw: the scoreboards keep their
+    # state on per-mode sub-managers and their no-argument path returns False.
+    # Only pass the argument when asked for, so the many plugins whose display()
+    # takes no display_mode keep working untouched.
     try:
-        plugin_instance.display(force_clear=True)
+        if args.display_mode:
+            try:
+                plugin_instance.display(display_mode=args.display_mode,
+                                        force_clear=True)
+            except TypeError as error:
+                if ("unexpected keyword argument" not in str(error)
+                        or "display_mode" not in str(error)):
+                    raise
+                logger.warning(
+                    "%s.display() does not accept display_mode; rendering its "
+                    "default screen instead", args.plugin)
+                plugin_instance.display(force_clear=True)
+        else:
+            plugin_instance.display(force_clear=True)
         logger.debug("display() completed")
     except Exception as e:
         logger.error("Error in display(): %s", e)

src/common/sports_card.py

@@ -17,10 +17,13 @@ deliberate difference is `crisp_size`, which takes the seven-plugin guard
 the extra guard only stops a None size raising TypeError.
 """
 
+import logging
 from datetime import datetime, timezone
 from typing import Any, Dict, Optional, Tuple
 from zoneinfo import ZoneInfo
 
+logger = logging.getLogger(__name__)
+
 __all__ = [
     "ELEMENT_FOR_FONT", "FAVORITE_RESULT_COLOR_DEFAULTS", "FONT_NAME_ALIASES",
     "FONT_PIXEL_GRID", "MONTH_ABBR", "WEEKDAY_ABBR",
@@ -395,7 +398,14 @@ def schema_font_size(schema_path: str, element_key) -> Optional[int]:
                 size = spec.get('properties', {}).get('font_size', {}).get('default')
                 if size is not None:
                     cache[key] = int(size)
-        except Exception:
+        except Exception as exc:
+            # See sports_shared._schema_font_size: an unreadable schema
+            # silently disables the pixel-grid snap for every element.
+            # Built once per schema path, so this cannot repeat per frame.
+            logger.warning(
+                "could not read %s (%s: %s); font sizes will skip their "
+                "pixel grid snap and may render a pixel narrow",
+                schema_path, type(exc).__name__, exc)
             cache = {}
         _SCHEMA_FONT_SIZE_CACHE[schema_path] = cache
     return cache.get(element_key)

src/common/sports_shared.py

@@ -211,17 +211,52 @@ class SportsCoreSharedMixin:
             return desired
         return max(grid, int(round(float(desired) / grid)) * grid)
 
+    #: Absolute path of this plugin's directory, declared by the plugin
+    #: itself. The mixin cannot work it out -- see _plugin_dir.
+    _PLUGIN_DIR: ClassVar[Optional[str]] = None
+
     def _plugin_dir(self) -> Optional[str]:
-        """Directory of the plugin that defines this class.
+        """Directory of the plugin that owns this instance.
 
         In sports.py these methods could just use ``__file__``. Here that is
-        src/common/, so the plugin's own directory has to be recovered from the
-        instance. ``type(self).__module__`` alone is not enough: SportsCore is
-        an ABC, so a subclass built with ``type(name, bases, ns)`` -- which the
-        plugins' own tests do -- reports its module as "abc". Walking the MRO
-        steps past those synthetic classes to the first one whose module sits
-        next to a config_schema.json, which is the real plugin.
+        src/common/, so the directory has to come from the plugin.
+
+        It is TOLD, not deduced. The first version walked the MRO for a class
+        whose module sits beside a config_schema.json. That works when a test
+        imports the plugin itself, and returns None under the real loader, for
+        a specific reason worth recording:
+
+            PluginLoader._namespace_plugin_modules renames every bare module a
+            plugin brought in (sports, game_renderer, ...) to
+            "_plg_<plugin_id>_<module>" and REMOVES the bare entry, so two
+            plugins owning a module of the same name cannot collide.
+
+        The class still reports ``__module__ == "sports"``, but
+        ``sys.modules["sports"]`` no longer exists, so the walk finds no
+        __file__ and falls off the end. The failure was silent and expensive:
+
+            _plugin_dir()       -> None
+            _schema_font_size() -> None for every element
+            -> a configured size equal to the schema default stops looking
+               like a default and is treated as a deliberate user choice
+            -> the snap to the font's pixel grid is skipped
+            -> 4x6-font.ttf renders at 6 instead of 7: 3px-wide glyphs
+               instead of 4px
+
+        On a 256x64 panel that made the odds, the team records and the date row
+        hard to read. It was found by a user counting pixels on the panel. No
+        gate here caught it: the tests imported plugins directly and the
+        safety harness loads them its own way, so neither reproduced the
+        loader's renaming.
+
+        The MRO walk stays as a fallback for hosts that declare no
+        _PLUGIN_DIR -- the plugins' own probe harnesses build classes with
+        ``type()`` -- but it is no longer the primary answer.
         """
+        declared = getattr(self, "_PLUGIN_DIR", None)
+        if declared and os.path.isfile(os.path.join(declared, "config_schema.json")):
+            return declared
+
         for cls in type(self).__mro__:
             module = sys.modules.get(getattr(cls, "__module__", ""), None)
             path = getattr(module, "__file__", None)
@@ -253,7 +288,23 @@ class SportsCoreSharedMixin:
                     size = spec.get('properties', {}).get('font_size', {}).get('default')
                     if size is not None:
                         cache[key] = int(size)
-            except Exception:
+            except Exception as exc:
+                # Say so. An unreadable schema is not cosmetic: every element's
+                # configured size then stops matching "the schema default", is
+                # treated as a deliberate user choice, and skips the snap to the
+                # font's pixel grid -- which renders 4x6-font.ttf at 6 instead
+                # of 7, a 3px-wide glyph instead of 4px. That shipped once,
+                # silently, and was found by a user counting pixels on a photo
+                # of the panel.
+                #
+                # Logged, not raised: a missing schema must not stop a plugin
+                # rendering. The cache is built once per class, so this cannot
+                # repeat per frame.
+                logger.warning(
+                    "%s: could not read config_schema.json (%s: %s); every font "
+                    "size will be treated as user-chosen and will skip its pixel "
+                    "grid snap. Font sizes may render a pixel narrow.",
+                    type(self).__name__, type(exc).__name__, exc)
                 cache = {}
             self.__class__._SCHEMA_FONT_SIZES = cache
         return cache.get(element_key)

src/display_manager.py

@@ -364,6 +364,7 @@ class DisplayManager:
             # Create image with the (logical) display dimensions
             self.image = Image.new('RGB', (self.matrix.width, self.matrix.height))
             self.draw = ImageDraw.Draw(self.image)
+            self.draw.fontmode = "1"  # 1-bit text: the panel has no partial brightness, so AA only smears glyphs.
             logger.info(f"Image canvas created with dimensions: {self.matrix.width}x{self.matrix.height}")
             
             # Initialize font with Press Start 2P
@@ -403,6 +404,7 @@ class DisplayManager:
 
             self.image = Image.new('RGB', (fallback_width, fallback_height))
             self.draw = ImageDraw.Draw(self.image)
+            self.draw.fontmode = "1"  # 1-bit text: the panel has no partial brightness, so AA only smears glyphs.
             # Simple fallback visualization so web UI shows a realistic canvas
             try:
                 self.draw.rectangle([0, 0, fallback_width - 1, fallback_height - 1], outline=(255, 0, 0))
@@ -705,6 +707,7 @@ class DisplayManager:
             # self.image, so swapping the buffer below is enough on its own.
             self.image = Image.new('RGB', (target_w, target_h))
             self.draw = ImageDraw.Draw(self.image)
+            self.draw.fontmode = "1"  # 1-bit text: the panel has no partial brightness, so AA only smears glyphs.
             yield
         finally:
             self.matrix = real_matrix
@@ -814,6 +817,7 @@ class DisplayManager:
                 
                 self.image = Image.new('RGB', (width, height))
                 self.draw = ImageDraw.Draw(self.image)
+                self.draw.fontmode = "1"  # 1-bit text: the panel has no partial brightness, so AA only smears glyphs.
                 logger.debug("Cleared display in fallback mode")
                 return
                 
@@ -825,6 +829,7 @@ class DisplayManager:
             # Create a new black image
             self.image = Image.new('RGB', (self.matrix.width, self.matrix.height))
             self.draw = ImageDraw.Draw(self.image)
+            self.draw.fontmode = "1"  # 1-bit text: the panel has no partial brightness, so AA only smears glyphs.
 
             if not self._capture_mode_active:
                 # Clear both canvases and the underlying matrix to ensure no artifacts.
@@ -1258,6 +1263,7 @@ class DisplayManager:
             try:
                 self.image = Image.new('RGB', (self.width, self.height))
                 self.draw = ImageDraw.Draw(self.image)
+                self.draw.fontmode = "1"  # 1-bit text: the panel has no partial brightness, so AA only smears glyphs.
             except (OSError, RuntimeError, ValueError, MemoryError):
                 logger.debug("Canvas reset during cleanup failed", exc_info=True)
         # Reset the singleton state when cleaning up

src/plugin_system/testing/visual_display_manager.py

@@ -70,6 +70,7 @@ class VisualTestDisplayManager:
         # Canvas
         self.image = Image.new('RGB', (width, height), (0, 0, 0))
         self.draw = ImageDraw.Draw(self.image)
+        self.draw.fontmode = "1"  # Match production: 1-bit text, so goldens show what the panel shows.
 
         # Matrix proxy (plugins access display_manager.matrix.width/height)
         self.matrix = _MatrixProxy(width, height)
@@ -184,6 +185,7 @@ class VisualTestDisplayManager:
         self.clear_called = True
         self.image = Image.new('RGB', (self._width, self._height), (0, 0, 0))
         self.draw = ImageDraw.Draw(self.image)
+        self.draw.fontmode = "1"  # Match production: 1-bit text, so goldens show what the panel shows.
 
     def update_display(self):
         """No-op for hardware; marks that display was updated."""
@@ -211,6 +213,7 @@ class VisualTestDisplayManager:
             self.matrix = _MatrixProxy(target_w, target_h)
             self.image = Image.new('RGB', (target_w, target_h), (0, 0, 0))
             self.draw = ImageDraw.Draw(self.image)
+            self.draw.fontmode = "1"  # Match production: 1-bit text, so goldens show what the panel shows.
             yield
         finally:
             self._width, self._height = prev_w, prev_h
@@ -569,6 +572,7 @@ class VisualTestDisplayManager:
         self.draw_calls = []
         self.image = Image.new('RGB', (self._width, self._height), (0, 0, 0))
         self.draw = ImageDraw.Draw(self.image)
+        self.draw.fontmode = "1"  # Match production: 1-bit text, so goldens show what the panel shows.
         self._scrolling_state = {
             'is_scrolling': False,
             'last_scroll_activity': 0,
@@ -582,3 +586,4 @@ class VisualTestDisplayManager:
         """Clean up resources."""
         self.image = Image.new('RGB', (self._width, self._height), (0, 0, 0))
         self.draw = ImageDraw.Draw(self.image)
+        self.draw.fontmode = "1"  # Match production: 1-bit text, so goldens show what the panel shows.

test/test_sports_shared.py

@@ -319,3 +319,58 @@ class TestLiveMixin:
         assert not hasattr(h, "_empty_live_streak")
         h._note_live_fetch(False)
         assert h._empty_live_streak == 1
+
+
+class TestPluginDirIsToldNotDeduced:
+    """The regression that shipped: _plugin_dir returned None in production.
+
+    The first version walked the MRO for a class whose module sits beside a
+    config_schema.json. That passes when a test imports the plugin directly --
+    which is how it was verified -- and returns None under the real plugin
+    loader, which imports modules by a path that leaves no such entry on the
+    MRO.
+
+    Silent, and expensive: no schema means _schema_font_size returns None for
+    every element, so a configured size equal to the schema default stops
+    looking like a default, is treated as a deliberate choice, and skips the
+    grid snap. 4x6-font.ttf then renders at 6 rather than 7 -- 3px-wide glyphs
+    instead of 4px. On a 256x64 panel that made the odds, the records and the
+    date row illegible. A user counted the pixels; no gate here caught it.
+    """
+
+    def test_a_declared_directory_is_used(self, tmp_path):
+        d = _write_plugin(tmp_path, "declared")
+        host = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})()
+        assert host._plugin_dir() == str(d)
+
+    def test_it_works_when_no_module_on_the_mro_helps(self, tmp_path, monkeypatch):
+        """The production case: nothing on the MRO sits beside a schema."""
+        d = _write_plugin(tmp_path, "loaderstyle")
+        # A class whose module is not importable by name, as the loader produces.
+        cls = type("Loaded", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})
+        cls.__module__ = "a.module.name.that.is.not.in.sys.modules"
+        assert cls.__new__(cls)._plugin_dir() == str(d), (
+            "the declared directory must win when the MRO walk cannot help")
+
+    def test_without_it_the_mro_walk_would_have_failed(self):
+        # Pin the precondition, so this test still means something if the
+        # fallback is ever changed.
+        cls = type("Orphan", (SportsCoreSharedMixin,), {})
+        cls.__module__ = "not.a.real.module"
+        assert cls.__new__(cls)._plugin_dir() is None
+
+    def test_a_declared_directory_without_a_schema_is_not_trusted(self, tmp_path):
+        # A stale or wrong path must not shadow the fallback.
+        empty = tmp_path / "noschema"
+        empty.mkdir()
+        d = _write_plugin(tmp_path, "realone")
+        monkey = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(empty)})
+        assert monkey.__new__(monkey)._plugin_dir() != str(empty)
+
+    def test_the_font_size_consequence(self, tmp_path):
+        """End to end: a declared directory restores the schema lookup."""
+        d = _write_plugin(tmp_path, "sizeconseq")
+        host = type("H", (SportsCoreSharedMixin,), {"_PLUGIN_DIR": str(d)})()
+        assert host._schema_font_size("score") == 16, (
+            "without the schema this is None, which is what made a configured "
+            "size look user-chosen and skipped the grid snap")