The ITerm variable is never initialized. It should be set to zero in the constructor PID::PID()

This may be helpful to anyone trying to understand what the parameters do...

The output of the PID in the Compute() method is:

      /*Compute PID Output*/
      double output = kp * error + ITerm- kd * dInput;

so, if you print out the values of Kp*error, ITerm and -kd * dInput you can see how each parameter makes up the output value.

Start by using parameters Kp=1.0, Ki=0.0, Kd=0.0 then try Kp=0.0, Ki=1.0, Kd=0.0 and so on.

It is useful to note that the output is just the ITerm when it reaches the setpoint. The PID will always reach the setpoint with only a Ki parameter, but it may take some time. The Kp and Kd parameters are used to speed up getting near the setpoint.

One way to simply do this is...
in PID:Compute(), after /* Compute PID Output */ add the following:

     // save each term of the output equation
      dispKp= kp * error;
      dispKi= ITerm;
      dispKd= 0-(kd * dInput);

then use the PID:GetKp(), PID:GetKi() and PID:Kd() to get those values in your main code

Yours,
TonyWilk

Here, there is this line of code:

double output = kp * error + ITerm- kd * dInput;

Shouldn't it be this?:

double output = kp * error + ITerm + kd * dInput;
//                                 ^ (switched from -)

Am I reading this wrong here?

First thanks for a great library. It helped me cook a great steak yesterday. :)

To get it working with my setup I had to modify it some though.

My setup is a temperature sensor (DS1820) that reads celsius and sets the Input variable each loop to the last read temperature.

To get it working with my solid state relay I had to change the less than sign to a greater than sign like below. Not sure of this is just a typo from your side or unique to my setup.

