mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 14:25:08 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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):
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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']
|
||||
|
||||
Reference in New Issue
Block a user