11 years, 3 months ago.

struggling with a rotary quadrature encoder and mbed... check my code ?

I must have re=written this code a dozen times now, always convincing myself that I'm making a stupid mistake, but the code never gets any more than barely close to usable...

Stripped to its bare bones I have a single quadrature encoder (2 bit grey code), connected to two pins on the mbed(11u24 version), I have a timer that checks the state of those two pins every 5 milliseconds, and based on the last and currently sensed state of those two pins, it _should_ give me a direction of rotation of the knob accumulated in a register, reading the accumulator register should reset the accumulator. As best as I see it, there are only four states that are valid and a progression from one state to the adjacent state is the only valid transition, and transitions in one direction or the other indicate a rotation in one direction or another. Debounce should not be a big problem if I only check every 5ms.

BUT... I see very random increments and decrements of the direction accumulator, often in the tens of hundreds for one detent on the encoder. I can only think its my code, but I've tried multipel versions of the algorithm ,some with more success than others (edge triggered interrupt driven state machines on the two pins, or as now timer driven polling of the inputs), neither gives me the nice smooth increments of decrements that I need for a simple user interface based on twisting a knob to set values.

here is my bare bones code... any comments welcome.. please.... Oh and it could be a really dumb mistake I'm just not seeing...

include "mbed.h"

DigitalIn qa(p13);      //phase a of the quadrature encoder
DigitalIn qb(p14);      //phase b of the quadrature encoder

Serial pc(USBTX,USBRX);
Ticker qt;              //create a timer to sample the keys
int dir=0;              //hold the signed value corresponding to the number of clicks left or right since last sample
int qslast=0;           //keep a record of the last actioned sample state of the Qb+Qa outputs for comparison on each interrupt

