I finished programming a PI loop today, and it worked. I’m hoping I could have it proofread before I move on to a full PID loop. I already know that this thing is pretty amateur, and it’s organization is a mess (I’ll try working on this soon). I’d appreciate it if any errors are pointed out to me.

task main()
{

int setpoint;
setpoint = 1535;
float error;
float power; // / //
float kP; // /-
float kI;
kI = 0.023;
kP = 0.0819;
float area;
float integral;
while(1==1)
{
error = setpoint - SensorValue[Potentiometer];
area = error*kI;
integral = integral + area;
power = error*kP + integral*kI;
if(power > 127)
{
power = 127;
}
if(power < -127)
{
power = 127;
}
if(error == 0)
{
integral = 0;
}
if(error > 127)
{
integral = 0;
}
motor[Arm] = power;//@Everyone
datalogDataGroupStart();
datalogAddValue( 0, setpoint );
datalogAddValue( 1, error );
datalogAddValue( 2, power );
datalogAddValue( 3, SensorValue[Potentiometer] );
datalogAddValue( 4, area );
datalogAddValue( 5, integral );
datalogDataGroupEnd();
wait1Msec(25);
}

You don’t want this:
if(power < -127)
{
power = 127;
}
I assume you meant to say power = -127 if power < -127?

You also don’t want this:
if(error == 0)
{
integral = 0;
}
I know some people for this for travel control, but if you are using this in, say, an application that lifts an arm or some other motion you don’t want to reset the integrator when your error == 0 because the integrator is the reason your error got to 0. The proportional can’t do that.

You can do this:
if(error > 127)
{
integral = 0;
}
This kind of thing is OK for the integrator, to reset based on how far away from the set point it is so you don’t get wind-up. However, you would want to do it for +/- most likely (depends upon what you are controlling). Looks like 127 is a magic number, It will most likely be determined with testing.

If power < -127, you’re setting it to 127. Why not -127?

You’re setting the integral to 0 in two cases, making sure it doesn’t blow up or stay high when it should drop. I get that. But what if error is 0. If error is 0, you’re where you want to be, right? So wouldn’t you want to set integral to 0 before setting power instead of afterward so that if you’re right where you want to be you stop?

This might be OK for moving to a position to stop, but it looks like his code, since it is using a pot, is an arm or something so you need to keep the integral at the value it settled on to get 0 error if there is a force on the arm.

The missing negative sign is a left over error from my P-Loop program, which this started as a copy of. I made the copy of the program before I fixed it, and I guess I forgot to change it in the copy alongside the original. That’s an embarrassing mistake. Thanks for pointing it out.

Also I’ve been considering removing the part where I set integral to 0 when error is 0. I’ll probably do that now. Thank you.

(I just realized that I forgot to edit out the meme names for the motor and potentiometer. That’s embarrassing.)

You appear to multiply by kI twice. Once when summing (area = error * kI) and also when you calculate power (power = error * kP + integral * kI). This means you’re effectively multiplying your integral by kI^2 which might make tuning a bit weird. You can solve this by either only multiplying when you integrate, or on calculation. So you could just turn area = error * kI; integral += area; into just integral += error; Regardless, having a separate area variable is also unnecessary.

Resetting your integral is fine if it’s on a lift that has rubber bands to keep it in place, however, depends on the circumstances (If your lift might be carrying a load, then it might be different). If you do choose to reset your integral, don’t do it when error is equal to 0, do it when your error changes sign. This is because your error will very rarely actually be equal to 0, and resetting on 0 exactly might lead to weird behaviour. To reset your error on sign change, all you have to do is keep a record of your previous error, and say:

if (sgn(error) != sgn(lastError)) {
integral = 0;
}

This will cause your integral to reset when your error has crossed the setpoint.

Aside from that, all looks fine. I do really recommend also getting derivative in your loop as it helps a lot.