Files
brewpi/components/pid/TODO.md
T
jensandClaude Sonnet 4.6 4256622e9d Make FSM hysteresis thresholds configurable
THRESH_HOLD_IDLE/HOLD_HEAT/IDLE_HEAT/IDLE_HOLD/HEAT_HOLD/HEAT_IDLE in
tc_constants.py were hardcoded module-level constants shared by every
controller instance, so tuning the IDLE/HOLD/HEAT hysteresis required
a code change.

Replace them with DEFAULT_THRESHOLDS plus an optional
TempCtrl.Thresholds config section, merged at construction time in
TempControllerBase.__init__ so configs that omit the section keep
behaving exactly as before. Documented the new section in
config.json.templ; left config.json.sim without it to exercise the
default-fallback path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-06-18 22:09:39 +02:00

2.4 KiB

components/pid design backlog

Findings from a design review, not yet acted on. See git history on claude_refactor for items already fixed (Smith predictor's delay correction enabled, the two TempController classes deduplicated into temp_controller_base.py).

  • FSM thresholds aren't configurable. Moved into an optional TempCtrl.Thresholds config section (HoldIdle, HoldHeat, IdleHeat, IdleHold, HeatHold, HeatIdle), merged over DEFAULT_THRESHOLDS in tc_constants.py so existing configs without the section keep working unchanged.

  • Pid.scale() is a gain-scheduling hack, not anti-windup. pid.py's scale(k) multiplies every gain by 1/heatrate_soll_set (called from temp_controller*.py's process()), but it's framed as overshoot compensation, conflated with the real anti-windup logic a few lines below. self.k also never resets in reset(), so a stale scale factor can survive a state-transition reset. Consider moving this scheduling logic out of the generic Pid class and into the temp controller, and resetting k in reset().

  • Three Kalman filters share one tuning. In temp_controller_smith.py, kalman_model, kalman_model_delay, and kalman_plant are all constructed with the same params['Kalman'] despite tracking signals with different noise characteristics. Allow independent var_P/var_Q/var_R per filter in config.

  • matplotlib imported at module level in production code. temp_controller.py, temp_controller_smith.py, and kalman.py all from matplotlib.pyplot import ... just to support their __main__ self-test plots, which couples the live server's import graph (and its dependency list) to a GUI plotting library it never uses at runtime. Move the plotting demos into separate scripts.

  • No automated tests. The __main__ blocks (and kalman_eval.py) are manual, eyeballed-plot harnesses. Add regression tests for FSM transitions, anti-windup clamping, and Kalman convergence — this code drives a physical heater.

  • Config access is unchecked dict[key] everywhere. params['Kalman'], model_params['gain'], etc., throughout, with no schema validation at load time. We already hit this bug class once (config.json.sim's gain/Model nesting mismatch). A small schema/dataclass validation layer at config-load would surface errors immediately instead of mid-__init__.