Rotary Encoder

Log in or register to post comments
Go To Last Post
26 posts / 0 new
Author
Message
harrod7159
harrod7159's picture
Level: Rookie
Joined: Thu. Jan 17, 2008
Posts: 29 View posts
#1
Posted by harrod7159: Mon. Mar 21, 2011 - 01:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi,

I've been running through a few tutorials to get my head around Rotary encoders whilst getting back into coding in AVR Studio instead of Arduino. Unfortunately I'm away and cant test the code, would you look at it and see if i've made any errors and or can improve on it? I've tried to comment it to keep it easy to understand.

All it does is setup PCINT6 to look for a logical change and call the ISR to track what change it was and either increase or decrease a variable.

Many thanks,

Dave

/*
 * RotaryEncoder.c
 *
 * Created: 3/21/2011 10:15:54 AM
 *  Author: daharrod
 */ 

#include 
#include 

#define STATE_PIN 5
#define PHASE_PIN 6
#define RENCBTN_PIN 7
#define RENC_DDR DDRB
#define RENC_PORT PORTB

#define SETBIT(ADDRESS,BIT) (ADDRESS |= (1<<BIT)) 
#define CLEARBIT(ADDRESS,BIT) (ADDRESS &= ~(1<<BIT)) 
#define FLIPBIT(ADDRESS,BIT) (ADDRESS ^= (1<<BIT)) 
#define CHECKBIT(ADDRESS,BIT) (ADDRESS & (1<<BIT)) 

volatile int RENC_POS;
volatile int RENC_PHASE_STATUS;

void SetupRENC() {
	
	//Setup I/O for STATE_PIN and PHASE_PIN
	RENC_DDR |= (1<<PB6) | (1<<PB5); //Set PB5/6 to Input
	
	//Enable Pin Change Interrupt on PB6
	PCIFR |= (1<<PCIF0);
	PCMSK0 |= (1<<PCINT6);
	
	//Check status of STATE_PIN, determine Rising or Falling Interrupts. 
	//Clear Interrupt Flag before changing 
	EIMSK |= (1<<INT6);
	
	//Setup Interrupt type (Change)
	EICRB |= (1<<ISC60);


	//Set start conditions
	RENC_PHASE_STATUS = (PINB & (1<<PB6));

	//Enable Global interrupts
	sei();
	
	//Setup Global int for RENC_POS
	RENC_POS = 0;
	
	
}

//PinChange Interrupt Handler
void ISR(PCINT0_vect) {
	int TEMP_STATE_STATUS;
	int TEMP_PHASE_STATUS;
	//Get New STATE_PIN Level
	TEMP_STATE_STATUS = (PINB & (1<<PB5));
	TEMP_PHASE_STATUS = (PINB & (1<<PB6));
	

	if ((RENC_PHASE_STATUS == 0) && (TEMP_PHASE_STATUS == 1))
	{
			if(TEMP_PHASE_STATUS) 
			{
				RENC_POS--;
			}
			else
			{
				RENC_POS++;
			}
	}
	if ((RENC_PHASE_STATUS ==1) && (TEMP_PHASE_STATUS == 0))
	{
			if(TEMP_PHASE_STATUS) 
			{
				RENC_POS++;
			}
			else
			{
				RENC_POS--;
			}
	}
	
	RENC_PHASE_STATUS = TEMP_PHASE_STATUS; //New ints to old ints.
	
};


int main(void)
{
	
	//Setup Functions
	SetupRENC();
	
    while(1)
    {
        
    }
}

Dave Harrod

Tags:

AVR Microcontrollers, megaAVR and tinyAVR
Top
Bronze Marktheusch
theusch's picture
Level: 10k+ Postman
Joined: Mon. Feb 19, 2001
Posts: 39995 View posts
Location: Wisconsin USA
Posted by theusch: Mon. Mar 21, 2011 - 02:01 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It is true that often when working with pin-change interrupts, it is desirable/necessary to keep "previous". That will allow one to sort out which of the bank caused the effect.

So your code may well be proper. In my encoder work, I try to side-step that and keep each pin on a different bank of pin-change interrupts (and/or on external interrupt INTn pins) to make the encoder ISR skinnier.

For a twisty-knob, it doesn't matter. On a motor shaft, the difference between 18 cycles and 20 cycles is 10% more max rate.

//
// **************************************************************************
// *
// *		P I N _ C H A N G E _ I S R 2
// *
// **************************************************************************
//

