Missed Count Problem when Reading Incremental Rotary Encoder withArduino Mega

Other Hardware Sensors
1

Hello everyone,

I am trying to use a incremental rotary encoder to measure the rotation of a shaft. The application for measuring human movement so the fastest rotation I am going to be measuring is 500 degrees/s. I am using a ES0S8-1024-6-L-5 and reading it using digital interrupts on an arduino Mega. I have a lot of experience in writing code in MATLAB, but not much experience with arduino. I have cobbled together an arduino script that uses the A and B pulses with interrupts to measure angular position based on examples, however I am observing a non-trivial amount of angular drift.

Based on what I have seen on this wonderful message board, it seems like the drift is most likely due to missed counts. Based on my understanding of this process, this is either because either Serial.print or digitalRead.

I found this solution online for the digitalRead issue. I have tried to implement the "digitalReadFast.h" library as suggested, however this has not remedied the issue.

With regards to the Serial.print, I am reading the serial output and displaying it live using a MATLAB GUI. This serial functionality is important for the final use case. Arduinos and rotary encoders seem to be used for much more complicated things than what I am attempting to do so this is really killing me.

My code is below, does anyone have any insight into what this issue is?

Thank you all in advance!

/* wire diagram
 *  red - 5V
 *  black - ground
 *  white - signal digital pin2
 *  green - signal digital pin3
 */
long pulses;
const int encoderA = 2; // black wire
const int encoderB = 3; // white wire
const int encoderZ = 21; // white wire
#include "digitalWriteFast.h" 

const int cortex = 8; // pin for cortex signal for syncing
int trigger = 0;
static const int WRITE_INTERVAL = 20; // 10ms - 100Hz
unsigned long last_millis;

void setup(){
  Serial.begin(115200);
  pinMode(encoderA, INPUT);
  pinMode(encoderB, INPUT);
  pinMode(cortex, INPUT);   
  last_millis = 0;
  attachInterrupt(0, A_CHANGE, CHANGE);
  attachInterrupt(1, B_CHANGE, CHANGE);
 
}//setup

void loop(){
    unsigned long this_millis = millis();
  if (this_millis - last_millis > WRITE_INTERVAL) {
    last_millis = this_millis;
    trigger=digitalReadFast(cortex);
    
    Serial.print(last_millis);
    Serial.print(';');
    Serial.print(trigger);
    Serial.print(';');
    Serial.println(pulses*-0.0876d);
  }
}

void A_CHANGE(){
  if( digitalReadFast(encoderB) == 0 ) {
    if ( digitalReadFast(encoderA) == 0 ) {
      // A fell, B is low
      pulses--; // moving reverse
    } else {
      // A rose, B is low
      pulses++; // moving forward
    }
 } else {
    if ( digitalReadFast(encoderA) == 0 ) {
      // A fell, B is high
      pulses++; // moving forward
    } else {
      // A rose, B is high
      pulses--; // moving reverse
    }
  }
}

void B_CHANGE(){
  if ( digitalReadFast(encoderA) == 0 ) {
    if ( digitalReadFast(encoderB) == 0 ) {
      // B fell, A is low
      pulses++; // moving forward
    } else {
      // B rose, A is low
      pulses--; // moving reverse
    }
 } else {
    if ( digitalReadFast(encoderB) == 0 ) {
      // B fell, A is high
      pulses--; // moving reverse
    } else {
      // B rose, A is high
      pulses++; // moving forward
    }
  }
}
2

One thing I see is pulses is changed in the ISRs. Variables affected by ISRs should have the volatile qualifier added at declaration.

3

And you can't safely read pulses in the main code without disabling interrupts briefly:

  noInterrrupts ();  // defer interrupts for as brief a time as possible:
  long my_pulses = pulses ;  // read all 4 bytes of the pulses variable atomically
  interrupts() ;
  Serial.println(my_pulses*-0.0876d);  // use my copy now that interrupts are flying again.
4

Hi All, Thank you for your response. Based on this feedback, I have changed my code as shown below. Unfortunately, it has not remedied the issue. Is my implementation of your suggestions correct?

/* wire diagram
 *  red - 5V
 *  black - ground
 *  white - signal digital pin2
 *  green - signal digital pin3
 */
long volatile pulses;
const int encoderA = 2; // black wire
const int encoderB = 3; // white wire
const int encoderZ = 21; // white wire
#include "digitalWriteFast.h" 

const int cortex = 8; // pin for cortex signal for syncing
int trigger = 0;
static const int WRITE_INTERVAL = 20; // 10ms - 100Hz
unsigned long last_millis;

