diff --git a/components/pid/TODO.md b/components/pid/TODO.md new file mode 100644 index 0000000..f4ac028 --- /dev/null +++ b/components/pid/TODO.md @@ -0,0 +1,48 @@ +# 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.** `tc_constants.py`'s + `THRESH_HOLD_IDLE`, `THRESH_HOLD_HEAT`, `THRESH_IDLE_HEAT`, + `THRESH_IDLE_HOLD`, `THRESH_HEAT_HOLD`, `THRESH_HEAT_IDLE` are + hardcoded module-level constants shared by every controller + instance. Move them into `config.json`'s `TempCtrl` section like the + PID gains, so mash schedules can tune hysteresis without code edits. + +- [ ] **`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__`.