From 55b40691f919eabe276ab92e3c39279eee78ec50 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Fri, 2 Oct 2026 22:10:08 -0400 Subject: [PATCH] refactor(display): run() stage 2 - an Arbiter decides the scheduled-off blank, follower and WiFi notice, no behaviour change Adds src/display_arbiter.py: Source, ArbiterInputs, ArbiterState, WifiNotice, ScreenPlan and a pure Arbiter.decide(state, inputs, now) (no I/O, no clock reads, nothing mutated). It applies the scheduled-off gate (blank unless an on-demand session overrides it, as #714 left it), then Follower, OnDemand and Wifi in that order; on-demand, live, Vegas and the rotation return a LEGACY plan that runs run()'s existing code. The WiFi notice's mid-screen rule moves there as wifi_notice_preempts. run() gathers the snapshot after the schedule and brightness bookkeeping (_arbiter_inputs; the notice is read only when it could win, since reading it has side effects), calls decide() and dispatches on plan.source. The dwells (60 s blank, 0.5 s notice) come from the plan. Side effects keep their order within a pass. The golden traces regenerate byte-identical. New test/test_display_arbiter.py: a written-out table of all 16 input combinations, the mid-screen table, purity checks and the controller's snapshot through an on-demand override. Three handover tests and a source-order test follow the helpers' new signatures. 23 mutants, all killed. display_arbiter is on the mypy ratchet. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 14 + docs/RUN_LOOP_REDESIGN.md | 69 ++++- mypy-clean.txt | 1 + src/display_arbiter.py | 185 ++++++++++++++ src/display_controller.py | 94 ++++--- test/test_display_arbiter.py | 239 ++++++++++++++++++ ...t_display_controller_shutdown_and_state.py | 2 +- test/test_handover_scroll_state.py | 18 +- 8 files changed, 582 insertions(+), 40 deletions(-) create mode 100644 src/display_arbiter.py create mode 100644 test/test_display_arbiter.py diff --git a/CHANGELOG.md b/CHANGELOG.md index af04e8c2e..7bb6ea062 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -208,6 +208,9 @@ policies are unchanged. this release: plugins reach it through `APIHelper` and `espn_dates`, and should not import it directly until a plugin-facing API ships (stage 3), so it sets no `ledmatrix_min_version` floor. +- `src/display_arbiter.py` -- the display loop's Arbiter (see Tooling). + Core-internal: plugins have no reason to import it, so it sets no + `ledmatrix_min_version` floor. ### Tooling @@ -225,6 +228,17 @@ policies are unchanged. (`_dispatch_first_frame`, `_resolve_durations`, `_resolve_active_mode`, `_needs_high_fps`, `_advance_after_screen` and others), and the traces are identical before and after the move. +- Display loop stage 2: an Arbiter decides who gets the panel. Each pass, + `run()` gathers a small snapshot (the schedule, the on-demand flag, the + sync follower, the pending WiFi notice) and calls + `Arbiter.decide(state, inputs, now)` in `src/display_arbiter.py`, a pure + function, which returns a `ScreenPlan`. It decides the scheduled-off + blank, the follower frame and the WiFi notice; on-demand, live priority, + Vegas and the rotation return a `LEGACY` plan and run the existing code. + The WiFi notice's mid-screen rule (`wifi_notice_preempts`) moves there + too. No behaviour change: the golden traces regenerate byte-identical. + `test/test_display_arbiter.py` tests `decide()` with a table of all 16 + combinations of its inputs. ### Fixes diff --git a/docs/RUN_LOOP_REDESIGN.md b/docs/RUN_LOOP_REDESIGN.md index 89452b131..73dd42214 100644 --- a/docs/RUN_LOOP_REDESIGN.md +++ b/docs/RUN_LOOP_REDESIGN.md @@ -44,7 +44,9 @@ Each pass, in order: 3. Poll on-demand requests and expiry, release plugins loaded only for on-demand, tick plugin updates, drop an expired WiFi notice, evaluate the schedule (an on-demand session overrides scheduled-off), apply the - brightness target. + brightness target. Then gather the Arbiter's inputs + (`_arbiter_inputs`) and call `Arbiter.decide()`, which picks one of + steps 4-6 or returns `LEGACY` for steps 7-9 (stage 2). 4. **Scheduled off:** blank, dwell up to 60 s. `_blank_while_scheduled_off` 5. **Follower:** render one frame from the leader. `_run_follower_frame` 6. **WiFi notice** (unless on-demand): draw it, dwell 0.5 s. `_show_wifi_notice`. @@ -70,8 +72,9 @@ Each pass, in order: next mode (`_advance_after_screen`). The helpers named above were extracted in stage 1 without changing -behaviour. The frame loops, the Vegas branch and every early exit are still -inline in `run()`. +behaviour. Since stage 2 the choice between steps 4, 5, 6 and the rest is +made by `Arbiter.decide()` in `src/display_arbiter.py`. The frame loops, the +Vegas branch and every early exit are still inline in `run()`. ## Target design @@ -160,7 +163,7 @@ that the harness patches in today. | 4 | Vegas as a Source driven by `run_frame()` | none intended | traces against the real coordinator; ledpi Vegas soak A/B | | 5 | Plugins declare `frame_policy` | DEBUG instead of INFO for the FPS line | traces; soak on a static-heavy rotation | -### Stage 1 (this PR) +### Stage 1 (#704) - `test/_run_loop_harness.py` builds a real `DisplayController` through `__init__` on in-memory fakes (plugins, cache, config service, plugin @@ -194,7 +197,7 @@ that the harness patches in today. - Twelve helpers were extracted from `run()` (listed under "What `run()` does today"). Breaking any one of them fails at least one golden trace. -### Stage 2: Arbiter, starting with Follower and Wifi +### Stage 2: Arbiter, starting with Follower and Wifi (done) 1. Add `ScreenPlan` and an `Arbiter` with the ScheduledOff gate, Follower and Wifi. Every other case returns a `LEGACY` plan, which means "carry on @@ -211,6 +214,41 @@ that the harness patches in today. Follower and Wifi go first because each is one self-contained branch that ends the pass. They prove the plumbing without touching the frame loops. +What shipped: + +- `src/display_arbiter.py` (on the mypy ratchet) holds `Source` + (`SCHEDULED_OFF`, `FOLLOWER`, `WIFI`, `LEGACY`), `ArbiterInputs`, + `ArbiterState`, `WifiNotice`, `ScreenPlan` and `Arbiter.decide`. + `ScreenPlan` has only the fields stage 2 uses: `source`, `max_duration` + (60 s for the blank, 0.5 s for the notice, the constants `run()` used to + hard-code) and `notice`. `mode`, `plugin`, the other durations, + `frame_policy` and `preemptible_by` arrive with the Sources that need them. +- `ArbiterState` is empty: no stage-2 Source remembers anything between + passes. `now` is passed but not read, because the top-of-pass WiFi check + never compared the expiry and must not start (the table pins this). +- `ArbiterInputs` holds `schedule_on`, `on_demand_active`, + `follower_active` and `wifi_notice`. `_arbiter_inputs` derives + `schedule_on` as `is_display_active and not on_demand_schedule_override`, + so the gate (blank when the schedule is off and no on-demand session + overrides it) blanks exactly when `is_display_active` is False, as before, + including #714's on-demand ending in off hours. It reads the WiFi notice + only when the notice could win, because `_check_wifi_status_message` has + side effects (its 1 Hz throttle, deleting an expired file) that those + passes never had. +- The mid-screen rule is `wifi_notice_preempts(notice, on_demand, now)`, + which `_wifi_notice_pending` calls; it does compare the expiry. +- `run()` still calls `_publish_current_mode_state_if_changed`, + `_apply_pending_vegas_init` and `process_deferred_updates` at the same + points relative to the branches, so the order of side effects in a pass + is unchanged. +- `test/test_display_arbiter.py`: the 16-row table (every combination of + the four inputs, written out), the mid-screen table, purity checks (no + clock reads, nothing mutated, no I/O imports), and the controller's + snapshot through an on-demand session that overrides the schedule and + ends. A mutation run broke 23 pieces once each (the gate, the order, each + Source, the dwells, the expiry comparison, the snapshot's reads, each + dispatch in `run()`); every one failed a test. + ### Stage 3: ScreenRunner and `PREEMPTED` Move the two frame loops, the make-up dwell and the dynamic-duration exit @@ -218,6 +256,27 @@ into `ScreenRunner.run(plan)` with an injected `FrameClock`. Replace the five re-checks with `PREEMPTED`. Add the OnDemand, Live and Rotation Sources so `LEGACY` is left meaning only Vegas. +Concretely, from where stage 2 left off: + +1. `ArbiterState` gains the rotation index, the on-demand mode list, index, + expiry and pin, and the live resume point (today `current_mode_index`, + `on_demand_*` and the live-priority stash). `ArbiterInputs` gains the + live modes (`_collect_live_modes`) and whether Vegas is enabled and keeps + live content in the ticker. +2. OnDemand returns its current mode with `_clamp_to_on_demand`'s bound, + reading `now` for the expiry. Live returns the next live mode + (round-robin). Rotation returns `available_modes[current_mode_index]`. + `ScreenPlan` gains `mode`, `plugin`, `min_duration`, `max_duration`, + `dynamic`, `frame_policy` and `preemptible_by`. +3. `ScreenRunner.run(plan)` returns an `ExitReason`; `state.after(plan, + outcome)` replaces `_advance_after_screen` and the live-resume + bookkeeping. Each mid-screen check asks `decide()` whether a Source in + `plan.preemptible_by` now wins, so `_screen_preempted`, + `_check_live_takeover` and `_wifi_notice_pending` become one call. +4. The control socket (`_drain_control_commands`, `_wait_for_control`) and + state publishing stay where they are; the runner calls them at its + service points. + This stage touches frame pacing (the 8 ms deadline sleep, the 1 ms yield), so it needs a frame soak on ledpi, A/B against main. Coordinate with whoever owns scroll performance (`docs/SCROLL_PERFORMANCE.md`). diff --git a/mypy-clean.txt b/mypy-clean.txt index d612e7166..b4f3f4f3e 100644 --- a/mypy-clean.txt +++ b/mypy-clean.txt @@ -47,6 +47,7 @@ src/config_service.py src/core_config_keys.py src/deprecation.py src/device_location.py +src/display_arbiter.py src/display_geometry.py src/dynamic_team_resolver.py src/exceptions.py diff --git a/src/display_arbiter.py b/src/display_arbiter.py new file mode 100644 index 000000000..9d45933e0 --- /dev/null +++ b/src/display_arbiter.py @@ -0,0 +1,185 @@ +"""What the panel shows next: the Arbiter of docs/RUN_LOOP_REDESIGN.md. + +``Arbiter.decide(state, inputs, now)`` takes a snapshot that +``DisplayController.run()`` gathers once per pass and returns a +:class:`ScreenPlan` naming the Source that gets the panel. It is a pure +function: no I/O, no clock reads (``now`` is passed in), no locks, and it +changes nothing it is given. That is what lets a plain table of cases test +the priority order, which used to exist only as the order of ``if`` blocks +in ``run()``. + +The full order is + + ScheduledOff (a gate), Follower, OnDemand, Wifi, Live, Vegas, Rotation + +Stage 2 decides the gate, Follower and Wifi. Every other case returns a +``LEGACY`` plan, meaning "carry on with run()'s existing code" (live +priority, Vegas, then one rotation screen). OnDemand is in the order already +because it outranks the WiFi notice: an active session is a ``LEGACY`` plan +even when a notice is pending. + +The Wifi Source's mid-screen rule, :func:`wifi_notice_preempts`, lives here +too, so both of its answers -- at the top of a pass and between frames -- +come from one module. +""" + +from dataclasses import dataclass +from enum import Enum +from typing import Optional + +__all__ = [ + "Arbiter", + "ArbiterInputs", + "ArbiterState", + "SCHEDULED_OFF_DWELL", + "ScreenPlan", + "Source", + "WIFI_NOTICE_DWELL", + "WifiNotice", + "wifi_notice_preempts", +] + +# How long one scheduled-off pass blanks the panel. The dwell ends early when +# on-demand starts or the schedule turns the panel back on. +SCHEDULED_OFF_DWELL = 60.0 + +# How long one WiFi-notice pass holds the notice before the next pass looks +# again; the notice stays up, pass after pass, until it expires. +WIFI_NOTICE_DWELL = 0.5 + + +class Source(Enum): + """Who gets the panel this pass.""" + + SCHEDULED_OFF = "scheduled-off" + FOLLOWER = "follower" + WIFI = "wifi" + # Not decided by the Arbiter yet: on-demand, live priority, Vegas and the + # rotation are still chosen by run()'s own code. Stage 3 adds the + # OnDemand, Live and Rotation Sources; stage 4 adds Vegas. + LEGACY = "legacy" + + +@dataclass(frozen=True) +class WifiNotice: + """A WiFi status message waiting to be drawn. + + ``expires_at`` is wall-clock time (``time.time()``), as written by the + WiFi manager. + """ + + message: str + expires_at: float + + +@dataclass(frozen=True) +class ArbiterState: + """What the Arbiter remembers between passes. + + Nothing yet: the stage-2 Sources decide from the inputs alone. The + on-demand index, the rotation index and the live resume point move here + with their Sources in stage 3. + """ + + +@dataclass(frozen=True) +class ArbiterInputs: + """One pass's snapshot, gathered by run() before it calls decide(). + + Attributes: + schedule_on: The display schedule has the panel on, not counting an + on-demand override of a scheduled-off window. + on_demand_active: An on-demand session is running. + follower_active: A sync leader is driving this panel. + wifi_notice: The pending WiFi notice, or None. run() reads it only + when it could win (the panel is on, and neither a follower nor + on-demand outranks it), because reading it has side effects: a + 1 Hz throttle and deleting an expired file. + """ + + schedule_on: bool + on_demand_active: bool + follower_active: bool + wifi_notice: Optional[WifiNotice] = None + + +@dataclass(frozen=True) +class ScreenPlan: + """The Arbiter's answer for one pass. + + Attributes: + source: The Source that gets the panel. + max_duration: How long the plan holds the panel, in seconds, at most + (its dwell ends early when what the panel should show changes). + None when the Source paces itself: a follower frame, or LEGACY. + notice: The WiFi notice to draw, for a WIFI plan. + """ + + source: Source + max_duration: Optional[float] = None + notice: Optional[WifiNotice] = None + + +SCHEDULED_OFF_PLAN = ScreenPlan(Source.SCHEDULED_OFF, max_duration=SCHEDULED_OFF_DWELL) +FOLLOWER_PLAN = ScreenPlan(Source.FOLLOWER) +LEGACY_PLAN = ScreenPlan(Source.LEGACY) + + +class Arbiter: + """Decides which Source gets the panel. Stateless; see the module docstring.""" + + @staticmethod + def decide(state: ArbiterState, inputs: ArbiterInputs, now: float) -> ScreenPlan: + """The plan for this pass, from the Sources in priority order. + + Args: + state: What the Arbiter remembers between passes (nothing yet). + inputs: This pass's snapshot. + now: Wall-clock time of the snapshot. No stage-2 Source reads it: + the top-of-pass WiFi check takes the notice as read, and only + the mid-screen check (:func:`wifi_notice_preempts`) compares + it with the expiry. It is in the signature for the Sources + stage 3 adds (on-demand expiry, durations). + + Returns: + The winning Source's plan, or LEGACY_PLAN when the winner is one + run() still decides itself. + """ + del state, now # not read by the stage-2 Sources; see the docstring + + # ScheduledOff is a gate, not a Source: a scheduled-off panel stays + # blank even for a follower, and only an on-demand session overrides + # it (#714 -- one ending in off hours blanks at the next pass). + if not inputs.schedule_on and not inputs.on_demand_active: + return SCHEDULED_OFF_PLAN + + # 1. Follower: a sync leader drives this panel, ahead of on-demand. + if inputs.follower_active: + return FOLLOWER_PLAN + + # 2. OnDemand: decided by run() until stage 3. It outranks the notice. + if inputs.on_demand_active: + return LEGACY_PLAN + + # 3. Wifi: a pending notice, held for one short dwell per pass. + if inputs.wifi_notice is not None: + return ScreenPlan(Source.WIFI, max_duration=WIFI_NOTICE_DWELL, + notice=inputs.wifi_notice) + + # 4-6. Live, Vegas, Rotation: still run()'s own code. + return LEGACY_PLAN + + +def wifi_notice_preempts(notice: Optional[WifiNotice], on_demand_active: bool, + now: float) -> bool: + """Whether a WiFi notice should end the current screen early. + + The Wifi Source's mid-screen rule, polled between frames, during dwells + and when a Vegas iteration yields. On-demand outranks the notice, as in + :meth:`Arbiter.decide`. Unlike the top-of-pass check it also compares + ``now`` with the expiry, because the 1 Hz read throttle can hand back a + notice that has expired since it was read. + """ + if on_demand_active or notice is None: + return False + return now < notice.expires_at diff --git a/src/display_controller.py b/src/display_controller.py index f5ee1659f..f076c05df 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -35,6 +35,9 @@ import pytz from src import display_watchdog +from src.display_arbiter import ( + Arbiter, ArbiterInputs, ArbiterState, Source, WifiNotice, wifi_notice_preempts, +) from src.display_manager import DisplayManager from src.config_manager import ConfigManager from src.config_service import ConfigService @@ -2754,11 +2757,12 @@ def _check_live_takeover(self) -> None: # into an Arbiter / ScreenRunner / Sources (docs/RUN_LOOP_REDESIGN.md). # test/test_run_loop_golden.py pins down what the loop does with them. - def _blank_while_scheduled_off(self) -> None: + def _blank_while_scheduled_off(self, dwell: float) -> None: """One pass while the schedule has the panel off: blank it and dwell. - The dwell returns early when on-demand starts or the schedule turns - the panel back on (see _sleep_with_plugin_updates). + The Arbiter's SCHEDULED_OFF plan; ``dwell`` is its max_duration. The + dwell returns early when on-demand starts or the schedule turns the + panel back on (see _sleep_with_plugin_updates). """ # Clear display when schedule makes it inactive to ensure blank screen # (not showing initialization screen) @@ -2771,7 +2775,7 @@ def _blank_while_scheduled_off(self) -> None: logger.info(f"Display not active (is_display_active={self.is_display_active}), sleeping...") self._publish_current_mode_state() - self._sleep_with_plugin_updates(60) + self._sleep_with_plugin_updates(dwell) def _run_follower_frame(self) -> None: """One frame while a sync leader drives this panel (follower mode). @@ -2850,46 +2854,69 @@ def _run_follower_frame(self) -> None: if remaining > 0: time.sleep(remaining) - def _show_wifi_notice(self) -> bool: - """Show a pending WiFi status message for one pass. + def _arbiter_inputs(self) -> ArbiterInputs: + """This pass's snapshot for Arbiter.decide, taken after _evaluate_schedule. + + _evaluate_schedule forces is_display_active on while an on-demand + session overrides a scheduled-off window, and flags that with + on_demand_schedule_override, so the schedule's own answer is "on and + not overridden". The WiFi notice is read only when it could win -- + the panel is on and neither a follower nor on-demand outranks it -- + because _check_wifi_status_message has side effects (its 1 Hz + throttle, deleting an expired or corrupt file) that such a pass + never had. + """ + schedule_on = self.is_display_active and not self.on_demand_schedule_override + on_demand = self.on_demand_active + follower = self.sync_manager.is_follower_active() + notice = None + if self.is_display_active and not follower and not on_demand: + notice = self._read_wifi_notice() + return ArbiterInputs(schedule_on=schedule_on, on_demand_active=on_demand, + follower_active=follower, wifi_notice=notice) + + def _read_wifi_notice(self) -> Optional[WifiNotice]: + """The pending WiFi notice (see _check_wifi_status_message), or None.""" + status = self._check_wifi_status_message() + if not status: + return None + return WifiNotice(message=status['message'], + expires_at=float(status['expires_at'])) + + def _show_wifi_notice(self, notice: WifiNotice, dwell: float) -> bool: + """Draw the Arbiter's WIFI plan and hold it for ``dwell`` seconds. Returns True when the message was drawn, and the pass ends there - (no rotation). On-demand outranks it, so nothing is checked while - on-demand is active; a message that fails to draw is treated as no - message. + (no rotation). A message that fails to draw is treated as no + message: the pass carries on as a LEGACY plan. """ - if self.on_demand_active: - return False - wifi_status_data = self._check_wifi_status_message() - if not wifi_status_data: - return False self._end_scroll_before_core_screen() - if not self._display_wifi_status_message(wifi_status_data): + if not self._display_wifi_status_message( + {'message': notice.message, 'expires_at': notice.expires_at}): # Display failed, clear the status and continue normally return False # The plugin that resumes afterwards must redraw # the whole panel, not paint over the message. self.force_change = True - self._sleep_with_plugin_updates(0.5) + self._sleep_with_plugin_updates(dwell) return True def _wifi_notice_pending(self) -> bool: - """True when a WiFi notice is waiting that _show_wifi_notice would draw. + """True when a WiFi notice should end the current screen early. Polled from the frame loops, the dwell sleep and after a Vegas iteration yields, so a notice preempts whatever is on the panel within about a second instead of waiting for the screen to end -- by which time a short notice has usually expired unseen. Cheap at frame rate: _check_wifi_status_message stats the file at most once - a second. On-demand outranks the notice, as in _show_wifi_notice. + a second. The rule is display_arbiter.wifi_notice_preempts; the + file is not read at all while on-demand, which outranks the notice, + is active. """ if self.on_demand_active: return False - status = self._check_wifi_status_message() - if not status: - return False - # The 1 s throttle can hand back a result that has expired since. - return time.time() < float(status['expires_at']) + return wifi_notice_preempts(self._read_wifi_notice(), self.on_demand_active, + time.time()) def _resolve_active_mode(self): """The mode this pass shows: the on-demand session's current mode @@ -3499,8 +3526,13 @@ def run(self): # is active). No repaint: this screen's first frame pushes it. self._apply_brightness_target() - if not self.is_display_active: - self._blank_while_scheduled_off() + # Who gets the panel this pass (src/display_arbiter.py). The + # Arbiter decides the scheduled-off gate, Follower and Wifi; + # a LEGACY plan carries on to the code below. + plan = Arbiter.decide(ArbiterState(), self._arbiter_inputs(), time.time()) + + if plan.source is Source.SCHEDULED_OFF: + self._blank_while_scheduled_off(plan.max_duration) continue self._publish_current_mode_state_if_changed() @@ -3513,7 +3545,7 @@ def run(self): # Multi-display sync: follower mode — render frames received from leader. # Plugin update() threads still run (via _tick_plugin_updates above) so # data is fresh when we return to standalone if the leader goes offline. - if self.sync_manager.is_follower_active(): + if plan.source is Source.FOLLOWER: self._run_follower_frame() continue @@ -3521,10 +3553,12 @@ def run(self): # This also cleans up expired updates to prevent memory leaks self.display_manager.process_deferred_updates() - # Check for WiFi status message (interrupts normal rotation, but respects on-demand) - # Priority: on-demand > wifi-status > live-priority > normal rotation - # Past this point no WiFi message is showing this pass. - if self._show_wifi_notice(): + # WiFi status message: interrupts the rotation, but on-demand + # outranks it (the Arbiter's order). Past this point no WiFi + # message is showing this pass: one that failed to draw + # carries on as a LEGACY plan. + if (plan.source is Source.WIFI and plan.notice is not None + and self._show_wifi_notice(plan.notice, plan.max_duration)): continue # Skip to next iteration, don't rotate # Check for live priority content and switch to it immediately. diff --git a/test/test_display_arbiter.py b/test/test_display_arbiter.py new file mode 100644 index 000000000..7ef7b6e8a --- /dev/null +++ b/test/test_display_arbiter.py @@ -0,0 +1,239 @@ +"""Arbiter.decide (src/display_arbiter.py): the stage-2 Sources as tables. + +decide() is pure, so every case is one row of (inputs) -> expected Source. +The rows are written out, not computed, so a change to the priority order +has to change a row here as well. The last class checks the controller's +side: the snapshot run() gathers gives the same scheduled-off answer as +is_display_active did before the Arbiter, through an on-demand session +that overrides the schedule and ends (#714). +""" + +import itertools +import os +from unittest.mock import MagicMock, patch + +import pytest + +os.environ.setdefault("EMULATOR", "true") + +from src import display_arbiter # noqa: E402 +from src.display_arbiter import ( # noqa: E402 + SCHEDULED_OFF_DWELL, + WIFI_NOTICE_DWELL, + Arbiter, + ArbiterInputs, + ArbiterState, + ScreenPlan, + Source, + WifiNotice, + wifi_notice_preempts, +) + +NOTICE = WifiNotice(message="Connected to HomeNet", expires_at=1_000.0) + +OFF = Source.SCHEDULED_OFF +FOLLOW = Source.FOLLOWER +WIFI = Source.WIFI +LEGACY = Source.LEGACY + +# (schedule_on, on_demand_active, follower_active, notice) -> Source. +# Every combination of the stage-2 inputs: 2 x 2 x 2 x 2 = 16 rows. +DECIDE_TABLE = [ + # The gate: scheduled off and no on-demand session -- blank, whatever + # else is pending, a follower and a WiFi notice included. + (False, False, False, None, OFF), + (False, False, False, NOTICE, OFF), + (False, False, True, None, OFF), + (False, False, True, NOTICE, OFF), + # Scheduled off, but on-demand overrides the gate. + (False, True, False, None, LEGACY), # on-demand: run() decides + (False, True, False, NOTICE, LEGACY), # on-demand outranks WiFi + (False, True, True, None, FOLLOW), # follower outranks on-demand + (False, True, True, NOTICE, FOLLOW), + # Scheduled on. + (True, False, False, None, LEGACY), # live / Vegas / rotation + (True, False, False, NOTICE, WIFI), + (True, False, True, None, FOLLOW), + (True, False, True, NOTICE, FOLLOW), # follower outranks WiFi + (True, True, False, None, LEGACY), + (True, True, False, NOTICE, LEGACY), # on-demand outranks WiFi + (True, True, True, None, FOLLOW), + (True, True, True, NOTICE, FOLLOW), +] + + +def _inputs(schedule_on, on_demand, follower, notice): + return ArbiterInputs(schedule_on=schedule_on, on_demand_active=on_demand, + follower_active=follower, wifi_notice=notice) + + +def test_the_table_covers_every_combination_once(): + keys = [row[:4] for row in DECIDE_TABLE] + combos = list(itertools.product([False, True], [False, True], + [False, True], [None, NOTICE])) + assert sorted(keys, key=repr) == sorted(combos, key=repr) + + +class TestDecide: + + @pytest.mark.parametrize("schedule_on,on_demand,follower,notice,expected", + DECIDE_TABLE) + def test_source(self, schedule_on, on_demand, follower, notice, expected): + plan = Arbiter.decide(ArbiterState(), + _inputs(schedule_on, on_demand, follower, notice), + 500.0) + assert plan.source is expected + + @pytest.mark.parametrize("schedule_on,on_demand,follower,notice,expected", + DECIDE_TABLE) + def test_plan_fields(self, schedule_on, on_demand, follower, notice, expected): + plan = Arbiter.decide(ArbiterState(), + _inputs(schedule_on, on_demand, follower, notice), + 500.0) + if expected is OFF: + assert plan == ScreenPlan(OFF, max_duration=SCHEDULED_OFF_DWELL) + elif expected is WIFI: + assert plan == ScreenPlan(WIFI, max_duration=WIFI_NOTICE_DWELL, + notice=NOTICE) + else: + # A follower paces itself; LEGACY is run()'s existing code. + assert plan == ScreenPlan(expected) + + def test_dwells_are_todays(self): + # The constants that run() used to hard-code. + assert SCHEDULED_OFF_DWELL == 60.0 + assert WIFI_NOTICE_DWELL == 0.5 + + @pytest.mark.parametrize("now", [0.0, NOTICE.expires_at - 1, + NOTICE.expires_at, NOTICE.expires_at + 1e6]) + def test_top_of_pass_wifi_takes_the_notice_as_read(self, now): + """Before the Arbiter, the top of the pass drew whatever notice + _check_wifi_status_message returned without comparing the expiry; + decide() must not start comparing it with ``now``.""" + plan = Arbiter.decide(ArbiterState(), _inputs(True, False, False, NOTICE), now) + assert plan.source is WIFI + + +class TestPurity: + + def test_decide_reads_no_clock(self): + boom = MagicMock(side_effect=AssertionError("decide read the clock")) + with patch("time.time", boom), patch("time.monotonic", boom), \ + patch("time.perf_counter", boom): + for *key, expected in DECIDE_TABLE: + assert Arbiter.decide(ArbiterState(), _inputs(*key), 1.0).source is expected + + def test_the_module_imports_no_io_or_clock(self): + for name in ("time", "os", "datetime", "threading", "json", "pathlib"): + assert not hasattr(display_arbiter, name), name + + def test_same_inputs_same_plan(self): + for *key, _expected in DECIDE_TABLE: + inputs = _inputs(*key) + first = Arbiter.decide(ArbiterState(), inputs, 1.0) + assert Arbiter.decide(ArbiterState(), inputs, 1.0) == first + assert inputs == _inputs(*key) # not mutated + + def test_inputs_and_plans_are_frozen(self): + inputs = _inputs(True, False, False, NOTICE) + with pytest.raises(Exception): + inputs.schedule_on = False # type: ignore[misc] + plan = Arbiter.decide(ArbiterState(), inputs, 1.0) + with pytest.raises(Exception): + plan.source = OFF # type: ignore[misc] + + +# (notice, on_demand_active, now) -> preempts. The mid-screen rule. +PREEMPT_TABLE = [ + (None, False, 0.0, False), + (None, True, 0.0, False), + (NOTICE, False, NOTICE.expires_at - 0.001, True), + (NOTICE, False, NOTICE.expires_at, False), # expired at expires_at + (NOTICE, False, NOTICE.expires_at + 5, False), # the throttle's stale copy + (NOTICE, True, NOTICE.expires_at - 0.001, False), # on-demand outranks it + (NOTICE, True, NOTICE.expires_at + 5, False), +] + + +@pytest.mark.parametrize("notice,on_demand,now,expected", PREEMPT_TABLE) +def test_wifi_notice_preempts(notice, on_demand, now, expected): + assert wifi_notice_preempts(notice, on_demand, now) is expected + + +class TestControllerSnapshot: + """DisplayController._arbiter_inputs, on a stub controller.""" + + def _controller(self, *, display_active=True, override=False, on_demand=False, + follower=False, status=None): + from src.display_controller import DisplayController + dc = object.__new__(DisplayController) + dc.is_display_active = display_active + dc.on_demand_schedule_override = override + dc.on_demand_active = on_demand + dc.sync_manager = MagicMock() + dc.sync_manager.is_follower_active.return_value = follower + dc._check_wifi_status_message = MagicMock(return_value=status) + return dc + + def test_reads_the_notice_when_it_can_win(self): + dc = self._controller(status={"message": "AP mode", "expires_at": 12, + "timestamp": 7, "duration": 5}) + inputs = dc._arbiter_inputs() + assert inputs == ArbiterInputs(schedule_on=True, on_demand_active=False, + follower_active=False, + wifi_notice=WifiNotice("AP mode", 12.0)) + + @pytest.mark.parametrize("kwargs", [ + {"display_active": False}, + {"follower": True}, + {"on_demand": True, "override": True}, + {"on_demand": True}, + ]) + def test_does_not_read_the_notice_when_it_cannot_win(self, kwargs): + """Reading it deletes an expired file and moves the 1 Hz throttle, + which these passes never did before the Arbiter.""" + dc = self._controller(status={"message": "x", "expires_at": 1e12}, **kwargs) + assert dc._arbiter_inputs().wifi_notice is None + dc._check_wifi_status_message.assert_not_called() + + def test_an_on_demand_override_is_not_the_schedule(self): + dc = self._controller(display_active=True, override=True, on_demand=True) + assert dc._arbiter_inputs().schedule_on is False + + def test_the_gate_matches_is_display_active_through_an_on_demand_session(self): + """Through _evaluate_schedule, as run() calls it: the Arbiter blanks + exactly when is_display_active is False, which is what run() tested + before. The schedule is 07:00-23:00.""" + from test.test_display_controller_schedule import at, make_controller + dc = make_controller({"schedule": {"enabled": True, "start_time": "07:00", + "end_time": "23:00"}, + "timezone": "UTC"}) + dc.on_demand_active = False + dc.on_demand_schedule_override = False + dc.sync_manager = MagicMock() + dc.sync_manager.is_follower_active.return_value = False + dc._check_wifi_status_message = MagicMock(return_value=None) + + def step(time_str): + p = at(time_str) + try: + dc._evaluate_schedule() + finally: + p.stop() + plan = Arbiter.decide(ArbiterState(), dc._arbiter_inputs(), 0.0) + assert (plan.source is OFF) is (not dc.is_display_active), time_str + return plan.source + + assert step("22:59:30") is LEGACY + assert step("23:00:00") is OFF # window ends + dc.on_demand_active = True + assert step("23:00:10") is LEGACY # on-demand overrides + assert dc.on_demand_schedule_override is True + assert step("23:01:00") is LEGACY # next minute, still on + dc._reset_on_demand_fields() # session ends + assert step("23:01:20") is OFF # same minute: blanks + dc.on_demand_active = True + assert step("06:59:00") is LEGACY + assert step("07:00:00") is LEGACY # schedule back on mid-session + dc._reset_on_demand_fields() + assert step("07:00:30") is LEGACY diff --git a/test/test_display_controller_shutdown_and_state.py b/test/test_display_controller_shutdown_and_state.py index c03b1269e..6d423706b 100644 --- a/test/test_display_controller_shutdown_and_state.py +++ b/test/test_display_controller_shutdown_and_state.py @@ -239,4 +239,4 @@ def test_pending_vegas_init_is_applied_by_the_helper_the_main_loop_calls(): def test_main_loop_applies_pending_vegas_init_before_the_follower_branch(): import inspect src = inspect.getsource(DisplayController.run) - assert src.index("self._apply_pending_vegas_init()") < src.index("self.sync_manager.is_follower_active()") + assert src.index("self._apply_pending_vegas_init()") < src.index("self._run_follower_frame()") diff --git a/test/test_handover_scroll_state.py b/test/test_handover_scroll_state.py index aa060afcd..dfcbf6df9 100644 --- a/test/test_handover_scroll_state.py +++ b/test/test_handover_scroll_state.py @@ -675,30 +675,40 @@ def test_the_schedule_off_blank_shows_none_of_the_ticker(self, dm, monkeypatch): _push(dm, (255, 0, 0)) # the ticker's last frame flags = _record_scrolling(dm, monkeypatch) c = _core_screen_controller(dm) - DisplayController._blank_while_scheduled_off(c) + DisplayController._blank_while_scheduled_off(c, 60.0) assert dm._presented[-1].getpixel((10, 30)) == (0, 0, 0) assert flags == [False] # a static frame, not a freeze assert not dm.is_currently_scrolling() c._sleep_with_plugin_updates.assert_called_once_with(60) def test_the_wifi_notice_shows_none_of_the_ticker(self, dm, monkeypatch): + from src.display_arbiter import WifiNotice from src.display_controller import DisplayController dm._scan_lag_bands = [(24, 48, 1)] dm.set_scrolling_state(True, 1) _push(dm, (255, 0, 0)) flags = _record_scrolling(dm, monkeypatch) c = _core_screen_controller(dm) - c._check_wifi_status_message.return_value = {"message": "x", "expires_at": 1e12} c._display_wifi_status_message.side_effect = lambda _s: _push(dm, (0, 0, 255)) and True - assert DisplayController._show_wifi_notice(c) is True + notice = WifiNotice(message="x", expires_at=1e12) + assert DisplayController._show_wifi_notice(c, notice, 0.5) is True assert dm._presented[-1].getpixel((10, 30)) == (0, 0, 255) assert flags == [False] def test_no_notice_leaves_the_scroll_alone(self, dm): + """With no notice the Arbiter does not pick the WiFi screen, so + nothing ends the scroll.""" + from src.display_arbiter import Arbiter, ArbiterState, Source from src.display_controller import DisplayController dm.set_scrolling_state(True, 2) c = _core_screen_controller(dm) + c.is_display_active = True + c.on_demand_schedule_override = False + c.sync_manager.is_follower_active.return_value = False + c._read_wifi_notice = types.MethodType(DisplayController._read_wifi_notice, c) c._check_wifi_status_message.return_value = None - assert DisplayController._show_wifi_notice(c) is False + inputs = DisplayController._arbiter_inputs(c) + assert inputs.wifi_notice is None + assert Arbiter.decide(ArbiterState(), inputs, 0.0).source is Source.LEGACY assert dm.is_currently_scrolling() assert dm._frame_hold == 2