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. Fixed: removed Pid.scale()/self.k entirely. Pid.process(err, d, scale=1.0) now takes the gain multiplier as a plain per-call argument instead of mutable state, so there's nothing to go stale across a reset(). temp_controller*.py compute hold_scale = 1.0/heatrate_soll_set if heatrate_soll_set > 0 else 1.0 themselves and pass it through process_pid(theta_err, heatrate_err, hold_scale) — the overshoot-compensation scheduling logic now lives entirely in the temp controller, not the generic Pid block.

• 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'], params['Td'], 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 that used to sit unused in config.json.templ/.sim (since deleted), and the Model.gain/Plant.gain keys that did the same in config.json.sim before that file was removed entirely — Pot dropped gain entirely, 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.