Strange encoder behaviour with PID

Hi,

Im trying to control the position of a DC motor (EMG30) with the rear shaft mounted encoder. The board used is an arduino Uno with a microbot dc motor shield driving the motor. Initially the code I was using to read the quadrature encoder used if statements however I have since adjusted this to use interrupts. When using if statements the motor would not intially turn unless by hand annd then get stuck at full speed where the encoder would fluctuate between 1 or 2 bits (Assuming this was missed digital reads). However once the motor had reached the setpoint, by hand turning or of its own accord it was responding as one would expect to disturbances.

When using interrupts however this same fluctating behaviour appears when being hand turned regardless of where the setpoint is. Now it cannot reach the setpoint.

To conclude the main issues are:

Encoder wont increment up to setpoint

Motor will not self start

#include <motorshield.h>
#include <PID_v1.h>
#define pinA    2
#define pinB    3

motorshield MotorShield;
double kp = 2 , ki =0 , kd =0 ;             // modify for optimal performance
double input = 0, output = 0, setpoint;
long temp;
int encoderPos =0;
PID myPID(&input, &output, &setpoint, kp, ki, kd, REVERSE);  // if motor will only run at full speed try 'REVERSE' instead of 'DIRECT'


void setup() {
  MotorShield.initialize(); // initialize the Motor Shield hardware  
  TCCR1B = TCCR1B & 0b11111000 | 1;                   // set 31KHz PWM to prevent motor noise
  myPID.SetMode(AUTOMATIC);
  myPID.SetSampleTime(1);
  myPID.SetOutputLimits(-255, 255);
  Serial.begin (115200);                              // for debugging
  pinMode(pinA, INPUT_PULLUP); 
  pinMode(pinB, INPUT_PULLUP); 
  attachInterrupt(digitalPinToInterrupt(2),pa1,CHANGE);
  attachInterrupt(digitalPinToInterrupt(3),pa2,CHANGE);
  encoderPos =0;
}

void loop() {
  setpoint=200;                       // modify to fit motor and encoder characteristics, potmeter connected to A0
  input = encoderPos ;                                // data from encoder 
  Serial.println(input);
  myPID.Compute();                                    // calculate new output
  pwmOut(output);                                     // drive L298N H-Bridge module
}

void pwmOut(int out) {                                // to H-Bridge board
  if (out > 0) {
 MotorShield.setMotorDir(1, 1);
 MotorShield.setMotorSpeed(1, out);
  }
  else {
   MotorShield.setMotorDir(1,0);
   MotorShield.setMotorSpeed(1, abs(out));
  }
}

void pa1()
{
if(digitalRead(pinA) == digitalRead(pinB))
{
  encoderPos++;
 }
else
{
  encoderPos--;
}
}

void pa2()
{
if(digitalRead(pinA)==digitalRead(pinB))
{
  encoderPos--;
 }
else
{
  encoderPos++;
}  
}

Strangely enouugh when the related code is removed the encoder works very well

int encoderPos =0;

If that is updated in an interrupt and read from the other code then shouldn't it be volatile?

if(digitalRead(pinA) == digitalRead(pinB))

If the encoder is spinning very fast then you will probably want to do this with direct port reads instead of digitalRead. It might be too slow.

https://www.arduino.cc/en/Reference/PortManipulation

Delta_G:

int encoderPos =0;

If that is updated in an interrupt and read from the other code then shouldn't it be volatile?

That is just to set the initial position as 0 in the setup. Should it still be volatile? I'll give your other suggestion a try

jaq252:
That is just to set the initial position as 0 in the setup. Should it still be volatile? I'll give your other suggestion a try

  1. It's not in setup, it's at global scope.

  2. The = 0 is for setting it to 0 but the "int encoderPos" part creates a variable that you are using in your ISR and also in your other code

  3. If a variable is used in an ISR and also in normal code then it needs to be volatile.

ok thanks, will make sure to change that. Any idea on the motor not self starting? The controller has an onboard set of LEDs that light up depending on which direction the motor is being driven and theyre usually correct depending on the setpoint