// Pin change 16-23 interrupt service routine
//	Used for a pin change interrupt on PCINT18 (PIND.2) for recognizing
//	encoder pulses (Hall switch channel A).
interrupt [HALLA_INT] void pin_change_isr2(void)
{
// An edge on HALLA was detected.  We use both edges (x2 quadrature)
//	as well as both edges on HALLB giving x4 quadrature
	if (!HALLA)
		{
		// A falling edge.

		if (!HALLB)
			{
			// Positive direction (arbitrary)
			position += rotation;
			}
		else
			{
			// Negative direction (arbitrary)
			position -= rotation;
			}

		}
	else
		{
		// A falling edge.

		if (HALLB)
			{
			// Positive direction (arbitrary)
			position += rotation;
			}
		else
			{
			// Negative direction (arbitrary)
			position -= rotation;
			}

		}

}

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101069 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. Mar 21, 2011 - 02:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just a point of clarity but most programmers will expect anything in all upper case to be a pre-processor macro. It's just confusing to give your variables upper case names too.

Top
harrod7159
harrod7159's picture
Level: Rookie
Joined: Thu. Jan 17, 2008
Posts: 29 View posts
Posted by harrod7159: Mon. Mar 21, 2011 - 02:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks theusch, I was going to look at compacting the code once id established I hadn't done anything major wrong. Ill look at your code and try to implement something similar.

Clawson, thanks I hadn't realised that. Is there a better naming convention, or is it just lower case for them?

Dave Harrod

Top
sparrow2
sparrow2's picture
Level: Raving Lunatic
Joined: Mon. Oct 7, 2002
Posts: 7100 View posts
Location: Denmark
Posted by sparrow2: Mon. Mar 21, 2011 - 02:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I don't have time at the moment to take a deep look, but the normal encoder error is when one channel is stabil and the other one go up and down, (you get that when you are at a stand still and vibrate), if your logic is wrong you will count up! (or down) and not up down up down ....

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101069 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. Mar 21, 2011 - 03:20 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Is there a better naming convention, or is it just lower case for them?

Many ways to skin a cat but here's some suggestions:

http://en.wikipedia.org/wiki/Nam...(programming)

Top
sparrow2
sparrow2's picture
Level: Raving Lunatic
Joined: Mon. Oct 7, 2002
Posts: 7100 View posts
Location: Denmark
Posted by sparrow2: Mon. Mar 21, 2011 - 11:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ok I had a look at the code
There must be something wrong!
you first have a if where (TEMP_PHASE_STATUS == 1) has to be true
and inside that there is a if else on the same!
Look at Lee code it will work.

To Lee:
How can you claim "x4 quadrature" when HALLB can change without a count!

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Tue. Mar 22, 2011 - 12:36 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can divide the quadrature waveform (called a 'pulse') into 4 quadrants, and depending on which edges and levels you are watching, you can get 1, 2, or 4 counts per pulse. I guess some engineer needs to decide whther he wants more speed or finer resolution. Obviously quad x1 is easiest... look for a rising edge on A, inc or dec depending on level of B. Simple?

Imagecraft compiler user

Top
sparrow2
sparrow2's picture
Level: Raving Lunatic
Joined: Mon. Oct 7, 2002
Posts: 7100 View posts
Location: Denmark
Posted by sparrow2: Tue. Mar 22, 2011 - 06:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see : it's how you def. quadrature.

Quote:
look for a rising edge on A, inc or dec depending on level of B. Simple?
That is the the logic that don't work! (up down up down on A and no change on B count up (or down) and we are at a stand still).

Top
kscharf
kscharf's picture
Level: Posting Freak
Joined: Wed. Aug 4, 2004
Posts: 1839 View posts
Location: Davie, FL
Posted by kscharf: Tue. Mar 22, 2011 - 01:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

A 2 bit quadrature sequence consists of 4 states. They are:
state 0: 0,0 state 1: 0,1 state 2: 1,1 state 3: 1,0 (and then back to state 0 again). To decode a quadrature encoder I built a state machine that reads and saves the new state (the two bits) and compares them to the previous stored state. Based on the above sequence there are three outcomes:
1: the encoder has moved forward one state, 2: the encoder has moved backwards one state, 3: the encoder has jumped more than one state and the direction cannot be determined. In case 1 or 2 I increment or decrement a counter, in case 3 I ignore the movement but save the new state for the next call to the state machine.

I OR the two pins as far as generating a change of state interrupt, a change of state interrrupt on either pin calls the state machine function.

An alternate way of doing things is to poll the encoder and call the state machine. In this case there is also the possibility that NO change is detected, this is handled the same as case 3 above. Use a hardware timer to set the poll interval, it must be fast enough to keep up with the expected rate of change in the encoder.

// change to the port YOUR enc is at
#define ENCPINS	PINC	
//change to the bits YOUR enc is at
#define ENCA		_BV(PC_0)
#define ENCB		_BV(PC_1)

