diff --git a/CHANGELOG.md b/CHANGELOG.md index a02f668a..1ad4093b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,44 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +### Fixed + +- The web preview and `/api/v3/display/current` no longer stay black for a + whole screen that draws its card once and then holds it. The snapshot is + written from `update_display()` at most once per write interval, so a frame + pushed inside that interval was skipped and left for the next + `update_display()` -- which such a screen never makes. Soccer's + recent/upcoming cards skip redundant redraws, and the first one after an + on-demand start lands a few milliseconds after the start's clear wrote a + black frame: on ledpi the preview showed 0 lit pixels for the whole 15 s + while the panel showed the card. `DisplayManager` now remembers a skipped + changed frame, and the render loop writes it (`write_owed_snapshot()`) + once the interval has passed. The cadence is unchanged, and nothing extra + runs when no frame is owed. + +### ESPN date-range fetches: fewer requests, fewer at once + +A soccer board (8 leagues, ESPN rejecting `dates=` ranges) logged ~90 +`NameResolutionError` lines and an `update() timed out` at every start on a +Pi: each league's fortnight-either-side window was 29 day requests, fetched +by several managers at once, ~40 in flight. Measured against live ESPN with +soccer-scoreboard 2.39.2, alternating runs: **~450 requests per start, peak +~45 in flight, ~75 DNS lookups -> 46 requests, peak 13, ~30 lookups**. + +- `fetch_espn_date_chunks()` asks for a window's partial edge month whole + when the window covers `ESPN_MONTH_COVER_MIN_DAYS` (7) or more of its days, + and trims the answer to the window's days by each event's US Eastern start + date -- the day ESPN's `dates=YYYYMMDD` means (417 of 417 live soccer + events matched). A 29-day window spanning two months is 2 requests instead + of 29. Short windows (a live poll's 1-2 days) stay day by day. A trimmed + month that comes back at the 500-event cap re-asks only the window's days. + An event with no readable date is kept. New: `espn_request_chunks()`. +- Chunk requests share one process-wide cap of `ESPN_CHUNK_WORKERS` (6) in + flight, across every window being fetched, instead of six per window. +- A new process starts as if a range had just been rejected, so it no longer + spends one doomed 400 per window at every start (eleven at once from a + soccer board); the range is still retried `RANGE_RETRY_SECONDS` in. + ### Cheap per-frame and per-fetch savings - `BaseOddsManager.get_odds()` no longer pretty-prints every odds response @@ -525,6 +563,34 @@ policies are unchanged. 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. +- `POST /api/v3/config/schedule` and `/config/dim-schedule` accept a + disabled per-day schedule with every day off. That is the shape + `config.template.json` ships, so posting back what GET returned on a fresh + install answered 400 "At least one day must be enabled". An enabled per-day + schedule still needs a day on. A day that is off now keeps the times it + was posted with (the schedule picker sends them). Before, saving dropped + them, so turning the day back on showed the defaults. +- `POST /api/v3/config/main` answers `restart_required: true` only when the + save changed a setting the running display does not apply by itself. + Brightness (`brightness.set` and the config watcher), the per-mode + durations and plugin sections are applied live. A brightness-only save, + such as the MQTT bridge's slider, or a save that changed nothing, no longer + shows the restart banner. Hardware, rotation order, timezone and every + other setting still ask for the restart. +- `GET /api/v3/health` reports `degraded` when the display service is + stopped. Before, only the sub-checks changed, and the overall status stayed + `healthy` for as long as the last preview frame was under 60 s old. + `checks.display_loop.status` is now `stopped` when three things agree: + systemd says the service is not active, the control socket does not + answer, and there is no live heartbeat. Where the platform has no socket + (Windows) or it is switched off, nothing changes. +- `GET /api/v3/display/current-status` no longer reports the stopped + display's last state (`is_display_active: true`) from the cache for up to + 120 s. When the control socket does not answer and the render loop's + heartbeat is absent, stale, or from a process that is gone (#726's rules), + the answer is unknown, with every field `null`. A display that still beats + without a socket, Windows and a socket switched off read the cache as + before. New `web_interface.display_state.display_gone()`. - 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/REST_API_REFERENCE.md b/docs/REST_API_REFERENCE.md index 0dce2c66..a69c331c 100644 --- a/docs/REST_API_REFERENCE.md +++ b/docs/REST_API_REFERENCE.md @@ -159,14 +159,16 @@ there an unchecked checkbox — which the browser omits — is saved as } ``` -`restart_required` is always true here: display hardware, rotation, -durations and general settings take effect when the display restarts, and -the web UI shows its restart banner on the flag. (Plugin sections saved -through this route reach the running plugin live, like -`POST /plugins/config`.) +`restart_required` is true when the save changed a setting that takes +effect when the display restarts: display hardware, rotation order, +timezone, general settings and the rest. The web UI shows its restart banner +on the flag. It is false when the save changed only what the running display +applies by itself, or nothing: `brightness`, the per-mode durations +(`duration__`, `display.display_durations`) and plugin sections, which +reach the running plugin live, like `POST /plugins/config`. -A saved `brightness` is the exception: it reaches the panel without a -restart. The route also sends it to the running display over the control +A saved `brightness` reaches the panel without a restart. The route also +sends it to the running display over the control socket (`brightness.set`), which puts it on the panel at once, and the response adds `"brightness_transport": "socket"`. Otherwise it is `"config"`, with `brightness_socket_error` giving the reason, and the @@ -246,7 +248,10 @@ Replace the schedule configuration. ``` A day whose `_enabled` key is absent counts as enabled, with default -times `07:00`-`23:00`. At least one day must be enabled. +times `07:00`-`23:00`. An enabled schedule needs at least one day enabled; a +disabled one (`"enabled": false`) may have every day off, as +`config.template.json` ships it. A day that is off keeps the times sent for +it, when they are valid `HH:MM`. **Response**: ```json @@ -343,7 +348,11 @@ control socket ([IPC_CONTROL_SOCKET.md](IPC_CONTROL_SOCKET.md)), and `cache` when it came from the `display_current_state` cache key (no socket: the display is stopped or older, or this is Windows). A display whose render loop has not refreshed its state for 120 seconds is reported with every -field `null`, either way. +field `null`, either way. So is a stopped display: when the socket does not +answer and the render loop's heartbeat +(`/run/ledmatrix/display-heartbeat.json`) is absent, stale or from a process +that is gone, the cache's last entry is not used. A display still beating +without a socket, Windows, or a socket switched off reads the cache. ### List Display Modes @@ -2281,7 +2290,11 @@ display snapshot. `data.status` is `healthy` or `degraded`, with (with `heartbeat_age_seconds`), `stalled` (no heartbeat for 60s: the panel is frozen even if the service is active; the status turns `degraded`), or `not_reported` when the display writes none (not started yet, the dev server, -Windows), which does not affect the status. Its `source` is `socket` when the +Windows), which does not affect the status, or `stopped` (with `source: +"service"`) when the display service is not active, the control socket does +not answer and there is no live heartbeat; the status then turns +`degraded`. A platform with no control socket (Windows) or a socket switched +off never reports `stopped`. Its `source` is `socket` when the age came from the display's state stream over the control socket (measured in memory by the display) and `heartbeat_file` when it came from `/run/ledmatrix/display-heartbeat.json`. @@ -2344,8 +2357,9 @@ Replace the dim schedule. `dim_brightness` is 0-100 (default 30). In `per-day` mode the days can be sent either as the `days` object that GET returns, or as the web form's flat fields (`monday_enabled`, `monday_start`, `monday_end`, ...). A day that is not sent counts as -enabled with default times `20:00`-`07:00`; at least one day must be -enabled. +enabled with default times `20:00`-`07:00`. As for the schedule above, an +enabled dim schedule needs at least one day enabled and a disabled one may +have every day off. --- diff --git a/src/common/README.md b/src/common/README.md index 9181db49..a0f46287 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -109,8 +109,11 @@ and the plugin test harness all use it. Most plugins get BDF text through [`espn_dates.py`](espn_dates.py). ESPN's site API rejects `dates=` ranges and truncates results when `limit` is above 500. `fetch_espn_scoreboard()` splits a range into month and day requests ESPN accepts and merges the -results; `espn_date_chunks()`, `fetch_espn_date_chunks()`, -`clamp_espn_limit()` and `merge_scoreboard_payloads()` are the pieces. +results; `espn_date_chunks()`, `espn_request_chunks()`, +`fetch_espn_date_chunks()`, `clamp_espn_limit()` and +`merge_scoreboard_payloads()` are the pieces. A window's partial edge months +are asked whole and trimmed to its days (US Eastern), and chunk requests share +one process-wide cap of `ESPN_CHUNK_WORKERS` in flight. Every request goes through [`fetch_service`](#fetch_service), the chunks counted against the plugin that asked. Scoreboard plugins also bundle a copy for older cores. diff --git a/src/common/espn_dates.py b/src/common/espn_dates.py index 3b34c9ea..850ac446 100644 --- a/src/common/espn_dates.py +++ b/src/common/espn_dates.py @@ -26,10 +26,30 @@ A month can hold more than 500 events (college baseball's March does), and ESPN answers that with exactly ``limit`` events and no hint that more exist. A month chunk that comes back full is therefore re-asked day by day. +A window's *partial* edge months are asked for whole, too, once the window +covers ``ESPN_MONTH_COVER_MIN_DAYS`` or more of their days, and the answer is +trimmed back to the window's days. A scoreboard's default fortnight either side +of today (29 days, two partial months) was 29 day requests per league; it is +now 2. Trimming needs ESPN's "game day", which is the event's start in US +Eastern time -- checked against the live API on 2026-10-03: 417 of 417 soccer +events across five leagues and three months (one of them spanning the end of +daylight saving) came back from exactly the day query their Eastern date +names. A short window (a live poll's one or two days) stays day by day, so it +never downloads a whole month to read a day of it. + +Chunk requests share one process-wide budget of ``ESPN_CHUNK_WORKERS`` in +flight, however many windows are being fetched at once. Each window used to get +its own six, so a scoreboard starting eight leagues -- each with a recent and +an upcoming manager -- had ~40 requests in flight, every one beyond a session's +pool a new connection and a new DNS lookup. On a Pi whose resolver could not +keep up, that was ~90 ``NameResolutionError`` lines within a minute of every +start. + Once a range has been rejected, later ranges skip straight to chunks for ``RANGE_RETRY_SECONDS`` instead of spending a doomed request first -- live scoreboards ask every 30 seconds. After that the range is tried again, so the -workaround retires itself if ESPN reverts. +workaround retires itself if ESPN reverts. A process starts inside that +period, as if a range had just been rejected. ONE CACHE KEY PER SCOREBOARD ---------------------------- @@ -55,7 +75,7 @@ import re import threading import time from concurrent.futures import ThreadPoolExecutor -from datetime import date, datetime, timedelta +from datetime import date, datetime, timedelta, tzinfo from functools import partial from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, cast @@ -100,16 +120,57 @@ RANGE_RETRY_SECONDS = 6 * 60 * 60 # pool_maxsize of 10 so the shared Session never has to discard connections. ESPN_CHUNK_WORKERS = 6 +#: An edge month the window covers at least this many days of is asked for +#: whole and trimmed, instead of one request per day (see module docstring). +#: Below it the days are cheaper than the month: a whole month is two to +#: three times the bytes of the half of it a fortnight window holds. +ESPN_MONTH_COVER_MIN_DAYS = 7 + +# Every chunk request in the process holds one of these while it is in flight +# -- the cap is per process, not per window (see module docstring). +_chunk_slots = threading.BoundedSemaphore(ESPN_CHUNK_WORKERS) + + +def _eastern_zone() -> Optional[tzinfo]: + """US Eastern, the zone ESPN's ``dates=YYYYMMDD`` means, or None when + this Python has no time zone data (no edge month is trimmed then).""" + zone: Optional[tzinfo] = None + try: + from zoneinfo import ZoneInfo + zone = ZoneInfo("America/New_York") + except Exception: # noqa: BLE001 - no zoneinfo module or no tz database + zone = None + if zone is not None: + return zone + try: + import pytz + return cast(tzinfo, pytz.timezone("America/New_York")) + except Exception: # noqa: BLE001 + return None + + +_EASTERN = _eastern_zone() + +# What _fetch_one_chunk returns for a month that came back at the cap. +_CAPPED: Any = object() + _range_lock = threading.Lock() -_ranges_rejected_until = 0.0 +# A process starts out assuming ranges are still rejected, as they have been +# since 2026-09-15, and tries one again RANGE_RETRY_SECONDS in. Starting +# from "unknown" cost one doomed range request per window at every start -- +# eleven 400s at once from a soccer board, each fetching before any had +# answered -- to learn what every start learns. +_ranges_rejected_until = time.monotonic() + RANGE_RETRY_SECONDS __all__ = [ "ESPN_MAX_LIMIT", "ESPN_CHUNK_WORKERS", + "ESPN_MONTH_COVER_MIN_DAYS", "RANGE_RETRY_SECONDS", "clamp_espn_limit", "parse_espn_date_range", "espn_date_chunks", + "espn_request_chunks", "merge_scoreboard_payloads", "fetch_espn_date_chunks", "fetch_espn_scoreboard", @@ -220,6 +281,79 @@ def espn_date_chunks(start: date, end: date) -> List[str]: return chunks +def espn_request_chunks( + start: date, + end: date, + month_cover_min_days: Optional[int] = None, +) -> List[Tuple[str, Optional[Tuple[date, date]]]]: + """The requests that fetch ``[start, end]``, as ``(dates, trim)`` pairs. + + :func:`espn_date_chunks`, except that a partial edge month with + ``month_cover_min_days`` (default ``ESPN_MONTH_COVER_MIN_DAYS``) or more + of its days in the window becomes one ``YYYYMM`` request whose ``trim`` + is the first and last of those days: its events that start outside them + (US Eastern) are dropped. ``trim`` is None for every other request. + Without time zone data nothing can be trimmed, so the edge days stay day + requests. + """ + if month_cover_min_days is None: + month_cover_min_days = ESPN_MONTH_COVER_MIN_DAYS + planned: List[Tuple[str, Optional[Tuple[date, date]]]] = [] + run: List[str] = [] + + def flush() -> None: + if (_EASTERN is not None and month_cover_min_days > 0 + and len(run) >= month_cover_min_days): + planned.append((run[0][:6], (_parse_day(run[0]), _parse_day(run[-1])))) + else: + planned.extend((day, None) for day in run) + run.clear() + + for chunk in espn_date_chunks(start, end): + if run and (len(chunk) != 8 or chunk[:6] != run[0][:6]): + flush() + if len(chunk) == 8: + run.append(chunk) + else: + planned.append((chunk, None)) + flush() + return planned + + +def _parse_day(text: str) -> date: + return date(int(text[:4]), int(text[4:6]), int(text[6:8])) + + +def _eastern_day(stamp: Any) -> Optional[date]: + """The US Eastern date of an ESPN event ``date`` ("2026-10-10T11:30Z"), + or None when it cannot be read.""" + if not isinstance(stamp, str) or _EASTERN is None: + return None + try: + moment = datetime.fromisoformat(stamp.strip().replace("Z", "+00:00")) + except ValueError: + return None + if moment.tzinfo is None: + return None + return moment.astimezone(_EASTERN).date() + + +def _trim_to_days(payload: Any, first: date, last: date) -> Any: + """Drop the events of a month payload that start outside ``[first, last]`` + (US Eastern). An event whose date cannot be read is kept: its day query + might well have returned it, and a game is never dropped on a guess. + """ + if not isinstance(payload, dict) or not isinstance(payload.get("events"), list): + return payload + kept = [] + for event in payload["events"]: + day = _eastern_day(event.get("date")) if isinstance(event, dict) else None + if day is None or first <= day <= last: + kept.append(event) + payload["events"] = kept + return payload + + def merge_scoreboard_payloads(payloads: List[Any]) -> Dict[str, Any]: """Fold chunk responses into one scoreboard payload. @@ -250,37 +384,54 @@ def merge_scoreboard_payloads(payloads: List[Any]) -> Dict[str, Any]: def _fetch_one_chunk( session, url: str, params: Dict[str, Any], headers, timeout, logger, chunk: str, cache_max_age: Optional[float] = None, -) -> Optional[Dict[str, Any]]: + trims: Optional[Dict[str, Tuple[date, date]]] = None, +) -> Any: """GET a single ``dates=`` chunk, or None when it failed. One bad chunk must not sink the rest of the season, so every error is logged and swallowed here rather than raised to the gather below. + + A month that comes back at the cap is truncated: it returns ``_CAPPED``, + its payload dropped here before it is ever held beside the others. A + month in ``trims`` loses its events outside the days given there. + + The request holds one of the process-wide ``_chunk_slots`` while it runs. """ try: - response = fetch_get( - session, - url, - params=dict(params, dates=chunk, limit=ESPN_MAX_LIMIT), - headers=headers, - timeout=timeout, - **_memo_kwargs(cache_max_age), - ) - response.raise_for_status() - return cast(Optional[Dict[str, Any]], response_json(response)) + with _chunk_slots: + response = fetch_get( + session, + url, + params=dict(params, dates=chunk, limit=ESPN_MAX_LIMIT), + headers=headers, + timeout=timeout, + **_memo_kwargs(cache_max_age), + ) + response.raise_for_status() + payload = response_json(response) except Exception as exc: # noqa: BLE001 - see docstring if logger: logger.warning("ESPN chunk %s failed, skipping it: %s", chunk, exc) return None + if len(chunk) == 6 and isinstance(payload, dict): + if len(payload.get("events") or []) >= ESPN_MAX_LIMIT: + return _CAPPED + trim = (trims or {}).get(chunk) + if trim is not None: + payload = _trim_to_days(payload, *trim) + return payload def _fetch_chunks( session, url: str, params: Dict[str, Any], headers, timeout, logger, chunks: List[str], cache_max_age: Optional[float] = None, -) -> List[Optional[Dict[str, Any]]]: + trims: Optional[Dict[str, Tuple[date, date]]] = None, +) -> List[Any]: """Fetch every chunk, returning payloads positionally aligned with ``chunks``. Requests go out ``ESPN_CHUNK_WORKERS`` at a time because a cold season is - over a hundred of them. The order they come back in is not significant -- + over a hundred of them -- and no more than that across every window the + process is fetching, which ``_fetch_one_chunk``'s slot enforces. The order they come back in is not significant -- callers keep ``chunks`` order from the returned list -- but it does mean the session is shared across threads, which is why this only ever issues GETs and never touches session state. @@ -293,7 +444,7 @@ def _fetch_chunks( return [] fetch = partial( _fetch_one_chunk, session, url, params, headers, timeout, logger, - cache_max_age=cache_max_age, + cache_max_age=cache_max_age, trims=trims, ) if len(chunks) == 1: return [fetch(chunks[0])] @@ -340,7 +491,9 @@ def fetch_espn_date_chunks( if span is None: return None - chunks = espn_date_chunks(*span) + planned = espn_request_chunks(*span) + chunks = [chunk for chunk, _ in planned] + trims = {chunk: trim for chunk, trim in planned if trim is not None} if logger: logger.debug( "Fetching ESPN date range %s as %d month/day chunks", @@ -349,32 +502,31 @@ def fetch_espn_date_chunks( results = _fetch_chunks( session, url, params, headers, timeout, logger, chunks, cache_max_age, + trims, ) attempted = len(chunks) - # A month that came back at the cap is truncated; its days replace it in - # place, so merged events stay in chunk order however the requests raced. + # A month that came back at the cap is truncated; its days (only the + # window's, for a trimmed edge month) replace it in place, so merged + # events stay in chunk order however the requests raced. Its payload was + # already dropped in the worker: a capped college-baseball month is ~2MB + # of parsed JSON, and holding four of them through ~120 day requests added + # ~25MB to the peak -- more than the concurrency itself. Low-memory boards + # (docs/LOW_MEMORY_BOARDS.md) have under 200MB of headroom. slots: List[Any] = results capped: Dict[int, List[str]] = {} for index, chunk in enumerate(chunks): - payload = slots[index] - if payload is None or len(chunk) != 6: + if slots[index] is not _CAPPED: continue - events = payload.get("events") if isinstance(payload, dict) else None - if len(events or []) >= ESPN_MAX_LIMIT: - if logger: - logger.info( - "ESPN month %s hit the %d-event cap; re-asking it day by day", - chunk, ESPN_MAX_LIMIT, - ) - capped[index] = _days_of_month(chunk) - # Drop the truncated month now rather than after its days arrive: - # a capped college-baseball month is ~2MB of parsed JSON, and - # holding four of them through ~120 day requests added ~25MB to - # the peak -- more than the concurrency itself. Low-memory boards - # (docs/LOW_MEMORY_BOARDS.md) have under 200MB of headroom. - slots[index] = None - payload = events = None + if logger: + logger.info( + "ESPN month %s hit the %d-event cap; re-asking it day by day", + chunk, ESPN_MAX_LIMIT, + ) + trim = trims.get(chunk) + capped[index] = (_days_of_month(chunk) if trim is None + else espn_date_chunks(*trim)) + slots[index] = None if capped: days = [day for index in sorted(capped) for day in capped[index]] diff --git a/src/display_controller.py b/src/display_controller.py index 5431872f..128f3b07 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -1230,6 +1230,13 @@ class DisplayController: note = getattr(self.plugin_manager, 'note_display_duration', None) if note is not None and plugin_id: note(plugin_id, time.monotonic() - started) + # A screen that drew once and holds makes no more + # update_display() calls, so a frame the preview throttle + # skipped would otherwise never reach the snapshot. + write_owed = getattr(getattr(self, 'display_manager', None), + 'write_owed_snapshot', None) + if write_owed is not None: + write_owed() def _health_tracker(self): """The plugin circuit breaker, or None when it is not enabled.""" diff --git a/src/display_manager.py b/src/display_manager.py index 3e211e9f..05b04cb7 100644 --- a/src/display_manager.py +++ b/src/display_manager.py @@ -317,6 +317,11 @@ class DisplayManager: # is handed to the writer; this only once it has been saved, so an # mtime touch never vouches for a frame still waiting to be written. self._saved_snapshot_digest: Optional[int] = None + # A changed frame reached _write_snapshot_if_due() inside the write + # interval and was skipped. Nothing writes it unless update_display() + # runs again, and a screen that draws once and holds never calls it + # again -- see write_owed_snapshot(). + self._snapshot_owed = False self._snapshot_dir_prepared = False # Background writer used mid-scroll; see _write_snapshot_if_due. self._snapshot_cond = threading.Condition() @@ -1788,9 +1793,10 @@ class DisplayManager: if frame_checksum is not None: digest = frame_checksum + frame_changed = digest != self._last_snapshot_digest action = snapshot_policy.decide( now, self._last_snapshot_ts, self._last_snapshot_touch_ts, - viewer_fresh, digest != self._last_snapshot_digest) + viewer_fresh, frame_changed) else: # Ask as if the frame had changed before paying to find out. # decide() is monotone in frame_changed -- a SKIP for a @@ -1802,22 +1808,36 @@ class DisplayManager: now, self._last_snapshot_ts, self._last_snapshot_touch_ts, viewer_fresh, True) if action is snapshot_policy.SnapshotAction.SKIP: + # Not hashed, so not known to be unchanged: owed until a + # later look finds it written or unchanged. + self._snapshot_owed = True return digest = zlib.adler32(self.image.tobytes()) - if digest == self._last_snapshot_digest: + frame_changed = digest != self._last_snapshot_digest + if not frame_changed: # Unchanged after all: the decision an unchanged frame gets. action = snapshot_policy.decide( now, self._last_snapshot_ts, self._last_snapshot_touch_ts, viewer_fresh, False) if action is snapshot_policy.SnapshotAction.SKIP: + # A changed frame inside the write interval stays owed: the + # next update_display() would write it, but a static screen + # may not make one -- write_owed_snapshot() covers that. + self._snapshot_owed = frame_changed return if (action is snapshot_policy.SnapshotAction.TOUCH and self._saved_snapshot_digest == digest): # mtime bump only: keeps the health check (snapshot age) # green without paying for a PNG encode of an unchanged frame + # (this frame is already on disk, so nothing is owed). + self._snapshot_owed = False os.utime(self._snapshot_path, None) self._last_snapshot_touch_ts = now return + # Owed until the write below succeeds: if it raises, the frame + # stays owed and write_owed_snapshot() retries it, rather than a + # held screen leaving the preview stale after one failed write. + self._snapshot_owed = True # (A TOUCH for a frame that isn't on disk yet -- still queued, or # its write failed -- is written instead: touching would make the # older file on disk look current.) @@ -1842,9 +1862,39 @@ class DisplayManager: self._last_snapshot_ts = now self._last_snapshot_touch_ts = now self._last_snapshot_digest = digest + self._snapshot_owed = False except Exception as e: self._log_snapshot_failure(e) + def write_owed_snapshot(self) -> None: + """Write a frame the snapshot throttle skipped, once it is due. + + The preview snapshot is only ever written from update_display(), and + at most once per write interval (snapshot_policy). A frame pushed + inside that interval is skipped, and is written by the next + update_display() that comes after it -- but a screen that draws its + card once and then holds it makes no further call. Its frame was on + the panel and never in the preview: soccer's recent/upcoming cards + skip redundant redraws, and the first one after an on-demand start + (pushed a few milliseconds after the controller's clear) left + /api/v3/display/current and the web preview black for the whole + screen while the panel showed the card. + + The render loop calls this after each frame. Cheap when nothing is + owed (one attribute read); otherwise the usual policy decides, so + the write still waits out the interval and an unchanged frame is + never re-encoded. + """ + if not self._snapshot_owed: + return + try: + if self._writes_suppressed(): + return + with self._update_lock: + self._write_snapshot_if_due() + except Exception as e: # pylint: disable=broad-except + self._log_snapshot_failure(e) + def _log_snapshot_failure(self, error: Exception) -> None: # Snapshot failures must never break display — but they must not # be silent either: the snapshot's mtime is the web UI's display diff --git a/test/test_api_v3_status_accuracy.py b/test/test_api_v3_status_accuracy.py new file mode 100644 index 00000000..c68f6d65 --- /dev/null +++ b/test/test_api_v3_status_accuracy.py @@ -0,0 +1,326 @@ +"""Four web answers that disagreed with the rig they describe (found on ledpi). + +1. POST /config/schedule refused the schedule GET returns on a fresh install + (config.template.json: per-day, every day off, schedule disabled) with + "At least one day must be enabled", as did /config/dim-schedule. A + disabled schedule needs no enabled day. +2. A brightness-only POST /config/main answered ``restart_required: true``, + though the display applies brightness live (brightness.set over the + socket, and the config watcher). The flag now says whether anything + changed that the running display does not pick up by itself. +3. /health stayed "healthy" with the display service stopped: only the + sub-checks changed. Service inactive, no socket and no live heartbeat + is now ``display_loop: stopped`` and "degraded". +4. /display/current-status kept answering ``is_display_active: true`` from + the cache for up to 120 s after the display stopped. With no socket and + no live heartbeat it is now unknown. +""" + +import copy +import json +import os +import sys +import time +from pathlib import Path +from unittest.mock import patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 + +from src import display_watchdog # noqa: E402 +from src.ipc import client as control_client # noqa: E402 +from web_interface import display_state # noqa: E402 + +REPO = Path(__file__).resolve().parent.parent +TEMPLATE = json.loads((REPO / 'config' / 'config.template.json').read_text(encoding='utf-8')) + + +@pytest.fixture +def store(api_v3_module, monkeypatch): + state = {'config': {}, 'saves': 0} + api_v3_module.api_v3.config_manager.load_config.side_effect = \ + lambda *a, **k: copy.deepcopy(state['config']) + + def fake_save(_manager, config, **_kwargs): + state['config'] = copy.deepcopy(config) + state['saves'] += 1 + return True, '' + + monkeypatch.setattr(api_v3_module, '_save_config_atomic', fake_save) + return state + + +# --- 1. schedules --------------------------------------------------------------- + +SCHEDULE_ROUTES = [('/api/v3/config/schedule', 'schedule'), + ('/api/v3/config/dim-schedule', 'dim_schedule')] + + +@pytest.mark.parametrize('route,section', SCHEDULE_ROUTES) +def test_the_templates_disabled_per_day_schedule_saves_back(api_v3_client, store, + route, section): + stored = copy.deepcopy(TEMPLATE[section]) + stored['mode'] = 'per-day' + assert stored['enabled'] is False + assert not any(day['enabled'] for day in stored['days'].values()) + store['config'] = {section: copy.deepcopy(stored)} + + read = api_v3_client.get(route).get_json()['data'] + resp = api_v3_client.post(route, json=read) + + assert resp.status_code == 200, resp.get_json() + saved = store['config'][section] + assert saved['enabled'] is False and saved['mode'] == 'per-day' + # The disabled days keep their times: switching one on finds them. + assert saved['days'] == stored['days'] + + +@pytest.mark.parametrize('route,section', SCHEDULE_ROUTES) +def test_an_enabled_per_day_schedule_still_needs_a_day(api_v3_client, store, route, section): + body = copy.deepcopy(TEMPLATE[section]) + body.update(enabled=True, mode='per-day') + resp = api_v3_client.post(route, json=body) + assert resp.status_code == 400 + assert 'At least one day must be enabled' in resp.get_json()['message'] + assert store['saves'] == 0 + + +@pytest.mark.parametrize('route', [r for r, _ in SCHEDULE_ROUTES]) +def test_the_pickers_form_post_with_every_day_off_saves(api_v3_client, store, route): + """What schedule-picker.js posts: flat hidden inputs, booleans as strings, + times for every day.""" + body = {'enabled': 'false', 'mode': 'per_day', 'start_time': '07:00', 'end_time': '23:00'} + for day in ('monday', 'tuesday', 'wednesday', 'thursday', 'friday', 'saturday', 'sunday'): + body.update({f'{day}_enabled': 'false', f'{day}_start': '06:30', f'{day}_end': '22:15'}) + resp = api_v3_client.post(route, json=body) + assert resp.status_code == 200, resp.get_json() + + +def test_an_invalid_time_on_a_disabled_day_is_dropped_not_refused(api_v3_client, store): + body = {'enabled': False, 'mode': 'per-day', + 'days': {'monday': {'enabled': False, 'start_time': 'soon', 'end_time': '22:00'}}} + resp = api_v3_client.post('/api/v3/config/schedule', json=body) + assert resp.status_code == 200, resp.get_json() + assert store['config']['schedule']['days']['monday'] == {'enabled': False, + 'end_time': '22:00'} + + +# --- 2. restart_required on /config/main ------------------------------------------ + +STORED_MAIN = { + 'timezone': 'America/Chicago', + 'display': { + 'hardware': {'rows': 32, 'cols': 64, 'chain_length': 2, 'brightness': 90, + 'disable_hardware_pulsing': False, 'inverse_colors': False, + 'show_refresh_rate': False}, + 'runtime': {'gpio_slowdown': 4}, + 'display_durations': {'clock': 15}, + 'use_short_date_format': False, + }, +} + + +@pytest.fixture +def main_store(store): + store['config'] = copy.deepcopy(STORED_MAIN) + return store + + +def _save_main(client, body): + with patch('web_interface.blueprints.api_v3.control_client.brightness_set', + side_effect=control_client.ControlError('no_socket', 'x')): + resp = client.post('/api/v3/config/main', data=json.dumps(body), + content_type='application/json') + assert resp.status_code == 200, resp.get_json() + return resp.get_json() + + +def test_a_brightness_only_save_needs_no_restart(api_v3_client, main_store): + body = _save_main(api_v3_client, {'brightness': 40}) + assert main_store['config']['display']['hardware']['brightness'] == 40 + assert body['restart_required'] is False + + +def test_a_brightness_save_on_a_config_without_a_display_section(api_v3_client, store): + """The route creates display.hardware and display.runtime on the way; + empty sections are not a change.""" + store['config'] = {} + assert _save_main(api_v3_client, {'brightness': 40})['restart_required'] is False + + +def test_the_display_form_with_only_brightness_changed_needs_no_restart(api_v3_client, + main_store): + hw = STORED_MAIN['display']['hardware'] + body = {'__form_section': 'display', 'rows': 32, 'cols': 64, 'chain_length': 2, + 'brightness': 55, 'gpio_slowdown': 4} + body.update({k: 'on' for k in ('disable_hardware_pulsing', 'inverse_colors', + 'show_refresh_rate') if hw[k]}) + assert _save_main(api_v3_client, body)['restart_required'] is False + + +def test_a_mode_duration_needs_no_restart(api_v3_client, main_store): + body = _save_main(api_v3_client, {'duration__clock': 40}) + assert main_store['config']['display']['display_durations']['clock'] == 40 + assert body['restart_required'] is False + + +@pytest.mark.parametrize('change', [{'rows': 64}, {'brightness': 40, 'chain_length': 3}, + {'gpio_slowdown': 2}, {'timezone': 'UTC'}]) +def test_a_setting_the_display_reads_at_startup_still_needs_one(api_v3_client, main_store, + change): + assert _save_main(api_v3_client, change)['restart_required'] is True + + +def test_restart_needed_compares_leaves(): + from web_interface.blueprints.api_v3.config import restart_needed + before = {'display': {'hardware': {'brightness': 90, 'rows': 32}}} + assert not restart_needed(before, copy.deepcopy(before)) + assert not restart_needed(before, {'display': {'hardware': {'brightness': 10, 'rows': 32}, + 'runtime': {}}}) + assert restart_needed(before, {'display': {'hardware': {'brightness': 90}}}) # removed + assert not restart_needed({}, {'clock': {'enabled': True}}, live_paths=[('clock',)]) + assert restart_needed({}, {'clockwork': {'enabled': True}}, live_paths=[('clock',)]) + + +# --- 3 and 4. a stopped display ----------------------------------------------------- + +@pytest.fixture +def no_display(monkeypatch, tmp_path): + """A Pi whose display service has stopped: the socket is expected here + but does not answer, and systemd took the heartbeat's directory away.""" + monkeypatch.setattr(display_state, 'socket_supported', lambda: True) + monkeypatch.setattr(display_state, 'client_socket_paths', lambda: [str(tmp_path / 'gone')]) + monkeypatch.setattr(display_state, 'read_state', lambda: None) + path = tmp_path / 'display-heartbeat.json' + monkeypatch.setattr(display_watchdog, 'HEARTBEAT_PATH', str(path)) + + def beat(age, pid=None): + path.write_text(json.dumps({'pid': os.getpid() if pid is None else pid, + 'mono': time.monotonic() - age, + 'wall': time.time() - age})) + return beat + + +@pytest.fixture +def service(monkeypatch): + status = {'active': False, 'returncode': 3, 'stdout': 'inactive', 'stderr': ''} + monkeypatch.setattr('web_interface.blueprints.api_v3.misc._get_display_service_status', + lambda: dict(status)) + return status + + +@pytest.fixture +def fresh_preview(tmp_path, monkeypatch): + """The preview frame the display left behind, under 60 s old: on its own + it kept the hardware check "connected".""" + from web_interface import display_preview + snapshot = tmp_path / 'preview.png' + snapshot.write_bytes(b'png') + monkeypatch.setattr(display_preview, 'SNAPSHOT_PATH', str(snapshot)) + + +def _health(client): + resp = client.get('/api/v3/health') + assert resp.status_code == 200, resp.get_json() + return resp.get_json()['data'] + + +class TestHealth: + def test_a_stopped_display_service_is_degraded(self, api_v3_client, no_display, service, + fresh_preview): + data = _health(api_v3_client) + assert data['services']['display_service']['status'] == 'inactive' + assert data['checks']['display_loop']['status'] == 'stopped' + assert data['status'] == 'degraded' + + def test_a_service_still_starting_is_not(self, api_v3_client, no_display, service, + fresh_preview): + """Active, before its socket and first heartbeat: not stopped.""" + service.update(active=True, stdout='active', returncode=0) + data = _health(api_v3_client) + assert data['checks']['display_loop']['status'] == 'not_reported' + assert data['status'] == 'healthy' + + def test_a_display_run_by_hand_is_not_stopped(self, api_v3_client, no_display, service, + fresh_preview): + """The service is off but a display process beats (sudo python3 run.py).""" + no_display(age=2) + data = _health(api_v3_client) + assert data['checks']['display_loop']['status'] == 'running' + assert data['status'] == 'healthy' + + @pytest.mark.parametrize('platform', ['no_unix_sockets', 'socket_off']) + def test_without_a_socket_to_expect_nothing_changes(self, api_v3_client, no_display, + service, fresh_preview, monkeypatch, + platform): + """Windows and the dev server (no systemd unit), or the socket + deliberately off: no heartbeat is no signal, as before.""" + if platform == 'no_unix_sockets': + monkeypatch.setattr(display_state, 'socket_supported', lambda: False) + else: + monkeypatch.setattr(display_state, 'client_socket_paths', lambda: []) + service.update(returncode=-1, stdout='', stderr='systemctl not found') + data = _health(api_v3_client) + assert data['checks']['display_loop']['status'] == 'not_reported' + assert data['status'] == 'healthy' + + def test_the_status_only_answer_says_degraded(self, api_v3_client, no_display, service, + fresh_preview, monkeypatch): + monkeypatch.setattr('web_interface.blueprints.api_v3.misc.request_is_authenticated', + lambda: False) + resp = api_v3_client.get('/api/v3/health') + assert resp.get_json()['data'] == {'status': 'degraded'} + + +class TestCurrentStatus: + CACHED = {'mode': 'clock', 'plugin_id': 'clock', 'is_display_active': True, + 'on_demand_active': False, 'last_updated': None} + + @pytest.fixture + def cached(self, api_v3_module): + entry = dict(self.CACHED, last_updated=time.time() - 30) + cache = api_v3_module.api_v3.cache_manager + cache.get.side_effect = lambda key, *a, **kw: ( + dict(entry) if key == 'display_current_state' else None) + return entry + + def _status(self, client): + resp = client.get('/api/v3/display/current-status') + assert resp.status_code == 200 + return resp.get_json()['data'] + + def test_a_stopped_display_is_not_reported_active(self, api_v3_client, no_display, cached): + data = self._status(api_v3_client) + assert not data.get('is_display_active') + assert data['mode'] is None and data['last_updated'] is None + assert data['source'] == 'cache' + + def test_a_stale_heartbeat_is_not_active_either(self, api_v3_client, no_display, cached): + no_display(age=display_watchdog.HEARTBEAT_STALE_SECONDS + 5) + assert self._status(api_v3_client)['mode'] is None + + @pytest.mark.skipif(os.name != 'posix', reason='process_exists answers only on POSIX') + def test_a_heartbeat_from_a_dead_process_is_not_active(self, api_v3_client, no_display, + cached): + no_display(age=1, pid=2 ** 22 + 12345) + assert self._status(api_v3_client)['mode'] is None + + def test_a_live_heartbeat_without_a_socket_reads_the_cache(self, api_v3_client, + no_display, cached): + """An older display with no socket, still running.""" + no_display(age=2) + data = self._status(api_v3_client) + assert data['mode'] == 'clock' and data['is_display_active'] is True + + @pytest.mark.parametrize('platform', ['no_unix_sockets', 'socket_off']) + def test_without_a_socket_to_expect_the_cache_answers(self, api_v3_client, no_display, + cached, monkeypatch, platform): + if platform == 'no_unix_sockets': + monkeypatch.setattr(display_state, 'socket_supported', lambda: False) + else: + monkeypatch.setattr(display_state, 'client_socket_paths', lambda: []) + data = self._status(api_v3_client) + assert data['mode'] == 'clock' and data['is_display_active'] is True diff --git a/test/test_espn_dates.py b/test/test_espn_dates.py index 33fa0c5e..253bf95e 100644 --- a/test/test_espn_dates.py +++ b/test/test_espn_dates.py @@ -489,3 +489,171 @@ class TestConcurrency: assert live["peak"] <= espn_dates.ESPN_CHUNK_WORKERS assert live["peak"] > 1, "chunks should actually overlap" + + +class TestEdgeMonths: + """A window's partial edge months are asked whole and trimmed. + + The default scoreboard window -- a fortnight either side of today -- spans + two partial months, so it used to cost 29 day requests per league. ESPN's + ``dates=YYYYMMDD`` means a US Eastern day (verified against the live API + on 2026-10-03, 417 of 417 soccer events), so a month answer trimmed to + the window's Eastern days is what the day requests returned. + """ + + def test_a_fortnight_either_side_is_two_requests(self): + planned = espn_dates.espn_request_chunks(date(2026, 9, 20), date(2026, 10, 18)) + assert planned == [ + ("202609", (date(2026, 9, 20), date(2026, 9, 30))), + ("202610", (date(2026, 10, 1), date(2026, 10, 18))), + ] + + def test_a_live_polls_two_days_stay_two_days(self): + planned = espn_dates.espn_request_chunks(date(2026, 10, 2), date(2026, 10, 3)) + assert planned == [("20261002", None), ("20261003", None)] + + def test_the_threshold_is_inclusive(self): + n = espn_dates.ESPN_MONTH_COVER_MIN_DAYS + short = espn_dates.espn_request_chunks(date(2026, 10, 1), date(2026, 10, n - 1)) + assert [chunk for chunk, _ in short] == [ + "202610%02d" % day for day in range(1, n)] + enough = espn_dates.espn_request_chunks(date(2026, 10, 1), date(2026, 10, n)) + assert enough == [("202610", (date(2026, 10, 1), date(2026, 10, n)))] + + def test_whole_months_and_short_edges_are_unchanged(self): + planned = espn_dates.espn_request_chunks(date(2026, 8, 30), date(2026, 10, 2)) + assert planned == [ + ("20260830", None), ("20260831", None), ("202609", None), + ("20261001", None), ("20261002", None), + ] + + def test_without_time_zone_data_edges_stay_days(self, monkeypatch): + monkeypatch.setattr(espn_dates, "_EASTERN", None) + planned = espn_dates.espn_request_chunks(date(2026, 9, 20), date(2026, 10, 18)) + assert len(planned) == 29 + assert all(trim is None for _, trim in planned) + + @pytest.mark.parametrize("start,end", [ + (date(2026, 9, 20), date(2026, 10, 18)), + (date(2026, 1, 25), date(2026, 3, 3)), + (date(2026, 12, 20), date(2027, 1, 9)), + (date(2026, 10, 5), date(2026, 10, 9)), + ]) + def test_the_planned_requests_still_cover_every_day_exactly_once(self, start, end): + covered = [] + for chunk, trim in espn_dates.espn_request_chunks(start, end): + if trim is None: + covered.extend(days_covered_by([chunk])) + else: + assert chunk == trim[0].strftime("%Y%m") == trim[1].strftime("%Y%m") + covered.extend(trim[0] + timedelta(days=offset) + for offset in range((trim[1] - trim[0]).days + 1)) + expected = [start + timedelta(days=offset) for offset in range((end - start).days + 1)] + assert covered == expected + + def test_a_trimmed_month_keeps_only_the_windows_eastern_days(self): + september = [ + # 03:30Z on the 20th is still the 19th in New York: outside. + {"id": "before", "date": "2026-09-20T03:30Z"}, + {"id": "first", "date": "2026-09-20T14:00Z"}, + {"id": "late", "date": "2026-09-30T23:30Z"}, + ] + october = [ + {"id": "oct1", "date": "2026-10-01T19:00Z"}, + # 03:30Z on the 19th is the evening of the 18th in New York: inside. + {"id": "last", "date": "2026-10-19T03:30Z"}, + {"id": "after", "date": "2026-10-19T14:00Z"}, + {"id": "undated"}, + ] + session = FakeSession({"202609": september, "202610": october}) + + data = fetch_espn_date_chunks(session, URL, params={"dates": "20260920-20261018"}) + + assert sorted(call["dates"] for call in session.calls) == ["202609", "202610"] + # An event with no readable date is kept, never dropped on a guess. + assert [e["id"] for e in data["events"]] == ["first", "late", "oct1", "last", "undated"] + + def test_eastern_standard_time_is_honoured_after_the_clocks_change(self): + # 2026-11-01 ends daylight saving: Eastern is UTC-5 from then on. + november = [ + {"id": "out", "date": "2026-11-15T04:30Z"}, # Nov 14, 23:30 EST + {"id": "in", "date": "2026-11-15T05:30Z"}, # Nov 15, 00:30 EST + ] + session = FakeSession({"202611": november}) + data = fetch_espn_date_chunks(session, URL, params={"dates": "20261115-20261121"}) + assert [e["id"] for e in data["events"]] == ["in"] + + def test_a_capped_edge_month_re_asks_only_the_windows_days(self): + full = [{"id": "cap%d" % i, "date": "2026-10-05T18:00Z"} for i in range(ESPN_MAX_LIMIT)] + by_chunk = {"202610": full} + by_chunk.update({"202610%02d" % day: [{"id": "o%02d" % day}] for day in range(1, 32)}) + session = FakeSession(by_chunk) + + data = fetch_espn_date_chunks(session, URL, params={"dates": "20261001-20261010"}) + + sent = [call["dates"] for call in session.calls] + assert sent[0] == "202610" + assert sorted(sent[1:]) == ["202610%02d" % day for day in range(1, 11)] + assert [e["id"] for e in data["events"]] == ["o%02d" % day for day in range(1, 11)] + + +class TestProcessWideChunkCap: + """The chunk cap holds across windows, not per window. + + A soccer board starting eight leagues fetches sixteen windows at once. + With a pool of ``ESPN_CHUNK_WORKERS`` each, ~40 requests were in flight + and every one past a session's pool opened a connection -- and a DNS + lookup. On ledpi that was ~90 NameResolutionErrors per start. + """ + + def test_concurrent_windows_share_one_budget(self): + live = {"now": 0, "peak": 0} + guard = threading.Lock() + + class CountingSession(FakeSession): + def get(self, url, params=None, headers=None, timeout=None): + with guard: + live["now"] += 1 + live["peak"] = max(live["peak"], live["now"]) + try: + time.sleep(0.01) + return super().get(url, params=params, headers=headers, timeout=timeout) + finally: + with guard: + live["now"] -= 1 + + sessions = [CountingSession() for _ in range(6)] + # Six leagues, so the fetch service cannot merge them into one, on a + # host with no token bucket: earlier tests may have spent ESPN's + # burst, and a bucket paced at 20/s would serialise these by itself. + threads = [ + threading.Thread(target=fetch_espn_date_chunks, + args=(session, "https://scores.example.test/league%d" % index), + kwargs={"params": {"dates": "20260101-20261231"}}) + for index, session in enumerate(sessions) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=30) + + assert all(len(session.calls) == 12 for session in sessions) + assert live["peak"] <= espn_dates.ESPN_CHUNK_WORKERS + assert live["peak"] > 1, "chunks should still overlap" + + +def test_a_fresh_process_skips_the_doomed_range_request(): + """Every start used to spend one 400 per window learning that ranges are + still rejected -- eleven at once from a soccer board. A new process now + starts inside the retry period instead.""" + import subprocess + import sys + from pathlib import Path + + out = subprocess.run( + [sys.executable, "-c", + "import src.common.espn_dates as e; print(e._ranges_known_rejected())"], + cwd=str(Path(__file__).resolve().parents[1]), + capture_output=True, text=True, timeout=60, + ) + assert out.stdout.strip() == "True", out.stderr diff --git a/test/test_fetch_service.py b/test/test_fetch_service.py index e25ec56a..d2c17f4c 100644 --- a/test/test_fetch_service.py +++ b/test/test_fetch_service.py @@ -670,14 +670,14 @@ class TestCallerIdentity: assert _counters(global_service, plugin="football-scoreboard")["requests"] == 1 def test_espn_chunks_on_worker_threads_count_against_the_caller(self, global_service): - from src.common.espn_dates import espn_date_chunks, fetch_espn_date_chunks, parse_espn_date_range + from src.common.espn_dates import espn_request_chunks, fetch_espn_date_chunks, parse_espn_date_range session = FakeSession(lambda url, kw: make_response(body=b'{"events": []}', url=url)) dates = "20260801-20261015" with plugin_scope("baseball-scoreboard"): fetch_espn_date_chunks(session, "https://site.api.espn.com/s/scoreboard", params={"dates": dates}) - chunks = len(espn_date_chunks(*parse_espn_date_range(dates))) + chunks = len(espn_request_chunks(*parse_espn_date_range(dates))) assert chunks > 1 assert len(session.calls) == chunks assert _counters(global_service, plugin="baseball-scoreboard")["requests"] == chunks diff --git a/test/test_snapshot_owed_frame.py b/test/test_snapshot_owed_frame.py new file mode 100644 index 00000000..2db890a1 --- /dev/null +++ b/test/test_snapshot_owed_frame.py @@ -0,0 +1,183 @@ +"""A frame the preview throttle skipped still reaches the snapshot. + +The preview snapshot (/api/v3/display/current, the web UI's live preview) is +only written from update_display(), at most once per write interval. A screen +that draws its card once and then holds it -- soccer's recent/upcoming cards +skip redundant redraws -- pushes exactly one frame. When that push lands inside +the interval, e.g. a few milliseconds after the on-demand start's clear wrote a +black frame, the throttle skips it and nothing ever writes it: on ledpi the +preview stayed black for soccer's whole 15 s screen while the panel showed the +card, and the next screen "rendered immediately". + +Runs the real DisplayManager on the emulator, like test_display_dirty_tracking. +""" + +import os +import sys +import types + +os.environ["EMULATOR"] = "true" + +import pytest +from PIL import Image + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + + +@pytest.fixture(scope="module") +def dm(tmp_path_factory): + from src.display_manager import DisplayManager + DisplayManager._instance = None + manager = DisplayManager({ + "display": { + "hardware": {"rows": 32, "cols": 64, "chain_length": 2, + "parallel": 1, "brightness": 90}, + "runtime": {"gpio_slowdown": 0}, + }, + }, suppress_test_pattern=True) + manager._snapshot_path = str( + tmp_path_factory.mktemp("owed_snapshot") / "led_matrix_preview.png") + yield manager + DisplayManager._instance = None + + +@pytest.fixture +def viewer(dm, monkeypatch, tmp_path): + """A preview is open (1 s write interval); fresh snapshot bookkeeping.""" + monkeypatch.setattr(dm, "_viewer_is_fresh", lambda now: True) + dm._viewer_was_fresh = True + dm._snapshot_path = str(tmp_path / "snap.png") + dm._last_snapshot_ts = 0.0 + dm._last_snapshot_touch_ts = 0.0 + dm._last_snapshot_digest = None + dm._saved_snapshot_digest = None + dm._snapshot_owed = False + dm.set_scrolling_state(False) + return dm + + +def _lit(path): + with Image.open(path) as img: + return sum(1 for p in img.convert("RGB").getdata() if max(p) > 20) + + +def _age_last_write(dm, seconds=2.0): + """As if `seconds` had passed since the last snapshot write.""" + dm._last_snapshot_ts -= seconds + dm._last_snapshot_touch_ts -= seconds + + +def _clear_then_draw_card(dm): + """The on-demand start's clear, then the card a few ms later.""" + dm.clear() + dm.update_display() # black frame: written + assert _lit(dm._snapshot_path) == 0 + dm.draw.rectangle([4, 4, 40, 20], fill=(255, 255, 0)) + dm.update_display() # the card: inside the interval + + +def _controller(dm): + from src import display_controller as dc_module + controller = dc_module.DisplayController.__new__(dc_module.DisplayController) + controller.plugin_manager = None + controller.display_manager = dm + return controller + + +class _HoldingPlugin: + """Already showing its card: display() returns True and draws nothing.""" + + plugin_id = "holding" + + def __init__(self): + self.calls = 0 + + def display(self, display_mode=None, force_clear=False): + self.calls += 1 + return True + + +def test_a_held_card_reaches_the_preview_on_the_next_frame(viewer): + dm = viewer + _clear_then_draw_card(dm) + assert _lit(dm._snapshot_path) == 0 # the throttle skipped the card + + controller = _controller(dm) + plugin = _HoldingPlugin() + _age_last_write(dm) + # The render loop's next frame: the plugin draws nothing and makes no + # update_display() call, as soccer's switch cards do. + assert controller._display_once(plugin, "soccer_eng.1_recent", True) is True + assert plugin.calls == 1 + assert _lit(dm._snapshot_path) > 0 + + +def test_the_owed_write_still_waits_out_the_interval(viewer, monkeypatch): + dm = viewer + _clear_then_draw_card(dm) + saves = [] + monkeypatch.setattr(dm, "_save_snapshot", lambda image: saves.append(image)) + dm.write_owed_snapshot() # still inside the interval + assert saves == [] + _age_last_write(dm) + dm.write_owed_snapshot() + assert len(saves) == 1 + # Written: nothing is owed, so later frames do no work and the unchanged + # frame is not encoded again. + assert dm._snapshot_owed is False + _age_last_write(dm) + dm.write_owed_snapshot() + assert len(saves) == 1 + + +def test_a_failed_owed_write_stays_owed_and_is_retried(viewer, monkeypatch): + dm = viewer + _clear_then_draw_card(dm) + _age_last_write(dm) + attempts = [] + + def failing_save(image): + attempts.append(image) + raise OSError("disk full") + + monkeypatch.setattr(dm, "_save_snapshot", failing_save) + dm.write_owed_snapshot() # the write fails + assert len(attempts) == 1 + assert dm._snapshot_owed is True # still owed: a held screen + saves = [] # makes no update_display() + monkeypatch.setattr(dm, "_save_snapshot", lambda image: saves.append(image)) + dm.write_owed_snapshot() # retried on the next frame + assert len(saves) == 1 + assert dm._snapshot_owed is False + + +def test_nothing_owed_after_a_frame_that_was_written(viewer, monkeypatch): + dm = viewer + dm.draw.rectangle([0, 0, 8, 8], fill=(0, 255, 0)) + dm.update_display() # due: written at once + assert dm._snapshot_owed is False + calls = [] + monkeypatch.setattr(dm, "_write_snapshot_if_due", + lambda *a, **k: calls.append(a)) + dm.write_owed_snapshot() + assert calls == [] + + +def test_an_unchanged_frame_inside_the_interval_is_not_owed(viewer): + dm = viewer + dm.draw.rectangle([0, 0, 8, 8], fill=(0, 0, 255)) + dm.update_display() + dm.update_display() # same frame, inside the interval + assert dm._snapshot_owed is False + + +def test_a_controller_without_the_hook_still_draws(): + """Controllers built without a display manager (tests) are unaffected.""" + from src import display_controller as dc_module + controller = dc_module.DisplayController.__new__(dc_module.DisplayController) + controller.plugin_manager = None + plugin = _HoldingPlugin() + assert controller._display_once(plugin, "x", True) is True + controller.display_manager = types.SimpleNamespace() + assert controller._display_once(plugin, "x", True) is True + assert plugin.calls == 2 diff --git a/test/test_state_stream_readers.py b/test/test_state_stream_readers.py index c7272da4..9c47dda6 100644 --- a/test/test_state_stream_readers.py +++ b/test/test_state_stream_readers.py @@ -612,6 +612,12 @@ class TestEndToEnd: cached['display_current_state'] = {'mode': 'from-cache', 'last_updated': 1} path = str(tmp_path / 'control.sock') monkeypatch.setenv(c.SOCKET_PATH_ENV, path) + # The display is this process, and its render loop is beating: the + # cache is then still its answer once the socket goes. + heartbeat = tmp_path / 'display-heartbeat.json' + heartbeat.write_text(json.dumps({'pid': os.getpid(), 'mono': time.monotonic(), + 'wall': time.time()})) + monkeypatch.setattr(display_watchdog, 'HEARTBEAT_PATH', str(heartbeat)) hub = _hub_with_everything() server = ControlServer(path, state_hub=hub, keepalive=0.2) assert server.start() @@ -638,3 +644,8 @@ class TestEndToEnd: break time.sleep(0.05) assert (data['mode'], data['source']) == ('from-cache', 'cache') + # Stopped: systemd takes the heartbeat's directory with it, and the + # cache's last answer is no longer anyone's. + heartbeat.unlink() + data = _data(client, '/api/v3/display/current-status') + assert (data['mode'], data['source']) == (None, 'cache') diff --git a/test/web_interface/test_web_process_runs_no_plugin_code.py b/test/web_interface/test_web_process_runs_no_plugin_code.py index 53841f25..5e993d9a 100644 --- a/test/web_interface/test_web_process_runs_no_plugin_code.py +++ b/test/web_interface/test_web_process_runs_no_plugin_code.py @@ -221,7 +221,9 @@ class TestTheWebProcessNeverRunsAPlugin: def test_saving_its_section_through_the_main_config(self, web): body = web.post("/api/v3/config/main", {PLUGIN_ID: {"message": "via main"}}) assert web.stored()["message"] == "via main" - assert body["restart_required"] is True + # The display's config watcher hands the section to the running + # plugin (on_config_change), as for /plugins/config: no restart. + assert body["restart_required"] is False assert web.ran() == [] def test_resetting_its_config(self, web): diff --git a/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index f2265370..91ee73ba 100644 --- a/web_interface/blueprints/api_v3/config.py +++ b/web_interface/blueprints/api_v3/config.py @@ -17,6 +17,8 @@ from src.pi5_matrix_support import is_raspberry_pi_5 from web_interface.cache import invalidate_cache from web_interface.auth import SECTION as _WEB_AUTH_SECTION, strip_auth_section import web_interface.blueprints.api_v3 as _pkg +import copy +from typing import Any, Dict, Iterable, Tuple # Read through the module rather than bound by value: tests patch these # as module attributes, and a value binding would not see the patch. @@ -33,6 +35,50 @@ FORM_SECTION_FIELD = '__form_section' GENERAL_FIELDS = ('timezone', 'city', 'state', 'country', 'web_display_autostart', 'plugins_directory', 'auto_update_enabled', 'auto_update_channel') +#: Settings in config.json the running display applies without a restart, +#: as key paths (a path covers everything under it). Brightness goes over the +#: control socket (brightness.set) and the config watcher's refresh +#: (DisplayController._refresh_config_cache); the per-mode durations are read +#: from the live config each time a mode starts (_get_display_duration). +#: Plugin sections are live as well (each plugin's on_config_change), and +#: save_main_config adds the ones a request saves. +LIVE_CONFIG_PATHS: Tuple[Tuple[str, ...], ...] = ( + ('display', 'hardware', 'brightness'), + ('display', 'display_durations'), +) + + +_MISSING = object() + + +def _config_leaves(config: Any, prefix: Tuple[str, ...] = ()) -> Dict[Tuple[str, ...], Any]: + """Every non-dict value in ``config``, by key path. An empty dict has none, + so a section created empty on the way to a field is not a change.""" + if not isinstance(config, dict): + return {prefix: config} + leaves: Dict[Tuple[str, ...], Any] = {} + for key, value in config.items(): + leaves.update(_config_leaves(value, prefix + (str(key),))) + return leaves + + +def restart_needed(before: Dict[str, Any], after: Dict[str, Any], + live_paths: Iterable[Tuple[str, ...]] = LIVE_CONFIG_PATHS) -> bool: + """Does going from config ``before`` to ``after`` need a display restart? + + True when anything changed outside ``live_paths``. A save that changes + only live settings, or nothing at all, does not. + """ + live = tuple(live_paths) + old, new = _config_leaves(before), _config_leaves(after) + for path in set(old) | set(new): + if old.get(path, _MISSING) == new.get(path, _MISSING): + continue + if not any(path[:len(prefix)] == prefix for prefix in live): + return True + return False + + #: Top-level fields save_main_config stores somewhere of its own (location, #: plugin_system, ...), never as a config key of the same name. _MAPPED_TOP_LEVEL_FIELDS = GENERAL_FIELDS + ( @@ -72,6 +118,22 @@ def _day_setting(data, day, flat_key, nested_key): return False, None +def _disabled_day_times(data, day, start_key, end_key): + """The times posted for a day that is off, the valid ones. + + Nothing reads them while the day is off, but the schedule picker posts + them and GET returns them, so keeping them means turning the day back on + finds what was there. An invalid one is dropped rather than refused, for + the same reason. + """ + times = {} + for field, key in (('start_time', start_key), ('end_time', end_key)): + value = _day_setting(data, day, key, field)[1] + if value and _validate_time_format(value)[0]: + times[field] = value + return times + + @api_v3.route('/config/main', methods=['GET']) def get_main_config(): """Get main configuration, with credentials redacted.""" @@ -257,11 +319,16 @@ def save_schedule_config(): day_config['start_time'] = start_time day_config['end_time'] = end_time + else: + day_config.update(_disabled_day_times(data, day, start_key, end_key)) schedule_config['days'][day] = day_config - # Validate that at least one day is enabled in per-day mode - if enabled_days_count == 0: + # An enabled per-day schedule needs a day to be on. A disabled + # one does not: every day off with the schedule off is what + # config.template.json ships, so refusing it meant a fresh + # install could not post back the schedule GET returned. + if enabled_days_count == 0 and enabled_value: return error_response( ErrorCode.VALIDATION_ERROR, "At least one day must be enabled in per-day schedule mode", @@ -465,11 +532,13 @@ def save_dim_schedule_config(): day_config['start_time'] = start_time day_config['end_time'] = end_time + else: + day_config.update(_disabled_day_times(data, day, start_key, end_key)) dim_schedule_config['days'][day] = day_config - # Validate that at least one day is enabled in per-day mode - if enabled_days_count == 0: + # As for the on/off schedule: only an enabled one needs a day on. + if enabled_days_count == 0 and enabled_value: return error_response( ErrorCode.VALIDATION_ERROR, "At least one day must be enabled in per-day dim schedule mode", @@ -557,6 +626,8 @@ def save_main_config(): # Merge with existing config (similar to original implementation) current_config = api_v3.config_manager.load_config() + # What was stored, to tell which settings this save changed. + stored_config = copy.deepcopy(current_config) was_auto_update_enabled = bool((current_config.get('auto_update') or {}).get('enabled')) is_general_update = any(k in data for k in GENERAL_FIELDS) @@ -1194,13 +1265,15 @@ def save_main_config(): message = f'{message}. {note}' except Exception: logger.warning("Automatic update setup could not be started", exc_info=True) - # Display hardware, rotation/durations and general settings take - # effect after a display restart; the UI shows its restart banner on - # this flag. - extra = {'restart_required': True} - # Brightness is the exception: the display applies a saved one - # without a restart. Over the control socket it lands at once, - # instead of when the config watcher next looks (up to ~2 s). + # Display hardware, rotation order and general settings take effect + # after a display restart; the UI shows its restart banner on this + # flag. Brightness, mode durations and plugin settings are applied by + # the running display (LIVE_CONFIG_PATHS), so a save that changed + # only those -- or nothing -- does not ask for one. + live_paths = LIVE_CONFIG_PATHS + tuple((plugin_id,) for plugin_id in plugin_keys_to_remove) + extra = {'restart_required': restart_needed(stored_config, current_config, live_paths)} + # Over the control socket a saved brightness lands at once, instead + # of when the config watcher next looks (up to ~2 s). if 'brightness' in data: saved = (current_config.get('display', {}).get('hardware', {}) or {}).get('brightness') if isinstance(saved, int) and not isinstance(saved, bool): diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index 163fbc21..a3da2acb 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -386,15 +386,22 @@ def get_current_display_status(): Read from the display's state stream over the control socket when it is available (``source: "socket"``). Otherwise from what the display publishes to the shared cache (display_controller._publish_current_mode_state) - when the active mode changes (``source: "cache"``). + when the active mode changes (``source: "cache"``). Unknown (every field + None) when the socket and the heartbeat both say the display is gone + (display_state.display_gone). """ - state = display_state.current_status(display_state.read_state()) + snapshot = display_state.read_state() + state = display_state.current_status(snapshot) source = 'socket' if state is None: source = 'cache' - cache = _cache_manager() - # memory_ttl=0: written by the display service; see get_on_demand_status. - state = cache.get('display_current_state', max_age=120, memory_ttl=0) + # A stopped display leaves its last answer in the cache, where it + # read as on (is_display_active: true) for the 120 s max_age. With + # no socket and no live heartbeat there is no display behind it. + if not display_state.display_gone(snapshot): + cache = _cache_manager() + # memory_ttl=0: written by the display service; see get_on_demand_status. + state = cache.get('display_current_state', max_age=120, memory_ttl=0) if state is None: state = { 'mode': None, diff --git a/web_interface/blueprints/api_v3/misc.py b/web_interface/blueprints/api_v3/misc.py index 5ef9aea0..dd089a19 100644 --- a/web_interface/blueprints/api_v3/misc.py +++ b/web_interface/blueprints/api_v3/misc.py @@ -102,6 +102,7 @@ def get_health(): # the only signal, as it always was. # The display reports the same beat's age over the control socket's # state stream, measured in memory; the file is the fallback. + snapshot = None try: snapshot = display_state.read_state() if snapshot is not None: @@ -132,6 +133,26 @@ def get_health(): 'error': 'see logs for details' } + # A stopped display service. The heartbeat's absence alone says + # nothing (the dev server, the emulator and Windows write none), so + # the overall status stayed "healthy" with the display down until the + # last preview frame it left aged past 60 s (hardware: stale). Together + # the three signals are definite: systemd says the service is not + # active, the control socket does not answer, and there is no live + # heartbeat (display_state.display_gone, which is never true where + # the platform has no socket or it is switched off). + try: + if (not display_service_status.get('active') + and display_state.display_gone(snapshot)): + health_status['checks']['display_loop'] = { + 'status': 'stopped', + 'note': 'The display service is not running', + 'source': 'service', + } + except Exception: + logger.warning("Health check could not tell whether the display is stopped", + exc_info=True) + # Check hardware connectivity (if display manager available) try: snapshot_path = display_preview.SNAPSHOT_PATH diff --git a/web_interface/display_state.py b/web_interface/display_state.py index 766f9e48..1c41694b 100644 --- a/web_interface/display_state.py +++ b/web_interface/display_state.py @@ -133,6 +133,39 @@ def on_demand_state(snapshot: Optional[Dict[str, Any]], return state +def display_gone(snapshot: Optional[Dict[str, Any]]) -> bool: + """Is there positively no display behind a fallback to the cache? + + True only when the socket should be there (this platform has one and it + is not switched off) but gave no ``snapshot``, and the render loop's + heartbeat file says nothing is running either: it is absent (systemd + removes its directory when the service stops), stale, or written by a + process that no longer exists -- #726's rules for the runtime snapshot. + Then what the display last left in the cache is a dead process's answer. + + False whenever the answer is in doubt: a snapshot came in, the socket is + off or unsupported (Windows, the test suite, a deliberate ``off``), or a + live heartbeat says the display is running without a socket (an older + display). Those read the cache exactly as before. + """ + if snapshot is not None: + return False + if not socket_supported() or not client_socket_paths(): + return False + from src import display_watchdog + from src.plugin_system.plugin_runtime import process_exists + heartbeat = display_watchdog.read_heartbeat(display_watchdog.HEARTBEAT_PATH) + if heartbeat is None: + return True + age = display_watchdog.heartbeat_age(heartbeat) + if age is None or age >= display_watchdog.HEARTBEAT_STALE_SECONDS: + return True + pid = heartbeat.get('pid') + if isinstance(pid, int) and not isinstance(pid, bool) and process_exists(pid) is False: + return True + return False + + def loop_heartbeat_age(snapshot: Optional[Dict[str, Any]]) -> Optional[float]: """The render loop's heartbeat age now; None when the display has no beat to report yet (or there is no snapshot).""" @@ -141,5 +174,5 @@ def loop_heartbeat_age(snapshot: Optional[Dict[str, Any]]) -> Optional[float]: return control_client.snapshot_loop_age(snapshot) -__all__ = ['current_status', 'loop_heartbeat_age', 'on_demand_state', 'read_state', - 'stop_subscription'] +__all__ = ['current_status', 'display_gone', 'loop_heartbeat_age', 'on_demand_state', + 'read_state', 'stop_subscription']