On KeyboardInterrupt, brewpi.py cancels all asyncio tasks (letting
HeaterTask's `with device.open()` finally block run), then calls
heater.close() as belt-and-suspenders before closing the event loop.
On shutdown HeaterHendi.activate(False) now sets the switch to 0 instead
of disabling remote control, so the Hendi stays in remote mode at 0 W
rather than dropping back to its local panel state. HendiCtrl.close()
does the same (setSwitch(0)) then closes the serial port.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a configurable IIR pre-filter (beta, default 0.05, τ≈19 s at dt=1 s)
applied to theta_ist before the derivative, so stirrer-induced temperature
noise is attenuated before being amplified by differentiation rather than
after. The shared filter/derivative/post-filter logic is factored into
TempControllerBase._compute_heatrate(); both Normal and Smith subclasses
call it, replacing their duplicated inline rate blocks.
beta is read from TempCtrl.beta in the config (default 0.05 if absent) and
added to config-sim.json.tpl and config-real.json.tpl.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In estimate()'s on_step_changed closure, the new 'pot = step.get("pot", {})'
local variable shadowed the outer 'pot' (Pot plant instance), causing
pot.set_plant_params() to be called on a plain dict and crash on load.
Renamed to 'step_pot' to avoid the collision.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T52JH848ojhXXHn1bAzdC3
Top-level pot_mass/pot_material/L/Td/grain_mass/water_mass and their
per-step overrides are now grouped under a 'pot' sub-object, matching
the structure already applied to sud_0010.json's parent level. Updated
all sude/*.json files and the parsing/consuming code in components/sud.py,
tasks/sud.py, components/sud_forecast.py, and scripts/demos/sud/demo_sud.py.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T52JH848ojhXXHn1bAzdC3
With TempControllerBase(APid, TempControllerFsm), APid.is_holding()
(abstract stub returning None) shadowed TempControllerFsm.is_holding()
in the MRO, so tc.is_holding() always returned None. The forecast
simulation never called sud.temp_reached() and always hit MAX_TICKS.
Swapping to TempControllerBase(TempControllerFsm, APid) puts the
concrete FSM methods before APid's abstract stubs in the MRO.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tqxrk8uj4M3w3d3eXm3xK8
States, DEFAULT_THRESHOLDS, the three PIDs, FSM state vars, process_fsm(),
on_state_entered(), set_enabled(), and is_holding() move to a new
TempControllerFsm class in temp_controller_fsm.py. TempControllerBase
inherits from both APid and TempControllerFsm; States is re-exported so
existing imports in temp_controller_smith.py are unaffected.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tqxrk8uj4M3w3d3eXm3xK8
All four name-dispatch factories repeated the same if/elif lazy-import
pattern. Replace with a ComponentFactory base class holding a callable
registry and a _lazy() helper for the common import-and-construct case.
PlantFactory is an orchestrator, not a dispatcher, and is left as-is.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tqxrk8uj4M3w3d3eXm3xK8
Same pattern as set_model_power: these methods only existed on the Smith
controller, forcing five hasattr guards at call sites. No-op stubs on the
base class give all controllers a stable interface; Smith overrides them.
Remove the now-redundant hasattr guards.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tqxrk8uj4M3w3d3eXm3xK8
The Normal controller had no set_model_power(), causing callers to
guard with hasattr() instead of relying on a stable interface. Adding a
no-op stub to the base class means all controllers have the method;
TempControllerSmith overrides it to actually update its internal model.
Remove the now-redundant hasattr guards at both call sites.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Tqxrk8uj4M3w3d3eXm3xK8
Re-enabling the temp controller landed unconditionally in HOLD and
relied on the next tick to correct it, but SudTask.on_process() reads
is_holding() synchronously in the same call chain that pushes a fresh
step's setpoint - so a restart could finish a hold-less step instantly
instead of actually ramping. Resolve the FSM against the real gap right
away instead. Also fix the status bar showing one step number below
what the Progress tab's plates show for the same step.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189FkmyJkY77HqsNomr1DwV
A glance at the Automatic tab's forecast plot couldn't show where the
brew actually stands within its own schedule - just a curve. The new
Progress tab stacks one StepPlate per step instead: an LED for its
status (off/done/active-ramping/active-holding), its resolved target
temp, masses and ramp rate, plus, for whichever step is currently
active, live actual temp and stirrer state (frozen at their last
value once that step finishes) and a forecast-based remaining-time
countdown.
The countdown needs to know where each step actually begins in the
forecast's timeline, which the server didn't track before -
SudForecastEstimator.estimate() now returns per-step start times
alongside t/theta, and SudTask threads them through send_forecast()/
_reanchor_forecast() the same way (including the same generation-guard
against the concurrent-call race) as a new 'StepStarts' field on the
'Forecast' message.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019qvu5giu7gvRCyEWzf2Vpx
Reanchoring used to only happen when a user confirmed a WAIT_USER
step, so anything the schedule advanced through on its own (a ramp
reaching target, a hold timing out) left the forecast showing a
stale, increasingly wrong prediction once reality diverged from it.
_reanchor_forecast() now fires from on_step_changed() on every real
transition, splicing in a fresh simulation anchored at the real
current temperature/elapsed time instead.
Also fixes two bugs surfaced while testing that change:
- estimate()'s early-return for an empty/not-yet-loaded schedule still
returned the old 4-tuple shape, crashing every connection before a
Sud was ever Loaded.
- send_forecast() and _reanchor_forecast() can run concurrently (e.g.
a fresh Start triggers both at once), and whichever resumed second
after its own worker-thread simulation would blindly splice its tail
onto whatever the other had already written, producing a spurious
connecting line across the plot. Both now carry a generation counter
and discard their result if a newer call has since committed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019qvu5giu7gvRCyEWzf2Vpx
heater_name/sensor_name/plant_name were three independent config keys,
each picked via its own factory - in practice sim and real hardware
are never actually mixed and matched, so this could (nonsensically)
disagree, e.g. a real heater paired with a simulated sensor.
Add PlantFactory (components/plant/plant_factory.py): plant_name alone
now builds the whole rig together. "sim" gets HeaterSim/Pot (the
modeled plant, as before)/TempSensorSim. Anything else gets
HeaterHendi/PotReal/TempSensor_max31865 - PotReal is a new, deliberately
unmodeled Pot for the real, physical kettle (components/plant/pot_real.py):
the real temperature comes straight from the real sensor, not from a
model, so it just accepts and ignores set_plant_params()/
set_ambient_temperature()/initial() and reports no temperature of its
own, satisfying PotTask/SudTask's interface with nothing to actually
simulate.
heater_name/sensor_name are gone from config.json.templ; server/brewpi.py
delegates to HeaterFactory/TempSensorFactory lazily from inside
PlantFactory, same as before, so real hardware's spidev/pyserial deps
still aren't needed just to import the module.
PotTask.on_process() only calls Pot.process() once Pot.is_configured()
- which required plant params (M/C/L/Td) that, by design, only ever
came from a Sud's own doc via SudTask.apply_plant_params(). So with no
Sud loaded (or the temp controller disabled), the simulated Pot just
sat inert: manually driving the heater never moved its temperature at
all.
EMPTY_SUD's pot_mass/pot_material defaulted to 0/None, which would
have made a zero-mass plant (a ZeroDivisionError in Pot.process()) -
they're actually the physical kettle's own properties, not really
per-recipe (every sude/*.json so far uses the same pot), so default
them to that real kettle's values instead. server/brewpi.py now seeds
the Pot with derive_plant_params() from the not-yet-loaded Sud right
away, same as ambient temperature already was - a real Sud's own doc
still overrides these the moment one is loaded.
SudForecastEstimator.estimate() was feeding tc.get_power() straight to
the plant (pot.set_power(heater_max_power * y)), bypassing the duty-
cycling across the heater's discrete power steps that the real run's
HeaterTask/device chain always applies (tasks/heater.py). That let
pid_heat's own oscillatory tendency reach the plant undamped, so the
forecast showed a violent on/off staircase that no actual brew run
ever produces - confirmed by comparing against real sud_log output,
which stays smooth because the real actuator chain duty-cycles the
same PID output down first.
estimate() now duty-cycles tc.get_power() across the heater's own
power steps (mirroring HeaterTask's PWM logic, duplicated rather than
imported to keep components/ from depending on tasks/) before handing
it to the plant, and also feeds the resulting discretized power back
into the Smith predictor's internal model via set_model_power(), same
as the real wiring in brewpi.py does. The constructor takes the
heater's get_powers() list and the configured sim_warp_factor (needed
to convert HeaterTask's 10-real-second PWM period into simulated
ticks) instead of a bare max power.
last_theta_ist was hardcoded to 20 in __init__, used to compute the very
first process() tick's heatrate as (theta_ist - 20)/dt*60 - a wildly bogus
spike whenever the real starting temperature wasn't actually 20 (e.g.
1200 deg/min for a 40-degree start). The exponentially-smoothed
heatrate_ist (alpha=0.1) takes several ticks to recover from that, during
which it distorts the PID's rate-error term and visibly throws off the
heating trajectory.
This was invisible everywhere this assumption was implicitly tested,
since ambient (and every start_theta used) was always exactly 20. It
surfaced once SudForecastEstimator.estimate() started being called with a
real, non-20 start_theta: it builds a fresh TempController on every call,
so its first tick always hits this bug fresh - unlike the real controller,
which only ever goes through this once near server startup and has long
since self-corrected (last_theta_ist = theta_ist runs every tick) by the
time any forecast comparison matters. That's exactly what showed up as a
forecast-vs-actual gap during the initial ramp.
last_theta_ist now starts None; process() treats the first tick's
heatrate as 0 (no real history yet) instead of computing it against a
hardcoded, usually-wrong baseline.
Verified: ambient=20 (the only case ever exercised before) is byte-
identical to before the fix. ambient=40/start=40 - reproducing the old
hardcoded-20 behavior side by side - shows the old version visibly lagging
~12 minutes behind the fixed one during the initial ramp before they
converge, matching the reported gap exactly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
send_forecast() was defaulting to a cold start at ambient (SudForecastEstimator.
estimate()'s default when start_theta isn't passed), so the forecast's t=0
never matched whatever temperature the pot actually was at - understating
how long the first ramp would really take whenever it wasn't already cold.
Anchoring at tc.get_theta_ist() seemed like the obvious fix but isn't
reliable right after Load: that's only ever updated inside process(), which
a model-based controller (Smith) doesn't run until its plant params are
configured - which now only happens once a Sud is loaded (see the
"inert until loaded" change) - so theta_ist can still be sitting at its
never-updated __init__ default of 0 the instant send_forecast() reads it.
Added TempControllerBase.get_theta_ist_set() (mirroring the existing
get_theta_soll_set()) - the raw sensor reading, updated the instant a
reading comes in regardless of whether process() has ever run - and used
that instead.
That still leaves a gap between Load and Start: the real temperature can
drift (time passing, manual heating via the Manual tab) between loading a
schedule and actually starting it, leaving the forecast anchored to a
now-stale temperature. recv()'s 'Start' handler now re-sends the forecast,
freshly anchored to the real temperature at that moment, on every fresh
start (not a Pause->resume, which keeps the already-established forecast
rather than discarding it mid-run).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
config.json.templ's component names already default to "sim" for every
backend (sensor/heater/stirrer/plant), making the separate .sim template
mostly redundant - it only differed in dt/sim_warp_factor/PID gain tuning,
which .templ now adopts directly. Removes config.json.sim, updates
brewpi.py's CLI args (-m/--logdir replaces the unused -d/--model/--sim
flags), and refreshes the README/TODO references that pointed at the now-
removed file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
Sud now parses top-level grain_mass/water_mass (sibling to pot_mass/
pot_material/L/Td, defaulting to 0/0) - what's actually in the pot before
the brew starts, as opposed to default.step's grain_mass/water_mass, which
is just inert template filler. All sude/*.json docs gain explicit values
matching what their first step already resolved to.
tasks/sud.py's apply_plant_params() now takes grain_mass/water_mass
directly instead of a step dict: on_step_changed() still passes the active
step's own (these vary as malt goes in/water boils off), but the Load
handler now reads Sud.grain_mass/water_mass directly instead of parsing
schedule[0]. The GUI's status-line mass preview (shown immediately on
Load, before Start) does the same.
Also fixes a latent bug found along the way: _continue_forecast_after_confirm()'s
reconstructed sub-doc was missing L/Td/grain_mass/water_mass entirely,
silently falling back to generic defaults instead of the real Sud's own
values - invisible only because every current sude/*.json happens to use
those same defaults.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
server/brewpi.py no longer pre-configures the real Pot's or Smith's model's
plant params at startup - there's no longer any generic baseline at all,
since the only source of truth is now a loaded Sud's own doc (applied via
tasks/sud.py's SudTask.apply_plant_params(), on Load and on every step).
Ambient temperature and PID gains stay configured at startup, since
they're independent of any Sud (global setting / hardware tuning, not
doc-derived).
Add Pot.is_configured() (already existed)/TempControllerBase.is_configured()/
TempController(Smith).is_configured(), and have tasks/pot.py's PotTask and
tasks/tempctrl.py's TcTask skip process() while not yet configured instead
of letting it raise - so plant and temp control are genuinely inert (not
crashing) until a Sud is loaded. "Normal" has no plant-model dependency,
so it stays active regardless.
Also refreshes README.md: removes stale tracer.py/.mat references (already
removed in an earlier commit), documents SudLogTask's JSON logs, the doc's
L/Td fields, and this inert-until-loaded behavior.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
sud.start() synchronously fires the first step's on_step_changed callback
before it even returns (assigning Sud.step triggers callbacks immediately),
which sets M/C/L/Td from this doc's own derive_plant_params() before any
pot.process()/tc.process() tick ever runs - so the constructor's
plant_params was being set, then immediately discarded, unused. theta_amb
stays, since it's not part of the Sud doc at all.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
Sud now parses top-level L/Td fields from sude/*.json (defaulting to 0.2/30,
matching the old hardcoded server constants), and derive_plant_params()
returns them alongside M/C - constant for the whole brew, unlike M/C which
vary per step with grain/water mass. All sude/*.json docs gain explicit
L/Td fields.
Callers (tasks/sud.py, SudForecastEstimator, demo_sud.py) switch from the
narrow set_thermal_params(M, C)/set_model_params(M, C) to the full
set_plant_params(params)/set_model_plant_params(params), since
derive_plant_params() now always returns all four keys; both narrow setters
are removed as dead code.
Caught along the way: Pot.set_plant_params() unconditionally rebuilt the
Delay ring buffer, which was harmless when only ever called once at
construction - but now running on every Sud step change, it was wiping
in-flight delayed power at each step boundary. Fixed to only rebuild when
Td actually changes; verified this restores the exact original forecast
result for sud_0010.json.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
TempController(Smith).__init__ no longer takes model_params/theta_amb -
set_model_plant_params() and the existing set_ambient_temperature() must be
called instead (mirrors components/plant/pot.py's own Pot constructor
refactor). temp_controller.py's now-pointless model_params/theta_amb
constructor placeholders (kept only for "compatibility" with Smith) are
dropped too.
Both TempController variants now raise a clear RuntimeError from process()
itself if set_params() wasn't called, instead of letting it surface deep
inside Pid.process() as an opaque "'NoneType' object is not subscriptable".
Smith additionally checks its internal models via Pot.is_configured() (new)
and raises if set_model_plant_params()/set_ambient_temperature() weren't
called either.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
Mirrors components/pid/pid.py's own Pid.set_params() pattern: params/
thresholds start out None, and set_params() configures pid_hold/pid_heat/
pid_cool from them. process()/process_fsm() fail naturally (TypeError on
None) if called before set_params() - same as Pid.process() already does -
rather than a new bespoke check. Updates all callers (server/brewpi.py,
SudForecastEstimator, and the pid/sud demo scripts) to call set_params()
right after construction.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
Pot.__init__ now only takes dt; set_plant_params() and set_ambient_temperature()
must be called before process(), which now raises RuntimeError if either is
missing instead of silently computing on whatever the constructor happened to
default to. set_ambient_temperature() only seeds the plant's starting
temperature the first time it's called, so changing ambient mid-run still
can't clobber an in-progress simulated temperature. Updates all callers
(server/brewpi.py, TempController(Smith)'s two internal models,
SudForecastEstimator, and the plant/pid/sud demo scripts) to call the setters
right after construction.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
A user_wait_for_continue step used to stop SudForecastEstimator.estimate()
dead, so the GUI's forecast plot only ever showed the first segment on Load.
Model the wait as a zero-delay auto-confirm instead, so the whole schedule's
projected curve is visible right away; correct that assumption piecewise as
real confirmations actually happen, anchored at the real elapsed time and
temperature.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk
Previously, the dynamic forecast's projected remainder was fully
recomputed on every step change, and a step requiring user
confirmation was simulated with zero added delay ("count it as
instant for estimation purposes"). Both undercut the actual goal of
the forecast: an honest comparison between real control behavior and
a simulation-based prediction. A forecast that keeps re-anchoring
itself to match reality isn't a useful baseline to compare reality
against, and assuming zero confirmation delay quietly understates the
schedule.
components/sud_forecast.py: SudForecastEstimator.estimate() now stops
simulating at the first step requiring user confirmation instead of
auto-confirming, returning the final SudState alongside the data so
the caller knows whether it stopped there or actually finished.
tasks/sud.py: SudTask now accumulates the forecast across piecewise
segments (forecast_t/forecast_theta). send_forecast() computes only
the first segment, at Load/Save. The real Confirm handler triggers
_continue_forecast_after_confirm(), which computes the next segment
anchored at the real elapsed time and real current temperature -
bridging the unforecastable wait with a flat segment rather than
guessing at its length - and sends the updated, stitched Forecast
(now carrying a Finished flag). The old per-step-change
RemainingForecast recompute is gone entirely, along with
remaining_schedule()/send_remaining_forecast().
client/brewpi_gui.py: SudForecastPlot redesigned around this - the
dashed line is the fixed, piecewise forecast (locked axes, untouched
except when a genuinely new segment arrives via show_forecast());
the solid line is purely the actual measured trace
(show_dynamic(), now only touching that). extend_forecast_while_
waiting() repeats the forecast's last value while the real Sud sits
in WAIT_USER, so the dashed line doesn't just stop, then snaps to the
new segment the moment the real confirmation appends one.
Verified via a raw-protocol test (segment boundaries, the Finished
flag, and real-time stitching across a confirm delay) and visually in
the GUI (locked dashed forecast, "X min so far total" title while
incomplete, solid/dashed comparison rendering with realistic
convergence/divergence).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LhiQe64F74uHV8jzuoSa5K
Window._elapsed_min() (client/brewpi_gui.py) reconstructed the dynamic
forecast's x-axis from wall-clock time times the server's nominal
configured sim_warp_factor. But that warp factor is only nominal -
real asyncio/Python scheduling overhead across the 7 concurrent tasks
means the actually-achieved speedup runs measurably below it (~80x
instead of 100x, confirmed by timing HoldRemaining countdowns against
real elapsed time), especially under heavy message/print load. Since
the GUI's reconstruction assumed a precise, constant mapping, the live
measured trace visibly drifted behind the forecast - most visible on
short, transition-dense test schedules where the timing slip is a
large fraction of the total duration.
components/sud.py: Sud now tracks 'elapsed' (tick-counted simulated
seconds since the run started, frozen while PAUSED/IDLE/DONE, reset on
start()) - the ground truth for run progress, immune to real-world
pacing jitter. tasks/sud.py broadcasts it as {'Sud': {'Elapsed': ...}}.
client/brewpi_gui.py: replaced sud_start_time/sud_paused_total/
sud_pause_started_at and all the wall-clock reconstruction in
_elapsed_min() with sud_elapsed_seconds, set directly from the
server's 'Elapsed' broadcasts - simpler too, since pause-freezing is
now handled server-side rather than needing separate client-side
bookkeeping.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LhiQe64F74uHV8jzuoSa5K
SudForecastEstimator.estimate() builds a fresh Pot/temperature
controller per call and never set its initial theta_soll, leaving it
at TempControllerBase's default of 0. For the upfront, full-schedule
Forecast this was harmless - the first real step always carries its
own 'temperature', overwriting it instantly. But the dynamic
RemainingForecast is recomputed from wherever the live run currently
is, and since ramping is no longer gated by a 'ramp' key, the
remaining schedule's first step often has no 'temperature' of its
own (e.g. resuming from a WAIT_USER pause into a hold-only step) - a
fresh tc has no persisted target to inherit it from like the real
run's tc does, so it span chasing 0 degrees, hit MAX_TICKS, and
produced a ~200,000-point result instead of the expected ~10,000.
That oversized payload (~6MB) is what was actually tripping
websockets' default 1MB max_size and disconnecting clients with
"1009 (message too big)" - not the GUI threading issue fixed
separately. Verified live: the same remaining-schedule case that
previously hit MAX_TICKS (200001 points) now resolves in 8348, and a
full driven run (load, start, auto-confirm both WAIT_USER steps,
completion) produces no oversized messages.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LhiQe64F74uHV8jzuoSa5K
Previously, whether a schedule step ramped at all was decided purely by
the presence of a 'ramp' key (components/sud.py's _advance()) - a
hold-only step jumped straight into its hold countdown, assuming the
plant was already at temperature. Now every step ramps toward
'temperature' first, for as long as the controller's own gap-tracking
FSM (TempControllerBase, already distinguishing HEAT/COOL/HOLD) says
the gap actually warrants it - via the new is_holding() (on APid and
TempControllerBase), which replaces a separate, redundant
TEMP_REACHED_TOLERANCE constant duplicated across tasks/sud.py,
components/sud_forecast.py, and scripts/demos/sud/demo_sud.py.
set_theta_soll() now recomputes the FSM eagerly so is_holding() can't
read stale HOLD for a tick after a much-further-away target is pushed.
components/sud.py's _build_step() always synthesizes a 'ramp' block
from default.step.ramp (so 'rate' is always available), but leaves
'temperature' undefaulted - every real sude/*.json's
default.step.temperature is inert template filler, and defaulting it
would send hold-only steps chasing 0 degrees. SudTask/
SudForecastEstimator/the demo only push a new theta_soll/heatrate_soll
when a step actually specifies its own temperature; otherwise the
controller keeps whatever the previous step left running.
Also fixes a related crash this surfaced: SudTask.remaining_schedule()'s
synthetic mid-hold step dropped 'ramp' to signal "don't re-ramp" - now
that every step always carries a 'ramp' dict, dropping it left 'rate'
missing the moment the synthetic step was re-resolved by
SudForecastEstimator. Drops 'temperature' instead, which is what
actually signals "no new target" under the new model.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LhiQe64F74uHV8jzuoSa5K
Captures the ramp/hold/user-confirm phase order and a couple of
non-obvious clarifications from a design discussion (ramp is gated
purely by the schedule's 'ramp' key, not actual-vs-target temp;
WAIT_USER is a distinct phase, not an extended hold), plus one open
gap: pure-hold steps never re-set the controller's target temperature.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LhiQe64F74uHV8jzuoSa5K
New components/sud_forecast.py: SudForecastEstimator simulates a whole
schedule with the same kind of plant/controller (and the same
configured params - ambient, plant params, pid_type, TempCtrl gains,
heater max power) the server uses for real, by driving a throwaway
Sud/Pot/controller trio through it exactly as tasks/sud.py's SudTask
would. It's pure CPU-bound iteration (no real time/IO), so a multi-
hour brew simulates in well under a second.
tasks/sud.py: SudTask now takes an optional forecast_estimator and
sends its result ({'Forecast': {'T': ..., 'Theta': ...}}) on the Sud
channel after every successful Save/Load, run in a worker thread via
run_in_executor so the ~100-500ms simulation doesn't stall the other
tasks. server/brewpi.py constructs one with the real server's config
and wires it in; AmbientTemp changes update it too.
client/brewpi_gui.py: the quick naive show_schedule() estimate (drawn
immediately on Load, before the server's simulation finishes) is now
superseded by show_precomputed_course() once the Forecast message
arrives - only while not actively running, so it doesn't fight the
dynamic re-anchored view.
Verified: matches a standalone run of the estimator (177 min for
sude/sud_0010.json) and replaces the old naive 164 min estimate live
within a couple seconds of loading.
HoldCool=0.1 was inherited from the pre-COOL-state era, where it meant
"eagerly give up active holding and coast" - harmless, since coasting
had no side effects. COOL is a real, actively-controlled state now
(its own pid_cool, reset on every entry), so a threshold this tight
relative to a real heater's discrete power steps (or even just sensor
noise) caused the system to flip in and out of COOL nearly every tick
once resting near a setpoint - resetting both pid_hold's and
pid_cool's integrators each time and never letting either actually
converge.
Confirmed via a full multi-task simulation (real Pot/Sensor/Heater/
TempCtrl tasks, not a simplified loop) of sude/sud_0010.json: 44+
TempCtrl state changes and the run not settling within hours at
HoldCool=0.1, dropping to 8 state changes and a clean 186.5 min finish
at HoldCool=1.0 (matching the other thresholds). This was the dominant
cause of real brews taking far longer than their nominal schedule
duration - not just a forecasting inaccuracy, an actual control-
quality bug.
start() only allowed a fresh run from IDLE, so once a brew reached
DONE, clicking Start again (the GUI correctly re-enables it once not
running) was silently a no-op. DONE now restarts the same way IDLE
does.
process_pid() was advancing pid_rate and pid_cool unconditionally on
every tick regardless of which one actually drove y. While COOL had
pid_cool driving, pid_rate kept silently integrating against the same
(very negative) heatrate_err with no way to act on it, building a
large stale windup. The moment the FSM returned to HOLD (which only
reset pid_hold), y read straight from that saturated pid_rate instead
of a fresh value, so the heater stayed off well past the target
before the windup finally unwound - an undershoot of about 1 degree
in scripts/demos/pid/demo_temp_controller.py's last (cooling) step.
Now only the PID currently driving y is process()'d each tick, and
pid_rate is also reset on IDLE->HOLD and COOL->HOLD (it was already
reset on the transitions into HEAT/COOL). Undershoot drops to ~0.06
degrees, in line with normal PID settling.
TempControllerBase.cooling/set_cooling() forced y=0 in advance of a
Sud ramp-down step; redundant now that the FSM's own COOL state
detects and handles a negative diff itself. Removed from the
controller, SudTask's anticipatory detection (and the Step message's
now-unused "Cooling" field), and demo_sud.py's mirror of it.
gui: the "Cool down" status-bar indicator now reflects the
controller's actual State ("States.COOL") via the TempCtrl channel,
rather than Sud's removed anticipatory flag - renamed
sud_cooling/set_sud_cooling to tc_cooling/set_tc_cooling to match.
IDLE conflated two unrelated things: "controller disabled" and "needs
to cool, which this heat-only actuator fakes via zero output." Split
them apart:
- IDLE now means disabled only - the FSM forces it whenever
enabled=False, regardless of the temperature gap, and process_pid()
zeroes y for it same as before.
- New COOL state (mirroring HEAT) takes over the negative-diff case,
with its own pid_cool (separate "Cool" gains, alongside Hold/Heat)
instead of reusing pid_rate. Its output can legitimately be negative
- it's the actuator (tasks/heater.py's actor(), already max(0, ...))
that clamps it to 0 because *this* plant (Pot) can only heat. A
plant with real cooling capability could one day honor it directly.
- Thresholds renamed accordingly (HoldIdle->HoldCool, HeatIdle->
HeatCool, new CoolHold/CoolHeat); config.json/.sim/.templ and the
hand-rolled ctrl_params in scripts/demos/pid/ and demo_sud.py
updated with a "Cool" params section (mirrors "Heat" for now, since
there's no real cooling actuator to tune against yet).
Also fixes a regression in demo_sud.py/the other PID demos: none of
them ever called set_enabled(True), so since enabled defaults to
False they never drove the heater at all - only caught because this
change's demo_sud.py re-run got stuck in RAMPING forever.
New TempControllerBase.enabled (default False) forces y=0 in
process_pid() when off, same mechanism as the existing cooling
override - the controller tries not to drive the heater at all.
tasks/tempctrl.py: new 'Enable' recv command and 'Enabled' broadcast
on the TempCtrl channel. tasks/sud.py: SudTask now enables the
controller on any non-IDLE/DONE Sud state and disables it (forcing
power back to 0) on IDLE/DONE, so automatic mode owns it for the
duration of a run and hands back manual control once it stops/finishes.
gui: new "Enabled" checkbox on the Manual tab's Controller group,
synced from the server; the temp/heatrate setpoint controls are
disabled whenever the controller itself is disabled.
New lineEdit_ambient next to the host/connect row - type a value and
press Enter to send {"System": {"AmbientTemp": ...}}. The server now
has a recv handler for the System channel (previously send-only):
updates the real Pot's ambient temperature and, where the controller
has an internal model (Smith), its model Pots too (new
set_ambient_temperature() on TempControllerSmith), then echoes the new
value back so the status bar label and entry itself stay in sync.
Several sude/*.json schedules can exist on disk; only one runs at a
time, chosen by a client Load. Sud no longer takes a path at all - it
starts empty (EMPTY_SUD) and the server config no longer names a
specific schedule file, removing the implicit link to sud_0010.json
in particular. Reading/writing sude/*.json is now entirely the
client's job (e.g. the GUI's Load/Save file dialogs); the server's Sud
only ever holds whatever was last Load()ed, in memory, until replaced
or the server restarts.
Updates demo_sud.py/demo_sud_save_load.py to construct an empty Sud()
and load() a schedule explicitly, matching the new constructor.
load() used to immediately persist whatever was loaded back to
self.path - the file the server was originally configured with. This
meant any Load (including "New Sud"'s empty schedule) silently
clobbered the currently-configured brew file, with no way to get it
back. load()/save() now operate purely on an in-memory copy; a client
that wants to persist a schedule does so explicitly itself (the GUI's
Save already prompts for a destination - this just removes the
hidden, automatic disk write load() was doing underneath it).
A step's target temperature applies to the whole step (it's what a
ramp ramps to and what a hold then holds at), not just its ramp phase
- promote it from a nested "ramp": {"temp": ...} to a step-level
"temperature" field, defaulted like descr/grain_mass/etc. Updates all
consumers (Sud._build_step, SudTask, the GUI's forecast estimator,
demo_sud.py) and migrates sude/sud_0010.json and the README.
jens