#define ENCMASK	(ENCA|ENCB)

#define STATE0		0
#define STATE1		ENCA
#define STATE2		ENCA|ENCB
#define STATE3		ENCB

int counter = 0;
static char state=0;

void EncoderStateMachine(void)
{
char newstate;

	newstate = ENCPINS&ENCMASK;
	
	switch(state)
	{
	case STATE0:
		if(newstate == STATE1) counter++;
		if(newstate == STATE3) counter--;
		break;
	case STATE1:
		if(newstate == STATE2) counter++;
		if(newstate == STATE0) counter--;
		break;
	case STATE2:
		if(newstate == STATE3) counter++;
		if(newstate == STATE1) counter--;
		break;
	case STATE3:
		if(newstate == STATE0) counter++;
		if(newstate == STATE2) counter--;
		break;
	}
	state=newstate;
}
Top
sparrow2
sparrow2's picture
Level: Raving Lunatic
Joined: Mon. Oct 7, 2002
Posts: 7100 View posts
Location: Denmark
Posted by sparrow2: Wed. Mar 23, 2011 - 08:32 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Nice code the only problem I can see is that the switch can take some extra clk cycles.
I have made the "same" code using a 4 bit lookup table on a 8051, but because tables are reletive slow on a AVR it can be done faster with logic on a AVR.
Depending of the C version it could be faster with function pointers.(but only if the pointer is stored in REG and it use IJMP)

Top
harrod7159
harrod7159's picture
Level: Rookie
Joined: Thu. Jan 17, 2008
Posts: 29 View posts
Posted by harrod7159: Wed. Mar 23, 2011 - 09:46 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sparrow, Thanks for pointing that out, would it be better to just use four different ifs, one for each state?

kscharf, that code looks very neat. Can you explain the following:

#define ENCA      _BV(PC_0) 
#define ENCB      _BV(PC_1)

I dont understand what the _BV() does.

Thanks.

Dave Harrod

Top
harrod7159
harrod7159's picture
Level: Rookie
Joined: Thu. Jan 17, 2008
Posts: 29 View posts
Posted by harrod7159: Wed. Mar 23, 2011 - 09:50 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Oh, and to get it fired on an interrupt would you just put a call to the EncoderStateMachine(void) in the ISR for the pinchange interrupt or is there a better way to do it?

Thanks

Dave Harrod

Top
danni
danni's picture
Level: Raving Lunatic
Joined: Wed. Sep 5, 2001
Posts: 2960 View posts
Posted by danni: Wed. Mar 23, 2011 - 09:59 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

https://www.avrfreaks.net/index.p...

Peter

Top
sparrow2
sparrow2's picture
Level: Raving Lunatic
Joined: Mon. Oct 7, 2002
Posts: 7100 View posts
Location: Denmark
Posted by sparrow2: Wed. Mar 23, 2011 - 10:12 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
EncoderStateMachine(void)
Should be a part of the ISR it self!

Top
sparrow2
sparrow2's picture
Level: Raving Lunatic
Joined: Mon. Oct 7, 2002
Posts: 7100 View posts
Location: Denmark
Posted by sparrow2: Wed. Mar 23, 2011 - 11:02 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
, would it be better to just use four different ifs
perhaps!
With if worst case are 2 compare (Lee's code), with a switch it's 4, unless you can force the compiler to make a binary search!

Top
kscharf
kscharf's picture
Level: Posting Freak
Joined: Wed. Aug 4, 2004
Posts: 1839 View posts
Location: Davie, FL
Posted by kscharf: Wed. Mar 23, 2011 - 12:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The _BV(num) is a macro that might be deprecated. It's the same as (1<<num).
You could inline the state machine code into the ISR if you are using a HW timer to poll the encoder since you have only one interrupt. If you use a pin change interrupt you might have TWO of them (I do since I'm using an atmega32 which does NOT have pin change interrupts for every pin, I'm using INT0 and INT1). The overhead of a call/return isn't really a problem. If the encoder is on a knob and only generates 16 cycles per rotation you can poll as slow as 100hz and it would work. If you are dealing with a high resolution encoder on a motor shaft you are better off with the pin change interrupts and might want to code this in assembler. In my experience the switch statement isn't less efficient than if's, but by coding using cascaded if statements you do have more control of what order things get tested in.

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101069 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Wed. Mar 23, 2011 - 01:19 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

that might be deprecated

Seems unlikely:

C:\WinAVR-20100110\avr\include\avr>grep _BV * | wc -l
   2511

Top
kscharf
kscharf's picture
Level: Posting Freak
Joined: Wed. Aug 4, 2004
Posts: 1839 View posts
Location: Davie, FL
Posted by kscharf: Wed. Mar 23, 2011 - 01:21 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:
Quote:

that might be deprecated

Seems unlikely:

C:\WinAVR-20100110\avr\include\avr>grep _BV * | wc -l
   2511

Good.
I remember there was a similar macro that HAD been deprecated, and I know _BV still works.

Top
gregsmithcts
gregsmithcts's picture
Level: Raving Lunatic
Joined: Thu. Nov 4, 2010
Posts: 2098 View posts
Location: Bristol, UK
Posted by gregsmithcts: Wed. Mar 23, 2011 - 02:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Seems unlikely:

Cliff, you know better than that :)

/*
grep anything and get a hit
*/

That is the silliest test I've seen you come up with to date ;)

For those that *don't* do unix, Cliff's test shows that the 'pattern' _BV exists two and a half thousand times, but, it does not put these 'hits' into any context. They 'could'' all have been commented out of the code.

--greg
Still learning, don't shout at me, educate me.
Starting the fire is easy; the hardest part is learning how to keep the flame!

Last Edited: Wed. Mar 23, 2011 - 09:14 PM
Top
kscharf
kscharf's picture
Level: Posting Freak
Joined: Wed. Aug 4, 2004
Posts: 1839 View posts
Location: Davie, FL
Posted by kscharf: Wed. Mar 23, 2011 - 02:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Guess it's still currently in vogue.

https://www.avrfreaks.net/index.p...
http://www.nongnu.org/avr-libc/u...
http://linux.die.net/man/3/_bv

Top
gregsmithcts
gregsmithcts's picture
Level: Raving Lunatic
Joined: Thu. Nov 4, 2010
Posts: 2098 View posts
Location: Bristol, UK
Posted by gregsmithcts: Wed. Mar 23, 2011 - 02:31 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Guess it's still currently in vogue.
I agree :) It's allmost as popular as doing it properly :) :) :) ;)

--greg
Still learning, don't shout at me, educate me.
Starting the fire is easy; the hardest part is learning how to keep the flame!

Top
kscharf
kscharf's picture
Level: Posting Freak
Joined: Wed. Aug 4, 2004
Posts: 1839 View posts
Location: Davie, FL
Posted by kscharf: Wed. Mar 23, 2011 - 02:37 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

gregsmithcts wrote:
Quote:
Guess it's still currently in vogue.
I agree :) It's allmost as popular as doing it properly :) :) :) ;)

I tend to use the (1<<num) and _BV(num) about equally. Both require about the same number of finger reaches for the shift key :D . If you don't know what _BV means I suppose (1<<num) makes the code easier to read. Or you could define all the required constants ahead of time with a _bm (bit mask) suffix.
IE: #define ENCA_BM (1<<num)

Top
Bronze Markbobgardner
bobgardner's picture
Level: 10k+ Postman
Joined: Wed. Sep 4, 2002
Posts: 26609 View posts
Location: Orlando Florida
Posted by bobgardner: Wed. Mar 23, 2011 - 05:27 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Consider this statement: ok=(PINA & 0b00001000)==0; The use of the binary term implies it is a bitmask. After twenty or thirty years, one just remembers that 0x08 is the same bit pattern. I know that the (1 << BITS) expression doesnt incur any runtime overhead, but a new guy might not, same with the BV macro. So I claim the example statement shown is the easiest to read.

Imagecraft compiler user

Top
gregsmithcts
gregsmithcts's picture
Level: Raving Lunatic
Joined: Thu. Nov 4, 2010
Posts: 2098 View posts
Location: Bristol, UK
Posted by gregsmithcts: Wed. Mar 23, 2011 - 09:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
So I claim the example statement shown is the easiest to read.
Ok, but this MUST be put in context, of course...
Consider setting a bit value in an IO or Direction register. Magic numbers can be, and, in my opinion are, bad news. (whatever base is used!)

I fear we've wandered somewhat off-topic :P

--greg
Still learning, don't shout at me, educate me.
Starting the fire is easy; the hardest part is learning how to keep the flame!

Top
Brutte
Brutte's picture
Level: Raving Lunatic
Joined: Thu. Oct 5, 2006
Posts: 3229 View posts
Location: Poland
Posted by Brutte: Thu. Mar 24, 2011 - 05:29 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
For those that *don't* do unix

Thanks! I was starting to worry it is some kind of a compiler option I do not understand.. Sometimes, when I am not sure about the spelling of some word, I use google and check which version is more frequent..

Anyway, previous discussion about software encoder interfaces:
https://www.avrfreaks.net/index.p...

No RSTDISBL, no fun!

Top