PIDController: setSetpoint causes onTarget to be false about allwpilib HOT 6 CLOSED

This was closed in #138, which was reverted... so I presume that this bug still exists? This should be reopened in that case.

from allwpilib.

Actually, we only need to keep a buffer of inputs. Here's some math:

a, b, c, and d are inputs. s is the setpoint. "s - a" is the error with respect to a.

Currently, we average the errors in GetAvgError():
avgError = ((s - a) + (s - b) + (s - c) + (s - d)) / 4
= (-(a - s) + -(b - s) + -(c - s) + -(d - s)) / 4
= -((a - s) + (b - s) + (c - s) + (d - s)) / 4
= -(a + b + c + d - 4 * s) / 4
= -(a + b + c + d - 4 * s) / 4
= -(a + b + c + d - 4 * s) / 4
= (4 * s - (a + b + c + d)) / 4
= s - (a + b + c + d) / 4

Therefore, GetAvgError() could instead return the current setpoint minus the input average. This would make anything using GetAvgError(), like OnTarget(), more robust to setpoint changes.

from allwpilib.

👍 for application of some basic math.

from allwpilib.

How should we go about fixing it this time? Apparently using doubles in a large FIR filter (moving average) causes rounding errors due to so many additions. We could approximate it with an IIR filter and avoid that problem. Using a cutoff frequency isn't necessarily intuitive, but it provides more tuning hints than the size of the FIR filter does. Teams would probably tune both by guessing and checking anyway.

from allwpilib.

Changing this to an IIR filter breaks the interface. If we're going to do interface breakage, I'd rather move the filter out of PIDController entirely because the state estimation should be separate from the controller. If the user does the following:

frc::Encoder encoder{1, 2};
frc::LinearDigitalFilter filter = frc::LinearDigitalFilter::SinglePoleIIR(&encoder, 0.3, 0.05);
frc::Spark motor{0};
PIDController pid{kP, kI, kD, &encoder, &motor};

the input is filtered instead of the error (which is more correct) and the user can decide what filtering would be best.

from allwpilib.

Fixed by #38.

from allwpilib.