镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(display): schedule windows end at the end time; on-demand ending in off hours blanks at once - #714

Merged
ChuckBuilds merged 3 commits into
mainfrom
claude/fix-schedule-boundaries
Oct 1, 2026
Merged

ChuckBuilds merged 3 commits into
mainfrom
claude/fix-schedule-boundaries

Conversation

@ChuckBuilds

Copy link
Copy Markdown
Owner

Fixes items 5 and 6 of "Behaviour the traces pin down that may be wrong" in docs/RUN_LOOP_REDESIGN.md, both recorded by #704's schedule golden trace.

Root causes

1. On-demand ending in scheduled-off hours left the panel on until the next minute.
_evaluate_schedule() lets an on-demand session override the schedule by setting is_display_active = True on top of _check_schedule()'s answer. When the session ends (expired or stopped), _clear_on_demand() -> _reset_on_demand_fields() drops the override flag, but is_display_active stays True: _check_schedule() is gated to once per clock minute (_schedule_checked_minute) and had already run that minute, so nothing recomputed it until the minute changed. Up to 60 s of rotation in off hours.

2. A window's end minute counted as on, and only sometimes.
_in_window() was closed, start <= now <= end, and now carries seconds. So at the end minute only a check landing on exactly hh:mm:00.000 saw "on"; the minute gate then held that answer for the whole minute. A check at any later second saw "off". The panel went off at 23:00 or 23:01 depending on when the minute's first check ran. (The golden harness's fake clock hits the boundary exactly, which is why the trace showed the late case.)

Fix (src/display_controller.py, two hunks)

  • _in_window() is half-open, [start, end): start <= now < end, and for windows that cross midnight now >= start or now < end. On from the start time, off at exactly the end time, whatever second the check runs.
  • _reset_on_demand_fields() clears _schedule_checked_minute, so the next _evaluate_schedule() (the next _service_pending_changes() pass, at most 0.25 s later, or the next loop pass) recomputes the real schedule answer and _blank_while_scheduled_off() takes over. Both expiry and stop go through this path.

The dim schedule shares _in_window(), so it gets the same [start, end) rule. It did not need the second fix: on-demand never overrides the dim answer. start == end is an empty window (never on). Before, it was on only for a check at exactly hh:mm:00.000, so effectively the same.

End-time semantics and docs

Owner direction was half-open [start, end). The web UI's schedule picker (schedule-picker.js, partials/schedule.html) only labels the fields "Start Time" / "End Time" and makes no claim either way. docs/CONFIG_REFERENCE.md called them "on/off times", which matches. Docs touched:

  • docs/CONFIG_REFERENCE.md: schedule now spells out "on from start_time up to, but not including, end_time", midnight crossing, per-day ("the entry for the current day decides"), and on-demand ending blanking within about a second. dim_schedule says it has the same boundaries.
  • docs/ARCHITECTURE.md: the schedule bullet notes the half-open windows and the immediate re-check when on-demand ends.
  • docs/RUN_LOOP_REDESIGN.md: removed items 5 and 6. Items 1-4 are untouched for the parallel wifi/live-takeover PRs.
  • CHANGELOG.md: two entries under Unreleased -> Fixes.

Golden trace diff

Only schedule.json changed. The other 14 goldens are byte-identical: the whole golden file passes with only this fixture regenerated (LEDMATRIX_REGEN_GOLDEN=1 pytest test/test_run_loop_golden.py -k schedule). The harness clock starts at 22:59:30. The schedule is 23:05-23:01 (off from 23:01 to 23:05) and dim is 23:00-23:01. On-demand weather starts at t=170 for 20 s. So t=90 is 23:01:00 (the end minute), t=190 is 23:02:40 (expiry) and t=330 is 23:05:00.

--- 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, "<off>", 20.0, "on-demand-start", 2, null],
+    [80.0, "clock", 10.0, "schedule-off", 11, true],
+    [90.0, "<off>", 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, "<off>", 120.0, "schedule-on", 2, null],
+    [190.0, "<off>", 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"]
   ]
 }

