Solved: Can't make encoder work correctly

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

Hello everyone,
I'm trying to make Arduino count number of encoder rotations and send this number to serial port. The actual program is if course more complicated, but for debugging 

purposes I've removed everything not related to encoder.
Of many algorithms for reading a rotary encoder, I've decided to choose this one, because it looks very elegant for me, and totally eliminates bouncing issue (well, at 

least in theory): https://www.circuitsathome.com/m...
But unfortunately this code (modified for Arduino) won't work: it seems to generate some random numbers, but related to real counts. Here's the code:

 

#define ENC_A 2
#define ENC_B 3
#define ENC_PORT PIND
volatile int encval = 0; //this variable will be changed by encoder input
volatile int encval2 = 0; //This var shows how many times an ISR has been called
volatile int old_AB = 0; //initial state of encoder pins
static const int8_t enc_states [] = {0, -1, 1, 0, 1, 0, 0, -1, -1, 0, 0, 1, 0, 1, -1, 0}; //array of possible encoder transition states
bool changed = true; //Send data flag
void setup()
{ /* Setup encoder pins as inputs */
 pinMode(ENC_A, INPUT);
 pinMode(ENC_B, INPUT);
 attachInterrupt(0, ENC_A_P, CHANGE);
 attachInterrupt(1, ENC_A_P, CHANGE);
 Serial.begin (115200);
 Serial.println("Start");
}
void loop()
{
 delay(7000); //program waits for 7 seconds, while calculating number of counts
 if (changed)
  {
   Serial.print("ISR called, times: ");
   Serial.println(encval2);
   Serial.print("Counter: ");
   Serial.println(encval );
   changed = false;
  }
}
void ENC_A_P (void)
{
 old_AB <<= 2; //remember previous state
 old_AB |= (ENC_PORT & 0x0C); //read pins 2 and 3 simultaneously
 encval += (enc_states[( old_AB & 0x0f )]);
 encval2++; //to see that ISR has been called
} 

Here's the output from serial monitor:

Start ISR called, times: 96 Counter: -77
Start ISR called, times: 1 Counter: -1
Start ISR called, times: 2 Counter: -1
Start ISR called, times: 14 Counter: 0
Start ISR called, times: 6 Counter: 1
Start ISR called, times: 7 Counter: 1
Start ISR called, times: 156 Counter: -14

I'm always rotating in one direction, so negative and positive bakes should not be present. Also, first time I've rotated encoder for 6 clicks, but fast, second time 

for 2 clicks, but slowly.
I don't have dual channel oscilloscope, but I have small logic analyzer, and it shows very nice output, with no bouncing at all when rotating slowly.
Do you have any ideas?

Last Edited: Sat. Aug 12, 2017 - 02:03 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Edited post with nice formatting

Last Edited: Tue. Jul 25, 2017 - 09:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

I can understand using interrupts for an encoder mounted to a motor, but surely one can poll a manual encoder via a timer tick much more reliably?

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

Logic Analysers are a very powerfull piece of debug equipment.

Is yours a Salaea clone? Have you used it with Sigrok / Pulseview?

 

I would start by using the SPI interface to write all intermediate values of the calculation in the interrupt.

 

Spi( uint8_t Val) {
    while ( !(SPSR & (1<<SPIF))); // Wait if SPI is still busy. do not overwrite.
    SPDR = Val;
}


ISR ( bla bla) {
 Spi( old_AB <<= 2); //remember previous state
 Spi( old_AB |= (ENC_PORT & 0x0C)); //read pins 2 and 3 simultaneously
 Spi( encval += (enc_states[( old_AB & 0x0f )]));
 Spi( encval2++); //to see that ISR has been called
 Spi( encval); // Post increment Spi's the old value :)
 
}

 

You can easily catch the Spi data with your logic analyser and verify if you made any errors in your calculation.

 

In your "nice formatting" you forgot to put

Changed = false;

on a separate line, I missed it in the first read.

Also, check out my little (extremely simple) debug "lib" for a way of switching fast between turning debugging on and off:

 

 

