Pid configurable thresholds #2

Merged
jens merged 11 commits from pid-configurable-thresholds into master 2026-06-19 15:03:52 +02:00
Owner
No description provided.
jens added 6 commits 2026-06-18 22:10:23 +02:00
Both controllers had nearly identical process_fsm(), process_pid(),
and all getters/setters; only Kalman/model setup and process()
genuinely differed, and the duplication had already drifted (the
Smith variant resets model state on entering HEAT, the plain one
didn't).

Add TempControllerBase with the shared logic and three small hooks
(init_kalman, on_state_entered, post_pid) subclasses use to plug in
their own Kalman/model behavior. Each subclass now contains only what
makes it different.

Also fixes a latent crash: TempController's constructor only accepted
(dt, params), but PidFactory/brewpi.py always call it with a third
model_params arg, so pid_type "Normal" would have raised TypeError.
The shared base's model_params=None default fixes this. Also
initializes the Smith controller's trace attributes in __init__
instead of leaving them undefined until the first process() call.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TempControllerBase no longer threads model_params through (only the
Smith subclass needs it, for its own Pot model and Kalman filters).

Enable the Smith predictor's actual delay-compensated error term
(theta_err now uses theta_ist_plant - theta_ist_model_delay +
theta_ist_model instead of the plain plant reading), which is the
correction this controller is named for.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PyQt5.Qwt isn't pip-installable; it only existed via the apt package
python3-pyqt5.qwt, so pip-installing client/requirements.txt's plain
PyPI PyQt5 always crashed with "No module named 'PyQt5.Qwt'".

Qwt was only used for 3 QwtSlider widgets (Slider_temp_soll,
Slider_pwr_soll, Slider_speed_soll). Swap them for stock QSlider in
brewpi.ui, regenerate brewpi_win.py via pyuic5, and update
brewpi_gui.py's setLowerBound/setUpperBound calls to QSlider's
integer-only setMinimum/setMaximum. The GUI now runs from a plain
`pip install -r client/requirements.txt` venv with no system PyQt5
packages required.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pure rename of the pyuic5-generated UI module plus its one import
site in brewpi_gui.py; regenerated from brewpi.ui, no content changes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Tracks open issues from a review that weren't fixed as part of this
branch's Smith-predictor and dedup work: non-configurable FSM
thresholds, Pid.scale()'s gain-scheduling hack, shared Kalman tuning
across the Smith controller's 3 filters, matplotlib imported at
module level in production code, no automated tests, and unchecked
config access.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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>
jens added 1 commit 2026-06-18 23:15:56 +02:00
Review of pot.py found a likely root cause for sim-vs-real control
mismatches: the heat-loss term carries the wrong units, masking
ambient cooling in simulation. Also notes the dropped sensor-noise
term, hardcoded ambient temp, and lag-vs-dead-time modeling gap.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jens added 4 commits 2026-06-19 14:43:26 +02:00
temp_controller.py, temp_controller_smith.py, and kalman.py imported
matplotlib at module level just to support eyeballed-plot __main__
blocks, coupling the live server's import graph to a GUI plotting lib
it never uses at runtime. Relocate those demos (and kalman_eval.py) to
scripts/demos/pid/ and strip the now-unused imports/__main__ blocks
from the production files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- remove HeatDiffusion
- use delay line for heat propagation modeling

[Temp Controller]
- remove Kalman
- added demo
temp_controller_smith.py still relied on Kalman filtering and Pot's old
get_temperature_intermediate(), both removed by the recent Pot/Kalman
simplification. Rebuild it using two Pot model copies (zero-delay and
delayed) and the same backward-difference heat rate as temp_controller.py,
combined into the classic Smith correction:
theta_ist = theta_model_fast + (theta_plant - theta_model_delay).

Also fix set_model_power() never being called, so the internal model
actually receives the controller's power output, and fill in
demo_temp_controller_smith.py to exercise it end-to-end with an extra
plot of plant vs. fast-model vs. delayed-model temperature.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jens merged commit e352c4ecf7 into master 2026-06-19 15:03:52 +02:00
jens deleted branch pid-configurable-thresholds 2026-06-19 15:04:00 +02:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jens/brewpi#2