Custom PI Loop not working

So I’ve created a PILoop class to handle PID (with no d) for different systems of the robot. I’ve got different methods for drive base, shooter angle, and intake articulation as all will need different Kp and Ki values. For some reason when I go to fill the set function of a motor to that PILoop function nothing happens. It’s definitely not the motor as I’ve written a dummy P loop inside the checking of the button press in teleop, and that works just fine. It’s only when I try to use the PILoop method that nothing happens. What am I missing?? Here’s the link to the github. Please take a look at the else if statement checking operator.getBaseBackLeft in teleop, as the code in that current logic block doesn’t actually move the arm. Can anyone help?

You have a SmartDashboard output in there. Is it showing 0? Or a number that you would expect to move the mechanism?

It actually spits out the output I would expect to run the motor.

I believe at least part of your problem is the else clause (lines 100-106) in your teleop code. When no buttons are pressed, it zeroes out your flywheels, overriding the values set based on the PI processing.

Why a PI loop? While I have heard of PIDs occasionally working out to have a zero or near-zero value for D based on mechanical friction, I don’t see any advantage to having a special class for PI.

Also, while this does not appear to be related to your observed problem, your PI class is not structured in a regular object-oriented fashion. Normally, you would only have one copy of the code in your “loop” class, and it would not know anything about any of the individual motors. Then, the subsystem classes (e.g. shooter) would each create its own instance of the calculator using its appropriate values. Much cleaner and easier to maintain.

there really is no reason for an if statement in a PID! PID’s in of their own aren’t really that complicated, and if you start adding if statements and things like that it can only have the potential to cause problems. if you don’t want to use the D component just don’t give it a constant, If you want negative feedback,make the constant negative. here is the class I wrote last year in c++. Simplicity is king!

class myPID{
private:
double Kp,Ki,Kd;
double error, error1 = 0;
double integral;
float value;
public:
void Update(double,double);
float GetValue(void);
void SetConstants(double,double,double);
};

void myPID::Update(double target,double actual){
	error = target - actual;
	integral += error*.01; //todo add actualy eleapsed time
	value = (error*Kp) + (integral*Ki) + ((error-error1)/(.01)*Kd);
	error1 = error;
}
float myPID::GetValue(){
	return value;
}

void myPID::SetConstants(double Kp,double Ki,double Kd){
        this->Kp=Kp;
        this->Ki=Ki;
        this->Kd=Kd;
}

I’ll take a look at re doing the PI class in the mean time. I don’t need the D variable as the shooter articulation and gearing is heavy enough that I don’t need the ramp down of velocity of angle change for my shooter. P will get it within range of my target setpoint, and I will eat up the remaining error so that I just don’t sit under my setpoint with stall torque. Also, the PIloop does nothing to control the flywheels. The flywheels are simply the shooter wheels that will spin up to 100% and give the boulder velocity when they exit the shooter. I am however passing control of the shooter angle over to just a joystick when no buttons are pressed. Again, if I write a dummy P loop in one of the methods checking the button press, it acts just like i want it to. It’s only when I use a function from the PILoop class that things don’t work.

Why would needing different Kp and Ki values for different instantiations require writing different methods?

It really wouldn’t but I’ll be honest and say that I was lazy and didn’t want to put in the effort to find a cleaner way to do it.

I would try making a “dummy PI” loop and make sure its working. looking at your file, there is so much “if” logic and booleans and everything for testing different scenarios, you would have to be a god to code that in one sitting without checking for functionality, and just have it it work. Most of all that logic isn’t even necessary.

simply write one class that works for everything once you get it working, and declare it for every application that you need it. In my experience I have never had code work when I just wrote everything in one sitting, you kinda have to test if its still working every time you add or change something, humans make mistakes. I could be wrong but thats why I think it isn’t working.

Thank you guys for all of the help. I’ll work through my code today, as it’s back to school for me and that means more time with the practice robot. I’ll post again when I get things sorted out letting you know what was going wrong.