PID temperature control troubles

Hi, I am fairly unfamiliar with the Arduino PID library, I have just been going off of the few examples in the Arduino Library. However, I figured I would give it a try before making my own PID controller if I have to (which I’ve done before, just never with C++, I’ve used LabView in combination with Matlab and simulink to develop a temp controller). That being said I am having trouble with the output of this particular PID. I have run several tests with different Kp, Ki, and Kd values but no matter what I use the Output of the PID steadily climbs until it reaches its max where it sits until the programs is manually restarted (in my code this is currently set to 2000) despite how close the input is to the setpoint. This means that my relay eventually is on ~100% of the time which causes the system to go out of control and temperatures would just climb until either the heating element reaches max temp or burns out, or I stop the program from running. I will also note that this PID is controlling a SSR which is why the output ranges from 0-2000. The code is set up so that the relay will be on for a portion of the 2000 ms and off for the rest. If anyone can lend some insight or if my errors are glaringly obvious please point them out. My automatic controls skills are a little rusty to say the least.

Here is my current code:

#include <PID_v1.h>
#include <LiquidCrystal.h>
#include "Adafruit_MAX31855.h"

//Define Variables we'll be connecting to
double Setpoint, Input, Output;
//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint,1,.1,.25, DIRECT);

int WindowSize = 2000;
unsigned long windowStartTime;
int RelayPin = 26;  //sets relay to pin 6
int thermoCLK = 3;
int thermoCS = 4;
int thermoDO = 5;
int up_1 = 22;    //sets switch to pin 22
int down_1 = 23;  //sets switch to pin 23
//int up_10 = 24;   //sets switch to pin 24
//int down_10 = 25; //sets switch to pin 25
int val_1 = 0;    // variable for pin state
int val_2 = 0;    // variable for pin state
//int val_3 = 0;    // variable for pin state
//int val_4 = 0;    // variable for pin state
int desired_temp = 200;  // sets initial desired temp
int currentState_1 = 0;
int previousState_1 = 0;
int currentState_2 = 0;
int previousState_2 = 0;
//int currentState_3 = 0;
//int previousState_3 = 0;
//int currentState_4 = 0;
//int previousState_4 = 0;
// Initialize the Thermocouple
Adafruit_MAX31855 thermocouple(thermoCLK, thermoCS, thermoDO);
// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);
byte deg[8] = {
  B1100,    //binary code created at: http://www.quinapalus.com/hd44780udg.html
  B10010,   //set desired character and insert the binary code created by the website	
  B10010,
  B1100,
  B0,
  B0,
  B0,
};

void setup() {
  windowStartTime = millis();
  pinMode(up_1,INPUT);  //sets up_1 as an input
  pinMode(down_1,INPUT);  //sets down_1 as an input
  pinMode(RelayPin,OUTPUT);  //sets RelayPin as an output
  //pinMode(up_10,INPUT);  //sets up_10 as an input
  //pinMode(down_10,INPUT);  //sets down_10 as an input
  // starts serial communication at 9600 bits per second:
  lcd.createChar(0, deg);
  // set up the LCD's number of columns and rows: 
  lcd.begin(16, 2);
  Serial.begin(9600);
  Setpoint = desired_temp;
  Input = 60;
  //tell the PID to range between 0 and the full window size
  myPID.SetOutputLimits(0, WindowSize);
  //turn the PID on
  myPID.SetMode(AUTOMATIC);
}

