ATTiny13 Timer + Rotary Encoder = Variable Results

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

Hi,
This is my first project working in any kind of micro environment, so bear with me :oops:

I'm working with a rotary encoder to modify a control value. If the encoder goes clockwise, increment control value, and if it's counter-clockwise, decrement control value.

It's almost perfect. I'm handling the encoder with a timer interrupt at 1kHz and testing values using some code pilfered from the net. When the trigger hits, it determines position and updates a bitfield for the main loop to process.

Now to the problem. When Pin B of the encoder is high, everyting works fine. The interrupt determines the rotation direction (if any) and sets the bitfield. Main loop catches the change, and outputs the new control value to the user. The bitfield is reset, and the timer keeps running. However, when Pin A of the encoder is high, the bitfield interrupt stays high, and the user sees a constantly incrementing control value. No idea why...

Anyway, here's the code, as that will probably make things much clearer:

Pin Configuration
Encoder Pin A on PB1
Encoder Pin B on PB2

F_CPU=1000000;
// set DATA, LATCH and CLOCK pins to OUTPUT for the bit shifter control
DDRB = 0;
DDRB |= (1 << PB2) | (1 << PB3) | (1 << PB4);

// enable pull-ups on input pins
PORTB |= (1 << PB0) | (1 << PB1);

// use timers to handle rotary encoder
// aiming for 1kHz clock with pre-scale of 64
TCCR0A |= (1 << WGM01);	 // Configure timer 1 for CTC mode
TCCR0B |= (1 << CS00) | (1 << CS01);	// prescale 64
TIMSK0 |= (1 << OCIE0A);	// Enable CTC interrupt
OCR0A =  16;						

// enable interrupts
sei();

Main Loop - watches for bitfield values and updates user

