Strange rotary encoder problem

Hi all,

I wrote a small function to read a rotary encoder. It works like a charm on most of the encoders I've tried it with, but some encoders increment or decrement the position value by TWO for each detent rather than just one.

Here's the encoder reading function (code not necessary to understand how this works has been left out for clarity):

#define ENC_A _BV(0) // encoder phase A (on PORTD) (mega pin 21)
#define ENC_B _BV(1) // encoder phase B (on PORTD) (mega pin 20)

volatile int32_t position; // encoder current position
volatile uint8_t state; // internal state of previous and current detent

// note both pin change interrupts are set to
// trigger on both rising and falling edges

// int0 is PORTD, bit 0
ISR (INT0_vect)
{
    updEncoder ();
}

// int1 is PORTD, bit 0
ISR (INT1_vect)
{
    updEncoder ();
}

void updEncoder (void)
{
    cli(); // interrupts off
    state >>= 2; // slide previous state down 2 places
    state |= (ENC_INP & ENC_A) ? 0b0100 : 0; // add in current phase a state
    state |= (ENC_INP & ENC_B) ? 0b1000 : 0; // add in current phase b state
    sei(); // interrupts on

    // now, "state" will have a value between 0 and 15, some of which
    // represent movement to the next detent and direction. Some "state"
    // values are invalid and ignored

    switch (state) {
        case 1: case 7: case 8: case 14: {
            position += 1;
            break;
        }

        case 2: case 4: case 11: case 13: {
            position -= 1;
            break;
        }

        default: { // invalid state, ignore
            break;
        }
    }
}

I can't seem to see anything wrong with the code. The way it works, it naturally rejects bogus bit values. But for some reason I'm getting TWO valid changes per encoder detent. I need a fresh set of eyes, 'cause I'm not seeing it.

Any ideas will be appreciated.

The code counts the gray scale pulses, not the detents. If you look at encoders with a scope you'll see that some have one set of pulses per detent, some have two, some have four, and who knows if there may be others. Just divide by 2 for that encoder and you're square.

By the way there's a WAY easier way to read the encoder. For the interrupt on one pin you just read the other pin. If it matches the pin that fired the interrupt you're going one way and if it doesn't then you're going the other way. For the interrupt on the other pin it will be exactly the same deal but the directions will be reversed.

Delta_G:
The code counts the gray scale pulses, not the detents. If you look at encoders with a scope you'll see that some have one set of pulses per detent, some have two, some have four, and who knows if there may be others. Just divide by 2 for that encoder and you're square.

Geez I feel SO stupid. That is exactly what's happening. The encoder pins (code) change TWICE for each detent. If I carefully turn the knob from detent to "between the detent" i I get a count, then going further to the next detent gives another count.

Why on earth would a manufacturer put mechanical detents into an encoder then output codes BETWEEN them as well?

As far as dividing by 2 to correct the output, I would have to change from an int32_t to a float or double for "position" because at each code change I would have to add or subtract 0.5 instead of 1.

The best bet may be just to toss those oddball encoders in the trash..... :slight_smile:

Delta_G:
By the way there's a WAY easier way to read the encoder. For the interrupt on one pin you just read the other pin. If it matches the pin that fired the interrupt you're going one way and if it doesn't then you're going the other way. For the interrupt on the other pin it will be exactly the same deal but the directions will be reversed.

Could you show me a quick and dirty example of what you mean?

When I first went to write the encoder function, I wrote it to act like a flip flop (with a "clock" and "data" input) because that's how to read an encoder with hardware. One phase goes to data and the other to clock. Then, if you rotate the encoder one way, a high gets clocked in, rotated the other way a low gets clocked in, therefore either the Q or Q-not outputs of the flip-flop tell direction and the pulses from the clock input to the flip-flop are "detent" (code change) events.

But it never worked right. I suspect I had contact bounce because sometimes it worked OK, other times I got several counts for one "detent".

The function I wrote uses the past state and the current state together to VALIDATE the code coming from the encoder. That is, if for example the state is a "2", then I know that the next state MUST either be an "8" or a "1" (depending on rotation direction) and any other code represents an error or a contact bounce and is therefore ignored.

If you draw a table with states from 0 to 15 and then look at each possible combination of outputs from the encoder, you will see that one number always follows a certain number for rotation in one direction and a different number follows for the other direction (and you also see what codes should never occur and you ignore these).

The function is very robust and reliable. I've driven an encoder with a drill motor and it never misses a count. I've pumped in simulated codes made up from digital outputs and blasted them in at several hundred kilohertz and it never misses.

I don't particularly like wasting two interrupts for one encoder, but the result is rock solid and reliable, so I use it.

If you have a better or simpler or cleaner method, I would appreciate seeing it.

Thanks!

krupski:
Could you show me a quick and dirty example of what you mean?

No problem!

Encoder hooked to pins 2 and 3 on an UNO counting all transitions in the gray code.

void attachMeToInt0(){

   if(digitalRead(3) == digitalRead(2)){
        encoderPos++;
   }
   else {
        encoderPos--;
   }
}


void attachMeToInt1(){

     if(digitalRead(3) == digitalRead(2)){
        encoderPos--;
   }
   else {
        encoderPos++;
   }
}

I usually use direct port reads instead of digitalRead, but that should get you the point.

The other cool thing about this is that for your oddball encoders, you just take one interrupt. OR if you have one that does 4 per detent, you take one transition on one interrupt, that is use RISING or FALLING instead of CHANGE.