Return PID temp_dState update logic to 2.0.x behaviour

[Triodes]

Description

This PR alters the PID algorithm slightly so it matches the behaviour it had in version 2.0.x

A while back up updated my firmware to version 2.1.x and noticed that my printer was not smoothly heating to it's target temp. Instead there was a small dip just before reaching the target temp. It turned out that as soon as the temp entered the PID_FUNCTIONAL_RANGE the heater would immediately and completely shut off. Debugging the PID values with M303 D I found that the dTerm aka work_d was going to around -5000 whereas in the 2.0.x version it would start off at about -10.

Digging through the code I found that the logic of the PID code was slightly changed. In version 2.0.x the value of the temp_dState would be set to the current temperature of the heater every iteration of the algorithm, regardless of whether the current temperature was within the bounds of PID_FUNCTIONAL_RANGE. In version 2.1.x this value is only set when the temperature was within these bounds. The result of this is that on the first iteration where the current temperature lies within the functional range, the difference between temp_dState and current is really large because temp_dState is initialized to 0 on boot. (for example: when heating to 200C with a functional range of 10, this difference would be 190). This in turn causes the work_d = work_d + PID_K2 * (Kd * (temp_dState - current) - work_d); formula to generate a rather large negative value (in my case around -5000). With the default PID_K1 smoothing factor of 0.95 it takes a little while for this large negative value to be corrected, which was the cause for the dip I experienced when heating.

Now I am a programmer, but I know very little about c++ or PID. I simply looked at the differences between the 2 versions and restored the original behaviour. Since I'm experiencing better results with the old behaviour I'm looking at this as a "regression", but I have no idea if this change in behaviour was intentional or not. I'm curious to know 😄 .

For reference, this is de code from version 2.0.x I compared to:

float Temperature::get_pid_output_hotend(const uint8_t E_NAME) {
  const uint8_t ee = HOTEND_INDEX;
  #if ENABLED(PIDTEMP)
    #if DISABLED(PID_OPENLOOP)
      static hotend_pid_t work_pid[HOTENDS];
      static float temp_iState[HOTENDS] = { 0 },
                    temp_dState[HOTENDS] = { 0 };
      static bool pid_reset[HOTENDS] = { false };
      const float pid_error = temp_hotend[ee].target - temp_hotend[ee].celsius;

      float pid_output;

      if (temp_hotend[ee].target == 0
        || pid_error < -(PID_FUNCTIONAL_RANGE)
        || TERN0(HEATER_IDLE_HANDLER, heater_idle[ee].timed_out)
      ) {
        pid_output = 0;
        pid_reset[ee] = true;
      }
      else if (pid_error > PID_FUNCTIONAL_RANGE) {
        pid_output = BANG_MAX;
        pid_reset[ee] = true;
      }
      else {
        if (pid_reset[ee]) {
          temp_iState[ee] = 0.0;
          work_pid[ee].Kd = 0.0;
          pid_reset[ee] = false;
        }

        work_pid[ee].Kd = work_pid[ee].Kd + PID_K2 * (PID_PARAM(Kd, ee) * (temp_dState[ee] - temp_hotend[ee].celsius) - work_pid[ee].Kd);
        const float max_power_over_i_gain = float(PID_MAX) / PID_PARAM(Ki, ee) - float(MIN_POWER);
        temp_iState[ee] = constrain(temp_iState[ee] + pid_error, 0, max_power_over_i_gain);
        work_pid[ee].Kp = PID_PARAM(Kp, ee) * pid_error;
        work_pid[ee].Ki = PID_PARAM(Ki, ee) * temp_iState[ee];

        pid_output = work_pid[ee].Kp + work_pid[ee].Ki + work_pid[ee].Kd + float(MIN_POWER);

        #if ENABLED(PID_EXTRUSION_SCALING) {...}
        #endif // PID_EXTRUSION_SCALING
        #if ENABLED(PID_FAN_SCALING) {...}
        #endif // PID_FAN_SCALING
        LIMIT(pid_output, 0, PID_MAX);
      }
      temp_dState[ee] = temp_hotend[ee].celsius;

    #else // PID_OPENLOOP {...}
    #endif // PID_OPENLOOP

    #if ENABLED(PID_DEBUG) {...}
    #endif

  #else // No PID enabled

    const bool is_idling = TERN0(HEATER_IDLE_HANDLER, heater_idle[ee].timed_out);
    const float pid_output = (!is_idling && temp_hotend[ee].celsius < temp_hotend[ee].target) ? BANG_MAX : 0;

  #endif

  return pid_output;
}

Note how temp_dState[ee] = temp_hotend[ee].celsius; is called outside of the if, else-if, else branch.

Benefits

Should result in more reliable PID heating? (it works on my machine 😆)

Configurations

I have no idea if this would help, but this is my current config:
Configuration.zip

Related Issues

Could not find any

[sjasonsmith]

@Triodes could you provide a link to the commit you think changed the behavior? I'd like to see what the reasoning was for the change at the time.

[Triodes]

@sjasonsmith

I haven't gotten around to this yet but I do have the intention to answer this question

[Triodes]

@sjasonsmith
I think it's this commit: 54e7b93

referring to this PR: #24389

[sjasonsmith]

I've reviewed that commit that you pointed out, and I agree that this looks like a regression, at least for the hotend.

The bed and chamber already had this behavior, which explains where the change came from when they were all merged into one template. I think the old bed/chamber behavior was likely incorrect, but with the slower heating and higher latency of those systems the disruption in power output likely went unnoticed.