void setup(){
  Serial.begin(115200);
  pinMode(encoderA, INPUT);
  pinMode(encoderB, INPUT);
  pinMode(cortex, INPUT);   
  last_millis = 0;
  attachInterrupt(0, A_CHANGE, CHANGE);
  attachInterrupt(1, B_CHANGE, CHANGE);
 
}//setup

void loop(){
    noInterrupts ();  // defer interrupts for as brief a time as possible:
    unsigned long this_millis = millis();
    interrupts() ;
  if (this_millis - last_millis > WRITE_INTERVAL) {
    noInterrupts ();  // defer interrupts for as brief a time as possible:
    last_millis = this_millis;
    trigger=digitalReadFast(cortex);
    interrupts() ;
    
    Serial.print(last_millis);
    Serial.print(';');
    Serial.print(trigger);
    Serial.print(';');
    Serial.println(pulses*-0.0876d);
  }
}

void A_CHANGE(){
  if( digitalReadFast(encoderB) == 0 ) {
    if ( digitalReadFast(encoderA) == 0 ) {
      // A fell, B is low
      pulses--; // moving reverse
    } else {
      // A rose, B is low
      pulses++; // moving forward
    }
 } else {
    if ( digitalReadFast(encoderA) == 0 ) {
      // A fell, B is high
      pulses++; // moving forward
    } else {
      // A rose, B is high
      pulses--; // moving reverse
    }
  }
}

void B_CHANGE(){
  if ( digitalReadFast(encoderA) == 0 ) {
    if ( digitalReadFast(encoderB) == 0 ) {
      // B fell, A is low
      pulses++; // moving forward
    } else {
      // B rose, A is low
      pulses--; // moving reverse
    }
 } else {
    if ( digitalReadFast(encoderB) == 0 ) {
      // B fell, A is high
      pulses--; // moving reverse
    } else {
      // B rose, A is high
      pulses++; // moving forward
    }
  }
}
5

frank99:

long volatile pulses;

I don't know how the compiler reacts to this but, I've only ever seen it done with volatile as the first term on the line.

6

It compiles and I was able to upload it to the mega. Thanks!

7

Hello All,

I made some changes based on your feedback as well as other posts I have seen on this forum. I am still observing the angular drift unfortunately. I have included my updated code below. Does anyone see any glaring issues?

Thanks in advance!

/* wire diagram
 *  red - 5V
 *  black - ground
 *  white - signal digital pin2
 *  green - signal digital pin3
 */
volatile long pulses;

const int encoderA = 2; // black wire
const int encoderB = 3; // white wire

#include "digitalWriteFast.h" ;

const int cortex = 8; // pin for cortex signal for syncing
int trigger = 0;
static const int WRITE_INTERVAL = 20; // 10ms - 100Hz
unsigned long volatile last_millis;

void setup(){
  Serial.begin(115200);
  pinMode(encoderA, INPUT);
  pinMode(encoderB, INPUT);
  pinMode(cortex, INPUT);   
  last_millis = 0;
  attachInterrupt(digitalPinToInterrupt(encoderA), A_CHANGE, CHANGE);
  attachInterrupt(digitalPinToInterrupt(encoderB), B_CHANGE, CHANGE);
 
}//setup

void loop(){
    noInterrupts ();  // defer interrupts for as brief a time as possible:
    unsigned long this_millis = millis();
    interrupts() ;
  if (this_millis - last_millis > WRITE_INTERVAL) {
    noInterrupts ();  // defer interrupts for as brief a time as possible:
    last_millis = this_millis;
    trigger=digitalReadFast(cortex);
    interrupts() ;
    
    Serial.print(last_millis);
    Serial.print(';');
    Serial.print(trigger);
    Serial.print(';');
    Serial.println(pulses); // to convert to degrees, multiply by *-0.0876d
    
    
  }
}

void A_CHANGE(){
  if( digitalReadFast(encoderA) != digitalReadFast(encoderB) ) {
    pulses ++;
  } else {
    pulses --;     
  }
}

void B_CHANGE(){
  if( digitalReadFast(encoderA) == digitalReadFast(encoderB) ) {
    pulses ++;
  } else {
    pulses --;     
  }
}
8

You are disabling interrupts when you don't need to, and may miss pulses. You are not making a protected copy of pulses. Printing at 100HZ seems very fast. What are you trying to do?

/* wire diagram
    red - 5V
    black - ground
    white - signal digital pin2
    green - signal digital pin3
*/
volatile long pulses;
long copy_pulses;

const int encoderA = 2; // black wire
const int encoderB = 3; // white wire

#include "digitalWriteFast.h" ;