if(Output > millis() - windowStartTime){

I'm running this library in reverse mode and i get this output to the serial console:

Target 40C - Temp 41C - Duty 29
Target 40C - Temp 41C - Duty 34
Target 40C - Temp 41C - Duty 39
Target 40C - Temp 41C - Duty 44
Target 40C - Temp 41C - Duty 49
Target 40C - Temp 41C - Duty 54
Target 40C - Temp 41C - Duty 59
Target 40C - Temp 41C - Duty 64
Target 40C - Temp 41C - Duty 69
Target 40C - Temp 41C - Duty 74
Target 40C - Temp 41C - Duty 79
Target 40C - Temp 41C - Duty 84
Target 40C - Temp 41C - Duty 89
Target 40C - Temp 41C - Duty 94
Target 40C - Temp 41C - Duty 99
Target 40C - Temp 41C - Duty 1044

Where the first value is the setpoint, the second is the input and the last is the output.
The output range is set from 14 to 255.

Idk why but when exceeding 100 the output becomes 1000+ and the program stops running.

This only happens when Ki is non-zero.

Hi,
Thank you for the excellent pid library! I have noticed that the way the iterm is initialized when switching from manual to automatic mode does not ensure a bumpless transfer because the kp*error term is not considered by the initialization.
Is it a bug? Is it a feature? It makes my life much easier, as I am using ki=0 and use the initialization to generate a static offset...

Thomas

Would overloading the PID function with volatile* doubles allow this library to be used inside interrupt functions?

I can see derivative term is calculated as Kd * (input-lastinput) while a PID derivative term is proportional to the derivative of the error signal over time.

Is that an error?

On line 58, there is a an error which makes the program useless.

Line 58: if (Output < millis() - windowStartTime) digitalWrite(RELAY_PIN, HIGH);

The output should be greater than not less than millis() - windowStartTime.

When warming up to the SetPoint, the output is 5000, which is the whole windowTime, so the relay would never turn on.

Does the 'RelayPin' the PID_RelayOutput.ino example need to be set to output in the 'setup' function?

I'm trying to use two independent instances of the PID controller (one for X axis, and one for Y axis) to control a ball-on-plate system (similar to https://www.youtube.com/watch?v=K-F_T59ZDPw&t=74s ). I've found that I can get the X-axis working, or the Y axis, but whenever I enable both controllers, only the first one that has it's Compute() method called will actually return an output. The second will always return a zero value for output. I have no idea why this is.

When I downloaded this library and put it into the library folder ,and I opened an example of this library ,It can't be complie.
The IDE shows this message to me .
"C:\Users\admin\Documents\Arduino\libraries\br3tt_rduino_PID_Library_fb095d8: Missing 'core-dependencies' from library
"

as you stated already by yourself, real 64bit double is overprecise and overtaxing for PID regulation, and actually on AVR your are using 32bit float instead because double is not existing by 64bit.
But on ARM cores there is a double data type of 64bit which has to be changed then to float, akward enough.
So why not stay with float for all platforms and cores from the start?

It is not possible to include directly the PID library under the Arduino folder call "libraries".
To do so, as you say in your readme.txt, you have to copy/paste it into the propriate folder to be recognize by the Arduino IDE.

It is a problem because it can not be automatically synchronize up and down with the GitHub repository without copy/paste in the other way.

The solution would be to delete the folder "PID_v1" and move all its files and folder into Arduino-PID-Library folder.

Hello,

I downloaded the library and tried to run the examples.I am printing the variables Input and Output on the serial monitor. The "Output" variable always reads 0. Could you tell me why the pidcompute is not able to generate the Output?

In this library, PID Output is calculated from
output = kp * error + ITerm - kd * dInput
(At line 63 in PID_v1.cpp)

But, I think PID Output is calculated from the following formula using previous step's output in discrete implementation.
output = old_output + kp * error + ITerm - kd * dInput

reference: https://en.wikipedia.org/wiki/PID_controller#Discrete_implementation

I get the following message when I try to compile PID-basic

This report would have more information with
"Show verbose output during compilation"
enabled in File > Preferences.
Arduino: 1.0.6 (Windows NT (unknown)), Board: "Arduino Uno"
C:\Users\PETER\Documents\Arduino\libraries\Arduino-PID-Library-master\PID_v1.cpp: In constructor 'PID::PID(double*, double*, double*, double, double, double, int)':
C:\Users\PETER\Documents\Arduino\libraries\Arduino-PID-Library-master\PID_v1.cpp:46: error: type 'PID' is not a direct base of 'PID'

1. Please add "lastTime = millis()-SampleTime;" to Initialize (bug)

2. Please consider to expose this private function to public use...in some use cases changing Kp,Ki,Kd to obtain optimized set there is reason to restart the controller.

3. May be my misunderstanding of the code but it looks like changing from auto mode to manual as the same like not calling compute in the loop?

4. Please consider fixing compute() : currently computation done in case of timeChange > SampleTime, the calculation however done based on K(i,d) based on SampleTime instead of the actual timeChange...
   this is causing fluctuations in the controller results.

5. After thorough testing and comparison to other PID simulations, I suspect the current compute method doesn't yield a valid PID output...a basic peudo compute code should be:

   outputSum += error * dt; //integral of Err/dt
   output += ki * outputSum; //Ki * integ(Err/dt)
   if(!pOnE) output -= kp * dInput;
   else output += kp * error; //Kp * Err
   output += kd * dError / dt; //Kd * Derr/Dt

   if(output > outMax) output = outMax;
   else if(output < outMin) output = outMin;
   *myOutput = output;

Unfortunately the current code doesn't not yield same result...

The default "notice text" is as follows, with my emphasis added:

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation,either version 3 of the License, or
(at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with this program. If not, see http://www.gnu.org/licenses/.

However, since the library only includes the words "This Library is licensed under a GPLv3 License", it's unclear whether that means GPL3-only (which is the "non-standard" way of licensing GPL3), or if it means this stock GPL3+ text. It's not critical now, since there's no GPL4 yet, but in the interest of cleaning up and including complete license notices, I'd like this point clarified.

Hi

Looking at https://github.com/br3ttb/Arduino-PID-Library/blob/master/PID_v1.cpp#L60:

     double dInput = (input - lastInput);

      /*Compute PID Output*/
      double output = kp * error + ITerm- kd * dInput;

Shouldn't dInput (D term) be the change in error rather than in input? That is, shouldn't dInput be equal to the difference between the current error and the last error?

Thanks

Since

ITerm += ki * error 

and

output = kp * error + ITerm - kd * dInput;

The i term can never be shut off.

Although Ki can be thought of as a constant of integration (thus it doesn't matter if it is inside or out side of the integral) it actually is not constant and can be adjusted by the user. If your tuning a PID loop and want to remove the i term by setting i to 0 your out of luck since the ITerm is never removed from the output equation. Here is a suggested fix:

      /*Compute all the working error variables*/
      double input = *myInput;
      double error = *mySetpoint - input;
      ITerm += error;
      double ki_x_ITerm = ki * ITerm;
      if(ki_x_ITerm > outMax) ITerm= outMax;
      else if(ki_x_ITerm < outMin) ITerm= outMin;
      double dInput = (input - lastInput);

      /*Compute PID Output*/
      double output = (kp * error) + (ki_x_ITerm) - (kd * dInput);

This should be functionally equivalent to existing code with the exception when i is set to 0. I'm testing this now.

Jacob

I have an error when compiling.
the solution is to change the line

include <WProgram.h>

To

include <Arduino.h>

On PIDlib.cpp

Or

if defined(ARDUINO) && ARDUINO >= 100

include "Arduino.h"

else

include "WProgram.h"

endif

The PID::PID() constructor prematurely accesses uninitialized members, which can potentially dereference an uninitialized pointer (this causes a Segmentation Fault when testing locally on my Linux box.)

  PID::PID(double* Input, double* Output, double* Setpoint,
          double Kp, double Ki, double Kd, int ControllerDirection)
  {
          PID::SetOutputLimits(0, 255);                           //default output limit corresponds to 

The SetOutputLimits() method depends on members "inAuto" "myOutput" and "ITerm", which have not yet been initialized by the time it is called by the constructor. If "inAuto" (uninitialized) happens to be true, SetOutputLimits() will read+write to a random memory location via the uninitialized pointer "myOutput".

The call to PID::SetControllerDirection() also prematurely accesses uninitialized members, but that appears to be harmless.

I ran into a problem when I alter Ki of a already running PID controller. Initially Ki was > 0 and ITerm cumulated some error.

I'm new to PID control but I think ITerm should never contain a value if Ki is zero.

Line #54
if (millis() - windowStartTime > WindowSize)
Should be:
if (millis() - windowStartTime >= WindowSize)

Existing code will drop the relay output briefly when the = event sometimes occurs

Is there any particular reason to use double as the floating point data type instead of float? I know it won't matter much since they are the same on the Arduino, but using this library kind of forces one to write all the other related arithmetic in terms of doubles. Considering that the Arduino platform now supports many more microcontrollers than just AVRs, it would be wise to change to floats.

I realize this change would break a good deal of existing code making use of this library. Hence, I'm willing to fork the library if this change is not acceptable in the upstream.

As a side note, I'd like to make a pull request in order to fix the indentation in the source code. I don't feel like opening a separate issue for this, so I'm asking what your preferred indentation level is (also, spaces or tabs).

Thank you for the great library!

Not a issue, just a suggestion... The Compute() function can return true when it computes a new PID output, and false otherwise. This lets one reset an input value that should count up from zero for every PID calculation.

Thanks for this great library.

first of thanks for this great library.

i am trying to use PID with a linear actuator that uses a Brushless DC motor.
i based my code of your adaptive example. when reaching the setPoint the linear actuator slowly down nicely. but i also need it to have a slow start up so that the motor does not jump from 0 to max speed.

my Input = current position
my SetPoint = target position
my Output is used with PWM that changes the speed

do you have any tips on how to make that happen?

thanks.

Hi,

Thanks for the cool and useful library.
I'm no expert but it seems to me that the Compute method has a bug. Specifically, it concerns line
double output = kp * error + ITerm- kd * dInput;
it seems to me that it should look like this
double output = kp * error + iTerm + kd * dInput;
for me it results directly from the formula
https://en.wikipedia.org/wiki/PID_controller
I would add that I am not entirely sure, and I have not tested it. Maybe the tests show that current version works better. If I'm wrong or missing something, please reject this issue.
Generally good job ;)

If you use a proportional pure (ki=0 && kd=0), each time you switch mode, you set an ITerm error equivalent to your output.

It could be prevented using this:
if(ki < -0.00001 || ki > 0.0001){ // meaning different from 0
ITerm = *myOutput;
}
if(kd < -0.00001 || kd > 0.0001){ // meaning different from 0
lastInput = *myInput;
}

Hi! First at all, thank you very much for your library, its really impressive.
Im using it to make an initially stabilized platform using the MPU-6050 (gyro+ accellerometer) and 2 servos controlling the pitch and the roll. Using two independent PID controllers from our library I could implement it with null error on permanent regime, which is great. However,the servos movement are very hard or "steppy", specially noted when moving at intermediary speeds (when I make large abrupt movements, it gets very quickly and straight to the final point, probably because it didnt had time to make 2 steps). Investigating, I figured out that, although the routine of myPID.Compute() is called at about 100Hz, the output variable is changing it value only at about 10Hz.
Can I make something to improve it? Is this the maximum computing speed of the compute function?
Thank you very much in advance.

I LOVE your PID library and the thorough explanation! Thank you.

I've been using it to control a small robotic arm and just ran into a random problem. I'm doing some current sensing along with driving the robot, and just recently tried to connect Arduino's AREF pin to 3.3V to get higher resolution. In order to do this, I need this line of code in the setup():

analogReference(EXTERNAL);

The weird thing is that although this should be completely separate from the PID loop and rest of the code, when I insert this line, the robotic arm joint angles get messed up and the behavior is erratic. If I use the exact same code and hardware setup, but comment out the above line, it works fine! Any thoughts on why this might be? It's a VERY weird, illogical error as far as I can tell. Thanks in advance!

Is this expecting values from 0-1024? I have an implementation where values are read and linearized and they result from 0-35.

I was trying to set as low sample rate as possible because I need fast reaction times, but I noticed that if I put anything lower than 14 into SetSampleTime() function, PID algorithm outputs NAN to the output variable. Could you please fix this issue? Thank you in advance.

I think it should be dError = error-lastError;

The way you have it written it is really an error chasing it tail with input-lastError. I could be wrong in the simplification of the math.

Playing few hours with lib and DC motor with encoder, it seems lib works, but with particular issue I am seeing - namely PID library calculates output always as positive Output value.
In the case of overshoot one would think this Output should be made negative and in that case implementation should reverse polarity of motor so:

  analogWrite(pwm1_M1, Output);
  analogWrite(pwm2_M1, 0);  

becomes

  analogWrite(pwm1_M1, 0);
  analogWrite(pwm2_M1, Output);  

Now I attached 2 buttons (up and down) to arduino, but PID motor logic works only in one direction...

Is there a way to handle those negative corrections - or how I am supposed to solve this? Am I missing something?
I guess I am not first one having this problem - HELP :)

I'm trying to control a quadrotor which works with mpu6050. I tried to tune only pitch control parameters with ziegler nichols method but PID controller gives sudden speed up. Here is my result for 0.6 and 1.5 (I tried 0.1 to 8.0 and system gives nearly same results). There is no enough
oscillation to apply ziegler nichols.

kp=0.6

kp=1.5

How can i fix this?

For proper bumpless transfer, the integral term needs to be set to zero on transition from manual to automatic. However, in the Initialize function the integral term is set to the current output value.

Recommended change:
On line 162 of PID_v1.cpp, change to ITerm = 0;

Hi. i have just swapped over from my own pdi implementation to yours. It's working fine and is a lot simpler code wise than what i had mashed together. However i'm missing an iMax parameter. i'm now using it for temperature control on a slow system that has to change temperatures at certain times. (Beer brewing). for this i need a relative small i value to keep the temp constant. but when i increase the setpoint the i term starts to build and when the setpoint is reached it takes to long to settle down again. for this i have implemented a i max parameter. Is this something you would like to have also or should i just keep it in my private fork?

I have added :
double GetIterm()
double GetIMax()
setIMax(double)
double getIMax()

in compute() ITerm validates to IMax and not outMax.
when outMax or IMax is changed i check to see that IMax <= outMax

i would have sent you a compare link but i used a different encoding and all the lines are green.

i'm unsure how this works on a reversed controller since i don't have anything to test with.

If you would like this i can make a proper pull request.

one more thing. i have made SetIMax to set double IMax but i think this should be set in either SetOutputLimits() or SetTunings() what do you think?

Sorry, not fork but just quick comment.

In Calculate method:
ITerm+= (ki * error);
Integration precision can be improved easily by using previous error value and integrate as trapezoid:
prevError = *mySetpoint - lastInput;
ITerm+= (ki * (error + prevError) / 2.0);

Second, you calculate derivative of input instead of derivative of error value as it should be done in canonical PID-regulator.
< dInput = (input - lastInput);

dError = (errror - prevError);

< double output = kp * error + ITerm- kd * dInput;

double output = kp * error + ITerm + kd * dError;

I'm using the PID Library for a balancing robot and it seems to be exhibiting strange behavior.

You can view the complete code here:

https://github.com/owhite/balancing_bot/blob/master/src/balancing.ino

but I believe the relevant section is here:

void loop() {  

 .
 .
 .
  tmp2 = micros();
  MPUupdate();
  tmp1 = micros();

  pid.Compute();

  tmp3 = tmp1 - tmp2;
  Serial.print(position);
  Serial.print(" ");
  Serial.print(output);
  Serial.print(" ");
  Serial.println(tmp3);
  
  // send output/PWM to the motors    

 .
 .
 .
}

The issue is this. MPUupdate() sets position by performing an I2C call to a MPU9250 module, and it returns position - the tilt of the MPU09250. Then the code calls pid.Compute(). The problem is that some calls to MPUupdate() take a lot longer than others and when it does the return values from pid.Compute() jump to some extreme value.

This is an example of print output:

179.87 3.82 72
179.87 3.82 72
179.87 3.82 71
179.87 3.82 74
179.87 3.82 70
179.87 41.32 757 (see below)
179.87 41.32 71
179.87 41.32 71
179.87 41.32 71
179.87 41.32 72
179.87 3.93 71
179.87 3.93 71
179.87 3.93 71
179.87 3.93 71
179.87 3.93 71
179.87 3.93 70
179.87 3.93 71
179.87 3.93 71
179.87 3.93 74
179.87 3.93 71
179.87 3.93 71
179.87 16.02 760
179.87 16.02 71
179.87 16.02 70
179.87 16.02 71
179.87 16.02 72
179.87 3.97 71
179.87 3.97 71
179.87 3.97 71
179.87 3.97 75
179.87 3.97 71
179.87 -10.11 757
179.87 -10.11 71
179.87 -10.11 75
179.87 -10.11 70
179.87 3.93 71
179.87 3.93 72
179.87 3.93 70

The fifth line down from that out is an example.

179.87 41.32 757 

Where I have an unusually long delay time and it gives me a much larger PID value even though the position is the same. So basically when there is a long call to the MPU it screws up my pid values, and then my balancing robot jitters. Any suggestions?

Would be useful to add a method to read back output limits. I'd submit a pull reuest, but it looks like those aren't being looked at...

Well it is late at night,

but wouldnt "SetControllerDirection"
https://github.com/br3ttb/Arduino-PID-Library/blob/master/PID_v1.cpp#L34

PID::SetControllerDirection(ControllerDirection);
PID::SetTunings(Kp, Ki, Kd);

work on uninitialised kp, ki, kd because SetControllerDirection tries to switch them from plus to minus within the constructor before they are filled up by "SetTunings"?
I hope I dont waste your time on this, if this is bogus. I saw some warning within Arduino and think this is the reason.

Greetings

line 63:
double output = kp * error + ITerm- kd * dInput;

replace "-" by "+"
double output = kp * error + ITerm + kd * dInput;
This should fix some instabilities ;-)

I'm trying to use the library to control a Quadcopter, and it looks like only the first PID instance declared actually works. For example, if I declare them in this order, only the yaw controller works:

PID yawPID(&yaw, &setYaw, &rxYaw, YAW_KP, YAW_KI, YAW_KD, REVERSE);
PID pitchPID(&pitch, &setPitch, &rxPitch, PITCHROLL_KP, PITCHROLL_KI, PITCHROLL_KD, REVERSE);
PID rollPID(&roll, &setRoll, &rxRoll, PITCHROLL_KP, PITCHROLL_KI, PITCHROLL_KD, REVERSE);

//Initialize PID Controller:
void initPID() {
  yawPID.SetOutputLimits(YAW_MIN, YAW_MAX); yawPID.SetSampleTime(PID_CYCLETIME);
  pitchPID.SetOutputLimits(PITCHROLL_MIN, PITCHROLL_MAX); pitchPID.SetSampleTime(PID_CYCLETIME);
  rollPID.SetOutputLimits(PITCHROLL_MIN, PITCHROLL_MAX); rollPID.SetSampleTime(PID_CYCLETIME);
  yawPID.SetMode(AUTOMATIC); pitchPID.SetMode(AUTOMATIC); rollPID.SetMode(AUTOMATIC);
}

Here is where I'm calling the Compute functions:

if(yawPID.Compute() || pitchPID.Compute() || rollPID.Compute()) {
  DBG.print("PID: YAW="); DBG.print(setYaw);
  DBG.print(", PITCH="); DBG.print(setPitch);
  DBG.print(", ROLL="); DBG.println(setRoll);
}

It seems to depend on which Compute() is called first in the loop.

The board I'm using is an Arduino Zero. Also, I agree with #39 since the Zero and Due treat doubles as 64-bit double-percision, and unfortunately neither have a floating-point unit.

hi,
I see glish on pid.Compute(); because input is updatd by an interruption happening on the middle of pid.Compute(), here 👍
"
double input = *myInput;
"
has myInput is a double it take 2 cycle to write it with 3000 interruption per second it can happen that interruption is called on the middle of the copy, creating a bad number, where the computation mmake a big jump.

if we replace by that it clean the issue :
noInterrupts();
double input = *myInput;
interrupts();

The gish : O move from 13.49 to -171.90
S:1000.00, I:1002.00, newPID=: 1, O:13.49, Kpid: 0.10,1.00,0.01,, GetDirection: 0,, GetMode: 1,loop
S:1000.00, I:1005.00, newPID=: 1, O:-171.90, Kpid: 0.10,1.00,0.01,, GetDirection: 0,, GetMode: 1,loop
S:1000.00, I:1161.00, newPID=: 1, O:2.29, Kpid: 0.10,1.00,0.01,, GetDirection: 0,, GetMode: 1,loop

Thanks for you work here. I was implementing a simpler version of this w/ only the P in PID and then found your work, and thought a more sophisticated solution couldn't hurt.

I am trying to PID control a magnetometer and a motor-controller to steer my car.

The interface to the steering of the car takes a value of -127-127, which translates to a full left turn to straight to a full right turn respectively.

I also have a magnetometer which will give me my heading in degrees 0-360.

Now, I'm not sure how to make this work w/ your library. Say for example Setpoint is 0 (north), then your library will never send an output signal b/c the input from the magnetometer will never have a value below 0 (0-360). So to workaround this, I scale my input to your library to the range -180-180 with the equation: ((theta + 180) % 360) - 180. This will, for example, convert 270 (east) to -90 and allow the library to give an output signal. For example if the current heading is -90 and setpoint is 0, then it give an output signal. However, if the current heading is +90 and the setpoint is 0 it will not give an output signal. In my case I need an output signal with the range of -127 -- +127 for an input range of -180 -- +180.

Any idea how to configure this?

related with Issue #23 ,
ITerm implementation currently is
ITerm+= (ki *error);
which is ok but there is a Bug if you define ki = 0;
because the implementation of the output

  /*Compute PID Output*/
      double output = kp * error +  ITerm - kd * dInput 

the integral term is no equal to zero, because it will keep the values untill the time when your change the Ki parameter

my proposal will be, calculate the ITerm like
ITerm+= (error);
and the output would look like this

  /*Compute PID Output*/
      double output = kp * error +ki*  ITerm - kd * dInput 

which look closer to the PID definition

wait for comments ;)

Please help me to install PID v1.2.1 to PlatformIO.

Thanks

/**********************************************************************************************

• Arduino PID Library - Version 1.1.1
• by Brett Beauregard [email protected] brettbeauregard.com
• This Library is licensed under a GPLv3 License
  **********************************************************************************************/