From 55020d9f38e1fdbf11e15ff4307ac9b9b6f32bed Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Sun, 21 Jun 2026 13:55:57 +0200 Subject: [PATCH] Fix COOL->HOLD undershoot caused by stale pid_rate windup process_pid() was advancing pid_rate and pid_cool unconditionally on every tick regardless of which one actually drove y. While COOL had pid_cool driving, pid_rate kept silently integrating against the same (very negative) heatrate_err with no way to act on it, building a large stale windup. The moment the FSM returned to HOLD (which only reset pid_hold), y read straight from that saturated pid_rate instead of a fresh value, so the heater stayed off well past the target before the windup finally unwound - an undershoot of about 1 degree in scripts/demos/pid/demo_temp_controller.py's last (cooling) step. Now only the PID currently driving y is process()'d each tick, and pid_rate is also reset on IDLE->HOLD and COOL->HOLD (it was already reset on the transitions into HEAT/COOL). Undershoot drops to ~0.06 degrees, in line with normal PID settling. --- components/pid/temp_controller_base.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/components/pid/temp_controller_base.py b/components/pid/temp_controller_base.py index 6a64db1..192848c 100644 --- a/components/pid/temp_controller_base.py +++ b/components/pid/temp_controller_base.py @@ -105,9 +105,12 @@ class TempControllerBase(APid): elif self.state == States.IDLE: # Just (re-)enabled - land in HOLD; the very next tick's # threshold check (below) moves it on to HEAT/COOL if the gap - # actually warrants it. + # actually warrants it. pid_rate was frozen (see process_pid()) + # and possibly stale for as long as we were disabled - start it + # clean rather than resuming wherever it last left off. state_next = States.HOLD self.pid_hold.reset() + self.pid_rate.reset() elif self.state == States.HOLD: if diff >= self.thresholds['HoldHeat']: state_next = States.HEAT @@ -129,6 +132,11 @@ class TempControllerBase(APid): elif diff >= -self.thresholds['CoolHold']: state_next = States.HOLD self.pid_hold.reset() + # pid_rate was frozen during COOL (see process_pid()) - + # resume it clean rather than from whatever it last held + # before COOL took over, which by now may be a stale fit + # for a completely different part of the curve. + self.pid_rate.reset() if state_next != self.state: self.state = state_next @@ -136,14 +144,19 @@ class TempControllerBase(APid): def process_pid(self, theta_err, heatrate_err, hold_scale=1.0): self.pid_hold.process(theta_err, -self.theta_ist, hold_scale) - self.pid_rate.process(heatrate_err, -self.heatrate_ist) - self.pid_cool.process(heatrate_err, -self.heatrate_ist) + # Only the PID actually driving y is advanced - otherwise the + # inactive one (e.g. pid_rate while COOL has pid_cool driving) + # would keep silently integrating against a heatrate_err that + # isn't actually under its control, building a stale windup that + # causes a discontinuity in y the moment it takes back over. if self.state == States.IDLE: self.y = 0 elif self.state == States.COOL: + self.pid_cool.process(heatrate_err, -self.heatrate_ist) self.y = self.pid_cool.get_y() else: + self.pid_rate.process(heatrate_err, -self.heatrate_ist) self.y = self.pid_rate.get_y() self.post_pid()