Attachment(s): 

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

Encoder complete part number?

Encoder has detents (clicks as in OP)?

If having detents, is a full-cycle type where BOTH CONTACTS are either open or closed at detented positions?

Or if having detents, is a quarter-cycle type where contacts could be any condition at detents?

 

Image from Bourns ECW Encoder

Last Edited: Tue. Jul 25, 2017 - 06:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

The code in the article has this line:

old_AB |= ( ENC_RD & 0x03 );

While your code has this line:

old_AB |= (ENC_PORT & 0x0C); //read pins 2 and 3 simultaneously

I suspect you need to shift the (ENC_PORT & 0x0C) value right by 2 bits before ORing it to old_AB.

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

Kartman wrote:
I can understand using interrupts for an encoder mounted to a motor, but surely one can poll a manual encoder via a timer tick much more reliably?

You mean check encoder input periodically upon timer tick? But why is it more reliable? Anyway, this is sort of self-education for me, so I want to implement it using interrupts, which I've never used before.

 

Paulvdh wrote:
Logic Analysers are a very powerfull piece of debug equipment. Is yours a Salaea clone? Have you used it with Sigrok / Pulseview?   I would start by using the SPI interface to write all intermediate values of the calculation in the interrupt.

Yes, a $6 clone, but it works OK. I've googled Sigrok, thanks, will try it out. So far I've worked with native Saleae Logic software. Thanks for the advice, I'll try it. I guess, however, I should initialize SPI before, by adjusting SPCR register?

 

Paulvdh wrote:
I missed it in the first read.

Fixed, thanks!

 

Paulvdh wrote:
Also, check out my little (extremely simple) debug "lib" for a way of switching fast between turning debugging on and off:

Thanks! I've downloaded the h file, but to my shame couldn't understand what it actually does... What's "MsgRf.Des"?

 

sbennett wrote:
Encoder has detents (clicks as in OP)? If having detents, is a full-cycle type where BOTH CONTACTS are either open or closed at detented positions?

Good point.. I guess it's neither. Encoder p/n is KY-040, and it have both pins either open or closed on detent.

 

 

christop wrote:
I suspect you need to shift the (ENC_PORT & 0x0C) value right by 2 bits before ORing it to old_AB.

Thanks, but I don't think it's an issue. In the article encoder is connected to pins 0 and 1 of PORTB, so this line:

old_AB |= ( ENC_RD & 0x03 );

Turns into:

old_AB |= ( PIND & 0b00000011 );

Thus he's reading first two pins in the port. But in my scheme encoder is connected to pins 2 and 3 in PORTD, so I need to mask them with 0b00001100, which is exactly 0xC.

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

Why is polling more reliable? Simply because you cannot control the rate the external interrupts are generated. If the interrupts happen too fast, the rest of your code gets starved for cycles.
Christop identified your problem - an index is formed by the previous two bits and the current two bits. Bits 2,3 are the previous and bits 0,1 are the new bits. Because you use port bits 2,3 when you 'or' them in, they overwrite the previous bits. So the code does not work as expected.

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

SPI port indeed has to be initialised before you can use it.

I omitted it on purpose to keep the post short.

If you want to use SPI, read the datasheet.

It's one of the simplest serial ports, even the datasheet is only 2 or 3 pages.

 

My debug lib consists of a few simple macro's.

The first 35 comment lines in the header file are the "user manual"

This is the important part, it explains how you can use it.

 

Line 36 to 60 are the macro's (Really that's all).

 

Everything below line 60 is also comment. "MsgRf.Dest" is a member name of a class in a completely unrelated project.

Just ignore it.

 

I do think christop's comment is important.

If you copy an example and change it to use other bits then you can inalidate assumptions made by the original author.

The best way to validate it still works is to take a pen and paper and do al the calculations by hand

 

 

 

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

you have this code:

 old_AB <<= 2; //remember previous state
 old_AB |= (ENC_PORT & 0x0C); //read pins 2 and 3 simultaneously

