Help with PID controller!

Hey guys,

Quick background. I'm driving 2 DC motors with a dual H-Bridge board. All of it is being controlled by an Arduino Uno. The DC motors have optical encoders on the shaft with 20 divisions per rotation.

I'm trying to spin both motors up to the same RPM using a couple of PID loops. When it's running, the motors seem to pulse (I'm not talking about PWM here). This pulsing on off, which is about 1 - 2 Hz and can be felt with my finger, prevents them spinning up to their full speed or allowing them to be PID controlled as they either can't get near the desired speed (when I have a small delay of 2ms) or they can get near the speed but the sampling rate is too slow (delay of about 100ms) to make for a decet PID controller. This delay is where I measure the RPM of the motors using the interrupts.

It seems like whatever PWM is written at the end of the main loop doesn't stay written on the next loop, so they turn off in between. However, adding another analogWrite at the beginning of the main loop does nothing. Does anyone know why the motors are pulsing like this?

(P.s. I know the code would be way simpler if I understood pointers but I'm not quite there yet)

Any help is much appreciated!

#include <PID_AutoTune_v0.h>
#include <AutoPID.h>
#include <PID_v1.h>

//Motor LEFT
#define enA 9
#define in1 6
#define in2 7
#define encL 2

//Motor RIGHT
#define enB 10
#define in3 5
#define in4 4
#define encR 3

//interrupt variables
volatile long countR = 0;
volatile long countL = 0;
volatile long currentTime;
volatile long previousTimeL = 0;
volatile long previousTimeR = 0;

//RPM Variables
double LRPM;
double RRPM;
long timeL;
long timeR;

//Define Variables we'll be connecting to
double Setpoint;
double InputL, OutputL;
double InputR, OutputR;

//Specify the links and initial tuning parameters
PID LPID(&InputL, &OutputL, &Setpoint,1,0, 0, DIRECT);
PID RPID(&InputR, &OutputR, &Setpoint,1000,0,0, DIRECT);

void setup() {  
  
  //Interrupt pins
  pinMode(encR, INPUT);
  pinMode(encL, INPUT);
  attachInterrupt (0, Lcounting, RISING);   //First argument corresponds to pin 2 as it's the 0th interupt pin
  attachInterrupt (1, Rcounting, RISING);

  //Connecting the address of RPM to input
  InputL = LRPM;
  InputR = RRPM;


  //Target Velocity
  Setpoint = 4000;

  
  //turn the PID on
  LPID.SetMode(AUTOMATIC);
  RPID.SetMode(AUTOMATIC);

  //Motor driver pin initialiasation LEFT
  pinMode(enA, OUTPUT);
  pinMode(in1, OUTPUT);
  pinMode(in2, OUTPUT);

  //Motor driver pin initialiasation RIGHT
  pinMode(enB, OUTPUT);
  pinMode(in3, OUTPUT);
  pinMode(in4, OUTPUT);

  
  // Set initial rotation direction
  digitalWrite(in1, LOW);
  digitalWrite(in2, HIGH);
  digitalWrite(in3, HIGH);
  digitalWrite(in4, LOW);

  Serial.begin(9600);

}

void loop(){
  
  
  //Sample Wheel Speed
  delay(10);


  //detach interrupts
  detachInterrupt(0);           //detaches the interrupt
  detachInterrupt(1);           //detaches the interrupt


  //Read the RPM of the wheels
  LRPM = getRPML();
  RRPM = getRPMR();
  Serial.print(LRPM);
  Serial.print(",");
  Serial.print(Setpoint);
  Serial.print("\n");


  //Perform PID calcs and write for LEFT
  InputL = LRPM;
  LPID.Compute();
  OutputL = (OutputL/1023)*255;
  analogWrite(enA, OutputL);

  
  //Perform PID calcs and write for RIGHT
  InputR = RRPM;
  RPID.Compute();
  OutputL = (OutputR/1023)*255;
  analogWrite(enB, 0);



  //Reattach the interrupt
  attachInterrupt (0, Lcounting, RISING);   
  attachInterrupt (1, Rcounting, RISING);

  


}

long getRPML (){
  long RPM;
  
  timeL = millis() - previousTimeL;                   //finds the time 
  RPM = ((countL*60000)/(timeL*20));                    //calculates rpm
  previousTimeL = millis();                          //saves the current time
  countL = 0;
  return(RPM);
  
}

long getRPMR (){
  long RPM;
  
  timeR = millis() - previousTimeR;                   //finds the time 
  RPM = ((countR*60000)/(timeR*20));                    //calculates rpm
  previousTimeR = millis();                          //saves the current time
  countR = 0;
  return(RPM);
  
}

void Lcounting() { //Known as an ISR - Interrupt Service Routine
  //1. Keep it short
  //2. Dont use delays
  //3. Don't serial print
  //4. Make variables also used in the main code volatile 

  countL++;
}

void Rcounting() { //Known as an ISR - Interrupt Service Routine
  countR++;
}

Why do you use delay at all is the question? Just remember the last time you checked the status of the interrupt and at the next loop (or after a given duration) do a delta with the current time to see how many ms have elapsed. Suspend interrupts just for a critical section where you duplicate the counts in copy variables and reset them, reactivate interrupts and calculate speed using the copies.

The functions delay() and delayMicroseconds() block the Arduino until they complete.
Have a look at how millis() is used to manage timing without blocking in Several Things at a Time.

And see Using millis() for timing. A beginners guide if you need more explanation.

Read data from variables that are updated in an ISR like this

noInterrupts();
   copyOfCountL = countL;
   countL = 0;
   copyOfCountR = countR;
   countR = 0;
interrupts();

then use the copy in your calculations.

Actually there is no need to set the count back to zero - just subtract the previous value from the new value.

There is no need to waste CPU cycles calculating RPM. PID calculations will work just as well with the number of counts within a period.

Have you tried the program with only the code for one motor - does it work better?

...R