Rotary Encoders Occasionally reading in reverse

Go To Last Post
20 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Currently have a project going involving 2 rotary encoders. 
I've implemented some hardware denouncing, and any jittering / noise is pretty well eliminated. 
Hardware implementation is based on this scheme, and works pretty well: https://imgur.com/a/1IxKBGV
 

I'm using hardware interrupts for both encoders, and most of the time, they work quite well. 
But, sometime, in the middle of turns, the encoders just start counting backwards instead of forwards, or vice versa. (clockwise instead of counterclockwise, if you prefer)

Is this implementation weird? It's basically just the standard 2 bit grey code implementation, but using one 8bit variable for both encoders. 
Is there anything in this code that could cause reverse reads that I'm just not seeing? 
Is there maybe a more effective/efficient way to do this?

Specs: 
MC: Atmel 2560 / Arduino Mega
Encoder:  https://www.digikey.com/product-detail/en/tt-electronics-bi/EN11-HSM1BF20/987-1398-ND/2620667
Thanks for Reading, 
-hal

/*

 */
#include <avr/io.h>
#include <avr/interrupt.h>

uint8_t topEncoderValue = 0;
uint8_t bottomEncoderValue = 0;
uint8_t topEncoderLastValue = 0;
uint8_t bottomEncoderLastValue = 0;

uint8_t encoderPortStates = 0;
//Bit 0 = pin2 -> bottom encoder PinA
//Bit 1 = pin3 -> bottom encoder PinB
//Bit 2 = pin4 -> top encoder PinA
//Bit 3 = pin5 -> top encoder PinB

//if a bit in this number is high when an interrupt is called,
//then we know we are turning that direction. If not, then set that bit. 

void initEncoders()
{

	//Interrupt pins - 2,3,4,5

	//setup rising edge detection on Int pins 2 and 3
	EICRA |=(1<<ISC31)|(1<<ISC30)|(1 << ISC21)|(1 << ISC20); 

	//same setup on pins 4 and 5
	EICRB |=(1<<ISC51)|(1<<ISC50)|(1 << ISC41)|(1 << ISC40);

	//enable all 4 interrupts through masking
	EIMSK |=(1<<INT2)|(1<<INT3)|(1<<INT4)|(1<<INT5);

}

ISR(INT2_vect)
{

	if(0b00000010&encoderPortStates)//this means Pin 2 is coming after pin 3
	{
		bottomEncoderValue--;
		encoderPortStates&=0b00001100; //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=0b00001101; //we want to set bit 1.
	}
}

ISR(INT3_vect)
{

	if(0b00000001&encoderPortStates)//this means Pin 3 is coming after pin 2
	{
		bottomEncoderValue++;
		encoderPortStates&=0b00001100; //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=0b00001110; //we want to set bit 2.
	}
}

ISR(INT4_vect)
{

	if(0b00001000&encoderPortStates)//this means Pin 4 is coming after pin 5
	{
		topEncoderValue++;
		encoderPortStates&=0b00000011; //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=0b00000111; //we want to set bit three.
	}
}

ISR(INT5_vect)
{

	if(0b00000100&encoderPortStates)//this means Pin 3 is coming after pin 2
	{
		topEncoderValue--;
		encoderPortStates&=0b00000011; //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=0b00001011; //we want to set bit two.
	}

}

void listenEncoders(myStuct inputStruct)
{
	if(topEncoderValue!=topEncoderLastValue)
	{
        //do some stuff to inputStruct
		topEncoderLastValue = topEncoderValue;
	}

	if(bottomEncoderValue!=bottomEncoderLastValue)
	{
        //do some stuff to inputStruct
		bottomEncoderLastValue = bottomEncoderValue;
	}
}

 

Last Edited: Sun. Jun 28, 2020 - 03:47 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
encoderPortStates|=0b00001101; //we want to set bit 1.

For example, here, you are setting multiple bits ..is that always correct, under all conditions...especially if some other bits correspond to the other encoder.

 

Also, bits are usually numbered bit7 down to bit 0 (lsb), but you can call 'em as you like.  Names would be much easier than all these bit paterrns

 

set_my_bits=  (1<< apple)  |  (0<< grape)  |  (1<< banana)  |  (0<< pear)  ...fruit farm

 

set_my_bits=  (0<< apple)  |  (1<< grape)  |  (1<< banana)  |  (1<< pear)  ...easy to read at the farm!!

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Sat. Jun 27, 2020 - 02:51 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I denounce your debounce! All those extra bits are pretty well superfluous. I'd only suggest some 100pF caps to address ESD.

 

Shared variables should be declared volatile. ie: topencodervalue and bottomencodervalue. As well, the is an atomicity issue - you do a test of the top/bottomencodervalue then you do an assignment. The value may change between these two operations.

 

For a twirly knob encoder, do you really need interrupts? The common method is to have a timer tick poll the encoder inputs and decode via a state machine.

Last Edited: Sat. Jun 27, 2020 - 03:28 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

halfordC wrote:
But, sometime, in the middle of turns, the encoders just start counting backwards instead of forwards, or vice versa. (clockwise instead of counterclockwise, if you prefer)

 