Row by row:

  • [80, clock]: was a full 20 s duration screen. Now cut at t=90 (23:01:00) with schedule-off: the end time turns the panel off. 11 frames = t=80-90 at 1 fps.
  • [100, weather], [120, clock], [140, weather … schedule-off]: removed. These were the whole 23:01 end minute being shown, because the check at exactly 23:01:00.000 counted it as "on" and the minute gate kept that until 23:02:00 (t=150).
  • [150, <off>, 20 …] -> [90, <off>, 80 …]: the off stretch now starts at the end time and runs until the on-demand request at t=170. The frame count is blank pushes: one per 60 s off pass (t=90, 150), plus the repaint at t=170 just before on-demand takes over, which the old row also had (t=150, 170).
  • [170, weather … on-demand-expired]: unchanged.
  • [190, weather, 20, schedule-off]: removed. After expiry at 23:02:40 the rotation showed weather until the 23:03:00 minute tick, the first bug.
  • [210, <off>, 120 …] -> [190, <off>, 140 …]: the panel blanks at the expiry itself. Blank pushes at t=190, 250, 310, one per 60 s pass (was t=210, 270).
  • Events schedule-off 150 -> 90 and 210 -> 190: the same two transitions, at the right times.
  • Dim: unchanged. Dimming at t=30 (23:00) is the window's start. Its end (23:01) coincides with the schedule turning the panel off, and brightness is only re-pushed when the panel comes back (t=170), as before.

Tests

test/test_display_controller_schedule.py:

  • TestScheduleWindows: test_boundaries_are_inclusive becomes test_window_is_half_open (08:59:59 off, 09:00 on, 16:59:59 on, 17:00 off). The midnight test now asserts 20:59:59 off, 21:00 on, 00:00 on, 06:59:59 on, 07:00 off.
  • TestEndMinuteBoundary: for a same-day window, a midnight-crossing window and a per-day midnight-crossing window, the first check of the end minute at :00 and at :59 is off. The minute gate keeps that answer for the rest of the minute. The start minute is on at both :00 and :59.
  • TestOnDemandEndsDuringScheduledOff: the override keeps the panel on, then _reset_on_demand_fields() followed by _evaluate_schedule() in the same clock minute is off. Ending in on hours stays on.
  • TestOnDemandEndsDuringScheduledOffRunLoop: runs the real run() loop through refactor(display): run() stage 1 - golden traces and extracted helpers, no behaviour change #704's harness. An expiry at 23:02:35 blanks at t=185 exactly. A stop request at 23:02:31 blanks within one second.
  • TestDimBoundaries: half-open same-day, midnight-crossing and per-day dim windows, plus a dim end minute whose first check is at :59.

test/test_display_pending_changes.py: two TestRunLoopBlanksWhenVegasHandsBack tests mean "off at 23:00:00" (off_at = t0 + 60) but wrote end_time: '22:59'. That only worked because the end minute counted as on. They now say '23:00'. These were the only new failures in the first full-suite comparison.

Mutation-checked. Reverting the gate reset fails the 3 on-demand tests and the schedule golden. Reverting _in_window to closed fails 11 tests and the golden.

Verification

  • Full suite on this branch vs a baseline run of origin/main in a separate worktree: the same 61 failed / 6 errors on both (the pre-existing Windows set: install_lowmem, pixlet download, starlark routes and ownership, and a few others), with no new and no fixed IDs. Branch: 7400 passed. The first branch run had 3 extra failures, the test_display_pending_changes.py tests above; after the fix the ID sets match exactly.
  • scripts/check_types.py (mypy 1.20.2, the CI pin): no issues in 92 modules.

Not changed: in per-day mode a window crossing midnight is judged by the entry for the day being checked. Monday's 22:00-07:00 does not carry into Tuesday 03:00; Tuesday's entry decides. That is existing behaviour, documented now, not altered.

🤖 Generated with Claude Code

ChuckBuilds and others added 2 commits October 1, 2026 15:14
…in off hours blanks at once

Two run-loop bugs recorded by #704's schedule golden trace
(docs/RUN_LOOP_REDESIGN.md items 5 and 6).

- _in_window() is half-open, [start, end). The closed end counted the end
  minute as on, but only for a check at exactly hh:mm:00.000, which the
  once-a-minute gate then held; so the panel went off at the start or the
  end of that minute depending on timing. Applies to the dim schedule too,
  which shares the helper. Midnight-crossing windows: now >= start or
  now < end.
- _reset_on_demand_fields() clears the schedule minute gate. On-demand
  forces is_display_active on over a scheduled-off answer; when the
  session expired or was stopped nothing recomputed it until the next
  clock minute, so rotation kept showing for up to 60 s in off hours.