and that is wrong if you don't use pin 0 and 1 ! (line 1 store bit 0 and 1 in 2 and 3 and then or the two new bit, but you need to shift those 2 down !) 

or you store the old bits by >>=2 and then or the new bits, but then I guess that the values in the LUT needs to be moved around

 

 

Last Edited: Wed. Jul 26, 2017 - 02:09 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello everybody,

It's a hobby project, so I didn't have time for a while... I've thoroughly read all your ideas, so that's what I've figured out:

 

christop wrote:

I suspect you need to shift the (ENC_PORT & 0x0C) value right by 2 bits before ORing it to old_AB.

Absolutely true, I'm so ashamed I didn't understood it myself. Thanks!

 

Kartman wrote:

Why is polling more reliable? Simply because you cannot control the rate the external interrupts are generated. If the interrupts happen too fast, the rest of your code gets starved for cycles.

It seems that my encoder bounces as hell, and that's generating serious problems. Will show it later.

 

Idea from sbennet, that my encoder is different from the one used by original author made me understand, that if single click (detent) causes both switches to change state, I cannot use that code at all. Guy from circuitsathome.com is using ISR to analyze current and previous states, but I need to compare transitions instead. So what I've implemented so far:

 

#define ENC_A PD2
#define ENC_B PD3
#define ENC_PORT PIND
#define ENC_PORT_D DDRD

#include <SPI.h>


volatile int encval = 0;      //this variable will be changed by encoder input

volatile byte cur_Transition = 0;
volatile byte prev_Transition = 0;

void setup()
{
  /* Setup encoder pins as inputs */
  pinMode(ENC_A, INPUT);
  pinMode(ENC_B, INPUT);
  attachInterrupt(0, ENC_A_P, CHANGE);
  attachInterrupt(1, ENC_A_P, CHANGE);
  SPI.begin ();
  /*
  cur_Transition and prev_Transitions store transitions of states of pins A and B in last 4 bits.
  For example, if at some time when ISR was called, pin A was high (0) and pin B was low (1), and during next ISR call both were high (1), then
  cur_Transition would be 0b00001011.
  */
  cur_Transition = (ENC_PORT >> 2) & 0x03; //read pins 2,3 and save their status
  prev_Transition = (ENC_PORT >> 2) & 0x03;//same
  PORTB &= ~(_BV(SS)); //Set Slave Select pin
}

void loop()
{
 //Do nothing here so far...
}

void ENC_A_P (void)
{
  cur_Transition = (cur_Transition << 2) & 0b00001111; //Store previous state of pins in bits 2 and 3, leaving bits 0 and 1 empty for further reading
  cur_Transition |= ((ENC_PORT >> 2) & 0x03); //Read status of pins 2 and 3, and store them into bits 0 and 1.
  /*
  We have only few valid state transitions. When one pin was high, and the other was low, and then both turned either high or low, means we made a next click (detent).
  All other transitions mean either half-state (between detent) or bouncing.
  */
  if ((cur_Transition == 0b00001011 && prev_Transition == 0b00000010) || (cur_Transition == 0b00000100 && prev_Transition == 0b00001101))
    encval++;
  else if ((cur_Transition == 0b00000111 && prev_Transition == 0b00000001) || (cur_Transition == 0b00001000 && prev_Transition == 0b00001110))
    encval--;
  prev_Transition = cur_Transition;
  SPI.transfer(encval);
}

More graphical comments on my code. Here's my encoder, I've marked transitions with dashed line, and ISR calls with dotted lines:

 

So for example, before ISR is called we've recorded

cur_Transition = (ENC_PORT >> 2) & 0x03;
prev_Transition = (ENC_PORT >> 2) & 0x03;

So both variables are now 0b00000011. Then ISR is called, and inside cur_Transition is changed,

cur_Transition = (cur_Transition << 2) & 0b00001111; //Changes to 0b00001100
cur_Transition |= ((ENC_PORT >> 2) & 0x03);// And now to 0b00001101

