diff --git a/CHANGELOG.md b/CHANGELOG.md index 46b42fe9..83cc0df2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,10 @@ to for one release are gone. process (`web_interface/on_demand_dispatch.py`) sends the request until the display acknowledges it, for up to 45 s (10 s for a service that is running but has no socket yet). `GET /display/on-demand/status` reports it - (`starting`, then the display's state, or `error` / `start-timeout`), and + (`starting`; still `starting` with `delivered: true` after the display + acknowledges it, until the display publishes the state for that + `request_id`, now part of its on-demand state, for at most 30 s; then the + display's state, or `error` / `start-timeout`), and `/display/current-status` adds `on_demand_pending`. A newer start replaces a pending one and a stop cancels it (`cancelled_request_id`). The web UI and the MQTT bridge treat `202` as taken. With the service stopped and diff --git a/docs/IPC_CONTROL_SOCKET.md b/docs/IPC_CONTROL_SOCKET.md index 40458ecb..c3679e6f 100644 --- a/docs/IPC_CONTROL_SOCKET.md +++ b/docs/IPC_CONTROL_SOCKET.md @@ -220,7 +220,8 @@ socket. }} ``` -- `display` and `on_demand` are the dicts the cache keys hold, `plugins` is +- `display` and `on_demand` are the dicts the cache keys hold (`on_demand` + includes `request_id`, the request it answers), `plugins` is the runtime snapshot (`build_runtime_snapshot`), and `brightness` is the configured level, what the panel shows now, and whether the dim schedule has it dimmed. A section not published yet is `null`. @@ -499,8 +500,18 @@ The outcome is reported where clients already look: (`source: "web"`, `status: "starting"`, or `status: "error"` with `error: "start-timeout"` or the socket's reason) until the display publishes something newer, and `GET /display/current-status` adds it as -`on_demand_pending`. Once the display has taken the request its own state -is reported, as for any start. +`on_demand_pending`. + +A delivered start keeps reading as `status: "starting"`, now with +`delivered: true`, until the display publishes the state that answers it. +The display acknowledges a start as soon as its socket opens, but its run +loop acts on it only after the first screen is built (about 5 s on ledpi, +while Vegas renders its first strip), and meanwhile it publishes its own +idle state. The display's on-demand state names the request it answers +(`request_id`), so "answers it" means the id matches. A display older than +that field answers with any state published after the delivery. Either +way the delivered start is reported for at most 30 s +(`DELIVERED_SHOWN_SECONDS`). Brightness and plugin reload never had a mailbox: without the socket, the config watcher applies the saved brightness and a reload becomes the diff --git a/docs/REST_API_REFERENCE.md b/docs/REST_API_REFERENCE.md index 83854c05..c3b4a6ab 100644 --- a/docs/REST_API_REFERENCE.md +++ b/docs/REST_API_REFERENCE.md @@ -510,7 +510,9 @@ with HTTP `202`. The web process sends the request until the display takes it, for up to `wait_seconds` (45 after a cold start, 10 when the service was already running). Follow it with `GET /api/v3/display/on-demand/status`: its `state` is `{status: "starting", source: "web", request_id, ...}` while -it waits, the display's own state once delivered, or `{status: "error", +it waits, and still `starting` with `delivered: true` once the display has +acknowledged it but not yet published the state for that `request_id` (at +most 30 s), then the display's own state, or `{status: "error", error: "start-timeout"}` (or the socket's reason) if it never was; `GET /api/v3/display/current-status` carries the same as `on_demand_pending`. A newer start replaces a pending one; a stop cancels it. diff --git a/src/display_controller.py b/src/display_controller.py index 40aeddc3..45e83ba3 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -1791,6 +1791,9 @@ class DisplayController: 'status': self.on_demand_status, 'error': self.on_demand_last_error, 'last_event': self.on_demand_last_event, + # The request this state answers: lets the web interface tell + # the outcome of a start it delivered from an older state. + 'request_id': self.on_demand_request_id, 'remaining': self._get_on_demand_remaining(), 'last_updated': time.time() } diff --git a/test/test_api_v3_on_demand_socket.py b/test/test_api_v3_on_demand_socket.py index ae4c66e8..ca6c5886 100644 --- a/test/test_api_v3_on_demand_socket.py +++ b/test/test_api_v3_on_demand_socket.py @@ -15,6 +15,7 @@ runs a real server on a temp socket (Linux/macOS only). import os import sys import time +import types from pathlib import Path from unittest.mock import patch @@ -294,6 +295,86 @@ class TestNoDisplayListening: status = api_v3_client.get("/api/v3/display/on-demand/status").get_json()["data"] assert status["state"]["status"] == "idle" and status["source"] != "web" + # -- delivered, but the display has not acted on it yet ---------------- + # The display acknowledges a start when its socket opens and acts on it + # seconds later (Vegas builds its first strip first: ~5 s on ledpi); + # meanwhile it publishes its own idle state, which must not flash. + + def _delivered(self, api_v3_client, service): + with patch(f"{CLIENT}.on_demand_start", + side_effect=_attempts(_no_socket(), "ack")): + data = api_v3_client.post(START_URL, json={"plugin_id": "weather"}) .get_json()["data"] + outcome = self._outcome() + assert outcome["status"] == "delivered" + return data["request_id"], outcome["last_updated"] + + @staticmethod + def _status(api_v3_client): + return api_v3_client.get("/api/v3/display/on-demand/status").get_json()["data"] + + def test_a_delivered_start_stays_starting_until_the_display_answers( + self, api_v3_client, service): + rid, delivered_at = self._delivered(api_v3_client, service) + # The display's startup state: idle, published after the ack, for + # no request (or an older one). + for older in (None, "an-older-request"): + service["cache"].get.return_value = { + "active": False, "status": "idle", "request_id": older, + "last_updated": delivered_at + 1} + data = self._status(api_v3_client) + assert data["source"] == "web", older + assert data["state"]["status"] == "starting" + assert data["state"]["delivered"] is True + assert data["state"]["request_id"] == rid + current = api_v3_client.get("/api/v3/display/current-status").get_json()["data"] + assert current["on_demand_pending"]["delivered"] is True + + def test_the_display_state_for_the_request_takes_over(self, api_v3_client, service): + rid, delivered_at = self._delivered(api_v3_client, service) + service["cache"].get.return_value = { + "active": True, "status": "active", "plugin_id": "weather", + "request_id": rid, "last_updated": delivered_at + 5} + data = self._status(api_v3_client) + assert data["source"] == "cache" and data["state"]["status"] == "active" + current = api_v3_client.get("/api/v3/display/current-status").get_json()["data"] + assert "on_demand_pending" not in current + + def test_its_error_takes_over_too(self, api_v3_client, service): + rid, delivered_at = self._delivered(api_v3_client, service) + service["cache"].get.return_value = { + "active": False, "status": "error", "error": "load-failed", + "request_id": rid, "last_updated": delivered_at + 5} + assert self._status(api_v3_client)["state"]["error"] == "load-failed" + + def test_an_older_display_answers_with_any_newer_state(self, api_v3_client, service): + # A display before request_id was published: timestamps decide. + _, delivered_at = self._delivered(api_v3_client, service) + service["cache"].get.return_value = {"active": False, "status": "idle", + "last_updated": delivered_at - 1} + assert self._status(api_v3_client)["source"] == "web" + service["cache"].get.return_value = {"active": True, "status": "active", + "last_updated": delivered_at + 1} + assert self._status(api_v3_client)["source"] == "cache" + + def test_a_delivered_start_is_shown_for_a_limited_time( + self, api_v3_client, service, monkeypatch): + from web_interface import on_demand_dispatch + _, delivered_at = self._delivered(api_v3_client, service) + service["cache"].get.return_value = {"active": False, "status": "idle", + "request_id": None, + "last_updated": delivered_at + 1} + cap = on_demand_dispatch.DELIVERED_SHOWN_SECONDS + assert cap == 30.0 + clock = types.SimpleNamespace(time=lambda: delivered_at + cap - 1, + monotonic=time.monotonic, sleep=time.sleep) + monkeypatch.setattr("web_interface.blueprints.api_v3.time", clock) + assert self._status(api_v3_client)["source"] == "web" + clock.time = lambda: delivered_at + cap + 1 + data = self._status(api_v3_client) + assert data["source"] == "cache" and data["state"]["status"] == "idle" + current = api_v3_client.get("/api/v3/display/current-status").get_json()["data"] + assert "on_demand_pending" not in current + def test_a_new_start_supersedes_the_pending_one(self, api_v3_client, service): service["state"]["active"] = False with patch(f"{CLIENT}.on_demand_start", side_effect=_no_socket()): diff --git a/test/test_ipc_display_hook.py b/test/test_ipc_display_hook.py index 37ee5e7f..841fb5e2 100644 --- a/test/test_ipc_display_hook.py +++ b/test/test_ipc_display_hook.py @@ -162,6 +162,14 @@ class TestLifecycle: assert status['on_demand']['plugin_id'] == 'clock' c.encode_message(status) # it has to fit on the wire + def test_the_state_names_the_request_it_answers(self, controller): + # The web interface keeps reporting a delivered start as "starting" + # until the display publishes state for that request id. + assert controller._on_demand_state()['request_id'] is None + controller._control_server = FakeServer(_start('sock-7')) + controller._poll_on_demand_requests() + assert controller._on_demand_state()['request_id'] == 'sock-7' + def test_cleanup_closes_the_socket(self, controller): server = FakeServer() controller._control_server = server diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index ed057594..d0b443d2 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -47,17 +47,43 @@ def _dispatcher(): def _pending_start_state(): - """A start the dispatcher is still delivering, or one it gave up on: - the state the status routes report instead of the display's. None when - there is none (or it was delivered, after which the display's own - state is the truth).""" + """A start the dispatcher is still delivering, one it delivered but the + display has not acted on yet, or one it gave up on: what the status + routes report instead of the display's own state (see _shadows). None + when there is none. + + A delivered start reads as ``status: "starting"`` with ``delivered: + true`` for at most DELIVERED_SHOWN_SECONDS: the display acknowledges it + when its socket opens and acts on it seconds later, and until then + publishes its own idle state, which would flash in the UI. + """ dispatcher = on_demand_dispatch.current() status = dispatcher.status() if dispatcher is not None else None - if status is None or status.get('status') not in ('starting', 'error'): + if status is None: + return None + if status.get('status') == 'delivered': + delivered_at = status.get('last_updated') or 0 + if _pkg.time.time() - delivered_at > on_demand_dispatch.DELIVERED_SHOWN_SECONDS: + return None + return dict(status, status='starting', delivered=True, delivered_at=delivered_at) + if status.get('status') not in ('starting', 'error'): return None return status +def _display_on_demand_state(snapshot): + """The display's own on-demand state: (state, source). From the socket's + snapshot, else the cache key it also writes; state None when neither.""" + state = display_state.on_demand_state(snapshot) + if state is not None: + return state, 'socket' + # memory_ttl=0: the display service writes this key, so only the file + # is current. This process's memory tier would keep serving the first + # copy it read for the full max_age -- "active" for two minutes after + # the display had already stopped. + return _cache_manager().get('display_on_demand_state', max_age=120, memory_ttl=0), 'cache' + + def _send_on_demand(payload): """Hand an on-demand request to the display over the control socket. @@ -200,20 +226,12 @@ def get_on_demand_status(): available (``source: "socket"``), else the cache key it also writes (``source: "cache"``). """ - state = display_state.on_demand_state(display_state.read_state()) - source = 'socket' + state, source = _display_on_demand_state(display_state.read_state()) pending = _pending_start_state() - if state is None: - source = 'cache' - cache = _cache_manager() - # memory_ttl=0: the display service writes this key, so only the file - # is current. This process's memory tier would keep serving the first - # copy it read for the full max_age -- "active" for two minutes after - # the display had already stopped. - state = cache.get('display_on_demand_state', max_age=120, memory_ttl=0) if pending is not None and _shadows(pending, state): - # A start the web process is still delivering, or gave up on - # (start-timeout): newer than anything the display has said. + # A start the web process is still delivering, has delivered but + # the display has not answered yet, or gave up on (start-timeout): + # newer than anything the display has said. state, source = pending, 'web' if state is None: state = { @@ -231,11 +249,24 @@ def get_on_demand_status(): } }) def _shadows(pending, state): - """Whether the web process's pending start (or its failure) is newer - than the display's on-demand ``state``. While it is still being sent it - always is; a failure is, until the display publishes something later.""" - if pending.get('status') == 'starting' or not isinstance(state, dict): + """Whether the web process's start (or its failure) is newer than the + display's on-demand ``state``. + + * Still being sent: always. + * Delivered: until the display publishes the state that answers it. + A display that names its request (``request_id``) answers when the + id matches; its startup state, published as the socket opens and so + possibly after the acknowledgement, names no request or an older one + and does not count. An older display without the field answers with + any state published after the delivery. + * Failed: until the display publishes something later. + """ + if not isinstance(state, dict): return True + if pending.get('status') == 'starting' and not pending.get('delivered'): + return True + if pending.get('delivered') and 'request_id' in state: + return state.get('request_id') != pending.get('request_id') shown = state.get('last_updated') if not isinstance(shown, (int, float)) or isinstance(shown, bool): return True @@ -482,8 +513,9 @@ def get_current_display_status(): } data = dict(state, source=source) pending = _pending_start_state() - if pending is not None: - # An on-demand start the web process is still delivering (or gave - # up on): what the panel is about to show, or why it will not. + if pending is not None and _shadows(pending, _display_on_demand_state(snapshot)[0]): + # An on-demand start the web process is still delivering, has + # delivered but the display has not acted on yet, or gave up on: + # what the panel is about to show, or why it will not. data['on_demand_pending'] = pending return jsonify({'status': 'success', 'data': data}) diff --git a/web_interface/on_demand_dispatch.py b/web_interface/on_demand_dispatch.py index 92cfe3f3..2e0c836a 100644 --- a/web_interface/on_demand_dispatch.py +++ b/web_interface/on_demand_dispatch.py @@ -44,6 +44,14 @@ RETRY_INTERVAL = 0.5 #: reported, so a client polling every few seconds sees it. OUTCOME_SECONDS = 120.0 +#: How long a delivered start is still reported as ``starting`` while the +#: display has not published the state that answers it. The display +#: acknowledges a start when its socket opens, but its run loop acts on it +#: only after its first screen is built (about 5 s on a Pi 4 in Vegas), and +#: until then it reports its own idle state. See +#: ``web_interface/blueprints/api_v3/display.py:_pending_start_state``. +DELIVERED_SHOWN_SECONDS = 30.0 + #: ``send(payload)`` hands the request to the display (the route's #: ``_send_on_demand``) and raises ``ControlError`` when it does not take it. Sender = Callable[[Dict[str, Any]], Any]