diff --git a/src/plugin_system/base_plugin.py b/src/plugin_system/base_plugin.py index dbe14a49..54aa7426 100644 --- a/src/plugin_system/base_plugin.py +++ b/src/plugin_system/base_plugin.py @@ -520,6 +520,38 @@ class BasePlugin(ABC): """ 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: """ Check if this plugin has live priority enabled. diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 0dc7a428..b72c4f02 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -779,15 +779,63 @@ class PluginManager: 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]: """ 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 - 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. + A plugin may implement ``get_update_interval()`` to vary its own cadence + at runtime, which the static manifest value cannot express. The case + this exists for: a sports scoreboard needs to poll every 15s while a + 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: return self._update_interval_cache[plugin_id] diff --git a/test/test_plugin_dynamic_update_interval.py b/test/test_plugin_dynamic_update_interval.py new file mode 100644 index 00000000..e87cf104 --- /dev/null +++ b/test/test_plugin_dynamic_update_interval.py @@ -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