mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
fix(plugins): let a plugin ask to be polled faster while it has live content (#555)
* fix(plugins): let a plugin ask to be polled faster while it has live content
Reported: "the football plugin with live games only updates the live game in
progress if I restart the display."
The data path was never the problem. NFLLiveManager fetches ESPN with no cache,
SportsLive.update() refreshes current_game in place when the game IDs are
unchanged, and the scorebug redraws from the game dict every frame -- which is
why the reporter's logs look healthy.
The problem is cadence. _get_plugin_update_interval() read only the manifest's
static update_interval, football's manifest pins that to 60, and the plugin's
own live_update_interval (15s) was invisible to the scheduler. Measured on a rig
during the fourth quarter of the game in the report:
23:21:49 23:22:50 23:23:50 23:24:50 23:25:50 <- exactly 60s apart
A clock and score up to a minute stale during a two-minute drill reads as a
frozen panel, and a restart is the one moment it is ever current.
A single static number cannot say "every 15 seconds while a game is on, every 15
minutes in July", and only the plugin knows which is true. get_update_interval()
lets it say so per tick; returning None means "no opinion" and the existing
manifest/config resolution applies, so every plugin that predates this is
unaffected.
Requests are clamped to MIN_DYNAMIC_UPDATE_INTERVAL (5s): a plugin returning 0
would otherwise be re-entered on every tick of the render loop, busy-waiting
against its own API. A hook that raises or returns a non-number is ignored
rather than propagated -- a scheduler that fails on one plugin's bug stops
updating all the others.
Deliberately NOT changed: the manifest still beats config in the static path.
That looked like the obvious fix -- user config being silently ignored -- until
checking a real rig, where football and baseball both carry update_interval 3600
in config against a manifest 60, and weather 1800 against 60. Those values are
stale precisely because nothing has been honouring them; making config win would
have slowed three plugins by 60x, turning a one-minute lag into an hour. The
dynamic hook makes the flip unnecessary. There is a test pinning the current
precedence with that reasoning attached.
Full suite: 4,283 passed, 68 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
* test(plugins): drive the real scheduler, not just the interval resolver
test_plugin_dynamic_update_interval.py asserts that
_get_plugin_update_interval() returns the number the plugin asked for. That is
not the same claim as "the plugin gets updated more often", and the gap between
those two is exactly where the original bug lived: the plugin knew it wanted
15s, said so in live_update_interval, and nothing downstream acted on it.
So this ticks the real run_scheduled_updates() through a simulated hour and
counts dispatches. Against pre-fix core it reports "10 updates in 10 minutes of
a live game" -- the 60s manifest cadence, matching what was measured on a rig
during the reported game. Against the fix it reports ~40.
Also pins the regression that would be worse than the bug: an idle hour must
still be ~60 updates, not 240. Asking for the live interval year-round would
poll ESPN four times a minute all summer.
Scope note, since it is easy to over-read this fix: the *switch* display path
already refreshed the manager immediately before drawing, via
_try_manager_display() -> _ensure_manager_updated(), which honours the manager's
own 15s interval. So a switch-mode card was already <=15s stale at draw time
before this change. What this fixes is the background cadence, which is what
live-priority detection, Vegas content and scroll preparation all read.
Full suite: 4,288 passed, 68 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
* fix(plugins): reject bool and -inf hook results in dynamic interval
get_update_interval() ran bool through float() (bool is an int subclass,
so True/False became 1.0/0.0) and only checked for +inf, not -inf. Both
cases landed on the MIN_DYNAMIC_UPDATE_INTERVAL floor by coincidence
instead of falling back to the static/manifest interval as invalid
input should.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017Tst9cied2ri9bH4QRWa6H
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,153 @@
|
||||
"""A plugin can ask to be polled faster while it has something live.
|
||||
|
||||
The manifest carries one static `update_interval`, and until now that was the
|
||||
only thing the scheduler would look at. A sports scoreboard needs 15 seconds
|
||||
while a game is in progress and 15 minutes when nothing is on, and no single
|
||||
number expresses that.
|
||||
|
||||
Measured consequence, on a live rig during an NFL game in its fourth quarter:
|
||||
football-scoreboard's manifest pins update_interval to 60, so ESPN was polled
|
||||
exactly once a minute --
|
||||
|
||||
23:21:49 23:22:50 23:23:50 23:24:50 23:25:50
|
||||
|
||||
-- while the plugin's own live_update_interval said 15. The clock and score on
|
||||
the panel therefore lagged by up to a minute during a two-minute drill, which
|
||||
reads to a viewer as a frozen display.
|
||||
|
||||
get_update_interval() lets the plugin say what it needs per tick. These tests
|
||||
pin the parts that are easy to get wrong: that the hook wins, that "no opinion"
|
||||
falls back cleanly, that a broken hook cannot stop a plugin updating, and that
|
||||
the hook is not accidentally cached (which would defeat the whole point).
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from src.plugin_system.plugin_manager import PluginManager
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def manager():
|
||||
pm = PluginManager.__new__(PluginManager)
|
||||
pm._update_interval_cache = {}
|
||||
pm.plugin_manifests = {"sports": {"update_interval": 60}}
|
||||
pm.config_manager = None
|
||||
pm.logger = MagicMock()
|
||||
return pm
|
||||
|
||||
|
||||
class _Plugin:
|
||||
def __init__(self, wants=None, raises=False):
|
||||
self._wants = wants
|
||||
self._raises = raises
|
||||
self.calls = 0
|
||||
|
||||
def get_update_interval(self):
|
||||
self.calls += 1
|
||||
if self._raises:
|
||||
raise RuntimeError("plugin is broken")
|
||||
return self._wants
|
||||
|
||||
|
||||
def test_the_hook_overrides_the_manifest(manager):
|
||||
"""The whole point: 15s while live, not the manifest's 60."""
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(wants=15)) == 15.0
|
||||
|
||||
|
||||
def test_none_means_no_opinion_and_the_manifest_applies(manager):
|
||||
"""A plugin with nothing live should not have to restate the default."""
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(wants=None)) == 60.0
|
||||
|
||||
|
||||
def test_a_plugin_without_the_hook_is_unaffected(manager):
|
||||
"""Every existing plugin predates this and must keep its manifest value."""
|
||||
assert manager._get_plugin_update_interval("sports", MagicMock(spec=[])) == 60.0
|
||||
|
||||
|
||||
def test_the_hook_is_consulted_every_time(manager):
|
||||
"""Caching the answer would make it exactly as static as the manifest.
|
||||
|
||||
The static path *is* cached -- reading config is expensive -- so it would be
|
||||
an easy mistake to let the dynamic answer ride along in that cache.
|
||||
"""
|
||||
plugin = _Plugin(wants=15)
|
||||
for _ in range(5):
|
||||
manager._get_plugin_update_interval("sports", plugin)
|
||||
assert plugin.calls == 5, f"hook called {plugin.calls} times in 5 lookups"
|
||||
|
||||
|
||||
def test_the_cadence_can_change_between_ticks(manager):
|
||||
"""A game going final must slow the polling back down without a reload."""
|
||||
plugin = _Plugin(wants=15)
|
||||
assert manager._get_plugin_update_interval("sports", plugin) == 15.0
|
||||
plugin._wants = None # game ended
|
||||
assert manager._get_plugin_update_interval("sports", plugin) == 60.0
|
||||
plugin._wants = 15 # another game started
|
||||
assert manager._get_plugin_update_interval("sports", plugin) == 15.0
|
||||
|
||||
|
||||
class TestABrokenHookCannotStopUpdates:
|
||||
"""A scheduler that propagates a plugin's bug stops every other plugin too."""
|
||||
|
||||
def test_a_raising_hook_falls_back_to_the_manifest(self, manager):
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(raises=True)) == 60.0
|
||||
|
||||
@pytest.mark.parametrize("junk", ["soon", object(), [], {}])
|
||||
def test_a_non_numeric_hook_falls_back(self, manager, junk):
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(wants=junk)) == 60.0
|
||||
|
||||
@pytest.mark.parametrize("junk", [float("nan"), float("inf"), float("-inf")])
|
||||
def test_nan_and_infinity_fall_back(self, manager, junk):
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(wants=junk)) == 60.0
|
||||
|
||||
@pytest.mark.parametrize("junk", [True, False])
|
||||
def test_a_bool_hook_falls_back(self, manager, junk):
|
||||
"""bool is a subclass of int, so it must be rejected before float()
|
||||
turns it into 0.0/1.0 and the floor silently swallows the mistake."""
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(wants=junk)) == 60.0
|
||||
|
||||
|
||||
class TestTheFloor:
|
||||
"""A plugin asking for 0 would be re-entered on every tick of the render
|
||||
loop, which is a busy-wait against whatever API it fetches."""
|
||||
|
||||
@pytest.mark.parametrize("wants", [0, 0.5, -10])
|
||||
def test_small_and_negative_requests_are_clamped(self, manager, wants):
|
||||
got = manager._get_plugin_update_interval("sports", _Plugin(wants=wants))
|
||||
assert got == PluginManager.MIN_DYNAMIC_UPDATE_INTERVAL
|
||||
|
||||
def test_a_reasonable_request_is_not_clamped(self, manager):
|
||||
assert manager._get_plugin_update_interval("sports", _Plugin(wants=15)) == 15.0
|
||||
|
||||
|
||||
def test_the_static_path_still_prefers_the_manifest_over_config():
|
||||
"""Deliberately unchanged, and not an oversight.
|
||||
|
||||
Config values that have sat inert behind the manifest are stale by
|
||||
definition -- nothing has been honouring them. On one rig, football and
|
||||
baseball both carry update_interval 3600 in config against a manifest 60;
|
||||
making config win would slow those plugins by 60x. The dynamic hook is the
|
||||
supported way for a plugin to vary its own cadence, so flipping this is
|
||||
both risky and no longer necessary.
|
||||
"""
|
||||
pm = PluginManager.__new__(PluginManager)
|
||||
pm._update_interval_cache = {}
|
||||
pm.plugin_manifests = {"sports": {"update_interval": 60}}
|
||||
cfg = MagicMock()
|
||||
cfg.get_config.return_value = {"sports": {"update_interval": 3600}}
|
||||
pm.config_manager = cfg
|
||||
pm.logger = MagicMock()
|
||||
assert pm._get_plugin_update_interval("sports", MagicMock(spec=[])) == 60.0
|
||||
|
||||
|
||||
def test_config_is_still_used_when_the_manifest_is_silent():
|
||||
pm = PluginManager.__new__(PluginManager)
|
||||
pm._update_interval_cache = {}
|
||||
pm.plugin_manifests = {"sports": {}}
|
||||
cfg = MagicMock()
|
||||
cfg.get_config.return_value = {"sports": {"update_interval": 120}}
|
||||
pm.config_manager = cfg
|
||||
pm.logger = MagicMock()
|
||||
assert pm._get_plugin_update_interval("sports", MagicMock(spec=[])) == 120.0
|
||||
Reference in New Issue
Block a user