mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-18 12:59:12 +00:00
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
This commit is contained in:
co-authored by
Claude Opus 5
parent
a29c84208e
commit
ad36d2efc3
@@ -520,6 +520,38 @@ class BasePlugin(ABC):
|
|||||||
"""
|
"""
|
||||||
return
|
return
|
||||||
|
|
||||||
|
def get_update_interval(self) -> Optional[float]:
|
||||||
|
"""
|
||||||
|
How often this plugin wants update() called, right now, in seconds.
|
||||||
|
|
||||||
|
The manifest's ``update_interval`` is a single static number, which
|
||||||
|
cannot say "poll me every 15 seconds while a game is in progress and
|
||||||
|
every 15 minutes when nothing is on". Only the plugin knows which is
|
||||||
|
true at any moment, so override this to say so.
|
||||||
|
|
||||||
|
Return None (the default) to accept the manifest/config value.
|
||||||
|
|
||||||
|
Two constraints, both because the scheduler calls this on every tick of
|
||||||
|
the render loop:
|
||||||
|
|
||||||
|
- It must be cheap. Attribute reads only -- no config lookups, no I/O,
|
||||||
|
no locks that a fetch might be holding.
|
||||||
|
- It must not raise. A raising hook is ignored and the static interval
|
||||||
|
used, but a hook that raises every tick also logs every tick.
|
||||||
|
|
||||||
|
Values below PluginManager.MIN_DYNAMIC_UPDATE_INTERVAL are clamped up:
|
||||||
|
a plugin asking for 0 would otherwise busy-wait against its own API.
|
||||||
|
|
||||||
|
Example::
|
||||||
|
|
||||||
|
def get_update_interval(self):
|
||||||
|
# Fast while something is actually live, manifest default otherwise.
|
||||||
|
if any(m.live_games for m in self._live_managers):
|
||||||
|
return self.config.get("live_update_interval", 15)
|
||||||
|
return None
|
||||||
|
"""
|
||||||
|
return None
|
||||||
|
|
||||||
def has_live_priority(self) -> bool:
|
def has_live_priority(self) -> bool:
|
||||||
"""
|
"""
|
||||||
Check if this plugin has live priority enabled.
|
Check if this plugin has live priority enabled.
|
||||||
|
|||||||
@@ -779,15 +779,63 @@ class PluginManager:
|
|||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
def _dynamic_update_interval(self, plugin_id: str, plugin_instance: Any) -> Optional[float]:
|
||||||
|
"""The interval a plugin asks for right now, or None if it has no view."""
|
||||||
|
hook = getattr(plugin_instance, 'get_update_interval', None)
|
||||||
|
if not callable(hook):
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
requested = hook()
|
||||||
|
except Exception as exc: # pylint: disable=broad-except
|
||||||
|
self.logger.debug(
|
||||||
|
"get_update_interval() failed for %s, using the static interval: %s",
|
||||||
|
plugin_id, exc)
|
||||||
|
return None
|
||||||
|
if requested is None:
|
||||||
|
return None
|
||||||
|
try:
|
||||||
|
requested = float(requested)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
self.logger.debug(
|
||||||
|
"get_update_interval() returned %r for %s, which is not a number",
|
||||||
|
requested, plugin_id)
|
||||||
|
return None
|
||||||
|
if requested != requested or requested == float('inf'): # NaN / inf
|
||||||
|
return None
|
||||||
|
return max(requested, self.MIN_DYNAMIC_UPDATE_INTERVAL)
|
||||||
|
|
||||||
|
#: Floor for a plugin-requested interval. A plugin asking for 0 (or a
|
||||||
|
#: negative) would otherwise be re-entered on every tick of the render
|
||||||
|
#: loop, which is a busy-wait against whatever API it fetches.
|
||||||
|
MIN_DYNAMIC_UPDATE_INTERVAL = 5.0
|
||||||
|
|
||||||
def _get_plugin_update_interval(self, plugin_id: str, plugin_instance: Any) -> Optional[float]:
|
def _get_plugin_update_interval(self, plugin_id: str, plugin_instance: Any) -> Optional[float]:
|
||||||
"""
|
"""
|
||||||
Get the data-fetch interval for a plugin (seconds between update() calls).
|
Get the data-fetch interval for a plugin (seconds between update() calls).
|
||||||
|
|
||||||
Result is cached per plugin_id after the first lookup to avoid calling
|
A plugin may implement ``get_update_interval()`` to vary its own cadence
|
||||||
config_manager.get_config() — which returns a full dict copy — on every
|
at runtime, which the static manifest value cannot express. The case
|
||||||
tick of the 30-fps display loop. The cache is invalidated when a plugin
|
this exists for: a sports scoreboard needs to poll every 15s while a
|
||||||
is loaded or unloaded.
|
game is in progress and every 15 minutes when nothing is on, and only
|
||||||
|
the plugin knows which is true right now. Returning None from the hook
|
||||||
|
means "no opinion", and the static resolution below applies.
|
||||||
|
|
||||||
|
The hook is called on every scheduling tick, so implementations must be
|
||||||
|
cheap — attribute reads, no config lookups and no I/O. A raising or
|
||||||
|
non-numeric hook is ignored rather than allowed to stop the plugin
|
||||||
|
updating, since a scheduler that propagates a plugin bug stops every
|
||||||
|
other plugin too.
|
||||||
|
|
||||||
|
The static result is cached per plugin_id after the first lookup to
|
||||||
|
avoid calling config_manager.get_config() — which returns a full dict
|
||||||
|
copy — on every tick of the 30-fps display loop. The cache is
|
||||||
|
invalidated when a plugin is loaded or unloaded. The dynamic hook is
|
||||||
|
deliberately *not* cached: caching it would defeat its only purpose.
|
||||||
"""
|
"""
|
||||||
|
dynamic = self._dynamic_update_interval(plugin_id, plugin_instance)
|
||||||
|
if dynamic is not None:
|
||||||
|
return dynamic
|
||||||
|
|
||||||
if plugin_id in self._update_interval_cache:
|
if plugin_id in self._update_interval_cache:
|
||||||
return self._update_interval_cache[plugin_id]
|
return self._update_interval_cache[plugin_id]
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,147 @@
|
|||||||
|
"""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")])
|
||||||
|
def test_nan_and_infinity_fall_back(self, manager, junk):
|
||||||
|
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