From 96fe7ce90c0333150b9d5a45f6f74702454d4d92 Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Mon, 22 Jun 2026 18:31:58 +0200 Subject: [PATCH] Require TempController(Smith)'s model params/ambient via setters; add comprehensive process() checks TempController(Smith).__init__ no longer takes model_params/theta_amb - set_model_plant_params() and the existing set_ambient_temperature() must be called instead (mirrors components/plant/pot.py's own Pot constructor refactor). temp_controller.py's now-pointless model_params/theta_amb constructor placeholders (kept only for "compatibility" with Smith) are dropped too. Both TempController variants now raise a clear RuntimeError from process() itself if set_params() wasn't called, instead of letting it surface deep inside Pid.process() as an opaque "'NoneType' object is not subscriptable". Smith additionally checks its internal models via Pot.is_configured() (new) and raises if set_model_plant_params()/set_ambient_temperature() weren't called either. Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01DkkuG48uHFCGKe6dPSERFk --- components/pid/temp_controller.py | 7 +++---- components/pid/temp_controller_base.py | 10 ++++++++++ components/pid/temp_controller_smith.py | 16 +++++++++++----- components/plant/pot.py | 8 ++++++++ components/sud_forecast.py | 6 +++++- scripts/demos/pid/demo_temp_controller_smith.py | 4 +++- scripts/demos/sud/demo_sud.py | 4 +++- server/brewpi.py | 8 +++++++- 8 files changed, 50 insertions(+), 13 deletions(-) diff --git a/components/pid/temp_controller.py b/components/pid/temp_controller.py index c9d3574..986fa3d 100644 --- a/components/pid/temp_controller.py +++ b/components/pid/temp_controller.py @@ -2,16 +2,15 @@ from components.pid.temp_controller_base import TempControllerBase class TempController(TempControllerBase): - def __init__(self, dt, model_params=None, theta_amb=20): - # model_params/theta_amb are accepted (but unused) for constructor - # compatibility with TempControllerSmith - PidFactory.create() calls - # either with the same arguments; "Normal" has no internal model. + def __init__(self, dt): TempControllerBase.__init__(self, dt) self.dt = dt self.last_theta_ist = 20 self.heatrate_ist = 0 def process(self): + self._require_params() + # Process Kalman self.theta_ist = self.theta_ist_set heatrate = (self.theta_ist - self.last_theta_ist)/self.dt*60 diff --git a/components/pid/temp_controller_base.py b/components/pid/temp_controller_base.py index be64529..765dd5b 100644 --- a/components/pid/temp_controller_base.py +++ b/components/pid/temp_controller_base.py @@ -65,6 +65,16 @@ class TempControllerBase(APid): self.pid_heat.set_params(params['Heat']) self.pid_cool.set_params(params['Cool']) + def _require_params(self): + """Called by each subclass's process() before doing anything else - + without it, a missing set_params() call would otherwise only + surface as an unhelpful "'NoneType' object is not subscriptable" + buried inside Pid.process() (components/pid/pid.py), once + process_pid() below gets to it.""" + if self.params is None: + raise RuntimeError( + "{}.process(): PID gains not set - call set_params() first".format(type(self).__name__)) + def on_state_entered(self, state): pass diff --git a/components/pid/temp_controller_smith.py b/components/pid/temp_controller_smith.py index 5091945..8996efe 100755 --- a/components/pid/temp_controller_smith.py +++ b/components/pid/temp_controller_smith.py @@ -3,7 +3,7 @@ from components.pid.temp_controller_base import TempControllerBase, States class TempController(TempControllerBase): - def __init__(self, dt, model_params, theta_amb=20): + def __init__(self, dt): TempControllerBase.__init__(self, dt) self.dt = dt self.last_theta_ist = 20 @@ -12,18 +12,18 @@ class TempController(TempControllerBase): # Fast model: same plant model but with zero transport delay, used # to predict the current temperature without the dead time. self.model = Pot(dt) - self.model.set_plant_params({**model_params, 'Td': 0}) - self.model.set_ambient_temperature(theta_amb) # Delayed model: keeps the plant's assumed transport delay, so it # can be compared like-for-like against the real (delayed) measurement. self.model_delay = Pot(dt) - self.model_delay.set_plant_params(model_params) - self.model_delay.set_ambient_temperature(theta_amb) self.theta_ist_plant = 0 self.theta_ist_model = 0 self.theta_ist_model_delay = 0 + def set_model_plant_params(self, model_params): + self.model.set_plant_params({**model_params, 'Td': 0}) + self.model_delay.set_plant_params(model_params) + def set_model_power(self, power): self.model.set_power(power) self.model_delay.set_power(power) @@ -46,6 +46,12 @@ class TempController(TempControllerBase): self.model_delay.process() def process(self): + self._require_params() + if not (self.model.is_configured() and self.model_delay.is_configured()): + raise RuntimeError( + "{}.process(): model plant params and/or ambient temperature not set - " + "call set_model_plant_params() and set_ambient_temperature() first".format(type(self).__name__)) + self.theta_ist_plant = self.theta_ist_set self.theta_ist_model = self.model.get_temperature() self.theta_ist_model_delay = self.model_delay.get_temperature() diff --git a/components/plant/pot.py b/components/plant/pot.py index 09353ef..3500eba 100644 --- a/components/plant/pot.py +++ b/components/plant/pot.py @@ -57,6 +57,14 @@ class Pot(APlant): def initial(self, temp): self.temp = temp + def is_configured(self): + """Whether both set_plant_params() and set_ambient_temperature() + have been called - lets a caller driving this Pot as an internal + model (e.g. TempController(Smith)) check upfront and raise its + own comprehensive error, rather than letting process() fail on + whichever of the two happens to be missing.""" + return self.delay is not None and self.theta_amb is not None + def activate(self, enable): pass diff --git a/components/sud_forecast.py b/components/sud_forecast.py index b3dc2c8..e6672c1 100644 --- a/components/sud_forecast.py +++ b/components/sud_forecast.py @@ -65,8 +65,12 @@ class SudForecastEstimator: pot.set_plant_params(self.plant_params) pot.set_ambient_temperature(self.theta_amb) pot.initial(start_theta) - tc = PidFactory.create(self.pid_type, self.dt, self.plant_params, theta_amb=self.theta_amb) + tc = PidFactory.create(self.pid_type, self.dt) tc.set_params(self.tempctrl_params) + if hasattr(tc, 'set_model_plant_params'): + tc.set_model_plant_params(self.plant_params) + if hasattr(tc, 'set_ambient_temperature'): + tc.set_ambient_temperature(self.theta_amb) tc.set_enabled(True) tc.set_theta_ist(pot.get_temperature()) # Seed the target at start_theta - this tc is a fresh, throwaway diff --git a/scripts/demos/pid/demo_temp_controller_smith.py b/scripts/demos/pid/demo_temp_controller_smith.py index 8bf602d..d03069f 100644 --- a/scripts/demos/pid/demo_temp_controller_smith.py +++ b/scripts/demos/pid/demo_temp_controller_smith.py @@ -43,8 +43,10 @@ if __name__ == '__main__': temp_ist = 0 temp_soll = 20 heatrate_soll = 1.25 - ctrl = TempController(dt, plant_params, theta_amb) + ctrl = TempController(dt) ctrl.set_params(ctrl_params) + ctrl.set_model_plant_params(plant_params) + ctrl.set_ambient_temperature(theta_amb) ctrl.set_enabled(True) plant = Pot(dt) plant.set_plant_params(plant_params) diff --git a/scripts/demos/sud/demo_sud.py b/scripts/demos/sud/demo_sud.py index aef4127..fc6da2a 100644 --- a/scripts/demos/sud/demo_sud.py +++ b/scripts/demos/sud/demo_sud.py @@ -48,8 +48,10 @@ if __name__ == '__main__': "Td" : 60 } - ctrl = TempController(dt, plant_params, theta_amb) + ctrl = TempController(dt) ctrl.set_params(ctrl_params) + ctrl.set_model_plant_params(plant_params) + ctrl.set_ambient_temperature(theta_amb) plant = Pot(dt) plant.set_plant_params(plant_params) plant.set_ambient_temperature(theta_amb) diff --git a/server/brewpi.py b/server/brewpi.py index fcc3131..0f72e93 100755 --- a/server/brewpi.py +++ b/server/brewpi.py @@ -88,8 +88,14 @@ if __name__ == '__main__': taskmgr.add(heater_task) # Temperature Controller - tc = PidFactory.create(config['Controller']['pid_type'], DT, DEFAULT_PLANT_PARAMS, theta_amb=theta_amb) + tc = PidFactory.create(config['Controller']['pid_type'], DT) tc.set_params(config['TempCtrl']) + # "Normal" has no internal model/ambient - see temp_controller.py vs. + # temp_controller_smith.py. + if hasattr(tc, 'set_model_plant_params'): + tc.set_model_plant_params(DEFAULT_PLANT_PARAMS) + if hasattr(tc, 'set_ambient_temperature'): + tc.set_ambient_temperature(theta_amb) tc_task = TcTask(tc, DT_TASK, dispatcher.msgio_get("TempCtrl")) taskmgr.add(tc_task)