Files
brewpi/components/TODO.md
T
jensandClaude Sonnet 4.6 2bdd3203cf Make Sud steps ramp based on the actual temperature gap, not a 'ramp' key
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
2026-06-22 09:35:53 +02:00

2.8 KiB

components/sud.py step lifecycle spec

Confirmed spec for a Sud schedule step's internal phase order (components/sud.py), from a design discussion clarifying how ramp/ hold/user-confirm interact, since implemented (gap-based ramping).

Phase order

ramp -> hold -> user-confirm. Every step ramps toward temperature first - this is not gated by whether the step has its own ramp block (_build_step() always synthesizes one from default.step.ramp, sud.py:35-52); _advance() (sud.py:192-204) always enters RAMPING. How long that actually takes depends on the real gap, decided by the temperature controller's own gap-tracking FSM (TempControllerBase.is_holding(), components/pid/temp_controller_base.py) rather than any fixed tolerance - SudTask.on_process() (tasks/sud.py:200-203) calls sud.temp_reached() once tc.is_holding() is true. set_theta_soll() recomputes the FSM eagerly (temp_controller_base.py:81-89) so is_holding() can't read one tick stale after a new target is pushed.

temperature is the one field deliberately not defaulted from default.step (sud.py:52) - every sude/*.json's default.step.temperature is inert template filler, never a real shared target. A step omitting it has no new target of its own: SudTask/ SudForecastEstimator/the demo driver all skip pushing set_theta_soll in that case (step['temperature'] is not None guard), so the controller just keeps whatever the previous step left running and the ramp resolves immediately. hold stays opt-in, gated by raw presence as before.

Duration 0

Supported for hold: hold_remaining = duration * 60.0 can be 0; tick() (sud.py:186-190) finishes the step on the next tick once hold_remaining <= 0.

End-of-step branching

_finish_step() (sud.py:212-216): no user_wait_for_continue -> advances immediately to the next step; user_wait_for_continue: true -> enters WAIT_USER, resumed only by confirm().

WAIT_USER is not "hold, prolonged": HOLDING and WAIT_USER are distinct SudState values. The hold countdown runs to completion before WAIT_USER begins - tick() only decrements hold_remaining while state == HOLDING, so the timer is frozen (not extended) during the wait. It delays the next step, but it's a separate third phase, not a longer hold.

The gating field is user_wait_for_continue, not "presence of a user message": user_message (display text) and user_wait_for_continue (the actual block) are independent fields - a step can show a message without blocking, or block silently.

Resolved

  • Pure-hold steps never (re-)set the controller's target temperature. Moot now that ramping isn't gated by the ramp key: SudTask.on_step_changed pushes set_theta_soll/set_heatrate_soll for any step that specifies its own temperature, not just ones with an explicit ramp block.