const int cortex = 8; // pin for cortex signal for syncing
int trigger = 0;
static const int WRITE_INTERVAL = 20; // 10ms - 100Hz
//unsigned long volatile last_millis;
unsigned long last_millis;

void setup() {
  Serial.begin(115200);
  pinMode(encoderA, INPUT);
  pinMode(encoderB, INPUT);
  pinMode(cortex, INPUT);
  last_millis = 0;
  attachInterrupt(digitalPinToInterrupt(encoderA), A_CHANGE, CHANGE);
  attachInterrupt(digitalPinToInterrupt(encoderB), B_CHANGE, CHANGE);

}//setup

void loop() {
  if (millis() - last_millis > WRITE_INTERVAL) {
    last_millis = millis();//this_millis;
    trigger = digitalReadFast(cortex);
    noInterrupts() ;
    copy_pulses = pulses;
    interrupts();

    Serial.print(last_millis);
    Serial.print(';');
    Serial.print(trigger);
    Serial.print(';');
EDIT : Correct error in posting
     Serial.println(copy_pulses);     
    //Serial.println(pulses); // to convert to degrees, multiply by *-0.0876d
  }
}

void A_CHANGE() {
  if ( digitalReadFast(encoderA) != digitalReadFast(encoderB) ) {
    pulses ++;
  } else {
    pulses --;
  }
}

void B_CHANGE() {
  if ( digitalReadFast(encoderA) == digitalReadFast(encoderB) ) {
    pulses ++;
  } else {
    pulses --;
  }
}
9

Thanks for your reply cattledog!

I introduced those interrupts based on previous feedback from earlier in the thread, however, as you noted, I don't think this is addressing the problem.

I doing serial prints at that frequency in order to log and display the data using a MATLAB GUI. I am interested in measuring data about human movement (both position and velocity) so I am attempting to collect data at 100Hz which seems to be the standard in that field.

I saw your response about rotary encoders where you mentioned that keeping updating the count from a previous value was important, however I didn't implement it because I thought adding additional steps would increase the chance for missing counts. Do you think that would help?

Thank you very much for your perspective on this!

cattledog:
Good job with the code tags on your first post.
You can read the encoder to count 1,2, or 4 of the available quadrature transitions. The resolution is a design choice for you to make. Robin2 suggests a routine to read 1 of the 4 transitions. Your original code was to count all 4 transitions so I have kept with that approach although the resolution sounds high(but achievable) for 330rpm. What is your application, and what are you going to do with the count?

There are many things wrong with your code. The switching of interrupt modes(RISING/FALLING) is wrong and can lead to problems. Use CHANGE and read the pin.

if (B = 1) {

You have several statements like this, which are wrong for two reasons. You must use == for the comparison, and you need to read the value of the B pin with digitalRead(B); There are faster routines than digitalRead() and I have used one of them in the code below.

 if (count != count)

To control you output, you need to use count and previous count, and assign the value of count to previousCount after the comparison. You also need to briefly stop the interrupts to grab a value of count which will not change as it is being read.

Here's a modified version of your code which addresses these issues.

const byte encoderPinA = 2;//outputA digital pin2

const byte encoderPinB = 3;//outoutB digital pin3
volatile int count = 0;
int protectedCount = 0;
int previousCount = 0;

#define readA bitRead(PIND,2)//faster than digitalRead()
#define readB bitRead(PIND,3)//faster than digitalRead()

void setup() {
  Serial.begin (115200);

pinMode(encoderPinA, INPUT_PULLUP);
  pinMode(encoderPinB, INPUT_PULLUP);
 
  attachInterrupt(digitalPinToInterrupt(encoderPinA), isrA, CHANGE);
  attachInterrupt(digitalPinToInterrupt(encoderPinB), isrB, CHANGE);
}

void loop() {
  noInterrupts();
  protectedCount = count;
  interrupts();
 
  if(protectedCount != previousCount) {
    Serial.println(protectedCount);
  }
  previousCount = protectedCount;
}

void isrA() {
  if(readB != readA) {
    count ++;
  } else {
    count --;
  }
}
void isrB() {
  if (readA == readB) {
    count ++;
  } else {
    count --;
  }
}

10

I made an error in my earlier post. Work with the protected copy of the interrupt value.

//Serial.println(pulses); // to convert to degrees, multiply by *-0.0876d
Serial.println(copy_pulses); // to convert to degrees, multiply by *-0.0876d

I saw your response about rotary encoders where you mentioned that keeping updating the count from a previous value was important, however I didn't implement it because I thought adding additional steps would increase the chance for missing counts. Do you think that would help?

I'm not sure what you mean by this?

Is the "drift" corrected? If not, can you document what you are seeing? Does the "drift" occur with both fast and slow rotation?