Skip to content

feat(display schedule): allow different on/off hours per weekday - #3447

Open
slider wants to merge 2 commits into
Screenly:masterfrom
slider:feat/per-day-display-schedule
Open

slider wants to merge 2 commits into
Screenly:masterfrom
slider:feat/per-day-display-schedule

Conversation

@slider

@slider slider commented Nov 2, 2026 •

Copy link
Copy Markdown

The display schedule (CEC power-off plus local blanking) used one pair of on/off times for every active day. Shops rarely keep the same hours all week.

Adds an optional per-weekday override stored as
display_power_day_times ('D=HH:MM-HH:MM', comma-separated). A weekday without an entry keeps using the shared on/off times, and the default is empty, so existing schedules behave exactly as before.

  • lib/display_power: parse/format helpers and should_be_on() taking the overrides. An overnight period still belongs to the day it starts, now with that day's own off time.
  • Settings page: a "Different hours per day" toggle that swaps the shared time fields for one row per weekday (day chip + on/off times). Switching it on seeds every row from the shared times.
  • v2 device settings: new display_power_day_times field on GET and PATCH (additive; malformed entries or a weekday given twice are a 400).

Checklist

  • I have performed a self-review of my own code.
  • New and existing unit tests pass locally and on CI with my changes.
  • I have done an end-to-end test for Raspberry Pi devices.
  • I have tested my changes for x86 devices.
  • I added a documentation for the changes I have made (when necessary).

The display schedule (CEC power-off plus local blanking) used one pair
of on/off times for every active day. Shops rarely keep the same hours
all week: a typical setup is Mon-Fri 07:30-18:00 and Sat 10:00-14:00.

Adds an optional per-weekday override stored as
display_power_day_times ('D=HH:MM-HH:MM', comma-separated). A weekday
without an entry keeps using the shared on/off times, and the default
is empty, so existing schedules behave exactly as before.

- lib/display_power: parse/format helpers and should_be_on() taking the
  overrides. An overnight period still belongs to the day it starts,
  now with that day's own off time.
- Settings page: a "Different hours per day" toggle that swaps the
  shared time fields for one row per weekday (day chip + on/off
  times). Switching it on seeds every row from the shared times.
- v2 device settings: new display_power_day_times field on GET and
  PATCH (additive; malformed entries or a weekday given twice are a
  400).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPcJJqJJ5aMsTa37NG4F7t
@slider
slider requested a review from a team as a code owner November 2, 2026 12:39
vpetersson-bot
vpetersson-bot previously approved these changes Nov 2, 2026

@vpetersson-bot vpetersson-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Approving.

What I checked:

  • should_be_on rewrite. With no per-day overrides, I compared the new function against the previous implementation on 20,000 random (on, off, days, now) combinations, including overnight windows and partial day sets. The results were identical, so existing schedules behave exactly as before. The new per-day and overnight-tail tests in tests/test_display_power.py pass locally. The "tail of yesterday's window first, then today's own window" ordering handles the mixed cases well: a shared overnight window running into an overridden morning, and an override rescuing shared times that can't be used.
  • Storage/API. parse_day_times is lenient for the beat and parse_day_time is strict for the v2 serializer, which matches how parse_hhmm/parse_days are already split. Rejecting a weekday given twice with a 400, instead of silently picking one, is the right call. The empty default keeps existing installs unchanged.
  • Settings form. Each set of day chips is disabled both server-side and through :disabled when it's hidden, so display_power_days is posted exactly once with or without JS. An invalid per-day row keeps that day's previous hours and shows an error, consistent with the shared time fields.

Non-blocking follow-ups:

  1. website/content/docs/display-schedule.md still describes a single on/off pair. The field reference table is missing display_power_day_times (format D=HH:MM-HH:MM, Monday=0, empty = shared times). The "Days and overnight schedules" section should also say that an overnight period ends on its starting day's off time. The checklist marks docs as done, but this page wasn't touched. Fine as a quick follow-up PR. (website/data/openapi.json is regenerated on deploy, so nothing to do there.)
  2. Nit: import re inside test_settings_renders_per_day_hours should move to the module imports.

@slider slider changed the title Display schedule: allow different on/off hours per weekday feat(display schedule): allow different on/off hours per weekday Nov 2, 2026
- display-schedule.md: add display_power_day_times to the field
  reference, a "Different hours per day" section, the overnight rule
  (a period ends at its starting day's off time) and an API example.
- tests: hoist `import re` in test_template_views.py to the module
  imports instead of importing it inside individual tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPcJJqJJ5aMsTa37NG4F7t
@sonarqubecloud

sonarqubecloud Bot commented Nov 2, 2026

Copy link
Copy Markdown

@vpetersson-bot vpetersson-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 3d14deb. The dismissal followed the new commit, which only changes docs and test imports.

  • display-schedule.md: I checked it against the code. The overnight example matches should_be_on: Friday 18:00 → 02:00 is still on at Sat 01:00, off at 02:00, and Saturday's own window starts at 10:00. "Switching it off clears the per-day hours" matches _apply_display_power_day_times. The API rules match validate_display_power_day_times: sorted HH:MM output, a 400 for a malformed entry, identical on/off times or a duplicate weekday, and blank clears every entry. "The form rejects identical times" matches too: the row is ignored with an error message and the day keeps its previous hours.
  • tests/test_template_views.py: the function-local import re statements move to a single module-level import. No behaviour change.

try:
day, window = display_power.parse_day_time(token)
except ValueError as exc:
raise serializers.ValidationError(str(exc)) from exc

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

False positive. str(exc) here is only ever one of the operator-readable ValueError messages raised in display_power.parse_day_time (format, weekday range, HH:MM, on == off). parse_hhmm returns None rather than raising, so no traceback or internal state reaches the response. It's the same 400-with-message pattern validate_display_power_days already uses. Safe to dismiss as "false positive"; my approval at 3d14deb stands.

This branch has not been deployed

No deployments
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.

3 participants