mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-06 07:15:09 +00:00
fix(web): a delivered on-demand start reads as starting until the display acts on it
On ledpi (three cold starts) the display acknowledged the start as its socket opened, then took ~5 s to act on it while Vegas built its first strip; the status routes meanwhile showed the display's own idle state, so a UI polling every 700 ms flashed idle. The display's on-demand state now names the request it answers (request_id). A delivered start keeps reading as status "starting" with delivered: true, in /display/on-demand/status and as on_demand_pending in /display/current-status, until the display publishes state for that request id (a display without the field: any state newer than the delivery), for at most DELIVERED_SHOWN_SECONDS (30 s). The display's startup state, which can be published after the acknowledgement, names no request and does not end it. Tests: stays starting against the startup idle state (no id, an older id); the matching active state and the matching error take over; an older display's newer state takes over; the 30 s cap; the display's state names its request. Mutation check: 11 mutants, 11 killed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+4
-1
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
|
||||
@@ -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()):
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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})
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user