brewpi/components/pid/TODO.md

components/pid design backlog

Findings from a design review, refreshed against the current state of master after the Kalman-filter removal and Smith-predictor rewrite (see git history for temp_controller.py/temp_controller_smith.py).

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

• matplotlib imported at module level in production code. temp_controller.py, temp_controller_smith.py, and kalman.py used to from matplotlib.pyplot import ... just to support their __main__ self-test plots. The plotting demos (and kalman_eval.py) moved to scripts/demos/pid/demo_*.py; production modules no longer import matplotlib.

• Three Kalman filters share one tuning. No longer applicable: temp_controller_smith.py was rewritten to drop Kalman filtering entirely (see below), so there's no shared tuning to split anymore.

• 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(). Still present.

• No automated tests. tests/components/pid/ was added once (stdlib unittest, no pytest) covering FSM transitions, anti-windup clamping, and Kalman convergence, but was removed again before the Kalman-filter removal/Smith rewrite, so none of the current temp_controller*.py behavior (backward-difference + low-pass filtered heat rate, the two-model Smith correction) has test coverage. This drives a physical heater — worth re-adding.

• set_model_power isn't defined on every controller, but brewpi.py wires it unconditionally. brewpi.py always does heater.set_on_changed("power_set", tc.set_model_power) regardless of Controller.pid_type. Only temp_controller_smith.py (the Smith predictor) defines set_model_power; temp_controller.py ("Normal") does not, so starting the server with "pid_type": "Normal" crashes at wiring time with AttributeError: 'TempController' object has no attribute 'set_model_power'. Either add a no-op set_model_power to TempControllerBase, or only wire it when the configured controller actually exposes a model to feed.

• Config access is unchecked dict[key] everywhere. params['Hold'], 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__ — and would have caught the dead TempCtrl.Kalman / Model.kn / Plant.kn keys still sitting in config.json.templ and config.json.sim (harmless today since extra keys are just ignored, but nothing reads them anymore after the Kalman/kn removal — see components/plant/TODO.md).

• kalman.py is now dead code in production. Neither temp_controller.py nor temp_controller_smith.py uses Kalman anymore; the only remaining references are scripts/demos/pid/demo_kalman.py and demo_kalman_eval.py. Decide whether to keep it as a documented standalone filtering example or delete it along with the demos.