PIDController integral error may be incorrect

[Ezward]

See https://github.com/autorope/donkeycar/blob/85958e59431ed142c40da1966f8b8654b60daacf/donkeycar/parts/transform.py#L97

• the totalError term does not include the current error
• the totalError term is being multiplied by delta-time, implying that the delta-time is constant. That may not be be true. I think it would be more accurate to make totalError the sum of all errors times their interval. So

  self.totalError += err * dt
  alpha += -self.Ki * self.totalError

I think we should also bound this total error OR keep a history queue of error values and bound that queue so we only use error from last 'n' iterations.

If error changes sign (we have 'crossed' our target value), then we would be justified is clearing the total error. For example, if the total error is accumulated on one side of the target value, then we make an adjustment that puts us on the other side of the target value, then that accumulated error no longer make sense as it would 'push away' from the target value, rather that make us converge on the target value. As it relates to the path_follow.py template, if cross track error has us to the right of the target, then we are accumulating error on that side of the target. If in the next frame we see that error has changed sign (we are now to the left of the target), then the accumulated error could possibly moves us farther to the left; we should zero because our error has changed sign.

In the path_follow.py template, the PIDController is used to calculate a new steering value. The implied target value is zero (no steering change, go straight). If crosstrack error is positive (we are to the 'right' of our path0 we want to steer left, so steering value is negative; -1 >= steering > 0. If crosstrack error is negative (we are the left of our path) we want to steer right, so steering value is positive; 0 < steering <= 1. The control relationship is a 'reverse' relationship. This is baked into the PID code by effectively multiplying the constants by -1 when they are applied. For example, note how the Ki term is negated when it is applied curr_alpha += -self.Ki * (self.totalError * dt). That makes this PIDController less general than it could be. I suggest that we have a boolean argument that declares whether the control relationship is reverse or not, rather than have this assumed to be reverse. The other alternative is to allow negative constants; we would need to look deeper to see if there any other pieces that assume positive constants only (like UI where they might be input, or in the twiddle() parameter optimizer.

[TCIII]

I had an extremely difficult time attempting to determine the correct P, I, and D values for my robot chassis when using the DC path_follow.py template. Using traditional guidelines to determine the correct P, I, and D values resulted in only marginal robot chassis performance when attempting to navigate a given path.

[Ezward]

I think it is probably worth adding a simple step controller to the path_follow template, since that type of controller is much easier to calibrate. This will work well if steering angles tend to be shallow and smooth. We can make this or the PID controller a choice in the config file.

[Ezward]

We have good success with the current PID controller. Closing this a won't do.