Arduino-PID-Library
Arduino-PID-Library copied to clipboard
br3ttb
Compute doesn't seem to calculate on an average of SampleTime
While testing for my project I created a "dashboard" where I'm monitoring all of the internal variables. One of them was TimeChange. Using the default 100ms (which works well for my project anyway), TimeChange was hovering around 88ms. I suspect the intent was to have Compute execute on an "average" of the SampleTime: i.e. if it executes at 105ms one time, it would execute at 95ms the next time. The way the code is not does not accomplish that.
lastTime = now;
should be:
lastTime += SampleTime;
If you monitor TimeChange while testing the code you'll see that it is not SampleTime (which I'm thinking it should be close)
I'm aware of this one. It's a trade-off. I decided it was better than the bug in the code you suggest:
Suppose that for a minute or two, the compute function is called once a second, even though the sample time is 100msec (say there's a comm lag or something.)
Now suppose that after that time, the compute gets called every 5msec. With your code, the output would go INSANELY adhesive; the compute code thinks 100msec have elapsed, and pushes accordingly, even though only 5msec have passed. It will keep doing this until lastTime increments up to current time.
On Tue, Apr 17, 2018, 8:23 AM Terry J Myers [email protected] wrote:
While testing for my project I created a "dashboard" where I'm monitoring all of the internal variables. One of them was TimeChange. Using the default 100ms (which works well for my project anyway), TimeChange was hovering around 88ms. I suspect the intent was to have Compute execute on an "average" of the SampleTime: i.e. if it executes at 105ms one time, it would execute at 95ms the next time. The way the code is not does not accomplish that. lastTime = now; should be: lastTime += SampleTime;
If you monitor TimeChange while testing the code you'll see that it is not SampleTime (which I'm thinking it should be close)
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/br3ttb/Arduino-PID-Library/issues/78, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWL8n0SmmeN4M2lWWAciFv3kRopYcL-ks5tpd6lgaJpZM4TYMbq .
Adhesive=> aggressive. On my phone.
On Tue, Apr 17, 2018, 8:56 AM Brett Beauregard [email protected] wrote:
I'm aware of this one. It's a trade-off. I decided it was better than the bug in the code you suggest:
Suppose that for a minute or two, the compute function is called once a second, even though the sample time is 100msec (say there's a comm lag or something.)
Now suppose that after that time, the compute gets called every 5msec. With your code, the output would go INSANELY adhesive; the compute code thinks 100msec have elapsed, and pushes accordingly, even though only 5msec have passed. It will keep doing this until lastTime increments up to current time.
On Tue, Apr 17, 2018, 8:23 AM Terry J Myers [email protected] wrote:
While testing for my project I created a "dashboard" where I'm monitoring all of the internal variables. One of them was TimeChange. Using the default 100ms (which works well for my project anyway), TimeChange was hovering around 88ms. I suspect the intent was to have Compute execute on an "average" of the SampleTime: i.e. if it executes at 105ms one time, it would execute at 95ms the next time. The way the code is not does not accomplish that. lastTime = now; should be: lastTime += SampleTime;
If you monitor TimeChange while testing the code you'll see that it is not SampleTime (which I'm thinking it should be close)
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/br3ttb/Arduino-PID-Library/issues/78, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWL8n0SmmeN4M2lWWAciFv3kRopYcL-ks5tpd6lgaJpZM4TYMbq .
yes you're right. That explains an odd behavior I foudn while testing. While having the pid in MANUAL mode for a while, then turning it to AUTO, integral would instantly saturate. I think I'll just add some reasonable limits such as 1.5X sample timer.
I've got the code to work reasonably well, but I'm unable to use the exact same numbers as my pid tuning spreadsheet gives, which honestly works 100% perfectly for CLX ladder pid dependant gain sets, so there must be something sligthly different under the hood. Going to keep plugging away at it though.
Something else you can check. This library doesn't use spans the way industrial ones do (I think they're legacy bs that isn't required with floating-point internals, but that's another subject.) if you're doing span-percentage scaling in your spreadsheet you'll need to correct for that.
On Tue, Apr 17, 2018, 7:28 PM Terry J Myers [email protected] wrote:
yes you're right. That explains an odd behavior I foudn while testing. While having the pid in MANUAL mode for a while, then turning it to AUTO, integral would instantly saturate. I think I'll just add some reasonable limits such as 1.5X sample timer.
I've got the code to work reasonably well, but I'm unable to use the exact same numbers as my pid tuning spreadsheet gives, which honestly works 100% perfectly for CLX ladder pid dependant gain sets, so there must be something sligthly different under the hood. Going to keep plugging away at it though.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/br3ttb/Arduino-PID-Library/issues/78#issuecomment-382189965, or mute the thread https://github.com/notifications/unsubscribe-auth/AAWL8vs91UIJUdTqo6VTwgt_XlPQ3SSWks5tpnqCgaJpZM4TYMbq .
Looks like I spoke too soon. Its working perfectly now. Trying different time constants and deadbands in my temperature simulator, all of the tuning methods seem to work perfectly now. I must have had some test code last night
- I modified the SetTunings to flip the internal ki calculation
- Changed Initialize to back calculate an appropriate ISum (your outputSum) based on what compute is ABOUT to do. And initialized the Low Pass Filter variables(d0 and d1)
- Modified the timing to be a a "good enough average"
- added a low pass filter for dInput with a time constnt of ~4 computes. (I've measured it to be 63% to value after 4 steps, 95% after 10, 99.5 after 17 steps, and 99.99 after 29 steps).
- Testing for what the I term will do first, then only updating it if the output of algorithm with it added will be < outMax.
- Switched to dependant gain sets (Kc, Ti, Td...but I kept the Kp, Ki, Kd because..who cares)
- Made all of the internal variables public as I wanted to access them all from my main program for a "dashboard"(giant block of nicely formatted text that allowed me to see all of the variables printed out every 250ms via serial..) For reference here is a copy of my modified spreadsheet (https://www.dropbox.com/s/loh19q5tcswqw4n/PID%20Tuning%20for%20MicroControllers.xlsx?dl=0).
Obviously I have a lot of testing and will probably find some "gotchas", as well as annotation clean up, and giving you proper credit in the header file, but here is a copy of my modified PID_v1.cpp for your reference in case you care to see the changes I made and point out all of the issues :).
/**********************************************************************************************
* Arduino PID Library - Version 1.1.1
* by Brett Beauregard <[email protected]> brettbeauregard.com
*
* This Library is licensed under a GPLv3 License
**********************************************************************************************/
#define UseDepentantGainSets 1
#if ARDUINO >= 100
#include "Arduino.h"
#else
#include "WProgram.h"
#endif
#include <PID_v1.h>
/*Constructor (...)*********************************************************
* The parameters specified here are those for for which we can't set up
* reliable defaults, so we need to have the user set them.
***************************************************************************/
PID::PID(double* Input, double* Output, double* Setpoint,
double Kp, double Ki, double Kd, int POn, int ControllerDirection)
{
myOutput = Output;
myInput = Input;
mySetpoint = Setpoint;
inAuto = false;
PID::SetOutputLimits(0, 255); //default output limit corresponds to
//the arduino pwm limits
SampleTime = 100; //default Controller Sample Time is 0.1 seconds
PID::SetControllerDirection(ControllerDirection);
PID::SetTunings(Kp, Ki, Kd, POn);
lastTime = millis()-SampleTime;
}
/*Constructor (...)*********************************************************
* To allow backwards compatability for v1.1, or for people that just want
* to use Proportional on Error without explicitly saying so
***************************************************************************/
PID::PID(double* Input, double* Output, double* Setpoint,
double Kp, double Ki, double Kd, int ControllerDirection)
:PID::PID(Input, Output, Setpoint, Kp, Ki, Kd, P_ON_E, ControllerDirection)
{
}
/* Compute() **********************************************************************
* This, as they say, is where the magic happens. this function should be called
* every time "void loop()" executes. the function will decide for itself whether a new
* pid Output needs to be computed. returns true when the output is computed,
* false when nothing has been done.
**********************************************************************************/
bool PID::Compute()
{
if(!inAuto) return false;
uint32_t now = millis();
if(now - SampleTimer>=SampleTime) {
timeChange = now - SampleTimer;
if (timeChange < SampleTimer * 1.5 || timeChange > SampleTimer * 0.5) { //sample time is reasonable
SampleTimer += SampleTime;
}
else { //sample time is not reasonable, reset it
SampleTimer = now;
Initialize(0);
}
/*Compute all the working error variables*/
error = *mySetpoint - *myInput;
dInput = (*myInput - lastInput);
//apply a light low Pass filter to smooth derivative
d0 = d1;
d1 = (c1 * dInput) + (c2 * d0);
dInput = d0 + d1;
//Test to see if the output is clamped to determine if integral should add any
if (ki != 0) { //protect against DIV/0
Itest = kp / ki * error; //See what the I term might be
if (kp * error + (ISum + Itest) - kp * kd * dInput < outMax) ISum += Itest; //calculate what the final output MIGHT be if this term is added
}
else { //clear out Isum when ki is 0. The user may be messing around with settings and its logical to assume they want it cleared out when they set Ki to 0
ISum = 0;
}
ISum = constrain(ISum, outMin, outMax);
/*Compute Rest of PID Output*/
// P + I + D
double output = kp * error + ISum - kp * kd * dInput;
*myOutput = constrain(output, outMin, outMax);
/*Remember some variables for next time*/
lastInput = *myInput;
return true;
}
else {
return false;
}
}
/* SetTunings(...)*************************************************************
* This function allows the controller's dynamic performance to be adjusted.
* it's called automatically from the constructor, but tunings can also
* be adjusted on the fly during normal operation
******************************************************************************/
void PID::SetTunings(double Kp, double Ki, double Kd, int POn)
{
if (Kp<0 || Ki<0 || Kd<0) return;
pOn = POn;
pOnE = POn == P_ON_E;
dispKp = Kp; dispKi = Ki; dispKd = Kd;
double SampleTimeInSec = ((double)SampleTime)/1000;
kp = Kp;
ki = Ki / SampleTimeInSec;
kd = Kd / SampleTimeInSec;
if(controllerDirection ==REVERSE)
{
kp = (0 - kp);
ki = (0 - ki);
kd = (0 - kd);
}
}
/* SetTunings(...)*************************************************************
* Set Tunings using the last-rembered POn setting
******************************************************************************/
void PID::SetTunings(double Kp, double Ki, double Kd){
SetTunings(Kp, Ki, Kd, pOn);
}
/* SetSampleTime(...) *********************************************************
* sets the period, in Milliseconds, at which the calculation is performed
******************************************************************************/
void PID::SetSampleTime(int NewSampleTime)
{
if (NewSampleTime > 0)
{
double ratio = (double)NewSampleTime
/ (double)SampleTime;
ki *= ratio;
kd /= ratio;
SampleTime = (unsigned long)NewSampleTime;
}
}
/* SetOutputLimits(...)****************************************************
* This function will be used far more often than SetInputLimits. while
* the input to the controller will generally be in the 0-1023 range (which is
* the default already,) the output will be a little different. maybe they'll
* be doing a time window and will need 0-8000 or something. or maybe they'll
* want to clamp it from 0-125. who knows. at any rate, that can all be done
* here.
**************************************************************************/
void PID::SetOutputLimits(double Min, double Max)
{
if(Min >= Max) return;
outMin = Min;
outMax = Max;
if(inAuto)
{
if(*myOutput > outMax) *myOutput = outMax;
else if(*myOutput < outMin) *myOutput = outMin;
if(ISum > outMax) ISum = outMax;
else if(ISum < outMin) ISum = outMin;
}
}
/* SetMode(...)****************************************************************
* Allows the controller Mode to be set to manual (0) or Automatic (non-zero)
* when the transition from manual to auto occurs, the controller is
* automatically initialized
******************************************************************************/
void PID::SetMode(int Mode)
{
bool newAuto = (Mode == AUTOMATIC);
if(newAuto && !inAuto)
{ /*we just went from manual to auto*/
PID::Initialize(*myOutput);
}
inAuto = newAuto;
}
/* Initialize()****************************************************************
* does all the things that need to happen to ensure a bumpless transfer
* from manual to automatic mode.
******************************************************************************/
void PID::Initialize(double value)
{
// outputSum = *myOutput;
ISum = value - (kp * error) + (kp * kd * dInput);
if (ISum > outMax) ISum = outMax;
else if (ISum < outMin) ISum = outMin;
lastInput = *myInput;
d0 = dInput / 2.0;
d1 = d0;
}
/* SetControllerDirection(...)*************************************************
* The PID will either be connected to a DIRECT acting process (+Output leads
* to +Input) or a REVERSE acting process(+Output leads to -Input.) we need to
* know which one, because otherwise we may increase the output when we should
* be decreasing. This is called from the constructor.
******************************************************************************/
void PID::SetControllerDirection(int Direction)
{
if(inAuto && Direction !=controllerDirection)
{
kp = (0 - kp);
ki = (0 - ki);
kd = (0 - kd);
}
controllerDirection = Direction;
}
/* Status Funcions*************************************************************
* Just because you set the Kp=-1 doesn't mean it actually happened. these
* functions query the internal state of the PID. they're here for display
* purposes. this are the functions the PID Front-end uses for example
******************************************************************************/
double PID::GetKp(){ return dispKp; }
double PID::GetKi(){ return dispKi;}
double PID::GetKd(){ return dispKd;}
int PID::GetMode(){ return inAuto ? AUTOMATIC : MANUAL;}
int PID::GetDirection(){ return controllerDirection;}
EDITS: Had trouble getting the code to paste with correct formatting...
I'm aware of this one. It's a trade-off
can we add a period check to overcome this "trade-off"?
if(timeChange>=SampleTime * 2)
lastTime = now;
else
lastTime += SampleTime;