void loop() {
  long T_F = thermocouple.readFarenheit();
  Setpoint = desired_temp;
  val_1 = digitalRead(up_1);  //reads input val of up_1
  val_2 = digitalRead(down_1);  //reads input val of down_1
  //val_3 = digitalRead(up_10);  //reads input val of up_10
  //val_4 = digitalRead(down_10);  //reads input val of down_10
  
   if (val_1 == HIGH) {
    currentState_1 = 1;
  }
  else {
    currentState_1 = 0;
  }
  if(currentState_1 != previousState_1) {
  if(currentState_1 == 1) {
  desired_temp = desired_temp + 1;
  delay(150);
  }
  }
  
  if (val_2 == HIGH) {
    currentState_2 = 1;
  }
  else {
    currentState_2 = 0;
  }
  if(currentState_2 != previousState_2) {
  if(currentState_2 == 1) {
  desired_temp = desired_temp - 1;
  delay (150);
  }
  }

//  if (val_3 == HIGH) {
//    currentState_3 = 1;
//  }
//  else {
//    currentState_3 = 0;
//  }
//  if(currentState_3 != previousState_3) {
//  if(currentState_3 == 1) {
//  desired_temp = desired_temp + 10;
//  }
//  }
//  
//  if (val_4 == HIGH) {
//    currentState_4 = 1;
//  }
//  else {
//    currentState_4 = 0;
//  }
//  if(currentState_4 != previousState_4) {
//  if(currentState_4 == 1) {
//  desired_temp = desired_temp - 10;
//  }
//  }

  // set the cursor to column 0, line 0
  // (note: line 0 is the first row, since counting begins with 0):
  lcd.setCursor(0, 0);
  lcd.print("Set Temp");
  // prints the current temp
  lcd.print(desired_temp);
  lcd.write(byte(0));
  lcd.print("F   ");
  lcd.setCursor(0, 1);
  lcd.print("Act Temp");
  // prints the current temp
  lcd.print(T_F);
  lcd.write(byte(0));
  lcd.print("F   ");
  
  double Input = T_F; 
  myPID.Compute();
 
  unsigned long now = millis();
  if(now - windowStartTime>WindowSize)
  { //time to shift the Relay Window
    windowStartTime += WindowSize;
  }
  if(Output > now - windowStartTime) {
  digitalWrite(RelayPin,HIGH);
  }
  else {
  digitalWrite(RelayPin,LOW);
  }
  Serial.println("output");
  Serial.println(Output);
  Serial.println("windowStartTime");
  Serial.println(windowStartTime);
  Serial.println("now");
  Serial.println(now);
  Serial.println("setpoint");
  Serial.println(Setpoint);
}

Thank you in advance for any help provided.

I haven't looked at your code, as you haven't provided a circuit diagram or described the individual components, but one of the most common errors in PID programming is to get a sign wrong on the feedback. Check P first. Put in debugging print statements to see what the error and correction terms actually are.

A schematic diagram showing the cartridge heater, SSR, thermocouple and thermocouple board, and how they are connected to my microcontroller is attached to this post. I did not include the two push-buttons controlling my Setpoint as I know they function as I want them to, I also did not include the LCD hookups.

Heat element with thrmcpl pic.bmp (1.05 MB)

   if (val_1 == HIGH) {
    currentState_1 = 1;
  }
  else {
    currentState_1 = 0;
  }
  if(currentState_1 != previousState_1) {
  if(currentState_1 == 1) {
  desired_temp = desired_temp + 1;
  delay(150);
  }
  }

Am I supposed
to be able
to follow the
logic of
this code
that jumps
all over the place?

You are aware, I think. that the Arduino is programmed in C++, rather than C = C + 1?

Where are previousState_1, previousState_2, etc. assigned values?

Aren't arrays great?

  double Input = T_F; 
  myPID.Compute();

Creating a local variable with the same name as a global variable will lead to frustration every time! Of course, it is NOT this variable that myPID is accessing, you know. Right?

I'm just asking for help. Yes the code is a bit messy as it is still under development. The first bit of code you highlighted has nothing to do with the PID. It allows me to adjust the desired temperature up and down by 1 using two buttons. That portion of the code was actually taken from another example provided on the arduino website. I also mentioned that I am UNFAMILIAR with the PID library from arduino. So no I did not know that it is not actually accessing the variable. If you care to explain I would appreciate it.

The first bit of code you highlighted has nothing to do with the PID.

Really? The setpoint has nothing to do with PID? That's hard to believe.

IN any case, PID does NOT exclude the Use of Tools + Auto Format, so that your code IS readable. Putting each { on a new line, and each } on a line by itself helps, too.

So no I did not know that it is not actually accessing the variable.