Does the direction fix eventually ?

A quad counter in SW usually has 4 states, and can rotate to adjacent states, but if you miss an edge, it can have an illegal transition.

It should recover, but the missed edge is the problem.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

hello,

 

Have you tried this way of deboucing and dealing with rotary encoders?

It seems so smart!

 

Technoblogy Bounce-Free Rotary Encoder

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Again, using external interrupts. Not very robust.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'll question this line:

encoderPortStates|=0b00001101; //we want to set bit 1.

The effect of the line sets more bits than your commented intention.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I want to ask what do you use the encoders for?

 

There have been many encoder threads here, and I will suggest (other than a "real" encoder), forget all about filtering and go for an encoder routine that is robust over for noise (other than count one up and then down again). 

Last Edited: Sat. Jun 27, 2020 - 12:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You don't do interrupts on the inputs. It will never work.

 

Poll the inputs cyclically with a high enough input frequency (depends on the resolution).

 

In depth article (in German, use google translate):  https://www.mikrocontroller.net/articles/Drehgeber .

The code samples should be clear anyhow.

 

br, Thomas

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You don't do interrupts on the inputs. It will never work.

I will not go that far.

But if you interrupt on inputs, you should ignore which input the interrupt came from, and just read both inputs. (use the interrupt only as an event) 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 It will never work.

It can work, because the encoder uses a gray code & only a certain sequence is allowed...so spurious irq's can (will) be ignored.  However, I wouldn't use IRQs either.  A bunch of ignored spurious interrupts, still uses time to reject them.

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

so spurious irq's can (will) be ignored

You can't because they all are legal! (if there is a change from last time you have to make one count in one direction.) 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can't because they all are legal! (if there is a change from last time you have to make one count in one direction.)

I might be out of kilter, but here's my thinking:

You have to go through a specific sequence.  So if you are at (AB) state 11, the next allowable state is either 01 or 10; 00 is not even considered & ignored.

 

AB

01

11  --present

10

 

So if you are at 11 and get another B irq with B=1, there will be no state change unless at that time A is 0.  You can get 1000 B irq's and they will all be ignored by code, until A is 0.

However, since the encoder only allows one bit to change, this effectively means all further B=1 irq's will effectively be ignored by code in the 11 state. The state will transition upwards only when a=0 irq happens.

So B can bounce all it wants, it will have no effect on the state.

 

Likewise, all further A=1 irqs will be ignored by code, only a b=0 irq will change the state downwards.

So A can bounce all it wants, it will have no effect on the state.

 

Do you agree? Am I missing something?  Probably a lack of sleep!

 

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Sun. Jun 28, 2020 - 05:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

if you get another B irq B MUST have been low for some time.

Then it's correct if the spike is shorter than an IRQ time it will not change.

All other cases (over about 1uS) you will give a count, and then when the spike is over you will count back in that IRQ.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

All other cases (over about 1uS) you will give a count, and then when the spike is over you will count back in that IRQ.

I see your excellent point, you might actually twiddle back & forth between two states. 

A thousand bounces wouldn't lose track of the value, you'd ping-pong between the values. 

 

 

 

 

When in the dark remember-the future looks brighter than ever.   I look forward to being able to predict the future!

Last Edited: Sun. Jun 28, 2020 - 06:50 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0


I was surprised that no-one posted a state diagram - so tried to find one. Google was not my friend.

 

The best I could find was this fully featured diagram: Credit where credit's due; it's part of a Cypress Semi Data sheet.

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It has been up many times, back when I used 8051 that is very fast to small LUT's I made it this way:   just add the value in the LUT, I used a timer IRQ

A and B old

a and b new

 

 

ABab  add

0000   0

0001   1

0010  -1

0011  error

0100  -1

0101   0

0110  error 

0111   1

1000   1

1001   error

1010   0

1011   -1

1100   error

1101   -1

1110    1

1111   0

(made on the fly not tested forward is 00 01 11 01)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hey all,

  Thanks for the replies!
I went ahead and made some changes to source based on some suggestions here, and things are running a lot more smoothly. 
Man, dumb mistake setting extra bits. I'm sure that was a decent part of the issue. That has been fixed, as well as using defines for easier reading.  
Still occasionally reading in the wrong direction, but much less frequently than before. 
I'll see if I can make some replies. 

Who-me wrote:

Does the direction fix eventually ?

A quad counter in SW usually has 4 states, and can rotate to adjacent states, but if you miss an edge, it can have an illegal transition.

It should recover, but the missed edge is the problem.

It does after a few turns. What I was finding during testing was turning once in the direction the encoder was inappropriately counting, then turning back the other direction fixes the issue.

 

sparrow2 wrote:

I want to ask what do you use the encoders for?

 

There have been many encoder threads here, and I will suggest (other than a "real" encoder), forget all about filtering and go for an encoder routine that is robust over for noise (other than count one up and then down again). 

They're kind of contextual. Mainly menu navigation, but sometimes incrementing and decrementing variables, depending on menu context.

 