So it is now 0b00001101. prev_Transition is still 0b00000011, as recorded at the beginning of the program. This combination can only be in half-state (between detents), so if-else condition is skipped entirely. Then we record current transition as previous, it becomes 0b00001101. At next ISR call we have:

 

cur_Transition = (cur_Transition << 2) & 0b00001111; //Changes from 0b00001101 to 0b00000100
cur_Transition |= ((ENC_PORT >> 2) & 0x03);// And now to 0b00000100 again

So now we have combination of cur_Transition == 0b00000100 and prev_Transition still equals 0b00001101, and this is a valid combination:

if ((cur_Transition == 0b00001011 && prev_Transition == 0b00000010) || (cur_Transition == 0b00000100 && prev_Transition == 0b00001101))
    encval++;

I won't show other combinations, but general idea, I guess, is clear: only after full detent we increment or decrement the encval.

 

Code works OK when I turn slowly, but firmly: it suppresses the bouncing. But if it occurs, I have problems. My guess is that due to heavy bouncing, ISR isn't called proper amount of times. See image from logic analyzer below:

 

 

Here it can be seen really clear. Orange channel is MOSI, red one is CLK, and yellow is encoder A. Encoder B is not shown, but it is already high, and steady for good number of milliseconds (two contacts never bounce simultaneously).

 

