From e4f8b15ab0644642956797c1f50189e1d0b25897 Mon Sep 17 00:00:00 2001 From: Jens Ahrensfeld Date: Tue, 23 Jun 2026 19:19:21 +0200 Subject: [PATCH] Route the forecast estimator's commanded power through the heater PWM chain SudForecastEstimator.estimate() was feeding tc.get_power() straight to the plant (pot.set_power(heater_max_power * y)), bypassing the duty- cycling across the heater's discrete power steps that the real run's HeaterTask/device chain always applies (tasks/heater.py). That let pid_heat's own oscillatory tendency reach the plant undamped, so the forecast showed a violent on/off staircase that no actual brew run ever produces - confirmed by comparing against real sud_log output, which stays smooth because the real actuator chain duty-cycles the same PID output down first. estimate() now duty-cycles tc.get_power() across the heater's own power steps (mirroring HeaterTask's PWM logic, duplicated rather than imported to keep components/ from depending on tasks/) before handing it to the plant, and also feeds the resulting discretized power back into the Smith predictor's internal model via set_model_power(), same as the real wiring in brewpi.py does. The constructor takes the heater's get_powers() list and the configured sim_warp_factor (needed to convert HeaterTask's 10-real-second PWM period into simulated ticks) instead of a bare max power. --- components/sud_forecast.py | 67 +++++++++++++++++++++++++++++++++++--- server/brewpi.py | 3 +- 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/components/sud_forecast.py b/components/sud_forecast.py index 3434f27..dc7846c 100644 --- a/components/sud_forecast.py +++ b/components/sud_forecast.py @@ -7,6 +7,27 @@ from components.sud import Sud, SudState # simulation forever - the estimate is simply cut off there. MAX_TICKS = 200000 +# Mirrors tasks/heater.py's own pulse_period_s - HeaterTask duty-cycles its +# device's discrete power steps over a rolling window this many *real* +# seconds wide, regardless of warp factor (see estimate()'s actuate() +# closure below). +PULSE_PERIOD_S = 10 + + +def _next_smaller_power(powers, power): + """Mirrors HeaterTask.get_next_smaller_power() (tasks/heater.py) - + duplicated rather than imported (components/ has no business depending + on tasks/) - keep the two in sync if HeaterTask's PWM logic changes.""" + p_list = [p for p in powers if p <= power] + return p_list[-1] if p_list else powers[0] + + +def _next_greater_power(powers, power): + """Mirrors HeaterTask.get_next_greater_power() (tasks/heater.py) - see + _next_smaller_power().""" + p_list = [p for p in powers if p > power] + return p_list[0] if p_list else powers[0] + class SudForecastEstimator: """Predicts how long a Sud schedule will actually take by simulating it @@ -20,14 +41,26 @@ class SudForecastEstimator: whenever a client needs an estimate - the naive "abs(delta)/rate" model the GUI used to compute itself has no way to see the real PID cascade's spin-up/settling lag, which is exactly why its estimate drifted so far - from reality (see README.md's "Forecast vs. actual duration").""" + from reality (see README.md's "Forecast vs. actual duration"). - def __init__(self, dt, theta_amb, pid_type, tempctrl_params, heater_max_power): + estimate() also duty-cycles the PID's continuous output across the + heater's own discrete power steps (see its actuate() closure), exactly + like the real run's HeaterTask/device chain does - feeding tc.get_power() + straight to the plant instead lets pid_heat's own oscillatory tendency + reach the plant undamped, producing a forecast far more jagged than any + real run actually is (the discrete steps end up duty-cycling it back + down to something close to the commanded average).""" + + def __init__(self, dt, theta_amb, pid_type, tempctrl_params, heater_powers, sim_warp_factor): self.dt = dt self.theta_amb = theta_amb self.pid_type = pid_type self.tempctrl_params = tempctrl_params - self.heater_max_power = heater_max_power + # The heater's own discrete output levels (AHeater.get_powers()), + # not just its max - duty-cycling needs the actual steps to pick the + # pair straddling the commanded power. + self.heater_powers = heater_powers + self.sim_warp_factor = sim_warp_factor def set_ambient_temperature(self, theta_amb): self.theta_amb = theta_amb @@ -87,6 +120,29 @@ class SudForecastEstimator: # hitting MAX_TICKS and producing a needlessly huge result. tc.set_theta_soll(start_theta) + # Ticks here are dt simulated seconds each, same as HeaterTask's own + # loop (one DT_TASK real seconds == dt simulated seconds, by warp + # factor's definition) - so the PWM period, in ticks, is the same + # pulse_period_s * sim_warp_factor regardless of dt. + pulse_period_count = PULSE_PERIOD_S * self.sim_warp_factor + pulse_counter = 0 + + def actuate(y): + """Duty-cycles y (tc.get_power(), in -1..1) across self. + heater_powers exactly like HeaterTask.on_process()'s loop does + with power_actor - see SudForecastEstimator's own docstring.""" + nonlocal pulse_counter + power = max(0, self.heater_powers[-1] * y) + power_low = _next_smaller_power(self.heater_powers, power) + power_high = _next_greater_power(self.heater_powers, power) + power_step = power_high - power_low + duty = (power - power_low) / power_step if power_step else 0.0 + on_count = pulse_period_count * duty + pulse_counter += 1 + if pulse_counter >= pulse_period_count: + pulse_counter = 0 + return power_low if (power == 0 or pulse_counter >= on_count) else power_high + def on_step_changed(step): if step is None: return @@ -114,7 +170,10 @@ class SudForecastEstimator: pot.process() tc.set_theta_ist(pot.get_temperature()) tc.process() - pot.set_power(max(0, self.heater_max_power * tc.get_power())) + power = actuate(tc.get_power()) + pot.set_power(power) + if hasattr(tc, 'set_model_power'): + tc.set_model_power(power) if sud.state == SudState.RAMPING: if tc.is_holding(): diff --git a/server/brewpi.py b/server/brewpi.py index 49da7f3..8d93b5e 100755 --- a/server/brewpi.py +++ b/server/brewpi.py @@ -109,7 +109,8 @@ if __name__ == '__main__': # it with the same kind of plant/controller (and params) as above - # see components/sud_forecast.py. forecast_estimator = SudForecastEstimator( - DT, theta_amb, config['Controller']['pid_type'], config['TempCtrl'], heater.get_power_max()) + DT, theta_amb, config['Controller']['pid_type'], config['TempCtrl'], + heater.get_powers(), config['Controller']['sim_warp_factor']) sud_task = SudTask(sud, tc, stirrer, pot, DT, DT_TASK, dispatcher.msgio_get("Sud"), forecast_estimator) taskmgr.add(sud_task) # Records every Sud run's measured data and forecast to their own