Golden: only schedule.json changes. Off at t=90 (23:01:00, the end time)
instead of t=150, and blank at t=190 (the on-demand expiry) instead of
t=210. All other goldens are byte-identical.

Docs: CONFIG_REFERENCE spells out [start, end) for schedule and
dim_schedule; ARCHITECTURE notes it; RUN_LOOP_REDESIGN items 5-6 removed;
CHANGELOG Unreleased -> Fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rt, end)

Both tests mean "off at 23:00:00" and wrote end_time 22:59, which only
worked because the end minute used to count as on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 57129476-a298-4162-91ae-6ae1e53492c0

📥 Commits

Reviewing files that changed from the base of the PR and between 77e0ea9 and 6593fd4.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • docs/ARCHITECTURE.md
  • docs/CONFIG_REFERENCE.md
  • docs/RUN_LOOP_REDESIGN.md
  • src/display_controller.py
  • test/fixtures/run_loop_golden/schedule.json
  • test/test_display_controller_schedule.py
  • test/test_display_pending_changes.py
  • test/test_run_loop_golden.py
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

ChuckBuilds added a commit that referenced this pull request Oct 1, 2026
…lds to it (#712)

A WiFi notice preempts the current screen within about a second (frame loops, post-loop check, make-up dwell), and an interrupted Vegas iteration that yielded for a notice ends the pass so the notice shows next. Goldens: wifi_notice, vegas. First of three run-loop fixes (#712, #713, #714), pre-tested together.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CHANGELOG keeps all Fixes entries. RUN_LOOP_REDESIGN.md: with #712 and
#713 in, removing items 5 and 6 empties the 'may be wrong' list, so the
section now says all six are fixed and by which PR. Goldens regenerated:
byte-identical to the merged files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ChuckBuilds
ChuckBuilds merged commit 41db488 into main Oct 1, 2026
13 checks passed
@ChuckBuilds
ChuckBuilds deleted the claude/fix-schedule-boundaries branch October 1, 2026 20:10
@ChuckBuilds

Copy link
Copy Markdown
Owner Author

Post-merge rig check on hdpi: SKIPPED (not run)

The run-loop checks were queued after the #702 soak, on origin/main 41db488, which contains #712, #713 and #714. They never ran.

At 20:07 EDT another session on hdpi made these changes:

  • 2x POST /api/v3/config/main (Vegas disabled, plugin order changed)
  • a display restart through POST /api/v3/system/action
  • POST /api/v3/plugins/config

My guard stopped the batch at that point. Their config was left as they set it, and HEAD was put back to 4ddc3a3.

Nothing about this PR was measured. For a re-run:

  • A WiFi notice can be triggered without touching the network or needing root. Write config/wifi_status.json with {message, timestamp, duration}; it is what _check_wifi_status_message reads.
  • hdpi runs Vegas all the time, so the static-screen case needs Vegas switched off for the test.
  • A favourite team was live during the window (NHL TB @ NYR, from 19:20 EDT), so the live-takeover check would have been possible.

🤖 Generated with Claude Code

@ChuckBuilds

Copy link
Copy Markdown
Owner Author

ledpi check (Pi 4, 192x48), main 746dcfc: PASS.

  • End-time boundary: the global schedule was set to 06:00–20:53 through POST /api/v3/config/schedule at 20:50:40.
    • Schedule activated: Display is now INACTIVE was logged at 20:53:00.983, and the panel blanked at 20:53:00.990.
    • display_current_state.is_display_active went False at 20:53:00.990.
  • On-demand in the off window (clock-simple, pinned), 3 rounds:
    • Start: on-demand was activated about 10 ms after the socket received the request, and the clock drew about 40 ms later.
    • Stop: the panel blanked (Display not active) 8–13 ms after the stop was received. is_display_active: false was published 19, 79 and 19 ms after the POST.

Observation (probably not from this PR):

  • /display/current-status keeps reporting the old is_display_active for up to 30 s after it turns True, if the mode name doesn't change. The value is only republished on a mode change or every 30 s (CURRENT_STATE_REFRESH_SECONDS).
  • This was seen after re-enabling the schedule, and after an on-demand start of the same mode in the off window.

🤖 Generated with Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant