Solving read skip problem- quadrature decoder

Hi everyone
I am having trouble with a quadrature decoder program, it seems to work- but when the encoder is spinning fast, i'm starting to get wrong values (for example, a full revolution yields a count of 1500~ instead of 2048)

Here's the code:

int pulses;
int deg = 0;
int encoderA = 2;
int encoderB = 3;

int pulsesChanged = 0;

void setup(){
Serial.begin(115200);
Serial.println("start");
pinMode(encoderA, INPUT);
pinMode(encoderB, INPUT);

attachInterrupt(0, A_CHANGE, CHANGE);
attachInterrupt(1, B_CHANGE, CHANGE);

}//setup

void loop(){
if (pulsesChanged != 0) {
pulsesChanged = 0;
Serial.println(pulses);
}
}

void A_CHANGE(){
if( digitalRead(encoderB) == 0 ) {
if ( digitalRead(encoderA) == 0 ) {
// A fell, B is low
pulses--; // moving reverse
} else {
// A rose, B is low
pulses++;
}
} else {
if ( digitalRead(encoderA) == 0 ) {
// A fell, B is high
pulses++; // "positive" spin direction
} else {
// A rose, B is high
pulses--; // "negaitve" direction
}
}

// tell the loop that the pulses have changed
pulsesChanged = 1;
}

void B_CHANGE(){
if ( digitalRead(encoderA) == 0 ) {
if ( digitalRead(encoderB) == 0 ) {
// B fell, A is low
pulses++; // pos
} else {
// B rose, A is low
pulses--; // neg
}
} else {
if ( digitalRead(encoderB) == 0 ) {
// B fell, A is high
pulses--; // pos
} else {
// B rose, A is high
pulses++; // neg
}
}

// tell the loop that the pulses have changed
pulsesChanged = 1;
}

My encoder is 2048P/R (though it says 1024 on it, but reality is reality...)
I cracked my head on this and can't find problems with this fairly simple code, it should be working.

I'd appreciate any help with finding issues in the code, or if anyone has advice on how to make changes to accommodate the high rates needed- suggestions are always welcome

Thank you

I cracked my head on this and can't find problems with this fairly simple code, it should be working.

It is. The problem is that every time the interrupt is called, you spend a long time determining the relationship between the pin number and the port and bit to read. That all happens in the convenience routine, digitalRead().

Use direct port manipulation, not digitalRead(), if you want to deal with high interrupt frequencies.

Thank you for your input, i am a complete beginner though and don't know how. Is it possible for you to give an example how to read using said method?
Usually i wouldn't even ask and google until my head falls off, but in this specific case time is running short.

It's good to know there are no mistakes though

Did you search for "direct port manipulation"? The first hit in google is Arduino - PortManipulation

It's a fact that some quadrature encoder manufacturers label their products with a Cycles Per Revolution number (CPR) and call it Pulses Per Revolution (PPR) whereas PPR = CPR * 4 because each "cycle" can produce 2 edges on Phase A and 2 others on Phase B.

For a maximum accuracy, you will have to record pulses on both Phase A and Phase B (1024 * 4 = 4096 PPR). The way to record these edges is an input capture with a Timer Counter. However, depending on the shaft RPM, you may loose some pulses with a "slow" uc (e.g. at 2000 RPM, that's 136533 pulses to record each and every second).

You can try several workarounds:

  • Record only Phase A rising and falling edges may be sufficiently correct if you always turn clockwise (2048 per revolution only)
  • Record only Phase A rising edges (1024 per revolution only)

or skip to a faster uc, see this thread, reply #84:

Use code tags when posting code, as described in the "How to use this forum" post.

To read digital pins 2 & 3 by direct port reads (about 50x faster than digitalRead()), use something like

pin2_val = PIND&(0x04); //PORTD, pin PD2
pin3_val = PIND&(0x08); //PORTD, pin PD3

Ard_newbie and jremington- thank you very much
I tried implementing the advice to my code, it turned out something like this:

int pulses;
int deg = 0;


int pulsesChanged = 0;

void setup(){
  Serial.begin(500000);
  Serial.println("start");
  
  attachInterrupt(0, A_CHANGE, CHANGE);
  attachInterrupt(1, B_CHANGE, CHANGE);

}//setup

void loop(){
  if (pulsesChanged != 0) {
    pulsesChanged = 0;
    Serial.println(pulses);
  }
}

void A_CHANGE(){
  if( PIND&(0x08)== 0 ) {
    if ( PIND&(0x04) == 0 ) {
      // A fell, B is low
      pulses--; // moving reverse
    } else {
      // A rose, B is low
      pulses++; 
    }
 } else {
    if ( PIND&(0x04) == 0 ) {
      // A fell, B is high
      pulses++; // "positive" spin direction
    } else {
      // A rose, B is high
      pulses--; // "negaitve" direction
    }
  }

 
  // tell the loop that the pulses have changed
  pulsesChanged = 1;
}

void B_CHANGE(){
  if ( PIND&(0x04) == 0 ) {
    if ( PIND&(0x08) == 0 ) {
      // B fell, A is low
      pulses++; // pos
    } else {
      // B rose, A is low
      pulses--; // neg
    }
 } else {
    if ( PIND&(0x08) == 0 ) {
      // B fell, A is high
      pulses--; // pos
    } else {
      // B rose, A is high
      pulses++; // neg
    }
  }


  // tell the loop that the pulses have changed
  pulsesChanged = 1;
}

Still not really working, might be even worse than before.
Is there something wrong here or might it be just a hardware limitation?

I see only one thing wrong with the code, but don't see how that could cause the problem.

The "wrong thing" is that the variable pulsesChanged is multibyte, and might be modified by the interrupt during a read operation in the main loop. To be safe in multibyte reads of an interrupt shared variable, you need to turn the interrupts off during the read operation. Or just use a byte variable.

How fast is "really fast"? State the RPM at which you observe the problem.

Does the program EVER skip steps at lower speeds?

Please post a link to the product page or data sheet for the exact encoder you are using.

I'm having a hard time seeing why pulsesChanged is an int, rather than a boolean. There was a pulse, or there wasn't. That's what the variable is saying.

Variables shared by the ISRs and loop() (or other functions) need to be declared volatile, or the loop() function COULD use a cached value, rather than the one just updated by the ISR.

Oops, I missed the lack of the required "volatile" keyword!

Try this instead:

volatile int pulses = 0; //recommend long here
volatile byte pulsesChanged = 0;

jremington:
Oops, I missed the lack of the required "volatile" keyword!

I did too, the first time I read the code.

Thank you, everyone
Paul- the variable was an int instead of a boolean (or byte) because i simply didn’t think about it.
The program didn’t skip at lower speeds, right now after deciding to drop one direction and changing the variables, it seems to work at higher speeds as well.

“Really fast” is about 3000rpm, the encoder i’m using is this one- Rotary Encoder - 1024 P/R (Quadrature) - COM-11102 - SparkFun Electronics
(It says 1024p/r even on the product itself, it’s 2048 in reality)

The code now looks like this-

//Version 3

volatile long pulses = 0;
int deg = 0;
//int rps= 0; //(for next stage)

void setup(){
  Serial.begin(1000000);
  Serial.println("start");
  attachInterrupt(1, B_CHANGE, CHANGE);
}//setup

void loop(){
  
  if (pulsesChanged != 0) {
    pulsesChanged = 0;
    Serial.println(pulses);

  }
}




void B_CHANGE(){
  if ( PIND&(0x04) == 0 ) {
    if ( PIND&(0x08) == 0 ) {
      // B fell, A is low
      pulses++; // moving forward
    } else {
      // B rose, A is low
      pulses--; // moving reverse
    }
 } else {
    if ( PIND&(0x08) == 0 ) {
      // B fell, A is high
      pulses--; // moving reverse
    } else {
      // B rose, A is high
      pulses++; // moving forward
    }
  }
   Make sure the pulses are between 0 and 2048
  if (pulses > 2047) {
    pulses = pulses - 2048;
  } else if (pulses < 0) {
    pulses = pulses + 2048;
  }

  // tell the loop that the pulses have changed
  pulsesChanged = 1;
}

It works well for displaying the number of pulses (resets to 0 at 2048)
The next step will be somehow getting an RPM measurement from this, i’ll have to change the loop to have a fixed time instead of triggering for every single signal

3000rpm with 2048 counts per revolution is 102,400 counts per second.

You can just about get away with this if you only interrupt on one of the pins CHANGEing, and
resign yourself to 1024 counts per revolution as a consequence. Direct port manipulation is
a requirement for this kind of speed, each digitalRead will take several microseconds.

Hey mark, please see the code in my last post, the program already uses direct port manipulation. This certain problem seems to have been resolved.
The next step is understanding how do i manage to get an RPM meter out of this, since i can not use the loop while it's triggered by the interrupts- i will need to loop a time interval, and figure how can i could the interrupts within each interval
When i fix the code to work like that i will post it here for further advice, which i have no doubt i will need....