Clear the Sud Load-rejection Error so it doesn't get replayed as stale
WsServerMultiUser.global_state persists every message ever broadcast
and replays the full accumulated state to any client that subscribes
- by design, so a client connecting fresh or reconnecting mid-run
always gets the same complete picture (see README's new "State replay
on connect" section). That mechanism has no concept of "transient":
the {'Sud': {'Error': ...}} added for Load-rejection got treated as
durable state just like everything else, so any client connecting
after the fact - even one that never touched Load - got the stale
error replayed on connect. Confirmed live before this fix.
tasks/sud.py: send {'Sud': {'Error': None}} immediately after the
error itself, clearing it back to neutral in global_state.
client/brewpi_gui.py: on_sud_changed() treats a falsy Error as a
no-op instead of popping an empty dialog.
README.md: documents the underlying principle (client state replay
on connect, and the corollary that one-shot events need explicit
clearing) under a new Architecture subsection, and refreshes the Sud
channel/forecast sections to match this session's other changes
(Elapsed-based dynamic forecast anchoring instead of wall-clock time
times the nominal warp factor, Load's Error reply).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LhiQe64F74uHV8jzuoSa5K
This commit is contained in:
@@ -113,6 +113,45 @@ Each component runs inside its own `ATask` at a configurable interval
|
||||
keyed WebSocket channel that any connected client can subscribe to and send
|
||||
commands back on (e.g. `{"TempCtrl": {"Soll": {"Temp": 65}}}`).
|
||||
|
||||
### State replay on connect
|
||||
|
||||
A fundamental assumption baked into this protocol: a client never knows what
|
||||
it's connecting to. The GUI might be the first thing to ever talk to a
|
||||
freshly started server, or it might connect to one that's been running
|
||||
unattended for hours, mid-brew, with a schedule already loaded and paused
|
||||
partway through step 4. A client can also disconnect (network hiccup, the
|
||||
user closing and reopening the GUI) and reconnect later, at any server state,
|
||||
with no special "resume" handshake. In every one of these cases, the client
|
||||
must end up with the *same* full picture of current state as one that's been
|
||||
connected the whole time - not just whatever changes happen after it joins.
|
||||
|
||||
This is what `WsServerMultiUser.global_state` (`ws/server/ws_server_multi_user.py`)
|
||||
exists for: every message ever broadcast over the dispatcher is merged into
|
||||
it (`ws/user.py`'s `update()`), keyed exactly like the live channels are.
|
||||
Subscribing to a channel (`{"+": "Sud"}`, sent automatically for every
|
||||
channel on connect - see `MessageDispatcherSync(auto_subscribe=True)`)
|
||||
doesn't just start a future stream of changes; the server immediately
|
||||
replays the *entire* currently-known state for that channel in one message,
|
||||
via `User.send()`'s `dpath` search over `global_state`. A reconnecting
|
||||
client gets exactly the same replay a brand-new one would, because the
|
||||
server doesn't distinguish between the two cases at all - there's no
|
||||
"first connect" special-casing anywhere in this path.
|
||||
|
||||
The corollary, easy to miss: this mechanism has no concept of "transient" -
|
||||
anything sent through it is implicitly durable state that will be replayed
|
||||
to whoever connects next, even long after the fact. A one-shot *event*
|
||||
(something that happened, rather than something that's currently true) has
|
||||
to be modeled as a value that gets explicitly cleared back to neutral right
|
||||
after sending, or it will linger in `global_state` and get handed to some
|
||||
unrelated future client as if it just happened to them. `tasks/sud.py`'s
|
||||
`SudTask.recv()` hit this directly: a `Load` rejected because a run is in
|
||||
progress sends `{"Sud": {"Error": "..."}}` to explain why, immediately
|
||||
followed by `{"Sud": {"Error": None}}` to clear it - without that second
|
||||
send, every client connecting afterwards, no matter how much later or how
|
||||
unrelated to the rejected `Load`, would immediately see that same stale
|
||||
error on connect. The client mirrors this by treating a falsy `Error` as a
|
||||
no-op rather than an empty dialog.
|
||||
|
||||
## Requirements
|
||||
|
||||
- Python 3.8+
|
||||
@@ -217,12 +256,17 @@ controller's `theta_soll`/`heatrate_soll` toward each step's target
|
||||
tolerance), counts down hold steps' `duration`, and applies each step's
|
||||
`stirrer` block (`interval_time`/
|
||||
`on_ratio` map directly onto the stirrer's cycle time/duty cycle). It
|
||||
exposes progress (current step, remaining hold time, state, any
|
||||
`user_message`) on the `"Sud"` WebSocket channel and accepts
|
||||
exposes progress (current step, remaining hold time, elapsed run time -
|
||||
tick-counted, not derived from wall-clock time times the warp factor, since
|
||||
the latter is only nominal and drifts under real scheduling load - state,
|
||||
any `user_message`) on the `"Sud"` WebSocket channel and accepts
|
||||
`{"Sud": {"Start": true}}` to begin the schedule (also restarts one that's
|
||||
already finished), `{"Sud": {"Pause": true}}` to freeze progress without
|
||||
losing it, and `{"Sud": {"Confirm": true}}` to acknowledge a pause and move
|
||||
to the next step.
|
||||
losing it, `{"Sud": {"Confirm": true}}` to acknowledge a pause and move to
|
||||
the next step, and `{"Sud": {"Load": <sud.json contents>}}` to replace the
|
||||
running schedule - refused (with an explicit, one-shot `Error` reply - see
|
||||
"State replay on connect" above) while a run is already in progress, rather
|
||||
than the client trying to guess that from its own view of the state.
|
||||
|
||||
The temperature controller has a master enabled switch (off by default -
|
||||
see `components/pid/temp_controller_base.py`): `SudTask` enables it for as
|
||||
@@ -249,8 +293,10 @@ The GUI's Automatic tab shows up to three time estimates for a schedule:
|
||||
second since it's pure CPU-bound iteration, no real time/IO involved. This
|
||||
replaces the naive preview a moment after a schedule is loaded.
|
||||
- Once running, a *dynamic* one: the already-elapsed part is the actual
|
||||
measured trace, and only the remaining steps are re-projected from the
|
||||
live temperature/step/`hold_remaining` each tick.
|
||||
measured trace, plotted against `Sud.elapsed` (see "State replay on
|
||||
connect" above) rather than wall-clock time, and only the remaining steps
|
||||
are re-projected from the live temperature/step/`hold_remaining` each
|
||||
tick.
|
||||
|
||||
The naive estimate is structurally optimistic - it assumes the plant
|
||||
instantly tracks the declared rate and that "reached" is instant once the
|
||||
@@ -265,7 +311,14 @@ finishes - if the two diverge by far more than that, suspect a real control
|
||||
issue rather than a forecasting one (see the `HoldCool` threshold note in
|
||||
`components/pid/temp_controller_base.py` - too tight a threshold there once
|
||||
caused exactly this kind of large, otherwise-unexplained gap, by making the
|
||||
controller chatter in and out of `COOL` and never actually converge).
|
||||
controller chatter in and out of `COOL` and never actually converge). Rule
|
||||
out a *display* artifact first, though: the dynamic trace used to position
|
||||
itself using wall-clock time times the configured `sim_warp_factor`, but
|
||||
that factor is only nominal - real asyncio/Python scheduling overhead means
|
||||
the actually-achieved speedup runs measurably below it (confirmed: ~80x
|
||||
instead of a configured 100x), which alone produced a divergence large
|
||||
enough to look like a control problem. `Sud.elapsed` (tick-counted, immune
|
||||
to wall-clock pacing) replaced that reconstruction for exactly this reason.
|
||||
|
||||
## Logging & analysis
|
||||
|
||||
|
||||
@@ -880,7 +880,13 @@ class Window(QtWidgets.QMainWindow, Ui_MainWindow):
|
||||
elif "Forecast" in key:
|
||||
self.on_sud_forecast_received(msg['Forecast'])
|
||||
elif "Error" in key:
|
||||
QtWidgets.QMessageBox.warning(self, "Sud", msg['Error'])
|
||||
# The server clears this back to None right after sending
|
||||
# it (see tasks/sud.py's recv()) so it doesn't linger in
|
||||
# global_state and get replayed as stale to a client
|
||||
# connecting later - the clearing message itself must be
|
||||
# a no-op here, not an empty dialog.
|
||||
if msg['Error']:
|
||||
QtWidgets.QMessageBox.warning(self, "Sud", msg['Error'])
|
||||
self.update_sud_actions()
|
||||
|
||||
def on_sud_forecast_received(self, forecast):
|
||||
|
||||
@@ -202,7 +202,17 @@ class SudTask(ATask):
|
||||
# silently dropping the request. The client has no
|
||||
# business pre-emptively guessing this itself from its
|
||||
# own (replicated, laggy) view of the state.
|
||||
#
|
||||
# Immediately cleared back to None - unlike every other
|
||||
# field here, this is a one-shot event, not state. The
|
||||
# dispatcher has no concept of "don't persist this into
|
||||
# global_state" (see ws/user.py's update()), so without
|
||||
# clearing it, any client connecting later - even one
|
||||
# that never touched Load - would get this stale error
|
||||
# replayed on connect, with nothing it just did to
|
||||
# explain why.
|
||||
await self.send({'Error': 'Cannot load a new schedule while a run is in progress - stop it first.'})
|
||||
await self.send({'Error': None})
|
||||
|
||||
async def send(self, data):
|
||||
await self.msg_handler.send(data)
|
||||
|
||||
Reference in New Issue
Block a user