Problem with PID when input is above Setpoint

Hi!

I’m trying to create PID regulated thermocontroller with stepper motor, MAX6675 thermocouple and arduino.

Seems that PID is working well except when stepper motor is fully opened and temperature goes above setpoint, PID does nothing, but I think that it should close stepper.

My logic is:

  1. temperature below setpoint - stepper opened;
  2. temperature above setpoint - stepper closed;
  3. temperature = setpoint - stepper is regulated;

Could somebody take a look on my code, maybe there is some mistake why PID not working as it should.

#include <max6675.h>
#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

//Specify the links and initial tuning parameters
double Kp=2, Ki=5, Kd=1;
PID myPID(&Input, &Output, &Setpoint, Kp, Ki, Kd, DIRECT);
double stepper_min = 0; //closed state
double stepper_max = 103; //opened state

int CLK = 52;
int CS = 22;
int SO = 50;
MAX6675 thermocouple(CLK, CS, SO);

int Step_count = 0;
int Step;



void setup()
{
  Serial.begin(9600);
  while (!Serial) {
     // wait for serial port to connect. Needed for native USB port only
  }
  digitalWrite(CS, LOW);
  delay(500);
  //initialize the variables we're linked to
  myPID.SetOutputLimits(stepper_min, stepper_max);//Set max and min limits
  
  Serial.println(thermocouple.readCelsius());
  Input = thermocouple.readCelsius();
  Setpoint = 100;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);
}

void open_stepper (int Step) {
   Serial.println("Open_stepper ");
   Serial.print("Steps = ");
   Serial.println(Step);

    if (Step_count == 103){
      Serial.println("Opened ");
    }
   
  
    for (int n = 0; n < Step; n++){
      Step_count = ++Step_count;
      //delay(500);
      Serial.println(Step_count);
    } 
    
    if (Step_count == 103){
      Serial.println("Opened ");
    }
 
}

void close_stepper (int Step) {
   Serial.println("close_stepper ");
   Serial.print("Steps = ");
   Serial.println(Step);

  if (Step_count == 0){
    Serial.println("Closed ");
  } 
    
  for (int n = 0; n < Step; n++){
   Step_count = --Step_count;
   //delay(500);
   Serial.println(Step_count);
    
  if (Step_count == 0){
   Serial.println("Closed ");
  }
  }

 }



void loop()
{
  delay(3000);
  Input = thermocouple.readCelsius();
  myPID.Compute();
  Serial.print("PID Output ");
  Serial.println(Output);


   if (Output == 0){
          Step = 0;
        }
        else
        {
           Step = Step_count - Output;
        }
    Serial.print("Actual temperature ");    
    Serial.println(Input);
    Serial.print("Steps that must be turned ");
    Serial.println(Step);
    if (Step < 0){
          Serial.println("To open position ");
          Serial.println(Step); 
            Step = Step * (-1);
            open_stepper (Step); 
            Serial.print("Actual position ");
            Serial.println(Step_count);       
        }
        else if (Step > 0){
          Serial.println("To close position "); 
          Serial.println(Step);
          close_stepper (Step); 
          Serial.print("Actual position ");
          Serial.println(Step_count); 
          
        }
        else {
           Serial.print("Do not turn ");
           Serial.print("Actual position ");
           Serial.println(Step_count);
        }
      }
double stepper_min = 0; //closed state
double stepper_max = 103; //opened state
  myPID.SetOutputLimits(stepper_min, stepper_max);//Set max and min limits

From this it appears that the intent is to directly control the position of the stepper. If that is the case, what the heck is this?

   if (Output == 0) {  // If the stepper should be fully closed...
          Step = 0;  // Don't move!?!
        }
        else
        {
           Step = Step_count - Output;
        }

WARNING! This operation has undefined results:

   Step_count = --Step_count;

Use "--Step_count;" or "Step_count = Step_count - 1;", NOT BOTH![/code]

Thank You!

You were right about this:

  if (Output == 0) {  // If the stepper should be fully closed...
          Step = 0;  // Don't move!?!
        }
        else
        {
           Step = Step_count - Output;
        }

Changed to this:

#include <max6675.h>
#include <PID_v1.h>

//Define Variables we'll be connecting to
double Setpoint, Input, Output;

//Specify the links and initial tuning parameters
double Kp=2, Ki=5, Kd=1;
PID myPID(&Input, &Output, &Setpoint, Kp, Ki, Kd, DIRECT);
double stepper_min = 0; //closed state
double stepper_max = 103; //opened state

int CLK = 52;
int CS = 22;
int SO = 50;
MAX6675 thermocouple(CLK, CS, SO);

int Step_count = 0;
int Step;



void setup()
{
  Serial.begin(9600);
  while (!Serial) {
     // wait for serial port to connect. Needed for native USB port only
  }
  digitalWrite(CS, LOW);
  delay(500);
  //initialize the variables we're linked to
  myPID.SetOutputLimits(stepper_min, stepper_max);//Set max and min limits
  
  Serial.println(thermocouple.readCelsius());
  Input = thermocouple.readCelsius();
  Setpoint = 100;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);
}

void open_stepper (int Step) {
   Serial.println("Open_stepper ");
   Serial.print("Steps = ");
   Serial.println(Step);

    if (Step_count == 103){
      Serial.println("Opened ");
    }
   
  
    for (int n = 0; n < Step; n++){
      Step_count = ++Step_count;
      //delay(500);
     // Serial.println(Step_count);
    } 
    
    if (Step_count == 103){
      Serial.println("Opened ");
    }
 
}

void close_stepper (int Step) {
   Serial.println("close_stepper ");
   Serial.print("Steps = ");
   Serial.println(Step);

  if (Step_count == 0){
    Serial.println("Closed ");
  } 
    
  for (int n = 0; n < Step; n++){
   --Step_count;
   //delay(500);
   //Serial.println(Step_count);
    
  if (Step_count == 0){
   Serial.println("Closed ");
  }
  }

 }



void loop()
{
  delay(3000);
  Input = thermocouple.readCelsius();
  myPID.Compute();
  Serial.print("PID Output ");
  Serial.println(Output);
    Step = Step_count - Output;
    Serial.print("Actual temperature ");    
    Serial.println(Input);
    Serial.print("Steps that must be turned ");
    Serial.println(Step);
    if (Step < 0){
          Serial.println("To open position ");
          Serial.println(Step); 
            Step = Step * (-1);
            open_stepper (Step); 
            Serial.print("Actual position ");
            Serial.println(Step_count);       
        }
        else if (Step > 0){
          Serial.println("To close position "); 
          Serial.println(Step);
          close_stepper (Step); 
          Serial.print("Actual position ");
          Serial.println(Step_count); 
          
        }
        else {
           Serial.print("Do not turn ");
           Serial.print("Actual position ");
           Serial.println(Step_count);
        }
      }

Now everything works great :slight_smile: Thanks!

You have another instance of the undefined behaviour John warned you about:

      Step_count = ++Step_count;