How could it? You aren't passing that variable to the Compute() method. You HAVE told the myPID instance to use the global variable of the same name.

The point is that there are bazillions of names to choose from for variables. Using the same name for two DIFFERENT variables just isn't a good idea.

There was a character on the Bob Newhart show, in the 70s, named Darryl. Darryl had two brothers, both named Darryl. The reason was to allow for jokes involving the wrong Darryl being expected to do/know something. It would not have been possible with Darryl, Bob, and Jack. Your code isn't supposed to be funny, so Input should not have a brother named Input.

Okay even if the setpoint is never touched and just left at the default 200, as I have it set, the PID doesn't work. I can confirm it doesn't have to do with the code for my buttons or setting a desired temp. Also as far as the variable previous_state_1 and previous_state_2 are set at the beginning of the code. The switches are normally open so they are set to 0.

How could it? You aren't passing that variable to the Compute() method. You HAVE told the myPID instance to use the global variable of the same name.

Are you saying not to set my Input = T_F and my Setpoint = desired_temp. (because that doesn't make sens to me)

Please excuse me if I seem stupid to you, but there is obviously something that I'm missing. All I have to go by are the couple examples on the arduino site for the PID library. I'm new to C++ and the arduino PID library.

Are you saying not to set my Input = T_F

No. I’m saying NOT to create a new variable.

   Input = T_F;

and

   double Input = T_F:

do different things. The first is right. The second (which you are doing) is wrong.

Also as far as the variable previous_state_1 and previous_state_2 are set at the beginning of the code. The switches are normally open so they are set to 0.

Then, the names are crap. They do NOT reflect the previous state of anything. They represent the setup state. The previous state, when loop() runs the second time would be the state that was read the previous time. That means that previousState_n needs to be set to the current reading at the end of loop.

Please excuse me if I seem stupid to you, but there is obviously something that I’m missing.

“I’m not getting something” is NOT the same as “I’m stupid”. Don’t go there. If, after reading this, you still don’t get something, I’ll keep trying.

PaulS is correct. The PID routine is accessing the global variable Input, which may or may not be initialized to some value (if so, it is probably zero) and never changes. You have defined a local variable also named Input, which is invisible to the PID routines.

I think this explains the observed behavior of the algorithm, but if you had taken a moment to print out the error term as I suggested, the problem would have instantly been obvious.

Jremington. I am so new to this (C++ mainly) and clearly have troubles sometimes. I did try what you suggested. I looked at the display functions for the PID library and I wasn't able to get them to print the error to my serial monitor. I realize I can be a bit slow with this but I am just trying to learn. Honestly the first time I have ever used C++ was for arduino which I just got not too long ago. In addition to that my programming experience is extremely limited. So where should I put this part of the code?:

PID myPID(&Input, &Output, &Setpoint,1,.1,.25, DIRECT);

Also since I want my input to be the temperature reading from the thermocouple should I just use "T_F" as my input variable. And since I want my setpoint to be the variable I call "desired_temp" should I also just set my setpoint as desired_temp?

So where should I put this part of the code?:

It's in the correct place now. Don't move it.

Also since I want my input to be the temperature reading from the thermocouple should I just use "T_F" as my input variable. And since I want my setpoint to be the variable I call "desired_temp" should I also just set my setpoint as desired_temp?

You could. Or you can assign values to SetPoint and Input, rather than defining new variables that you assign values to, that don't get used.

Again, I have NOT checked all the way through your code, but PaulS originally pointed out a major difficulty, namely the keyword "double" in the line

   double Input = T_F;

get rid of it, and the value of T_F will be stored instead in the global variable Input.

Now, as to the PID algorithm itself, and how to debug it. A little bit of investigation will take you a long way, and it isn't that hard! The code for PID::compute is all of about 15 lines long, and I'm taking a moment to reproduce the single line of that code that comprises the heart of the algorithm, below. Since you have some experience programming controllers, do you recognize any of the terms in that line?

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

If I were curious as to how well this code were functioning, immediately after the line beginning with "double output",
I would insert a couple of lcd.print or Serial.print statements to display the contents of the variables dInput, error, ITerm and output.

Now, as to the PID algorithm itself, and how to debug it. A little bit of investigation will take you a long way, and it isn’t that hard! The code for PID::compute is all of about 15 lines long, and I’m taking a moment to reproduce the single line of that code that comprises the heart of the algorithm, below. Since you have some experience programming controllers, do you recognize any of the terms in that line?

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

Yes that would be the equation that determines the duty-cycle for a PWM output. It incorporates the three components of a PID controller. The proportional component which is based on error (dif between actual and desired value), then the integral which is based on the integral of the error over time, and lastly the derivative component is based on the change in error Although it looks to be in a slightly different form than I have seen before.

Also I got it to work with some rough Kp, Ki, Kd parameters and the following revised code (thank you for the help so far):

#include <PID_v1.h>
#include <LiquidCrystal.h>
#include "Adafruit_MAX31855.h"

//Define Variables we'll be connecting to
double desired_temp, T_F, Output;
//Specify the links and initial tuning parameters
PID myPID(&T_F, &Output, &desired_temp,2,.1,.2, DIRECT);

unsigned long WindowSize = 2000;
unsigned long windowStartTime;
int RelayPin = 26;  //sets relay to pin 6
int thermoCLK = 3;
int thermoCS = 4;
int thermoDO = 5;
int up_1 = 22;    //sets switch to pin 22
int down_1 = 23;  //sets switch to pin 23
//int up_10 = 24;   //sets switch to pin 24
//int down_10 = 25; //sets switch to pin 25
int val_1 = 0;    // variable for pin state
int val_2 = 0;    // variable for pin state
//int val_3 = 0;    // variable for pin state
//int val_4 = 0;    // variable for pin state
//int desired_temp = 200;  // sets initial desired temp
int currentState_1 = 0;
int previousState_1 = 0;
int currentState_2 = 0;
int previousState_2 = 0;
//int currentState_3 = 0;
//int previousState_3 = 0;
//int currentState_4 = 0;
//int previousState_4 = 0;
// Initialize the Thermocouple
Adafruit_MAX31855 thermocouple(thermoCLK, thermoCS, thermoDO);
// initialize the library with the numbers of the interface pins
LiquidCrystal lcd(7, 8, 9, 10, 11, 12);
byte deg[8] = {
  B1100,    //binary code created at: http://www.quinapalus.com/hd44780udg.html
  B10010,   //set desired character and insert the binary code created by the website	
  B10010,
  B1100,
  B0,
  B0,
  B0,
};

void setup() {
  windowStartTime = millis();
  pinMode(up_1,INPUT);  //sets up_1 as an input
  pinMode(down_1,INPUT);  //sets down_1 as an input
  pinMode(RelayPin,OUTPUT);  //sets RelayPin as an output
  //pinMode(up_10,INPUT);  //sets up_10 as an input
  //pinMode(down_10,INPUT);  //sets down_10 as an input
  // starts serial communication at 9600 bits per second:
  lcd.createChar(0, deg);
  // set up the LCD's number of columns and rows: 
  lcd.begin(16, 2);
  Serial.begin(9600);
  T_F = thermocouple.readFarenheit();
  desired_temp = 200;
  //tell the PID to range between 0 and the full window size
  myPID.SetOutputLimits(0, WindowSize);
  //turn the PID on
  myPID.SetMode(AUTOMATIC);
}

void loop() {
  T_F = thermocouple.readFarenheit();
  
  val_1 = digitalRead(up_1);  //reads input val of up_1
  val_2 = digitalRead(down_1);  //reads input val of down_1
  //val_3 = digitalRead(up_10);  //reads input val of up_10
  //val_4 = digitalRead(down_10);  //reads input val of down_10

  if (val_1 == HIGH) {
    currentState_1 = 1;
  }
  else {
    currentState_1 = 0;
  }
  if(currentState_1 != previousState_1) {
    if(currentState_1 == 1) {
      desired_temp = desired_temp + 1;
      delay(150);
    }
  }

  if (val_2 == HIGH) {
    currentState_2 = 1;
  }
  else {
    currentState_2 = 0;
  }
  if(currentState_2 != previousState_2) {
    if(currentState_2 == 1) {
      desired_temp = desired_temp - 1;
      delay (150);
    }
  }

  //  if (val_3 == HIGH) {
  //    currentState_3 = 1;
  //  }
  //  else {
  //    currentState_3 = 0;
  //  }
  //  if(currentState_3 != previousState_3) {
  //  if(currentState_3 == 1) {
  //  desired_temp = desired_temp + 10;
  //  }
  //  }
  //  
  //  if (val_4 == HIGH) {
  //    currentState_4 = 1;
  //  }
  //  else {
  //    currentState_4 = 0;
  //  }
  //  if(currentState_4 != previousState_4) {
  //  if(currentState_4 == 1) {
  //  desired_temp = desired_temp - 10;
  //  }
  //  }

  // set the cursor to column 0, line 0
  // (note: line 0 is the first row, since counting begins with 0):
  lcd.setCursor(0, 0);
  lcd.print("Set Temp");
  // prints the current temp
  lcd.print(desired_temp);
  lcd.write(byte(0));
  lcd.print("F   ");
  lcd.setCursor(0, 1);
  lcd.print("Act Temp");
  // prints the current temp
  lcd.print(T_F);
  lcd.write(byte(0));
  lcd.print("F           ");
 
  myPID.Compute();

  unsigned long now = millis();
  if(now - windowStartTime>WindowSize)
  { //time to shift the Relay Window
    windowStartTime += WindowSize;
  }
  if(Output > now - windowStartTime) {
    digitalWrite(RelayPin,HIGH);
  }
  else {
    digitalWrite(RelayPin,LOW);
  }
    Serial.println("output");
    Serial.println(Output);
  //  Serial.println("windowStartTime");
  //  Serial.println(windowStartTime);
  //  Serial.println("now");
  //  Serial.println(now);
  //  Serial.println("setpoint");
  //  Serial.println(Setpoint);
  
}

however a new problem seems to have arose. The output will climb until the actual temp T_F reaches the desired_temp. The temp overshoots slightly then oscillates about the desired_temp until it stabilizes. Like it should (could be better but that will come with more tuning of the paramters). BUT the output then seems to randomly produce a “nan” error. I don’t believe it is due to a loose wire or intermittent connection as everything is mounted solid and nothing is being moved or vibrating while the system runs.

Again, a few print statements in the right place(s) would shed an enormous amount of light on why NaN should appear!

Again, a few print statements in the right place(s) would shed an enormous amount of light on why NaN should appear!

I am still unsure of how I would get error to print. I see the GetKp(), GetKi(), and GetKd() commands but wont that those just return the initial Kp, Ki, and Kd values? If you could maybe write an example line of how I would go about writing the error to the serial monitor that could be helpful.

How is Output being used to affect the process? Do you understand what meaning the Output value has?

Yes that would be the equation that determines the duty-cycle for a PWM output.

And, yet:

  if(Output > now - windowStartTime) {

you are using Output as though it was somehow related to time.

How is Output being used to affect the process? Do you understand what meaning the Output value has?

Quote
Yes that would be the equation that determines the duty-cycle for a PWM output.
And, yet:
Code:
if(Output > now - windowStartTime) {
you are using Output as though it was somehow related to time.

PaulS I said it earlier, I am basing this off of examples in the arduino library. That is all I had to go by as far as using their PID library goes. The Output is made to be related to time. If you look earlier in the code:

unsigned long WindowSize = 2000;
 myPID.SetOutputLimits(0, WindowSize);

The output is set from 0-WindowSize. Which in my case is 2000. The 2000 representing 2000 ms. The relation between Output and time occurs here:

unsigned long now = millis();
  if(now - windowStartTime>WindowSize)
  { //time to shift the Relay Window
    windowStartTime += WindowSize;
  }
  if(Output > now - windowStartTime)

That is my understanding of the example provided on the arduino website.

I also mentioned that I did get the program functioning.