From c6b064b2207118d2631d6f296c28c3af84bdff01 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 22:20:26 -0400 Subject: [PATCH] fix(on-demand): show a named live mode; end a session that cannot resume A request naming a *_live mode (football-scoreboard / ncaa_fb_live with 15 college games on) answered 200 and showed nfl_recent. The session's mode list kept live modes only when has_live_content() said so, which is the live-priority question and is answered for favourite teams only. A mode the request names now leads the session; display() decides whether it has anything to draw, and an empty one moves on to the plugin's next mode as any empty on-demand mode does. The name is saved in display_on_demand_config (named_mode) so a restart resumes on it. A bare plugin-id request still skips quiet live modes, as before. A restart during a session whose plugin then failed to load (clock-simple failed config validation after a crash on ledpi) left the session active with no modes and its cached request in place. It now ends at startup with status error / restore-failed, and the cached request is dropped; likewise when the plugin system fails to start. Golden traces: two new scenarios (on_demand_named_live, on_demand_restore_failed); every existing trace is unchanged. The harness's restore_on_demand takes named_mode and logs a failed restore. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 20 +++ docs/RUN_LOOP_REDESIGN.md | 5 +- src/display_controller.py | 53 +++++++- test/_run_loop_harness.py | 9 +- .../run_loop_golden/on_demand_named_live.json | 29 +++++ .../on_demand_restore_failed.json | 10 ++ test/test_on_demand_live_and_restore.py | 119 ++++++++++++++++++ test/test_run_loop_golden.py | 28 +++++ 8 files changed, 264 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/run_loop_golden/on_demand_named_live.json create mode 100644 test/fixtures/run_loop_golden/on_demand_restore_failed.json create mode 100644 test/test_on_demand_live_and_restore.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 35ac9182..35b6bb0e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -488,6 +488,26 @@ policies are unchanged. ### Fixes +- An on-demand request that names a `*_live` mode now shows that mode. On + ledpi, `{"plugin_id": "football-scoreboard", "mode": "ncaa_fb_live"}` with + 15 college games on answered 200 and showed `nfl_recent`. The session's + mode list kept a live mode only when the plugin's `has_live_content()` + said so. That method answers the live-priority question, and the sports + plugins answer it for favourite teams only. A mode the request names + (not one resolved from a bare plugin id) now leads the session, with the + plugin's other modes after it. If it has nothing to draw, the session + moves on to the next of those modes, like any empty on-demand mode. The + name is saved with the session (`named_mode` in + `display_on_demand_config`), so a restart resumes on it. +- A restart during an on-demand session whose plugin then fails to load no + longer leaves a session with no modes. On ledpi, `clock-simple` failed + config validation after a crash. The display logged `No valid display + modes found for on-demand plugin 'clock-simple' after restoration` and + kept reporting the session as active until its first pass ended it as + `idle`. The cached request stayed behind for the next restart. The session + now ends at startup with status `error` and error `restore-failed`, which + `/display/on-demand/status` reports, and the cached request is dropped. The + same applies when the plugin system itself fails to start. - The garbage-collection timer (`GcMonitor`, above) no longer prints `Exception ignored while calling GC callback ... 'NoneType' object has no attribute 'perf_counter'` when the display service or a test run exits. diff --git a/docs/RUN_LOOP_REDESIGN.md b/docs/RUN_LOOP_REDESIGN.md index 73dd4221..0b58cd53 100644 --- a/docs/RUN_LOOP_REDESIGN.md +++ b/docs/RUN_LOOP_REDESIGN.md @@ -181,13 +181,14 @@ that the harness patches in today. - dynamic duration (cycle complete, plugin cap, global cap) - live priority taking over and handing back; live round-robin - on-demand start/stop/expiry; pinned on-demand; a session resumed after - a restart + a restart, and one that cannot resume (its plugin did not load); a + request naming a live mode the plugin's live check would drop - schedule off and dim, with an on-demand override during downtime - WiFi notice; sync follower - Vegas, with and without `live_in_ticker` - Each trace row is `[start, mode, duration, exit_reason, frames, force_clear]`. The exit reason is the event that decided what came next. -- All 16 tests run in under a second. The goldens were generated from +- All 18 tests run in under a second. The goldens were generated from main's `run()` before any code moved. - Vegas uses `FakeVegas`, which implements only the contract the controller depends on: `run_iteration()` returns True after its duration and False diff --git a/src/display_controller.py b/src/display_controller.py index 8756518a..f4ac77d0 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -354,6 +354,10 @@ class DisplayController: self.on_demand_last_error: Optional[str] = None self.on_demand_last_event: Optional[str] = None self.on_demand_schedule_override = False + # The mode the request named, when it named one (not a mode resolved + # from a bare plugin id). Shown even when the plugin's live checks + # would leave it out of the session (_on_demand_modes_for_plugin). + self._on_demand_named_mode: Optional[str] = None # Plugins that are disabled in config and loaded only because an # on-demand request named them. The main loop unloads each one once # on-demand has moved off it (_release_on_demand_plugins). @@ -547,6 +551,10 @@ class DisplayController: except Exception: # pylint: disable=broad-except logger.exception("Plugin system initialization failed") self.plugin_manager = None + if self.on_demand_active: + # A restored session has no plugin to resume on. + self.cache_manager.clear_cache('display_on_demand_config') + self._set_on_demand_error('restore-failed') # Its state machine no longer describes what runs; let the last # snapshot go stale (readers then say unknown) rather than keep # refreshing it. @@ -1620,6 +1628,7 @@ class DisplayController: self.on_demand_expires_at = None self.on_demand_pinned = False self.on_demand_schedule_override = False + self._on_demand_named_mode = None # While the session ran, _evaluate_schedule may have forced # is_display_active on over a scheduled-off answer. Drop the minute # gate so the next _check_schedule recomputes it; otherwise the panel @@ -1786,6 +1795,7 @@ class DisplayController: self.on_demand_pinned = on_demand_config.get('pinned', False) self.on_demand_requested_at = on_demand_config.get('requested_at') self.on_demand_expires_at = on_demand_config.get('expires_at') + self._on_demand_named_mode = on_demand_config.get('named_mode') self.on_demand_status = 'active' self.on_demand_schedule_override = True logger.info("On-demand mode detected during initialization: resuming on plugin '%s'; " @@ -2272,13 +2282,25 @@ class DisplayController: return modes[0] return plugin_id - def _on_demand_modes_for_plugin(self, plugin_id: str) -> List[str]: + def _on_demand_modes_for_plugin(self, plugin_id: str, + named_mode: Optional[str] = None) -> List[str]: """Every loaded display mode belonging to `plugin_id`, in rotation order. Live modes that actually have content lead, then the rest, then live modes with nothing to show -- so an on-demand request for a sports plugin opens on a game in progress rather than an empty live screen. Returns an empty list when the plugin has no loaded modes. + + `named_mode` is a mode the request asked for by name. It is always + in the list, first when the checks below would have dropped it. + Those checks ask has_live_content(), which is the live-priority + question -- "should this plugin take the panel from the rotation?" + -- and the sports plugins answer it for favourite teams only. Asking + for ncaa_fb_live with fifteen games on and no favourite playing got + a 200 and nfl_recent on the panel. The plugin's display() is what + knows whether the mode has anything to draw; when it has not, the + session moves to the plugin's next mode like any empty on-demand + mode. """ plugin_modes = self.plugin_display_modes.get(plugin_id, []) if not plugin_modes: @@ -2319,6 +2341,12 @@ class DisplayController: # Only live modes available but no content - use them anyway ordered_modes = live_modes + if (named_mode and named_mode in available_plugin_modes + and named_mode not in ordered_modes): + logger.info("On-demand: showing %s as requested; plugin '%s' reports no " + "live-priority content for it", named_mode, plugin_id) + ordered_modes = [named_mode] + ordered_modes + return ordered_modes def _apply_on_demand_pin(self, ordered_modes: List[str], resolved_mode: Optional[str], @@ -2347,10 +2375,20 @@ class DisplayController: plugin_id = self.on_demand_plugin_id - ordered_modes = self._on_demand_modes_for_plugin(plugin_id) + ordered_modes = self._on_demand_modes_for_plugin(plugin_id, self._on_demand_named_mode) if not ordered_modes: - logger.warning("No valid display modes found for on-demand plugin '%s' after restoration", plugin_id) - self.on_demand_modes = [] + # The plugin did not load this time (seen on a rig: its config + # failed validation after the crash that caused the restart), so + # there is nothing to resume. Leaving the session active with no + # modes published it as active for a plugin that was not running + # until the first pass ended it as an ordinary 'idle', and kept + # the cached request for the next restart to trip over. End it + # as a failure the status endpoint reports, and drop the cache. + logger.error("On-demand session for plugin '%s' cannot resume after the " + "restart: the plugin has no loaded display modes (did it " + "fail to load?); ending it", plugin_id) + self.cache_manager.clear_cache('display_on_demand_config') + self._set_on_demand_error('restore-failed') return # A restart must not silently un-pin: the pin is part of the request @@ -2551,7 +2589,10 @@ class DisplayController: if resolved_mode in self.available_modes: self.current_mode_index = self.available_modes.index(resolved_mode) - ordered_modes = self._on_demand_modes_for_plugin(resolved_plugin_id) + # Named: the request gave this mode itself, rather than a plugin id + # (or a mode the plugin doesn't have) that resolved to a default. + named_mode = resolved_mode if mode == resolved_mode else None + ordered_modes = self._on_demand_modes_for_plugin(resolved_plugin_id, named_mode) if not ordered_modes: logger.error("No valid display modes found for plugin '%s'", resolved_plugin_id) self._set_on_demand_error("no-modes") @@ -2568,6 +2609,7 @@ class DisplayController: self.on_demand_requested_at = now self.on_demand_expires_at = (now + duration) if duration else None self.on_demand_pinned = pinned + self._on_demand_named_mode = named_mode self.on_demand_status = 'active' self.on_demand_last_error = None self.on_demand_last_event = 'started' @@ -2596,6 +2638,7 @@ class DisplayController: 'mode': resolved_mode, 'duration': duration, 'pinned': pinned, + 'named_mode': named_mode, 'requested_at': now, 'expires_at': self.on_demand_expires_at } diff --git a/test/_run_loop_harness.py b/test/_run_loop_harness.py index 6d866760..80b04e5c 100644 --- a/test/_run_loop_harness.py +++ b/test/_run_loop_harness.py @@ -724,11 +724,14 @@ class RunLoopHarness: self.clock.at(t, post) def restore_on_demand(self, plugin_id: str, mode: Optional[str] = None, - duration: Optional[float] = None, pinned: bool = False): + duration: Optional[float] = None, pinned: bool = False, + named_mode: Optional[str] = None): """Start with an on-demand session resumed from the cache, as after a restart: the state _select_startup_plugins restores, then - _populate_on_demand_modes_from_plugin, as __init__ calls it.""" + _populate_on_demand_modes_from_plugin, as __init__ calls it. A + session that cannot resume is logged as ``on-demand-error``.""" dc = self.controller + dc._on_demand_named_mode = named_mode dc.on_demand_active = True dc.on_demand_plugin_id = plugin_id dc.on_demand_mode = mode @@ -739,6 +742,8 @@ class RunLoopHarness: dc.on_demand_status = 'active' dc.on_demand_schedule_override = True dc._populate_on_demand_modes_from_plugin() + if dc.on_demand_status == 'error': + self.log("on-demand-error", dc.on_demand_last_error) def wifi_message(self, t: float, message: str, duration: float = 5): def write(): diff --git a/test/fixtures/run_loop_golden/on_demand_named_live.json b/test/fixtures/run_loop_golden/on_demand_named_live.json new file mode 100644 index 00000000..8b4ac876 --- /dev/null +++ b/test/fixtures/run_loop_golden/on_demand_named_live.json @@ -0,0 +1,29 @@ +{ + "screens": [ + [0.0, "clock", 5.0, "on-demand-start", 6, false], + [5.0, "sports_live", 15.0, "duration", 15, true], + [20.0, "sports_recent", 15.0, "duration", 15, true], + [35.0, "sports_upcoming", 5.0, "on-demand-requested-stop", 6, true], + [40.0, "clock", 20.0, "duration", 20, true], + [60.0, "sports_live", 15.0, "display-false", 11, true], + [75.0, "sports_recent", 15.0, "duration", 15, true], + [90.0, "sports_upcoming", 10.0, "on-demand-start", 11, true], + [100.0, "sports_live", 0.0, "empty", 1, true], + [100.0, "sports_recent", 15.0, "duration", 15, true], + [115.0, "sports_upcoming", 15.0, "duration", 15, true], + [130.0, "sports_live", 0.0, "empty", 1, true], + [130.0, "sports_recent", 10.0, "on-demand-requested-stop", 11, true], + [140.0, "sports_upcoming", 15.0, "duration", 15, true], + [155.0, "clock", 5.0, "horizon", 5, true] + ], + "events": [ + [5.0, "request", "start:n1"], + [5.0, "on-demand-start", "sports"], + [40.0, "request", "stop:n2"], + [40.0, "on-demand-requested-stop"], + [100.0, "request", "start:n3"], + [100.0, "on-demand-start", "sports"], + [140.0, "request", "stop:n4"], + [140.0, "on-demand-requested-stop"] + ] +} diff --git a/test/fixtures/run_loop_golden/on_demand_restore_failed.json b/test/fixtures/run_loop_golden/on_demand_restore_failed.json new file mode 100644 index 00000000..ed7a3a0d --- /dev/null +++ b/test/fixtures/run_loop_golden/on_demand_restore_failed.json @@ -0,0 +1,10 @@ +{ + "screens": [ + [0.0, "clock", 20.0, "duration", 20, false], + [20.0, "weather", 20.0, "duration", 20, true], + [40.0, "clock", 20.0, "horizon", 20, true] + ], + "events": [ + [0.0, "on-demand-error", "restore-failed"] + ] +} diff --git a/test/test_on_demand_live_and_restore.py b/test/test_on_demand_live_and_restore.py new file mode 100644 index 00000000..2b7d9738 --- /dev/null +++ b/test/test_on_demand_live_and_restore.py @@ -0,0 +1,119 @@ +"""Two on-demand edges seen on a rig. + + * A request naming a ``*_live`` mode got HTTP 200 and a different mode on + the panel. The session's mode list kept live modes only when the plugin's + has_live_content() said so, and that is the live-priority question, + which the sports plugins answer for favourite teams only: fifteen college + games on, no favourite playing, and ``ncaa_fb_live`` became + ``nfl_recent``. + * A restart during a session whose plugin then failed to load (its config + no longer validated) logged "No valid display modes found ... after + restoration" and left the session active with no modes: published as + active for a plugin that was not running, with its cached request kept + for the next restart. +""" + +from unittest.mock import MagicMock + +import pytest + +SPORTS_MODES = ['nfl_live', 'nfl_recent', 'nfl_upcoming', + 'ncaa_fb_live', 'ncaa_fb_recent', 'ncaa_fb_upcoming'] + + +def _sports_plugin(has_live_content=False): + plugin = MagicMock(spec=['display', 'has_live_content', 'has_live_priority', + 'get_live_modes']) + plugin.has_live_content.return_value = has_live_content + plugin.has_live_priority.return_value = True + plugin.get_live_modes.return_value = [] + return plugin + + +def _register(controller, plugin_id, modes, plugin): + controller.plugin_display_modes[plugin_id] = list(modes) + for mode in modes: + controller.plugin_modes[mode] = plugin + controller.mode_to_plugin_id[mode] = plugin_id + if mode not in controller.available_modes: + controller.available_modes.append(mode) + + +@pytest.fixture +def football(test_display_controller): + c = test_display_controller + _register(c, 'football-scoreboard', SPORTS_MODES, _sports_plugin()) + return c + + +class TestANamedLiveModeIsShown: + + def test_it_is_the_first_screen(self, football): + football._activate_on_demand({'plugin_id': 'football-scoreboard', + 'mode': 'ncaa_fb_live'}) + assert football.on_demand_active + assert football.current_display_mode == 'ncaa_fb_live' + assert football.on_demand_mode == 'ncaa_fb_live' + + def test_the_plugins_other_modes_follow_it(self, football): + football._activate_on_demand({'plugin_id': 'football-scoreboard', + 'mode': 'ncaa_fb_live'}) + assert football.on_demand_modes[0] == 'ncaa_fb_live' + assert set(football.on_demand_modes[1:]) == { + 'nfl_recent', 'nfl_upcoming', 'ncaa_fb_recent', 'ncaa_fb_upcoming'} + + def test_pinned_holds_it(self, football): + football._activate_on_demand({'plugin_id': 'football-scoreboard', + 'mode': 'ncaa_fb_live', 'pinned': True}) + assert football.on_demand_modes == ['ncaa_fb_live'] + + def test_a_bare_plugin_request_still_skips_quiet_live_modes(self, football): + """Only a mode asked for by name is kept: a plugin-only request + resolves to the plugin's first mode (nfl_live), and opening on an + empty live screen there is what the ordering exists to avoid.""" + football._activate_on_demand({'plugin_id': 'football-scoreboard'}) + assert not any(m.endswith('_live') for m in football.on_demand_modes) + + def test_the_named_mode_survives_a_restart(self, football): + football._activate_on_demand({'plugin_id': 'football-scoreboard', + 'mode': 'ncaa_fb_live'}) + saved = football.cache_manager.set.call_args_list[-1] + assert saved.args[0] == 'display_on_demand_config' + config = saved.args[1] + assert config['named_mode'] == 'ncaa_fb_live' + + football._reset_on_demand_fields() + football._select_startup_plugins(['football-scoreboard'], config) + football._populate_on_demand_modes_from_plugin() + assert football.on_demand_modes[football.on_demand_mode_index] == 'ncaa_fb_live' + + +class TestARestoreWithNothingToResume: + + @pytest.fixture + def restored(self, test_display_controller): + c = test_display_controller + c.config['clock-simple'] = {'enabled': True} + c._select_startup_plugins(['clock-simple'], + {'plugin_id': 'clock-simple', 'mode': 'clock-simple'}) + assert c.on_demand_active + # The plugin's load then fails: nothing is registered for it. + c.cache_manager.clear_cache.reset_mock() + c._populate_on_demand_modes_from_plugin() + return c + + def test_the_session_ends(self, restored): + assert not restored.on_demand_active + assert restored.on_demand_plugin_id is None + assert not restored.on_demand_schedule_override + + def test_it_is_reported_as_an_error(self, restored): + assert restored.on_demand_status == 'error' + assert restored.on_demand_last_error == 'restore-failed' + published = restored.cache_manager.set.call_args_list[-1] + assert published.args[0] == 'display_on_demand_state' + assert published.args[1]['status'] == 'error' + assert published.args[1]['error'] == 'restore-failed' + + def test_the_cached_request_is_dropped(self, restored): + restored.cache_manager.clear_cache.assert_any_call('display_on_demand_config') diff --git a/test/test_run_loop_golden.py b/test/test_run_loop_golden.py index 9e5adae1..1cbeb197 100644 --- a/test/test_run_loop_golden.py +++ b/test/test_run_loop_golden.py @@ -133,6 +133,32 @@ def scenario_on_demand_restored(h: RunLoopHarness): h.restore_on_demand("sports", mode="sports_upcoming", duration=40) +def scenario_on_demand_named_live(h: RunLoopHarness): + # Games are on until t=70, but none involves a favourite, so + # has_live_content() (the live-priority answer) stays False throughout. + # A request naming sports_live still opens on it (it opened on + # sports_recent); asked for again after the games end, it has nothing to + # draw and the session moves on to the plugin's next mode. + h.add_plugin(FakePlugin("clock", ["clock"], duration=20)) + h.add_plugin(FakePlugin( + "sports", ["sports_live", "sports_recent", "sports_upcoming"], duration=15, + live_priority=True, + content=lambda t, mode: mode != "sports_live" or t < 70)) + h.on_demand_request(5, "n1", plugin_id="sports", mode="sports_live") + h.on_demand_request(40, "n2", action="stop") + h.on_demand_request(100, "n3", plugin_id="sports", mode="sports_live") + h.on_demand_request(140, "n4", action="stop") + + +def scenario_on_demand_restore_failed(h: RunLoopHarness): + # A restart during a session whose plugin then fails to load: the + # session ends as an error before the first screen, and the rotation + # runs normally from the top. + h.add_plugin(FakePlugin("clock", ["clock"], duration=20)) + h.add_plugin(FakePlugin("weather", ["weather"], duration=20)) + h.restore_on_demand("gone", mode="gone", duration=40) + + def scenario_schedule(h: RunLoopHarness): # The clock starts at 22:59:30. Off from 23:01 until 23:05 (the window # spans midnight); dimmed from 23:00 until 23:01. @@ -194,6 +220,8 @@ SCENARIOS = { "on_demand": (scenario_on_demand, 240), "on_demand_pinned": (scenario_on_demand_pinned, 160), "on_demand_restored": (scenario_on_demand_restored, 100), + "on_demand_named_live": (scenario_on_demand_named_live, 160), + "on_demand_restore_failed": (scenario_on_demand_restore_failed, 60), "schedule": (scenario_schedule, 400), "wifi_notice": (scenario_wifi_notice, 150), "follower": (scenario_follower, 80),