I thought I’d report some troubles we found with the pid controller code while we’re waiting for our regional.
We wanted to use the PIDController class to control a Jaguar for rate control. (And yes, if we’d done CAN protocol, we wouldn’t need this, but we’re not that cool a team…yet).
We ended up wrapping the Jaguar class with another class that implemented PIDWrite as a delta, not an absolute, and that gave us reasonable results, and also seemed to be a more theoretically correct implementation. (My PID experts insist that the goal of PID code should be to drive error to 0, not to have an I term that provides a perpetual error correction).
Also, the OnTarget() method consistently crashed for us; my guess is that the use of GetError(), and the doubled critical sections cause that crash.
We’re using a very slightly modified version of the PIDController class quite successfully for speed control on 3 shooter wheels. The only modification is to change the integral anti-windup measures to work slightly differently. We’re using P, I, and FF terms and it’s working well. A velocity PID like you’re using is technically more correct, but whatever works.
What kind of crashes were you getting with the OnTarget method? We’re using that to determine when to exit the shooter speed setting command, so it’s getting called every teleop loop, and we haven’t had any problems.
It’s still quite simple just to make your own OnTarget() method as well as GetError(). Even though it is more redundant, it should not lock the thread like it will otherwise. Simply, use GetSetpoint() and PIDGet() from your PIDSource object to calculate the error. From there, you can tune your OnTarget() error. For example…
//PIDSource is called source, PIDController is called controller.
float error = controller->GetSetpoint() - source->PIDGet();
float tolerance = error * .05; //any percentage value
return fabs(tolerance) < 5; // a tuned value
If I recall correctly, it was noted to the WPILib authors and in one of their logs in wpilib.screenstepslive.com, they had mentioned they fixed this problem. You can just resubmit a ticket to them at this link.
Yeah, good point, that’s where I should add a note after we retest. I find the firstforge web site a bit deceptive; it has good activity, but then bits of cruft. What’s nice about posting here is that you get immediate answers
Our team also had issues with the WPILib PID implementation, and we ended up doing pretty much the same thing: wrapping the Talon class to use PID output as a delta rather than an absolute. So far it’s been working alright, but it’s ugly.
