Is there something wrong with this PID code?

VEX V5 General Discussion Technical Discussion
1

I made this from George Gilliards guide. When I tried different values there is no change and let alone any oscillations to tune the PID. I have even tried extreme values like 50 for kP leaving kI and kD in zero and no visible change has happened.

void move(int setpoint, double kP, double kI, double kD){

	double integral = 0;
	double prevError = 0;

	while(getDrivePos() <= setpoint){
		double error = setpoint - getDrivePos();
		datalogAddValueWithTimeStamp(0, error);
		integral += error;
		if(error == 0 || error < 0){
			integral = 0;
		}
		if(error > 200){
			integral = 0;
		}
	double derivative = error - prevError;
	prevError = error;
	double power = error*kP + integral*kI + derivative*kD;
	motor[ulDrive] = power;
	motor[llDrive] = power;
	motor[urDrive] = power;
	motor[lrDrive] = power;
	delay(15);
	}

}
2

Shouldn’t the value of kP be less than one?

3

I was wondering about that when I read the kP=50 bit. But the OP says

So we’re left unsure just what values were actually chosen most of the time. It could be worthwhile to post the actual sets of values used. Also, how far away are your targets and for how long did you let it run? If you’re in a situation where your target is very far away, this code gives a huge error, which forces integral to 0. In such a case, if you don’t run it for long, kI will be irrelevant. Meanwhile, if kP isn’t small, derivativekD won’t be able to overcome errorkP. You’ll just keep the motors maxed out. We could know a lot better if we could see value choices and the datalog.

4

Missed the “extreme values”…

5

There is also the issue that everything defined here is positive (and the k’s are supposed to be positive), so without negatives in the k’s, values are only being added and will likely always max out the motors if the k’s aren’t small. But, again, we can’t see the k’s.

But now that I look over this again, I wonder why PID is being used here, especially with the statements in the original question. The OP makes it sound like a PID loop is trying to match the velocities, with the integral and derivative representing displacement and acceleration. But the PID loops itself isn’t looking to match velocities, but just go a target distance. That makes it look like the actual goal should be slew rate control, slowing down when the robot gets close to its goal so as not to overshoot. So I think the bigger question, before looking at the k’s and the datalog, is what is the goal of this? What is the OP trying to do?

Rephrasing this, it looks like the code isn’t using error for an error, but rather for a distance to target calculation type thing. But the idea in PID that the goal is to maintain error at 0. You fix things when there is an error. But that’s not at all what that while loop looks like it’s trying to do. It looks like it’s working with a huge error and driving until the error reaches 0, which isn’t using error in the same sense as is expected in PID.

6

George’s guide’s example is for slowing down as you approach your target, not for making left & right sides equal. That’s why his error is (target distance - distance now). To make left & right equal, you need error = (encoderLeft - encoderRight). Then your drive function each time through the loop would be
motor[left] = drivePower
motor[right] = drivePower + Kp * error + …

7

I am actually trying to stop as I approach my target not make both sides equal. And what I meant with that I tried extreme values is that I cannot see a visible change in behavior and in the graph when I do this so I do not know if it is actually working.

8

Well, it will work, but I still don’t think it’s what the OP is describing. Why would the OP mention oscillation? That would be oscillation around error=0, so oscillation around the end point. But if the goal is to slow down while approaching a goal, why does the OP expect an oscillation around the endpoint. The while statement already guarantees there won’t be oscillation around the endpoint because the loop ends when it first reaches or crosses the endpoint.

This is why I said what I said. The OP makes this sound like one thing, while the code is using a PID approach to slew control. This brings up another issue: the OP may not even be trying to do the desired thing. Maybe this is what is desired, though, and the OP is doing the desired thing but is evaluating it against something different.

For clarity, I’m not saying the code is for matching velocities. I very specifically said that, and that “error” isn’t an error. I’m saying the OP description sounds like that’s the goal, though. So I’m waiting for some more information from the OP.

9

So I mentioned oscillations because apparently a way to tune a PID is to increase P until the system oscillates and then increase the others. What I said was that the system was not oscillating even when I had high values for P.

10

Right, but look at what I wrote above:

You’re expecting normal PID behavior while using PID code for slew rate control. Did it not slow down and stop when you got to your target?

Just what it is you expect to see oscillating?

11

I came up with a good analogy to help explain things.

You have cruise control in your car. You’ve read about cruise control and how your speed will oscillate around the speed you set it to. You set it to 55 mph, but then you have to stop for a stop light. The light turns green, and you turn your cruise control back on, still set to 55 mph. The cruise control sees you’re far below 55 mph and keeps speeding you up. Then, right as you get to 55 mph you turn off the cruise control and wonder why you never saw it oscillating.

You’re doing the equivalent of speeding up from 0 mph to 55 mph using cruise control to do it, and when you get there you turn it off. Then you’re saying something seems wrong because you’re not seeing any oscillation around the target value.

12

ik that this won’t solve anything but for the first if statement under the while loop you could put


if(error<=){

instead of


if(error == 0 || error < 0){

to make the code more precise

13

I know this doesn’t affect the issue at hand, but @iperez is probably going to want to say


if(abs(error) < (some small value))

instead, because error will never be exactly 0, but in a case where you want to go backward, your integral will always be set to 0. Basically, integral should reset if the error is really close to zero (though, the value read will almost never be exactly zero).