Am I right about the reason? (Hope somebody would read through to this point laugh

 

Thanks a lot to everybody, again!

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

It have a hard time to follow the flow in your program, but I fear that the SPI prevent you from updating the encoder.

Those two needs to be independent, so encoder can be updated, and values over SPI can have bigger steps than +-1  

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

Thanks, sparrow2

I've tried to send encval in the main loop every 100 ms - much slower, but it is the same. Sometimes readings are correct, sometimes not. Even without SPI, ISR routine takes some time, during which some transitions can possibly be lost. My code was something like:

void loop()
{
  SPI.transfer(encval);
  delay(300);
}

And still sometimes a get proper readings, and sometimes not. And I always get amount lower, then expected, never got encval to increase more, than actual amount of clicks.

 

Am I right, that using interrupts on such a heavily bouncing hardware is generally a bad idea? Sometimes it looks like this:

If I zoom, I can see 15 contact status change overall, way too much..

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

What kind of ISR are you using? (and make sure it don't call any functions! )

 

I general it's hard to guess whats wrong when you don't show the code!

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

sparrow2, there is a code in my previous post. Here's the ISR:

void ENC_A_P (void)
{
  cur_Transition = (cur_Transition << 2) & 0b00001111; //Store previous state of pins in bits 2 and 3, leaving bits 0 and 1 empty for further reading
  cur_Transition |= ((ENC_PORT >> 2) & 0x03); //Read status of pins 2 and 3, and store them into bits 0 and 1.
  /*
  We have only few valid state transitions. When one pin was high, and the other was low, and then both turned either high or low, means we made a next click (detent).
  All other transitions mean either half-state (between detent) or bouncing.
  */
  if ((cur_Transition == 0b00001011 && prev_Transition == 0b00000010) || (cur_Transition == 0b00000100 && prev_Transition == 0b00001101))
    encval++;
  else if ((cur_Transition == 0b00000111 && prev_Transition == 0b00000001) || (cur_Transition == 0b00001000 && prev_Transition == 0b00001110))
    encval--;
  prev_Transition = cur_Transition;
  SPI.transfer(encval);
}

It is called if pins 2 or 3 change their value:

attachInterrupt(0, ENC_A_P, CHANGE);
attachInterrupt(1, ENC_A_P, CHANGE);

So ISR doesn't call any functions, except for SPI.transfer, for debugging purposes. But as I said, moving it from ISR to main loop doesn't change the behavior.

Can I assume, that time from the moment when pin changes it's condition, till the beginning of SPI transmission start, is exactly time of ISR function execution? This time is always constant, and is about 6.25 - 6.50 microseconds, which is about 100 clock cycles.

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

but what does SPI.transfer do if you call this fast (when it's still transferring the last value), does it have a buffer or wait or ?

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

You've been tinkering about this for a week now.

Maybe it's time for a change of plan?

I would ditch the arduino stuff, It might set you up for a bit of steep learning buy you'll be gratefull in the end.

I couln't find a main() entry pont in your software nor a sei().

And who knows how much overhead that "attachinterrupt(), causes?

 

Arduino is trying to learn people to skate real fast over thin ice above very murky water.

It goes great untill the ice breaks.

I very much dislike the arduino stuff, and yes, it's personal and you have the right to disagree.

 

If you can't get something to work then it helps to take smaller steps.

Don't  try to do 2 bits at once (And in the same interrupt ???) just because "it looks elegant".

 

For what it's worth, here 2 (almost?) identical interrupts of which at least the first works.

1 bit at a time, less "elegant", but robust and easy to understand.

First just use the first interrupt, then maybe extend and add the 2nd.

The 2nd is apparently not enabled in this piece of my test software.

//=============================================================== Interrupts.
ISR ( INT1_vect) {
/* Interrupt for the Encoder. Increment or decrement the position counter
 for each flank of the _A_ bit. */
	if( ENCODER_PIN & ENCODER_A_BIT) {	// If the Interrupt pin turned high.
		if( ENCODER_PIN & ENCODER_B_BIT)	// Can be any IO pin.
			Position++;
		else
			Position--;
	} else {
		// Else interrupt pin went low.
		if( ENCODER_PIN & ENCODER_B_BIT)
			Position--;
		else
			Position++;
	}
}

//---------------------------------------------------------------------------
#if 0
ISR ( INT0_vect) {
/* Interrupt for the Encoder. Increment or decrement the position counter
 for each flank of the _B_ bit. */
	if( ENCODER_PIN & ENCODER_A_BIT) {	// If the Interrupt pin turned high.
		if( ENCODER_PIN & ENCODER_B_BIT)	// Can be any IO pin.
			Position++;
		else
			Position--;
	} else {
		// Else interrupt pin went low.
		if( ENCODER_PIN & ENCODER_B_BIT)
			Position--;
		else
			Position++;
	}
}
#endif

//===========================================================================

 

Or another way:

Draw a piece of quadrature signals (As in #5) on a piece of paper and calculate a bunch of transitions yourself.

Follow your own algorithm and write it all down on paper.

This is a good way to discover differences between what you think your code should do and what your code acutally does.

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

sparrow2 wrote:

but what does SPI.transfer do if you call this fast (when it's still transferring the last value), does it have a buffer or wait or ?

 

SPI.h, located at Arduino_folder\packages\arduino\hardware\avr\1.6.19\libraries\SPI\src\ has the following code:

 

inline static uint8_t transfer(uint8_t data)
{
    SPDR = data;
    /*
     * The following NOP introduces a small delay that can prevent the wait
     * loop form iterating when running at the maximum speed. This gives
     * about 10% more speed, even if it seems counter-intuitive. At lower
     * speeds it is unnoticed.
     */
    asm volatile("nop");
    while (!(SPSR & _BV(SPIF))) ; // wait
    return SPDR;
}

 

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

How many (real) pulses per second do you expect from this encoder? Then ask yourself if you need the microsecond response of an external interrupt. Run a timer tick, say at 10ms and sample the encoder inputs, then feed that into the state machine. Job done.

If you want to persist with external interrupts, then add a rc network to filter most of the bouncing and EMI.

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

Bouncing is not unusual for mechanical encoders, it is suggested you use an R/C filter to suppress the bounces.

Optical encoders are usually cleaner, but cost much more, so I guess you get what you pay for.

 

Jim

 

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

but the code OP use don't care about bouncing (at least in a timer ISR) that will just make an extra +1 and then -1 the next time, only problem is that only one channel can change at the time.  

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

Arduino is trying to learn people to skate real fast over thin ice above very murky water.
It goes great untill the ice breaks.
I very much dislike the arduino stuff, and yes, it's personal and you have the right to disagree.

That's probably true, but for me personally Arduino has opened a way to studying MCU's. Without it, I would never even try to learn electronics and programming from scratch at the age of 30, without any relevant background and education. Before Arduino, I've never wrote a single line of code, and barely understood how electricity works at all. Now I'm trying to move away from Arduino, as a start to understand all the drawbacks of the built-in functions, but just one step at a time) I've already learn what is direct port manipulation, how timers work, now it's turn for external interrupts.

 

Draw a piece of quadrature signals (As in #5) on a piece of paper and calculate a bunch of transitions yourself.
Follow your own algorithm and write it all down on paper.
This is a good way to discover differences between what you think your code should do and what your code acutally does.

I did that, several times, and it seems that my code does what I've supposed it to do. The problem, as I see it, is that interrupt handler function misses some pin change events due to their high frequency. It is obvious from the picture I receive from logic analyzer: sometimes there are 10-15 events, and only 2-3 ISR calls.

 

but the code OP use don't care about bouncing (at least in a timer ISR) that will just make an extra +1 and then -1 the next time, only problem is that only one channel can change at the time. 

The original code won't work for me, that's I've already understood. The reason was mentioned in post #5: I have a totally different encoder behavior. But my code in #11 also works OK in case there is not more than 2-4 bounces.

 

I guess I'll use timer-based approach, suggested by Kartman.

 

Thank you all, guys, I really appreciate the help. Now I don't know which post to mark as a solution, all of you helped me a lot!

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

One of the easy ways   
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It seems that everybody (including me) have skipped over the possibility of hardware problems.

First of: What sort of encoder are you using? I assume such a 12 to 20 pulses/revolution encoder made for knobs in front panels?

Logic Analysers always show clean signals "0" or "1" but it is a poor reflection of the harsh outside world.

Do you have a scope? (If so, show the analog encoder output).

Or you can measure if you have at least steady state full swing signals with a DMM if you rotate the encoder slowly.

You should see (almost) only 0V and 5V on your DMM.

 

I can not make much sense of your logic analyser screen shots.

Brown & Red seem to be some SPI capture, Is that the debug info I mentioned earlier?

Is the lowest "light brown" / (almost) "Orange" one of the A / B encoder signals?

Give your channels a name, and show those names in the screenshots.

 

Forget the debouncing and "right" encoder output for a while.

 

Just simply toggle an I/O pin in the interrupt, do nothing more.

Capture that together with the A / B channels and check if you trigger the interrupt when it should be triggered.

Then slowly extend, take baby steps.

 

You could also try a 400 pulse/revolution encoder such as the link below.

These usually have clean output signals.

Beware that there are different versions and they do not all work on 5V.

https://www.aliexpress.com/whole...

 

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

Last Edited: Tue. Aug 1, 2017 - 02:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Paulvdh, it's KY-040, this one: http://henrysbench.capnfatz.com/.... So as I've said before, it's different from the one used in the circuitsathome.com. I don't have a good scope, only cheap DSO-138. But I've tested my encoder with DMM, and it sure works like on the link.

Red is SPI clock, brown is SPI data (MOSI). I'm sending current value of encoder number of clicks in the end of ISR function. The lowest is one of the encoder channels. Here's the global view of a single transition (click, or detent):

 

If I zoom on the left red/orange spike, we see the following:

 

Clean and nice transition of one channel, and ISR is called only once spitting out current value, which is 2. This is correct, now it should wait till full click is finished (yellow channel goes high). So if I zoom on the right transition, we see:

 

Now full transition took place, and ISR correctly decrements encval, it is now has value of 1.

 

Just simply toggle an I/O pin in the interrupt, do nothing more.
Capture that together with the A / B channels and check if you trigger the interrupt when it should be triggered.
Then slowly extend, take baby steps.

Thanks, will give it a try!

You could also try a 400 pulse/revolution encoder such as the link below.
These usually have clean output signals.
Beware that there are different versions and they do not all work on 5V.

 Thanks, but that's probably an overkill for a simple user input)))) I guess this encoder would be the most heavy and bulky part of the entire device. I guess such encoders are used to get fast motors RPM?

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

OK, I did what you've said. Now ISR looks so:

void ENC_A_P (void)
{
  cur_Transition = (cur_Transition << 2) & 0b00001111;
  cur_Transition |= ((ENC_PORT >> 2) & 0x03);
  if ((cur_Transition == 0b00001011 && prev_Transition == 0b00000010) || (cur_Transition == 0b00000100 && prev_Transition == 0b00001101))
    encval++;
  else if ((cur_Transition == 0b00000111 && prev_Transition == 0b00000001) || (cur_Transition == 0b00001000 && prev_Transition == 0b00001110))
    encval--;
  prev_Transition = cur_Transition;
  DEBUG_PORT ^= (_BV(DEBUG_PIN)); //Toggle debug pin. Before ISR initially set to HIGH.
}

And strange things turned out. For example, this is exactly one click, general view:

 

ENC_A and ENC_B show legitimate behavior, but not debug pin, which should go low on green transition and back to high on yellow transition. Close look:

While ENC_B channel really should cause 3 ISR calls, only 2 seem to get recorded...

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

Actually muce more stranger stuff is going on.

@ 73us Interrupt not triggered when it should have been.

@ 75us Interrupt not triggered when it hsould have been. (Do you trigger on both flanks ? ).

@ 80us Interrupt triggered .... no wait.

 

Looking from the "pin 8" the interrupt seems to be griggered while there is no trigger happening.

Cause: The time your uC spends in the interrupt is probably around 8.5us

So the first interrupt is triggered @ 73us.

@ 75us Or at 79us the interrupt flag is probably set again while the last interrupt is still being executed.

When the interrupt is exiting (@ around 79us) it is triggered immediately (well, almost, it executes a single instruction of the normal program) because the interrupt flag is already set.

So it is not strange at all. It's all very logical and explanable. Just right for the guy with the pointy ears and the antenna in his ear.

 

First hint: Move

Zhenek wrote:
DEBUG_PORT ^= (_BV(DEBUG_PIN)); //Toggle debug pin. Before ISR initially set to HIGH.
  to the beginning of the interrupt function.

The start of the interrupt is more important on the timeline than the exit point.

Or even better. Set the pin when you enter the interrupt, and clear it again as the last line of the interrupt routine.

 

Solutions:

Best is to add hardware to make sure there are no such short bouncing pulses.

Adding RC migt work, or a Flipflop such as Goorman suggests.

You can also make the software more robust to ignore those bouncing contact jitter thingies.

 

Zhenek wrote:
totally eliminates bouncing issue (well, at least in theory ...

Guess you'll have to re-read that theory then :)

 

Tip:

You can drag the border between the grey text area and the logic analyser trace to waste less space on the big gray text area.

 

Paul van der Hoeven.
Bunch of old projects with AVR's:
http://www.hoevendesign.com

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

Hi

Another Solution is Simple digital filter instead of RC

Running encoder routine in timer ISR  e.g.  Every 0.0002s.

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

I don't know how to express this in C, I'll only provide a description here.

 

        Rotary Encoder Phases

 

With this encoder both pins change state when crossing a detent, so there is no need to have an ISR from each pin. When one pin changes the other should have already passed through bouncing and be solid. Therefore a pin-change interrupt from pin A could immediately take the pin B state and the final debounced state of pin A and compare the two. If both are the same state the rotation was CCW, if different then CW.

 

Stan

Last Edited: Tue. Aug 1, 2017 - 06:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,

 

Again, big thanks to everyone, guys. You were of great help.

Finally I've made timer-based poll of encoder pins with 400 Hz frequency, and implemented algorithm from the circuitsathome.com. Works perfectly.

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

So for completeness and to help others using your same encoder, how about posting your code?

 

Ross McKenzie ValuSoft Melbourne Australia