for(;;)
{
  // clockwise turn of peg, increase control value
  if(isr_flag.encoder_cw)
  {
    cli();
    ++controlVal;

    showVal(controlVal); // output routine, takes 1 second
    isr_flag.encoder_cw=0;
    sei();
  }

  // counter-clockwise turn of peg, increase die size
  if(isr_flag.encoder_ccw)
  {
    cli();
    --controlVal;

    showVal(controlVal); // output routine, takes 1 second
    isr_flag.encoder_ccw=0;
    sei();
  }

Timer ISR - determines rotation of encoder

ISR(TIM0_COMPA_vect)
{
  static uint8_t oldA = 0;
  static uint8_t oldB = 0;
  uint8_t a = CHECKBIT(PINB,PB1);
  uint8_t b = CHECKBIT(PINB,PB0);
  int8_t delta = getDelta(a, oldA, b, oldB);
  oldA = a;
  oldB = b;
  if (delta < 0) isr_flag.encoder_ccw = 1;
  if (delta > 0) isr_flag.encoder_cw = 1;
}

static int8_t g_delta[16] PROGMEM = {
  0, 1, -1, 0, -1, 0, 0, 1, 0, 0, 0, -1, 0, -1, 0, 0
};

// returns the direction the rotary encoder was rotated: -1, 1 or 0
static int8_t getDelta(uint8_t a, uint8_t oldA, uint8_t b, uint8_t oldB)
{
  return pgm_read_byte(&g_delta[oldB | (b << 1) | (oldA << 2) | (a << 3)]);
}

That's it. Pin A always seems to leave the bitfield set and the control value increments non stop, until Pin A goes low. Then everything works perfectly.

I'm completely stumped.
Any help would be greatly appreciated.

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

Are the elements of isr_flag volatile?

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

Yes, they're volatile

typedef struct bitfield {
	unsigned char roll:1;
	unsigned char encoder_cw:1;
	unsigned char encoder_ccw:1;
}bitfield;

volatile bitfield isr_flag;
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Here's yet another way. Might be simpler?

//quadrature mode:
//1: count up or down on one edge of a depending on state of b
//2: count up or down on either edge of a depending on state of b
//4: count up or down on either edge of a depnding on b, or
//   either edge of b depending on a

//encoder a,b,z on pinb 3,2,1
//   ------    ------
//A  |    |    |    |        ->cw
//----    ------    -------
//      ------   ------
//B     |    |   |    |
//------     -----    -------

unsigned int pos,posl;
unsigned char a,al,b,bl,z,zl,an,af,bn,bf;
//-------------------------------------------
#pragma interrupt_handler int0_isr:iv_INT0
void int0_isr(void){
//int0 pb2 just went hi
char b;

  b=(PINB & 0x08)==0x08; //pd3 hi or lo
	if(b)  pos++;
	if(!b) pos--;
}

//--------------------------
void encoder(void){
//read encoder
//max rate: 2000ppr, 2usec per pulse->4ms per rev->250rps->15000rpm
unsigned char c,tmp;

  cprintf("encoder loop   q=quit\n");
	c=0;
	pos=posl=0;
  while(c != 'q'){
	  if(kbhit()){
		  c=getchar();
			if(c==' ') pos=0;
		}
		tmp=PINB;  //<-need to trigger from an edge interrupt
		a=(tmp & 0x04)==0x04; //pb2 is hi
		b=(tmp & 0x02)==0x02; //pb1
		z=(tmp & 0x01)==0x01; //pb0
		an =  a && !al; //a on  (rising edge)
		af = !a &&  al; //a off (falling edge)
		al=a; //a last pass
		if(mode==1){ //one edge of a        500 ppr
		  if(an && !b) pos++;
		  if(an &&  b) pos--;
  		if(z && (pos > 500)) pos -= 500;
		}
		if(mode==2){ //either edge of a      1000ppr
		  if((an && !b)||(af &&  b)) pos++;
		  if((an &&  b)||(af && !b)) pos--;
  		if(z && (pos > 1000)) pos -= 1000;
		}	
		if(mode==3){ //either edge of a or b  2000ppr
  		bn =  b && !bl; //b on  (rising edge)
	  	bf = !b &&  bl; //b off (falling edge)
		  bl=b; //b last pass
			
		  if((an && !b)||(af &&  b)) pos++;
		  if((an &&  b)||(af && !b)) pos--;
		  if((bn &&  a)||(bf && !a)) pos++;
		  if((bn && !a)||(bf &&  a)) pos--;
  		if(z && (pos > 2000)) pos -= 2000;
		}	
		if(pos != posl){
		  cprintf("pct 5u \r",pos);   //this takes too long
		}
		posl=pos;	
	}
}

//--------------------------
void raw(void){
//read rawbits
unsigned char c,tmp;

  cprintf("raw loop   q=quit\n");
	c=0;
  while(c != 'q'){
	  if(kbhit()){
		  c=getchar();
		}
		tmp=PINB;
		cprintf(" pct 02x  pct04x \r",tmp,pos);
	}	
}

Imagecraft compiler user

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

Thanks for the alternative option. I'm using INT0 for a button press, so I don't have it for the encoder. Plus, the timer seems to be handling debounce nicely.

I'm hoping someone can see a glaring bug that I'm just missing. The Lookup table works well, as I can store it in progmem instead of ram, saving space.

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

Quote:

I'm using INT0 for a button press

but that's a massively bad idea - so why not move the button elsewhere and free up INT0 for something that could really benefit from using it?

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

Imho its much easier to use a Pinchange interrupt. Fire it from one encoder pin and evaluate if both pins are the same or differ. If the are of the same level increase the value else decrease.

#define ENCA PB0
#define ENCB PB1
volatile uint8_t controlValue;
int main(void) {
PCMSK = (1 << ENCA);
GIMSK |= (1 << PCIE);
sei();

for (;;) {
   // do somethin' in the main loop
  }
return 0;
}
ISR(PCINT_vect){
_delay_ms(1); // debounce
if (PinB & (1 << ENCA)) {
   if (PinB & (1 << ENCB)) controlValue++ 
   else controlValue--;
   } else {
   if (PinB & (1 << ENCB)) controlValue--
   else controlValue++
   }
}

Add more debouncing at will :P Code is not tested but i can't find serious errors. I normally program the small Tinys with assembler :wink:

Last Edited: Sun. Oct 16, 2011 - 06:34 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

but that's a massively bad idea - so why not move the button elsewhere and free up INT0 for something that could really benefit from using it?

I'm still really new to micro stuff, and I wanted to trigger on a rising edge for the button press. INT0 was the only thing available for rising edge, everything else is pinchange.

What makes using INT0 for button press bad?

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

mschoeldgen wrote:
Imho its much easier to use a Pinchange interrupt. Fire it from one encoder pin and evaluate if both pins are the same or differ. If the are of the same level increase the value else decrease.

#define ENCA PB0
#define ENCB PB1
volatile uint8_t controlValue;
int main(void) {
PCMSK = (1 << ENCA);
GIMSK |= (1 << PCIE);
sei();

for (;;) {
   // do somethin' in the main loop
  }
return 0;
}
ISR(PCINT_vect){
_delay_ms(1); // debounce
if (PinB & (1 << ENCA)) {
   if (PinB & (1 << ENCB)) controlValue++ 
   else controlValue--;
   } else {
   if (PinB & (1 << ENCB)) controlValue--
   else controlValue++
   }
}

Add more debouncing at will :P Code is not tested but i can't find serious errors. I normally program the small Tinys with assembler :wink:

I like the look of that, much less code all around, but I thought it was a 'no-no' to call _delay_ms() from an ISR? Or does it really matter since I'm not really dealing with anything super time sensitive?

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

Quote:
What makes using INT0 for button press bad?

Because INT0 is the IRQ with the highest priority in the MCU. There might be some important additions later on which require it.
Quote:
INT0 was the only thing available for rising edge, everything else is pinchange.
But it is very easy to see if it was a rising edge :

ISR(PCINT_vect) {
 if (PINB & (1 << MyPin)) {
 // do my stuff when the value of Mypin is high
  }
} // else we simply leave the ISR

Note that by ignoring one edge of the encoder you will loose half of its resolution.

Why can't we call _delay_ms() in ISRs ? Many of my C programs do it ( including a BLDC motor control) without sideeffects ( that i know of :P )

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

Quote:
What makes using INT0 for button press bad?
Buttons bounce, using interrupts for buttons does not take bounce into account. If you write the interrupt to handle the bounce, you end up doing it the way you would have done without interrupts without any real benefit (and very likely end up impeding other operations in the process).

Regards,
Steve A.

The Board helps those that help themselves.

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

Thanks for all the help. I'm going to move over to the pinchange for encoder reading. I can live with less resolution. Still don't know why the timer route wouldn't work. Oh well.

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

See the advantages :D :
the timer is free for other tasks.
the ISR only needs time when the encoder is operated amd does not regularly interrupt the MCU.

Have fun!

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

Regarding bounce (and why buttons and external interrupts do not mix) read Ganssle:

http://www.ganssle.com/debouncin...

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

I will repeat (as I expect some of the links will show), interrupt code like the one mschoeldgen show can have a wrong count at stand still (and when change direction), it's the good old atari520 mouse run away error.
The problem is that pulses on the interrupt pin (noise at stand still), will be countet in one direction (the level of the other pin give direction). And not the correct one up , one down .....

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

'Noise' on any input pin ( wether its interupt driven or not) will always give erratic results. As with all digital equipment its up to the hardware design to avoid spikes, noise or whatever on a digital input pin. This is not limit to encoders but to any input. Use strong pullups ( the AVR built-ins are weak and not well defined ) or pulldowns and add RFI protection. Not only will your MCU be less sensitive to cellphones or other strong RF signals but it will infest its environment less. I cannot stress enough that running any MCU without proper decoupling is not recommended, even for DIY projects. Adding small decoüpling capacitors to e.g. encoders is not a bad idea. In my projects i use e.g. 2n2 or so and strong ( 1k or 2k2) pullups to get well defined signals. Often i add a 74HC14 Schmitt for fast slew rates and protection purposes ( because i have loads of them in my shack :wink: ). Adding a little hardware does payoff in predictable signals and simplifys tracing the real software errors.

Quote:
good old atari520 mouse run away error.
This is not limited to Ataris :D My Logitech optical mouse (!) suffers from this quite often.

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

You don't get the point, a non moving encoder (to make easy lets say it's placed on something that vibrate), a pogram with interrupt on only one pin will fail, and no pullups will help you! (it's bad logic).
A real encoder routine should count up down up down in that situation.

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

Quote:
a non moving encoder (to make easy lets say it's placed on something that vibrate)
Yeah,i see what you mean but i never encountered such 'bad' encoders and i use them a lot - from the costly 20$ down to the 50 cent pieces. Quite a few of them are built into musical equipment ( i'm playing bass in a rockband ) and the MCU-PreAmp sits on top of a 400 W speaker driven by a 240 W PA. Never ever have i encountered any change in the settings , only the top starts to wander around with the low frequencies... If i ever encounter such an encoder i'd probably trash it for a better one.

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

You don't get the problem if you take the encoder and do:
one step cw
one step ccw
one step cw
one step ccw

Now you should be at the same place (up down up down),
Your code will eiter not move at all (the step was on the other channel)
Or you step 4 forward or 4 back depending of the level of the other channel!

Edit other channel means the channel without interrupt

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

Sorry , took some time here to test for that 'misbehaviour' mentioned by sparrow2 , i had a server failure and also had a holiday from the band where the AVR bassamp is located . It might be the encoder i've used but the pinchange interrupt routines there don't show that behaviour - one click to the left decreases my value, one click to the right increases it. So for me at least, the routines do work. It might be worth noticing though, that i do most of the debouncing in hardware with an RC network and a 47HC14 schmitt so that my (assembler ) program uses a very fast plausibility check only.