PID dc motor control

Hi,
I got a 24v DC motor with an encoder that produces 360 pulses for every revolution and goes around 3800 rpm. I had hooked up a L293 chip and using the PWM from the arduino I am controlling the rpm. I got a reading from the encoder using interrupts every 500ms and everything is good so far. When i tried to use the PID library from the site and maintain the rpm steady but it seems that it doesn’t work and the motor keeps starting and stopping every 500ms. Does anyone know how could i improve my project?

my motor specs
http://www.ebay.com/itm/JAPAN-SERVO-DS48BE25-2-DC-SERVO-MOTOR-ENCODER-/250400518741?pt=LH_DefaultDomain_0&hash=item3a4d08b255#ht_1922wt_1180
my l293 kit

code:

#include <PID_v1.h>
#include <TimedAction.h>

#define InA1            10                  // INA motor pin
#define InB1            11                  // INB motor pin
#define PWM1            6                   // PWM motor pin
#define sensor          2                   //The pin location of the sensor

TimedAction timedAction = TimedAction(500,blink);                            
unsigned long time;
double Setpoint, Input, Output,Calc,pulses;
//Define the aggressive and conservative Tuning Parameters
double aggKp=4, aggKi=0.2, aggKd=1;
double consKp=1, consKi=0.05, consKd=0.25;
//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint, consKp, consKi, consKd, DIRECT);

void rpm ()                                //This is the function that the interupt calls
{
  pulses++;
}

void setup()
{
  pinMode(InA1, OUTPUT);
  pinMode(InB1, OUTPUT);
  pinMode(PWM1, OUTPUT);
  pinMode(pulses, INPUT);
  digitalWrite(pulses, HIGH);
  Serial.begin(9600);
  attachInterrupt(0, rpm, RISING);
   //initialize the variables we're linked to
  Setpoint =1000;
  //turn the PID on
  myPID.SetMode(AUTOMATIC);
}
void loop ()
{
  pulses = 0; //Set pulses to 0 ready for calculations
  sei(); //Enables interrupts
  timedAction.check();

  
 }
void motorForward(int PWM_val) 
{
  analogWrite(PWM1, PWM_val);
  digitalWrite(InA1, LOW);
  digitalWrite(InB1, HIGH);
}
void motorBackward(int PWM_val)
{
  analogWrite(PWM1, PWM_val);
  digitalWrite(InA1, HIGH);
  digitalWrite(InB1, LOW);
}

