From 41db488c73a899384d22f52c78466b22192a0a9f Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:10:04 -0400 Subject: [PATCH] fix(display): schedule windows end at the end time; on-demand ending in off hours blanks at once (#714) Schedule and dim windows are half-open [start, end): on from the start time, off at exactly the end time, whatever second the check runs. An on-demand session that ends in scheduled-off hours (expiry or stop) clears the once-a-minute schedule gate, so the panel blanks within about a second. Golden: schedule; two test_display_pending_changes.py tests now say end_time 23:00. Merged with #712 and #713: with all three in, docs/RUN_LOOP_REDESIGN.md's 'may be wrong' list is empty, so that section now records that all six items are fixed and by which PR. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 ++ docs/ARCHITECTURE.md | 5 +- docs/CONFIG_REFERENCE.md | 11 +- docs/RUN_LOOP_REDESIGN.md | 26 ++- src/display_controller.py | 19 +- test/fixtures/run_loop_golden/schedule.json | 14 +- test/test_display_controller_schedule.py | 206 +++++++++++++++++++- test/test_display_pending_changes.py | 4 +- test/test_run_loop_golden.py | 3 +- 9 files changed, 265 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cb46f66..03dff2f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -110,6 +110,17 @@ policies are unchanged. screen showed first and the game came after it. Each check also asks each plugin `has_live_content()` once, where a plugin registered under several modes used to be asked once per mode. +- The display schedule turns the panel off at exactly the end time. A window + now runs from its start time up to, but not including, its end time: with + 07:00-23:00 the panel is on at 07:00 and off at 23:00. Before, the end + minute counted as on, and because the schedule is checked once a minute, + the panel went off at 23:00 or at 23:01 depending on when in the minute + that check ran. Windows that cross midnight and per-day schedules follow + the same rule, and so does the dim schedule. +- An on-demand session that ends during scheduled-off hours, by expiring or + being stopped, blanks the panel within about a second. It used to stay on + until the next minute, because the once-a-minute schedule check had + already run that minute and the session had overridden its answer. ## 3.8.0 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 0a652888..70ccb2e2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -211,7 +211,10 @@ plan for restructuring this loop and lists its golden trace tests. to it, rotating between several live games. - **Schedule and dim schedule.** `_check_schedule()` reads `schedule`; `_check_dim_schedule()` reads `dim_schedule` and - `display.hardware.brightness`. Both are re-evaluated once a minute. + `display.hardware.brightness`. Both are re-evaluated once a minute, and + both windows are half-open: on (or dimmed) from the start time, off at + the end time. When an on-demand session ends, the on/off schedule is + re-checked at once rather than at the next minute. - **Long screens.** While a screen is showing (a dwell, a scroll, a Vegas iteration), `_service_pending_changes()` repeats the on-demand, schedule and brightness checks every 0.25 s, so a change does not wait for the diff --git a/docs/CONFIG_REFERENCE.md b/docs/CONFIG_REFERENCE.md index 31ae053e..fd33d14b 100644 --- a/docs/CONFIG_REFERENCE.md +++ b/docs/CONFIG_REFERENCE.md @@ -31,6 +31,13 @@ tooling against it. | `start_time` / `end_time` | `"HH:MM"`, `07:00`–`23:00` | Global-mode on/off times | | `days..{enabled,start_time,end_time}` | per-day objects | Per-day-mode overrides | +The display is on from `start_time` up to, but not including, `end_time`: +with `07:00`–`23:00` it turns on at 07:00 and off at 23:00. An end earlier +than the start crosses midnight (`22:00`–`07:00` is on overnight). In +per-day mode, the entry for the current day decides. An on-demand session +keeps the display on during off hours; once it ends or is stopped, the +display blanks within about a second. + Read by `DisplayController._check_schedule()` (`src/display_controller.py`). Managed in the web UI under Schedule. @@ -44,7 +51,9 @@ Same shape as `schedule` (the template sets its `mode` to `"global"`), plus: Read by `DisplayController._check_dim_schedule()` (`src/display_controller.py`; saved via `POST /api/v3/config/dim-schedule`). The display returns to -`display.hardware.brightness` outside the window. +`display.hardware.brightness` outside the window. The window has the same +boundaries as `schedule`: dimmed from `start_time` up to, but not including, +`end_time`. ## `display.hardware` — matrix panel hardware diff --git a/docs/RUN_LOOP_REDESIGN.md b/docs/RUN_LOOP_REDESIGN.md index 7c0e557c..668d7d93 100644 --- a/docs/RUN_LOOP_REDESIGN.md +++ b/docs/RUN_LOOP_REDESIGN.md @@ -257,13 +257,21 @@ and its per-screen INFO line drops to DEBUG. ## Behaviour the traces pin down that may be wrong -These are recorded as they are today. Each one should be fixed in its own -PR, which updates the affected trace and explains why. None of them is -changed by the restructure. +Stage 1 recorded six behaviours as they were, each to be fixed in its own +PR that updates the affected trace and explains why. All six are fixed: -1. **An on-demand session that expires during scheduled-off keeps the panel - on** until the next minute boundary, because the schedule check runs at - most once a minute (`schedule`, t=190-210). -2. **A schedule window's end minute is inclusive**, and whether the panel - turns off at the start of that minute or the end depends on when in the - minute the first check runs. +- A WiFi notice was only checked between screens, and Vegas yielded to one + and then showed a rotation screen instead. Notices now preempt within + about a second, and Vegas yields straight to them (#712; `wifi_notice`, + `vegas`). +- A live game only took over between screens, and Vegas yielded to one and + then showed a rotation screen first. Games now take over within about a + second, and Vegas yields straight to them (#713; `live_priority`, + `vegas`). +- An on-demand session that ended during scheduled-off kept the panel on + until the next minute, and a schedule window's end minute counted as on + only sometimes. Windows are now half-open `[start, end)`, and the panel + blanks as soon as on-demand ends in off hours (#714; `schedule`). + +A new one found later goes the same way: record it here with the trace that +shows it, then fix it in its own PR, not inside a restructure stage. diff --git a/src/display_controller.py b/src/display_controller.py index 0bb32d73..fec82702 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -728,10 +728,18 @@ class DisplayController: @staticmethod def _in_window(start, end, now) -> bool: - """Whether ``now`` is within [start, end], a window that may span midnight.""" + """Whether ``now`` is within [start, end), a window that may span midnight. + + Half-open: on from the start minute, off at exactly the end minute. + A closed end made the end minute count as inside, and because ``now`` + carries seconds, only a check at hh:mm:00.000 saw it that way -- so + whether the panel went off at the start or the end of that minute + depended on when the minute's one check ran. ``start == end`` is an + empty window, as it effectively was before. + """ if start <= end: - return start <= now <= end - return now >= start or now <= end + return start <= now < end + return now >= start or now < end def _check_schedule(self): """Check if display should be active based on schedule.""" @@ -1424,6 +1432,11 @@ class DisplayController: self.on_demand_expires_at = None self.on_demand_pinned = False self.on_demand_schedule_override = False + # 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 + # stayed on until the next clock minute. + self._schedule_checked_minute = None def _advance_on_demand(self) -> None: """Move an active on-demand session to its next mode and publish it. diff --git a/test/fixtures/run_loop_golden/schedule.json b/test/fixtures/run_loop_golden/schedule.json index e86f0f56..06a562a3 100644 --- a/test/fixtures/run_loop_golden/schedule.json +++ b/test/fixtures/run_loop_golden/schedule.json @@ -4,14 +4,10 @@ [20.0, "weather", 20.0, "duration", 20, true], [40.0, "clock", 20.0, "duration", 20, true], [60.0, "weather", 20.0, "duration", 20, true], - [80.0, "clock", 20.0, "duration", 20, true], - [100.0, "weather", 20.0, "duration", 20, true], - [120.0, "clock", 20.0, "duration", 20, true], - [140.0, "weather", 10.0, "schedule-off", 11, true], - [150.0, "", 20.0, "on-demand-start", 2, null], + [80.0, "clock", 10.0, "schedule-off", 11, true], + [90.0, "", 80.0, "on-demand-start", 3, null], [170.0, "weather", 20.0, "on-demand-expired", 20, true], - [190.0, "weather", 20.0, "schedule-off", 20, true], - [210.0, "", 120.0, "schedule-on", 2, null], + [190.0, "", 140.0, "schedule-on", 3, null], [330.0, "clock", 20.0, "duration", 20, true], [350.0, "weather", 20.0, "duration", 20, true], [370.0, "clock", 20.0, "duration", 20, true], @@ -19,13 +15,13 @@ ], "events": [ [30.0, "brightness", 30], - [150.0, "schedule-off"], + [90.0, "schedule-off"], [170.0, "request", "start:s1"], [170.0, "on-demand-start", "weather"], [170.0, "schedule-on"], [170.0, "brightness", 90], [190.0, "on-demand-expired"], - [210.0, "schedule-off"], + [190.0, "schedule-off"], [330.0, "schedule-on"] ] } diff --git a/test/test_display_controller_schedule.py b/test/test_display_controller_schedule.py index 22a72c63..5f4d110b 100644 --- a/test/test_display_controller_schedule.py +++ b/test/test_display_controller_schedule.py @@ -6,7 +6,8 @@ test_display_controller_optimizations.py::TestScheduleMinuteGate already covers the once-per-minute gating; this file covers what it doesn't: midnight-crossing windows, mode selection (global / per-day / legacy inference), per-day disabled days, invalid time strings, unknown -timezones, boundary equality, and the transition-tracking flags. +timezones, the half-open [start, end) boundaries, on-demand ending during +scheduled-off, and the transition-tracking flags. Both methods read only self.config and a handful of instance attributes, so a bare stub via object.__new__ (the test_display_controller_vegas_tick @@ -41,12 +42,13 @@ def make_controller(config=None, *, normal_brightness=90): def at(time_str, day="monday"): - """Context manager patching the controller module's clock.""" + """Patch the controller module's clock to ``HH:MM`` or ``HH:MM:SS``.""" patcher = patch("src.display_controller.datetime") mock_dt = patcher.start() mock_dt.strptime = datetime.strptime + fmt = "%H:%M:%S" if time_str.count(":") == 2 else "%H:%M" mock_dt.now.return_value.time.return_value = ( - datetime.strptime(time_str, "%H:%M").time()) + datetime.strptime(time_str, fmt).time()) mock_dt.now.return_value.strftime.return_value.lower.return_value = day mock_dt.now.return_value.hour = int(time_str.split(":")[0]) mock_dt.now.return_value.minute = int(time_str.split(":")[1]) @@ -98,10 +100,13 @@ class TestScheduleWindows: assert check_at(dc, "20:00") is False assert check_at(dc, "08:59") is False - def test_boundaries_are_inclusive(self): + def test_window_is_half_open(self): + # [start, end): on from the start minute, off at the end minute. dc = make_controller(self._config("09:00", "17:00")) - assert check_at(dc, "09:00") is True # now == start - assert check_at(dc, "17:00") is True # now == end + assert check_at(dc, "08:59:59") is False + assert check_at(dc, "09:00") is True # now == start + assert check_at(dc, "16:59:59") is True + assert check_at(dc, "17:00") is False # now == end def test_midnight_crossing_window(self): # 21:00 -> 07:00: active late evening AND early morning, inactive @@ -110,8 +115,11 @@ class TestScheduleWindows: assert check_at(dc, "23:00") is True assert check_at(dc, "03:00") is True assert check_at(dc, "12:00") is False - assert check_at(dc, "21:00") is True # boundary - assert check_at(dc, "07:00") is True # boundary + assert check_at(dc, "20:59:59") is False + assert check_at(dc, "21:00") is True # start + assert check_at(dc, "00:00") is True # midnight itself + assert check_at(dc, "06:59:59") is True + assert check_at(dc, "07:00") is False # end def test_no_schedule_config_is_always_active(self): dc = make_controller({"timezone": "UTC"}) @@ -296,3 +304,185 @@ class TestDimSchedule: assert dc._was_dimmed is True dim_at(dc, "12:00") assert dc._was_dimmed is False + + +def check_in_same_minute(dc, time_str, day="monday"): + """Run _check_schedule WITHOUT resetting the minute gate, as the loop does.""" + p = at(time_str, day) + try: + dc._check_schedule() + finally: + p.stop() + return dc.is_display_active + + +class TestEndMinuteBoundary: + """The panel goes off at the end minute whichever second the check runs. + + The loop evaluates the schedule once per clock minute, on the first check + in it. With a closed [start, end] window only a check at hh:mm:00.000 saw + the end minute as inside, so the panel went off at the start or the end + of that minute depending on timing. + """ + + WINDOWS = { + "same_day": ({"start_time": "09:00", "end_time": "17:00"}, + "monday", "16:59", "17:00"), + "midnight_crossing": ({"start_time": "22:00", "end_time": "07:00"}, + "monday", "06:59", "07:00"), + "per_day_midnight_crossing": ( + {"mode": "per-day", "start_time": "09:00", "end_time": "17:00", + "days": {"wednesday": {"enabled": True, "start_time": "22:00", + "end_time": "07:00"}}}, + "wednesday", "06:59", "07:00"), + } + + def _controller(self, window): + return make_controller({"schedule": {"enabled": True, **window}, + "timezone": "UTC"}) + + @pytest.mark.parametrize("name", sorted(WINDOWS)) + @pytest.mark.parametrize("second", ["00", "59"]) + def test_off_for_the_whole_end_minute(self, name, second): + window, day, last_on, end = self.WINDOWS[name] + dc = self._controller(window) + assert check_at(dc, f"{last_on}:59", day) is True + # First check of the end minute, at :00 or at :59. + assert check_in_same_minute(dc, f"{end}:{second}", day) is False + + @pytest.mark.parametrize("name", sorted(WINDOWS)) + def test_gated_minute_keeps_the_off_answer(self, name): + window, day, last_on, end = self.WINDOWS[name] + dc = self._controller(window) + assert check_at(dc, f"{last_on}:30", day) is True + assert check_in_same_minute(dc, f"{end}:00", day) is False + assert check_in_same_minute(dc, f"{end}:59", day) is False + + @pytest.mark.parametrize("second", ["00", "59"]) + def test_on_for_the_whole_start_minute(self, second): + dc = self._controller({"start_time": "22:00", "end_time": "07:00"}) + assert check_at(dc, "21:59:59") is False + assert check_in_same_minute(dc, f"22:00:{second}") is True + + +class TestOnDemandEndsDuringScheduledOff: + """An on-demand session ending in off hours blanks the panel at once, + not when the once-a-minute schedule check next runs.""" + + def _controller(self): + 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 + return dc + + def _evaluate(self, dc, time_str): + p = at(time_str) + try: + dc._evaluate_schedule() + finally: + p.stop() + return dc.is_display_active + + def test_session_end_in_off_hours_blanks_within_the_minute(self): + dc = self._controller() + assert self._evaluate(dc, "23:30:05") is False + dc.on_demand_active = True + assert self._evaluate(dc, "23:30:10") is True # override + assert dc.on_demand_schedule_override is True + dc._reset_on_demand_fields() # expired or stopped + assert self._evaluate(dc, "23:30:40") is False # same minute + assert dc.on_demand_schedule_override is False + + def test_session_end_in_on_hours_stays_on(self): + dc = self._controller() + assert self._evaluate(dc, "12:00:05") is True + dc.on_demand_active = True + assert self._evaluate(dc, "12:00:10") is True + dc._reset_on_demand_fields() + assert self._evaluate(dc, "12:00:40") is True + + +class TestOnDemandEndsDuringScheduledOffRunLoop: + """The same through the real run() loop (test/_run_loop_harness.py). + + The harness clock starts at 22:59:30; the schedule below is off from + 23:01 (t=90) until 23:05 (t=330). The sessions end mid-minute, so the + old behaviour (on until the next minute) would show as a gap.""" + + def _harness(self, tmp_path): + from test._run_loop_harness import FakePlugin, RunLoopHarness + h = RunLoopHarness(tmp_path, horizon=260) + h.config["schedule"] = {"enabled": True, "start_time": "23:05", + "end_time": "23:01"} + h.add_plugin(FakePlugin("clock", ["clock"], duration=20)) + h.add_plugin(FakePlugin("weather", ["weather"], duration=20)) + return h + + @staticmethod + def _first_off_after(trace, t): + return [row for row in trace["screens"] + if row[1] == "" and row[0] >= t][0] + + def test_expiry_blanks_at_once(self, tmp_path): + h = self._harness(tmp_path) + # 15 s from t=170 ends at t=185, 23:02:35. + h.on_demand_request(170, "x1", plugin_id="weather", duration=15) + trace = h.run() + session = [r for r in trace["screens"] if r[0] == 170.0][0] + assert session[1:4] == ["weather", 15.0, "on-demand-expired"] + assert self._first_off_after(trace, 170)[0] == 185.0 + + def test_stop_blanks_at_once(self, tmp_path): + h = self._harness(tmp_path) + h.on_demand_request(170, "x1", plugin_id="weather") + h.on_demand_request(181, "x2", action="stop") # 23:02:31 + trace = h.run() + assert 181.0 <= self._first_off_after(trace, 170)[0] <= 182.0 + + +class TestDimBoundaries: + """The dim schedule shares _in_window, so it is half-open too.""" + + def _config(self, start, end, **extra): + return {"dim_schedule": {"enabled": True, "start_time": start, + "end_time": end, "dim_brightness": 25, + **extra}, + "timezone": "UTC"} + + def test_same_day_dim_window_is_half_open(self): + dc = make_controller(self._config("13:00", "14:00")) + assert dim_at(dc, "12:59:59") == 90 + assert dim_at(dc, "13:00") == 25 + assert dim_at(dc, "13:59:59") == 25 + assert dim_at(dc, "14:00:00") == 90 + assert dim_at(dc, "14:00:59") == 90 + + def test_midnight_crossing_dim_window(self): + dc = make_controller(self._config("20:00", "07:00")) + assert dim_at(dc, "19:59:59") == 90 + assert dim_at(dc, "20:00") == 25 + assert dim_at(dc, "00:00") == 25 + assert dim_at(dc, "06:59:59") == 25 + assert dim_at(dc, "07:00:00") == 90 + assert dim_at(dc, "07:00:59") == 90 + + def test_per_day_dim_end_minute(self): + dc = make_controller(self._config("20:00", "07:00", mode="per-day", days={ + "friday": {"enabled": True, "start_time": "23:00", + "end_time": "06:00"}, + })) + assert dim_at(dc, "05:59:59", day="friday") == 25 + assert dim_at(dc, "06:00:00", day="friday") == 90 + assert dim_at(dc, "06:00:59", day="friday") == 90 + + def test_dim_end_minute_checked_late_in_the_minute(self): + dc = make_controller(self._config("20:00", "07:00")) + assert dim_at(dc, "06:59:30") == 25 + p = at("07:00:59") # first check of the end minute; gate not reset + try: + assert dc._check_dim_schedule() == 90 + finally: + p.stop() diff --git a/test/test_display_pending_changes.py b/test/test_display_pending_changes.py index f30f29bd..0a2443a7 100644 --- a/test/test_display_pending_changes.py +++ b/test/test_display_pending_changes.py @@ -477,7 +477,7 @@ class TestRunLoopBlanksWhenVegasHandsBack: c._cleanup_expired_wifi_status = MagicMock() c._refresh_config_cache({ 'display': {'hardware': {'brightness': 90}}, - 'schedule': {'enabled': True, 'start_time': '07:00', 'end_time': '22:59'}, + 'schedule': {'enabled': True, 'start_time': '07:00', 'end_time': '23:00'}, }) c.vegas_coordinator = vegas_coordinator(c) c.vegas_coordinator._pending_config_update = False @@ -518,7 +518,7 @@ class TestRunLoopBlanksWhenVegasHandsBack: c._refresh_config_cache({ 'display': {'hardware': {'brightness': 90}, 'display_durations': {'ticker': 120}}, - 'schedule': {'enabled': True, 'start_time': '07:00', 'end_time': '22:59'}, + 'schedule': {'enabled': True, 'start_time': '07:00', 'end_time': '23:00'}, }) c.plugin_manager.plugin_executor.execute_display.side_effect = ( lambda target, plugin_id, force_clear=False, display_mode=None, **kw: diff --git a/test/test_run_loop_golden.py b/test/test_run_loop_golden.py index db3a34eb..9e5adae1 100644 --- a/test/test_run_loop_golden.py +++ b/test/test_run_loop_golden.py @@ -141,7 +141,8 @@ def scenario_schedule(h: RunLoopHarness): "end_time": "23:01", "dim_brightness": 30} h.add_plugin(FakePlugin("clock", ["clock"], duration=20)) h.add_plugin(FakePlugin("weather", ["weather"], duration=20)) - # An on-demand request during scheduled downtime overrides it. + # An on-demand request during scheduled downtime overrides it; when it + # expires the panel blanks at once, not at the next minute. h.on_demand_request(170, "s1", plugin_id="weather", duration=20)