From 54fb42180f7498cdbee0f158bd012af9eedf694d Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Sat, 3 Oct 2026 22:11:32 -0400 Subject: [PATCH] fix(web): schedule saves, restart_required, health and current-status agree with the rig - POST /config/schedule and /config/dim-schedule accept a disabled per-day schedule with every day off (config.template.json's own shape); a day that is off keeps its posted times. - POST /config/main sets restart_required from what the save changed: brightness, mode durations and plugin sections are applied live. - /health is degraded (display_loop: stopped) when the service is inactive, the socket does not answer and there is no live heartbeat. - /display/current-status no longer serves a stopped display's cached state when the socket and heartbeat both say it is gone (display_gone). Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 28 ++ docs/REST_API_REFERENCE.md | 38 ++- test/test_api_v3_status_accuracy.py | 326 +++++++++++++++++++++ test/test_state_stream_readers.py | 11 + web_interface/blueprints/api_v3/config.py | 95 +++++- web_interface/blueprints/api_v3/display.py | 17 +- web_interface/blueprints/api_v3/misc.py | 20 ++ web_interface/display_state.py | 37 ++- 8 files changed, 542 insertions(+), 30 deletions(-) create mode 100644 test/test_api_v3_status_accuracy.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 35ac9182..ca64f00a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -488,6 +488,34 @@ policies are unchanged. ### Fixes +- `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/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_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/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index 41720c96..f8cf641a 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", @@ -553,6 +622,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) @@ -1190,13 +1261,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 cbc83946..49036aeb 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -340,15 +340,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..1d041664 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,25 @@ 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. 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']