void motorStop()
{
  analogWrite(PWM1, 0);
  digitalWrite(InA1, LOW);
  digitalWrite(InB1, LOW);
}
void blink()
{
  cli(); //Disable interrupts
  Calc = ((pulses * 120)/360); // calculating pulses
  
  Input = Calc;
  
  double gap = abs(Setpoint-Input); //distance away from setpoint
  if(gap<10)
  {  //we're close to setpoint, use conservative tuning parameters
    myPID.SetTunings(consKp, consKi, consKd);
  }
  else
  {
     //we're far from setpoint, use aggressive tuning parameters
     myPID.SetTunings(aggKp, aggKi, aggKd);
  }
  
  myPID.Compute();
  motorForward(Output);
  Serial.print("Time: ");
  Serial.println(time);
  Serial.print (" rpm\r\n");
  Serial.println (Calc, DEC); //Prints the number calculated above
  Serial.print (pulses, DEC);//Prints 
  Serial.println (" pulses\r\n");

Because "pulses" is used in an ISR you should declare it 'volatile':

double Setpoint, Input, Output,Calc;
volatile double pulses;

I don't know if that will fix the problem but it can't hurt.

I'd worry that the double Output coming out of the PID loop is NOT an integer value from 0 to 255 suitable for PWM. Is it, perhaps, a value from -1.0 to 1.0? You'd have to read more about the PID_v1 library to find out.

thank you for your interest. I will try it later. found some more mistakes in my code.Will post later.

#include <PID_v1.h>

#define InA1            10                  // INA motor pin
#define InB1            11                  // INB motor pin
#define PWM1            6                   // PWM motor pin
#define sensor          2                   //The pin location of the sensor
                            
unsigned long time;
double Setpoint, Input, Output;
volatile double pulses,Calc;

//Define the aggressive and conservative Tuning Parameters
double aggKp=4, aggKi=0.2, aggKd=1;
double consKp=1, consKi=0.05, consKd=0.25;
PID myPID(&Input, &Output, &Setpoint, consKp, consKi, consKd, DIRECT);


void setup()
{
  pinMode(InA1, OUTPUT);
  pinMode(InB1, OUTPUT);
  pinMode(PWM1, OUTPUT);
  pinMode(pulses, INPUT);
  digitalWrite(pulses, HIGH);
  Serial.begin(115200);
  attachInterrupt(0, rpm, RISING);
  Setpoint =1000;//initialize the variables we're linked to
  SetSampleTime(500); //
  myPID.SetMode(AUTOMATIC);//turn the PID on
}

void loop ()
{
  sei(); //Enables interrupts
  delay(500);
  cli(); //Disable interrupts
  Calc = ((pulses * 120)/360); // calculating pulses
  Input = Calc;
  double gap = abs(Setpoint-Input); //distance away from setpoint
  if(gap<10)
  {  //we're close to setpoint, use conservative tuning parameters
    myPID.SetTunings(consKp, consKi, consKd);
  }
  else
  {
     //we're far from setpoint, use aggressive tuning parameters
     myPID.SetTunings(aggKp, aggKi, aggKd);
  }
  myPID.Compute();
  motorForward(Output);
  Serial.print("Time: ");
  Serial.println(time);
  Serial.print (" rpm\r\n");
  Serial.println (Calc, DEC); 	//Prints the number calculated above
  Serial.print (pulses, DEC);	//Prints pulses
  Serial.println (" pulses\r\n");
  pulses = 0; //Set pulses to 0 ready for calculations

  
 }
void motorForward(int PWM_val) //move forward with val PWM
{
  analogWrite(PWM1, PWM_val);
  digitalWrite(InA1, LOW);
  digitalWrite(InB1, HIGH);
}
void rpm () //This is the function that the interupt calls
{
  pulses++;
}

You still haven’t allowed for the fact that Output is a (signed) double value and analogWrite requires an unsigned input in the range 0 to 255. You need to replace the line:

  motorForward(Output);

with something that does the conversion. This may do the job:

  int temp =(int)Output;    // could use "(int)Output + 128" here instead
  motorForward((temp < 0) ? 0 : (temp > 255) ? 255 : temp);

my code isn’t working the way i would like to and the output of Compute() is either 0 or 255.

#include <PID_v1.h>
#include <TimedAction.h>
TimedAction timedAction = TimedAction(125,blink);//125

#define InA1            10                  // INA motor pin
#define InB1            11                  // INB motor pin
#define PWM1            6                   // PWM motor pin
#define hallsensor      2                   //The pin location of the sensor

int Calc;                                
volatile double Pulses;
//Define Variables we'll be connecting to
double Setpoint, Input, Output;


//Specify the links and initial tuning parameters
PID myPID(&Input, &Output, &Setpoint,2,2,1, DIRECT);

void setup()
{
  pinMode(InA1, OUTPUT);
  pinMode(InB1, OUTPUT);
  pinMode(PWM1, OUTPUT);
  pinMode(hallsensor, INPUT);
  digitalWrite(hallsensor, HIGH);
  //initialize the serial link with processing
  Serial.begin(9600);
  attachInterrupt(0, rpm, RISING);
  //initialize the variables we're linked to
  Setpoint = 1500;
  Input=0;

  //turn the PID on
  myPID.SetMode(AUTOMATIC);
}
void blink()
{
  cli(); //Disable interrupts
  Calc = (Pulses * 1.33333);//1,333
  Input = Calc;
  myPID.Compute();
  int temp =(int)Output;
  motorForward(temp);
  Serial.println (Calc, DEC); //Prints the number calculated above
  Pulses = 0; 
}

void rpm ()                                //This is the function that the interupt calls
{
  Pulses++;
}
void motorForward(int PWM_val) 
{
  analogWrite(PWM1, PWM_val);
  digitalWrite(InA1, LOW);
  digitalWrite(InB1, HIGH);
}
void loop()
{
  sei(); //Enables interrupts
  timedAction.check();
}

You should only disable interrupts for the smallest amount of time possible, ie only when you are accessing the volatile variables that require uninterrupted access. In this case, you should be re-enabling interrupts immediately after you perform your 'Calc'. There's no reason to wait till you are back in Loop().

What is the value of pulses each time blink() is called? If your output isn't what you're expecting, then the next course of action is determine what the input that generates that output looks like. You should also look at the values of Calc and Input as well. You are mixing datatypes in your operations, and how the processor handles mixed operations isn't necessarily intuitive. You often have to be more explicit about how you want those operations to be performed with casts (though, ironically enough the one explicit cast you are doing is redundant).

Also, if Calc = (Pulses * 1.33333);//1,333 is your attempt at calculating the RPM your motor is running at, it is incorrect. A quick google should provide a number of links on how to calculate RPM from an encoder. That's likely the primary source of your problem, as the PID can't compute a reasonable output to reach a setpoint of 1500rpm when the input calculation is incorrect.

Advice:

1) 360 encoder pulses at 3800 rpm is almost 23,000 interrupts per second. That's a lot of ISR calls, which will slow down the processing of the rest of the code and may effect the accuracy of things like millis() and the TimedEvent calls. Keep that in mind when relying on them to do your timing for you. If you get input faster than the arduino can process the interrupts, then you will miss encoder pulses and the measured rpm will top out below the actual rpm. It will also probably bring your sketch to a halt, or at least a crawl. Print out the pulses in your main loop. Make sure they make sense. At 1500 rpm with a sample every 1/8 of a second you should be seeing about 1000 pulses.

2) The best type for Pulses is 'volatile unsigned long', since it is in a ISR, is never negative and is always a whole number. This will cut down the amount of time spent in the ISR as well, since floating point operations take a lot of time compared to ints. Access it like this:

unsigned long temp;
cli()
temp = Pulses;
Pulses = 0;
sei()
// use temp for calculations

3) I understand how you are calculating RPM. It may be off by a millisecond or two because of point #1. You could calculate the interval yourself and then you wouldn't have to re-tune if the timing changes. You could also use the micros() call if you need better timing.

4) You haven't tuned your PID factors. They are probably much too large. First set Ki and Kd to 0, and set Kp to 1. If the output doesn't change fast enough, then Kp is too small. If it swings between 0 and 255, then it is too large. Once you get a decent Kp, then you can add in a Kd to damp oscillations. For your application, you probably don't even want a Ki since you don't care about cumulative error - leave it as 0 unless the rpm starts drifting.

Tuning PID factors is neither simple or intuitive. They are also very sensitive to timing. If the timing of the code changes and your sample rate or the rate at which you read the Pulses is changed then you will have to re-tune them.

5) It is useless to call Compute faster than the SampleTime. The PID library simply ignores data that comes in faster than the sample rate.

Hope this helps,

just to conclude

OP concludes in the PIDLIB group thread about this subject:

OP:
yes i tried and finally it worked. My parameters where far smaller
than i thought so. kp=0,08 and ki-0,2. Working like a charm.