I'll see if I can make this work with a timer interrupt instead of Hardware interrupts, and how much of a change I see in stability. TommyZ's article seems like a good place to start. 
(It'll take a bit to switch everything over, but I'll report back with results, if anyone is interested.)
Thanks for the feedback everyone!

 

/*

 */
#include <avr/io.h>
#include <avr/interrupt.h>

#define topEncoderPinA 0
#define topEncoderPinB 1
#define bottomEncoderPinA 2
#define bottomEncoderPinB 3

volatile uint8_t topEncoderValue = 0;
volatile uint8_t bottomEncoderValue = 0;
uint8_t topEncoderLastValue = 0;
uint8_t bottomEncoderLastValue = 0;

volatile uint8_t encoderPortStates = 0;

//if a bit in this number is high when an interrupt is called,
//then we know we are turning that direction. If not, then set that bit. 

void initEncoders()
{

	//Interrupt pins - 2,3,4,5

	//setup rising edge detection on Int pins 2 and 3 (would maybe want all pin states if this doesn't work).
	EICRA |=(1<<ISC31)|(1<<ISC30)|(1 << ISC21)|(1 << ISC20); 

	//same setup on pins 4 and 5
	EICRB |=(1<<ISC51)|(1<<ISC50)|(1 << ISC41)|(1 << ISC40);

	//enable all 4 interrupts through masking
	EIMSK |=(1<<INT2)|(1<<INT3)|(1<<INT4)|(1<<INT5);

}

ISR(INT2_vect)
{

	if((1<<topEncoderPinB)&encoderPortStates)//this means Pin 2 is coming after pin 3
	{
		bottomEncoderValue--;
		encoderPortStates&=(1<<bottomEncoderPinB)|(1<<bottomEncoderPinA);//reset our two pins to low.
	}
	else
	{
		encoderPortStates|=(1<<topEncoderPinA); //we want to set bit 0.
	}
}

ISR(INT3_vect)
{

	if((1<<topEncoderPinA)&encoderPortStates)//this means Pin 3 is coming after pin 2
	{
		bottomEncoderValue++;
		encoderPortStates&=(1<<bottomEncoderPinA)|(1<<bottomEncoderPinB); //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=(1<<topEncoderPinB); //we want to set bit 1.
	}
}

ISR(INT4_vect)
{

	if((1<<bottomEncoderPinB)&encoderPortStates)//this means Pin 4 is coming after pin 5
	{
		topEncoderValue++;
		encoderPortStates&=(1<<topEncoderPinA)|(1<<topEncoderPinB); //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=(1<<bottomEncoderPinA); //we want to set bit 2.
	}
}

ISR(INT5_vect)
{

	if((1<<bottomEncoderPinA)&encoderPortStates)//this means Pin 3 is coming after pin 2
	{
		topEncoderValue--;
		encoderPortStates&=(1<<topEncoderPinA)|(1<<topEncoderPinB); //reset our two pins to low.
	}
	else
	{
		encoderPortStates|=(1<<bottomEncoderPinB); //we want to set bit 3.
	}
}

void listenEncoders(myStruct *inputStruct)
{
	if(topEncoderValue!=topEncoderLastValue)
	{
	    //do stuff with internal struct
	    topEncoderLastValue = topEncoderValue;
	}

	if(bottomEncoderValue!=bottomEncoderLastValue)
	{
	    //do stuff with internal struct
		bottomEncoderLastValue = bottomEncoderValue;
	}
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

halfordC wrote:

Hey all,

Who-me wrote:

Does the direction fix eventually ?

A quad counter in SW usually has 4 states, and can rotate to adjacent states, but if you miss an edge, it can have an illegal transition.

It should recover, but the missed edge is the problem.

It does after a few turns. What I was finding during testing was turning once in the direction the encoder was inappropriately counting, then turning back the other direction fixes the issue.

What does 'a few turns' mean ?  As per the state diagram above, normally the MCU tracks the clicks and follows to an adjacent state CW or CCW. Rarely it might lose one, and find itself 180' out instead of +/- 90'.

That error should recover on the next click, unless you are in bounce-jackpot land, where the bounce time is matched to your SW-missed-it time...

 

 

halfordC wrote:

I'll see if I can make this work with a timer interrupt instead of Hardware interrupts, and how much of a change I see in stability.

A small tip for timer based encode, is to sample the pins once only, and then use that sample to build the [previous:current] action table, as listed above.

Common is to shift the previous 2 pin sample left 2 bits, and merge with current sample, then use the 4 bit / 16 possible values to index a lookup table. (as in #17)

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

That is the way I have done it for years, and on a 8051 it's a small fast code. (and the best thing it's easy to understand)

But have a table on a AVR cost way more.

The best (small and fast) on a AVR is this : (I know of)

 

There need to be a change, (else do nothing) 

if ( (newA ^ oldA) == (newA ^ newB) )

  inc

else

 dec

 

old =new

 

 

Add:

Note this logic count the opposite direction than my table! (flip inc and dec for same direction). 

 

Last Edited: Mon. Jun 29, 2020 - 09:40 AM