void qdec (void){
    int qas=qa.read();          // read the state of phase a                qa=  __|-----|_____|-----|_____
    int qbs=qb.read();          // read the state of phase b                qb=  -----|_____|-----|_____|-
    int qs=qbs<<1+qas;          // create a two bit value out of qb and qa, qs=  10 11 01 00 10 11 01 00
    switch (qslast) {           // depending on the last actionable state     =   2  3  1  0  2  3  1  0
        case 0:                 // if last state was 0,                            <- left     right->    (direction of know rotation)
            if (qs==1) {        //      and the currently sampled state is 1, then the knob was rotated left, 
                dir--;          //      so subtract one from the diretion accumulator        
                qslast=1;       //      and set the new last state to 1 and  leave the if statement   
            } else if (qs==2) { //      otherwise if the currently sampled state is 2, then the knob was rotated right,
                dir++;          //      so increase the rotation accumulator
                qslast=2;       //      and set the new last state to 
        case 1:
            if (qs==3) {        
            } else if (qs==0) {
        case 2:
            if (qs==0) {
            } else if (qs==3) {
        case 3:
            if (qs==2) {
            } else if (qs==1) {

int getrot() {          // function to read the rotation accumulator, and once read, reset it to zero
    int res=dir;        // this allows the knob to be rotated "while im not looking at it' and still return the 
    dir=0;              // actual number of clicks that have been rotated since last time I was checking it.
    return (res);

int main() {
    int counter=0;

    qa.mode(PullUp);                                    // qa is set with a built in pull-up resistor
    qb.mode(PullUp);                                    // qb is set with a built in pull-up resistor    
    qt.attach_us(&qdec,5000);                           // make the quadrature decoder function check the knob once every 1000us = 1ms
    while (1)                                           // loop forever
        printf("dir = %d,%d\r\n",dir,qslast);           // print the value of the rotation accumulator register
        wait_ms(200);                                   // every 200 ms.

3 Answers

11 years, 3 months ago.

Rob is correct, the breaks are wrong. I also recommend that you add external pullup Rs of 1k instead of the much higher internal pullups. In addition you should add small capacitors of 10n between the inputpins and gnd. This provides much better defined levels and debouncing. Declare the dir and qslast as volatile. They are updated inside an interrupt driven function. The compiler may not understand this and optimise out some of the code.

Accepted Answer

I should have mentioned, the cap and pullups are there too, again didn't seem to make much difference, I used 10k as this will ultimately be a battery driven project, so I'm trying to keep power draws down as much as possible. Thanks for the tip on the volatile declaration.

posted by Peter Wilson 31 Dec 2012
11 years, 3 months ago.

There are already several Quadrature Encoding Interfaces written for the mbed. Have a look at: https://mbed.org/users/aberk/code/QEI/file/5c2ad81551aa/QEI.cpp

11 years, 3 months ago.

The problem with your code is the positioning of your break statements - for any given value of qlast the code will fall through the case statements until it finds a matching value of qs and then it will execute those statements and then break out of the switch statement, so then general arrangemen of your code should be

switch( qLast ) { case 0: if( qs == 1 ) { } else if( qs == 3 ) { } break;

case 1: add code here break; }

A much better implementation would use the mBed edge detection logic

InterruptIn inPhaseA( p5 ); InterruptIn inPhaseB( p6 );

set interrupt on each encoder edge inPhaseA.rise( &encoderEdge ); inPhaseB.rise( &encoderEdge ); inPhaseA.fall( &encoderEdge ); inPhaseB.fall( &encoderEdge );

static void encoderEdge() {

  1. define PREV_MASK 0x1 Mask for the previous state in determining direction of rotation.
  2. define CURR_MASK 0x2 Mask for the current state in determining direction of rotation.
  3. define INVALID 0x3 XORing two states where both bits have changed.

static int previousState = 0;

int currentState = ( inPhaseA.read() << 1 ) | inPhaseB.read();

Entered a new valid state. if( ((currentState ^ previousState) != INVALID) && (currentState != previousState) ) { 2 bit state. Right hand bit of prev XOR left hand bit of current gives 0 if clockwise rotation and 1 if counter clockwise rotation. pulses += (previousState & PREV_MASK) ^ ((currentState & CURR_MASK) >> 1) ? -1 : 1; }

previousState = currentState; }

thanks Rob...

But.. moving the breaks as you recommended doesn't fix the problem. The code is still responding like the transitions are super bouncy and direction is even muddled sometimes (count up or down by tens or even hundreds of counts (usually in the right direction at least, but never by just one 'tick'). In fact sometimes, the counts seem to lock up and continue to assert that I'm spinning the knob when i'm not. Weirdest thing.

My original versions where all edge interrupt driven, and exhibited essentially the same issue.

I have tried at least a couple of encoders from each of three different sources, $0.50 nameless ones from China via E-bay, the few dollar ones from SparkFun and top of the line optical ones at $50 from Grayhill... no real difference in the results, its got to be my code that is the problem.

posted by Peter Wilson 29 Dec 2012


I have tested the following code with my encoder (connected to pins 5 and 6

  1. include "mbed.h"

DigitalIn phaseA( p5 ); DigitalIn phaseB( p6 );

int encoderClickCount = 0; int previousEncoderState = 0;

void quadratureDecoder( void ) { int currentEncoderState = (phaseB.read() << 1) + phaseA.read();

if( currentEncoderState == previousEncoderState ) { return; }

switch( previousEncoderState ) { case 0: if( currentEncoderState == 1 ) { encoderClickCount; } else if( currentEncoderState == 2 ) { encoderClickCount++; } break;

case 1: if( currentEncoderState == 3 ) { encoderClickCount; } else if( currentEncoderState == 0 ) { encoderClickCount++; } break;

case 2: if( currentEncoderState == 0 ) { encoderClickCount; } else if( currentEncoderState == 3 ) { encoderClickCount++; } break;

case 3: if( currentEncoderState == 2 ) { encoderClickCount; } else if( currentEncoderState == 1 ) { encoderClickCount++; } break;

default: break; } previousEncoderState = currentEncoderState; }

read the rotation accumulator, and once read, reset it to zero int getClicks( void ) { int res = encoderClickCount;

encoderClickCount = 0;

return res; }

int main() { Serial pc( USBTX, USBRX ); Ticker sampleTicker; int currentClicks;

phaseA.mode( PullUp ); phaseB.mode( PullUp );

sampleTicker.attach_us( &quadratureDecoder, 1000 );

pc.printf( "Encoder test started\r\n" ); while( 1 ) { currentClicks = getClicks(); if( currentClicks != 0 ) { pc.printf( "click count = %d,%d\r\n", currentClicks, previousEncoderState ); } wait_ms( 200 ); } }

If this code does not work, then you need to check your encoder


posted by Rob Greenhill 30 Dec 2012

I wanted to give your code a try but it looks weirdly formatted, looks like its editted out of a bigger program with some odd formatting and wrong syntax and misssing actions as a result of the pasting probably.

posted by Peter Wilson 31 Dec 2012

Thats what happens when you dont put code tags around it, it is his complete program probably though. But is it now fixed with volatile declaration?

posted by Erik - 31 Dec 2012


ok, I have published the program here



posted by Rob Greenhill 31 Dec 2012