soloam / ha-pid-controller

PID Controller to Home Assistant
MIT License
97 stars 12 forks source link

Wrong saturation prevention in I-part #43

Open wsw2016 opened 1 year ago

wsw2016 commented 1 year ago

Current implementation of saturation prevention creates deadlocks. Assume we have an I-controller (P & D are 0). The error is big and the I-term is growing with a step of 10: step n: i_term == 95, last_output == 95 step n + 1: i_term == 105, last_output == 100 Now, the current evaluation of i_term:

if self._last_output is None or (
            self._last_output > 0 and self._last_output < 100
        ):
     ...   update i_term

the i_term will never be updated again as the last_output == 100.

The correct implementation would be:

  self._i_term += i_term_delta
  self._i_term = self.clamp_value(self._i_term, (0, 100))

without any conditions.

Another issue is the windup, which is claimed to be The maximum value to increment in the integral portion of the PID. Current implementation: self._i_term = self.clamp_value(self._i_term, self._windup) sets the maximum value of the integral part of PID itself. The correct implementation would be (together with the previous code snippet):

# Calculate I and avoid saturation  
  i_term_delta = self._ki * error * delta_time
  i_term_delta = self.clamp_value(i_term_delta, self._windup)
  self._i_term += i_term_delta
  self._i_term = self.clamp_value(self._i_term, (0, 100))