Rotary encoder debouncing

Hi,

we're currently struggling to get a rotary encoder working the way we'd like to.
We connected the encoder according to the image below.

(Actual wiring)

(Example circuit we found in the datasheet of our rotary encoder.)

Since we didn't have 10k resistors and 0.01µ capacitors we used 1k and 0.1µ which should give us the same values. We also tried 2k resistors to slow down the charging of the capacitor. Both values didnÄt work properly.

It's definitely working better than before (without the debouncing circuit) but we still get contact bouncing sometimes.

Here's our code:

int dPinLatch = 11;
int dPinClock = 12;
int dPinData = 10;

int dPinRotEnc1A = 14;
int dPinRotEnc1B = 15;

volatile bool bits[8];
int numbers[5] = {1, 2, 4, 8, 16};

void setup() {
  Serial.begin(9600);
  Serial.println("Arduino online");

  pinMode(dPinLatch, OUTPUT);
  pinMode(dPinClock, OUTPUT);
  pinMode(dPinData, OUTPUT);

  pinMode(dPinRotEnc1A, INPUT);
  pinMode(dPinRotEnc1B, INPUT);

  attachInterrupt(digitalPinToInterrupt(dPinRotEnc1A), doEncoder, CHANGE);
}

void loop() {
}

volatile int counter = 0;
void doEncoder() {    
    if (digitalRead(dPinRotEnc1A) == digitalRead(dPinRotEnc1B)) {
      counter++;
    } else {
      counter--;
    }
    
    if(counter > 4){
      counter = 0;
    }
    if(counter < 0){
      counter = 4;
    }
    writeNumberToLed(numbers[counter]);
}

void writeNumberToLed(int number){
  Serial.print("Number: ");
  Serial.println(number);
  digitalWrite(dPinLatch, LOW);
  shiftOut(dPinData, dPinClock, LSBFIRST, number);
  digitalWrite(dPinLatch, HIGH);
}

Sometimes the code registers two forward "clicks" instead of one, and sometimes it registers one forward and one backward.

Any ideas on how to fix this?

I've found the filter circuit at the bottom right of page 2 here to work well. The one on your datasheet seems too simple

It's what I use on my Standalone Programmer card.
You can see the 6 components right above the encoder:
http://www.crossroadsfencing.com/BobuinoRev17/

Isn't the one you linked the exact same circuit I linked in the OP?

Why are you trying to use interrupts?

De-bouncing of mechanical interrupters should be performed in software, not hardware.

Ben Buxton has a good explanation of how to handle a rotary encoder and code that works well.

There is a simpler adaptation version of his library (no interrupts) in my code repository, link below.

In the end we'll have 3 rotary encoders and 12 Buttons. We won't be able to read all this input within the loop.

We also tried to debounce via software like so:

volatile int counter = 0;
volatile int lastEncoding = 0;
void doEncoder() {
  int currMillis = millis();
  if (currMillis - lastEncoding > 50) {
    
    if (digitalRead(dPinRotEnc1B) == HIGH) {
      counter++;
    } else {
      counter--;
    }
    
    if(counter > 4){
      counter = 0;
    }
    if(counter < 0){
      counter = 4;
    }
    writeNumberToLed(numbers[counter]);
    lastEncoding = currMillis;
  }
}

Oh, down at the bottom of that datasheet, yes. I stopped at the one in the middle as I couldn't read the text with it.

We just ordered an more expensive and sturdy rotary encoder. The one we are using now gives us signal just by pressing against it.

Mazon:
In the end we'll have 3 rotary encoders and 12 Buttons. We won't be able to read all this input within the loop.

Why would you think that? Are you not able to think with more than four variables, or just what is the problem?

Mazon:
We also tried to debounce via software like so:

That code does not even compile for a start. It has no setup() and no loop().

It most certainly contains no evidence whatsoever of debouncing. Did you imagine it did?

:roll_eyes: :roll_eyes: This looks like being a long haul!

Paul__B:
Why would you think that? Are you not able to think with more than four variables, or just what is the problem?

Ok then.. enlighten me: How are we supposed to read all these inputs in a single loop, without missing a ton of button clicks/user inputs? For example, at a certain point in my loop I read the state of a certain digital input pin. What if i push the button at a moment, where my loop is busy reading other input?
That's why we thought we'd need interrupts.

Paul__B:
That code does not even compile for a start. It has no setup() and no loop().

It most certainly contains no evidence whatsoever of debouncing. Did you imagine it did?

I thought it'd be clear for someone who read at least the OP, that this is not the whole sketch, but another version of the same function in the sketch from the OP.
But I'll put it in context for you:

int dPinLatch = 11;
int dPinClock = 12;
int dPinData = 10;

int dPinRotEnc1A = 14;
int dPinRotEnc1B = 15;

volatile bool bits[8];
int numbers[5] = {1, 2, 4, 8, 16};

void setup() {
  Serial.begin(9600);
  Serial.println("Arduino online");

  pinMode(dPinLatch, OUTPUT);
  pinMode(dPinClock, OUTPUT);
  pinMode(dPinData, OUTPUT);

  pinMode(dPinRotEnc1A, INPUT);
  pinMode(dPinRotEnc1B, INPUT);

  attachInterrupt(digitalPinToInterrupt(dPinRotEnc1A), doEncoder, CHANGE);
}

void loop() {
}

volatile int counter = 0;
volatile int lastEncoding = 0;
void doEncoder() {
  int currMillis = millis();
  if (currMillis - lastEncoding > 50) {
    
    if (digitalRead(dPinRotEnc1B) == HIGH) {
      counter++;
    } else {
      counter--;
    }
    
    if(counter > 4){
      counter = 0;
    }
    if(counter < 0){
      counter = 4;
    }
    writeNumberToLed(numbers[counter]);
    lastEncoding = currMillis;
  }
}

void writeNumberToLed(int number){
  Serial.print("Number: ");
  Serial.println(number);
  digitalWrite(dPinLatch, LOW);
  shiftOut(dPinData, dPinClock, LSBFIRST, number);
  digitalWrite(dPinLatch, HIGH);
}

To your second point: How I imagined this would debounce the input? I'd save the time of the last function call in lastEnconding, now everytime the interrupt function is called, I check if the last call is atleast 50ms ago.

Paul__B:
:roll_eyes: :roll_eyes: This looks like being a long haul!

I'm afraid so.

Mazon:
Ok then.. enlighten me: How are we supposed to read all these inputs in a single loop, without missing a ton of button clicks/user inputs? For example, at a certain point in my loop I read the state of a certain digital input pin. What if I push the button at a moment, where my loop is busy reading other input?

Then the button will be read on the next cycle through the loop, as it should be. In the process of de-bouncing, there is by definition, no hurry to detect the initial press.

Mazon:
That's why we thought we'd need interrupts.

A common mistake is to fail to conceptualise just how fast the loop() runs. If properly written, it should cycle at least 5000 times per second - at least five times per millisecond - and generally much more than that. Unless your encoder is driven by a power drill, that should be more than fast enough.

It is only the operations that take significant time that you need to manage - such as printing. But actually printing some information generally signifies that you do not need to collect further information at the time of printing. More about that in a moment. :grinning:

Mazon:
But I'll put it in context for you:

Ooh, that is nasty! As best I understand that code - in either version - the interrupt executes a function which calls another function which uses Serial.print. So you are attempting to execute Serial.print within an interrupt. :roll_eyes:

Mazon:
To your second point: How I imagined this would debounce the input? I'd save the time of the last function call in lastEnconding, now every time the interrupt function is called, I check if the last call is at least 50ms ago.

You appear to be saving the time only after Serial.print is called within the interrupt routine. Given how long Serial.print takes to execute, this could be an awful lot longer than 50 ms and all sorts of things might have happened in that interval.

The general concept is not too bad, you make a note of when the encoder state changes (on only one pin - you are sensing half steps) and keep checking on every bounce (of that pin) in either direction, waiting for the interval to elapse in which case you act on whether the other encoder pin is high or not.

No, that is not going to work. If you insist in responding to every bounce, you must completely characterise the change of state to determine how to interpret it. 50 ms is a long time (a usual bounce period would be more like 5 ms for an encoder and 10 ms for a pushbutton) and all sorts of things could happen in that interval of which you would be entirely ignorant.


My method of de-bouncing is to poll the input(s) every time the loop cycles. If a change of state is detected, a decision is made as to whether the change is sustained since the last poll, otherwise it reverts to the previous state. If the change is sustained for every poll over the debounce interval (the timer is only reviewed if a change has been so maintained since the last cycle through the loop), then the corresponding action is taken. In so doing, changes that revert within the debounce interval are completely ignored and the interval is only started again when the next change is sensed. An example of the code is below.

By the way, if by

if(counter > 4){
      counter = 0;
    }
    if(counter < 0){
      counter = 4;
    }

you really meant to restrict the range to 0 to 3, it could be abbreviated to

  counter &= 3;

and lastEncoding and currMillis should be unsigned long and 50 should be 50UL


Debouncing:

// Binary cycling multiple LEDs

const int led1Pin =  13;    // LED pin number
const int led2Pin =  11;
const int button1 =  4;
char led1State = LOW;        // initialise the LED
char led2State = LOW;
char allState = 0;
char bstate1 = 0;
unsigned long bcount1 = 0; // button debounce timer.  Replicate as necessary.

// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
boolean timeout(unsigned long *marker, unsigned long interval) {
  if (millis() - *marker >= interval) {
    *marker += interval;    // move on ready for next interval
    return true;
  }
  else return false;
}

// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
  switch (*butnstate) {               // Odd states if was pressed, >= 2 if debounce in progress
    case 0: // Button up so far,
      if (button == HIGH) return false; // Nothing happening!
      else {
        *butnstate = 2;                 // record that is now pressed
        *marker = millis();             // note when was pressed
        return false;                   // and move on
      }

    case 1: // Button down so far,
      if (button == LOW) return false; // Nothing happening!
      else {
        *butnstate = 3;                 // record that is now released
        *marker = millis();             // note when was released
        return false;                   // and move on
      }

    case 2: // Button was up, now down.
      if (button == HIGH) {
        *butnstate = 0;                 // no, not debounced; revert the state
        return false;                   // False alarm!
      }
      else {
        if (millis() - *marker >= interval) {
          *butnstate = 1;               // jackpot!  update the state
          return true;                  // because we have the desired event!
        }
        else
          return false;                 // not done yet; just move on
      }

    case 3: // Button was down, now up.
      if (button == LOW) {
        *butnstate = 1;                 // no, not debounced; revert the state
        return false;                   // False alarm!
      }
      else {
        if (millis() - *marker >= interval) {
          *butnstate = 0;               // Debounced; update the state
          return false;                 // but it is not the event we want
        }
        else
          return false;                 // not done yet; just move on
      }
    default:                            // Error; recover anyway
      {
        *butnstate = 0;
        return false;                   // Definitely false!
      }
  }
}

void setup() {
  pinMode(led1Pin, OUTPUT);
  pinMode(led2Pin, OUTPUT);
  pinMode(button1, INPUT);
  digitalWrite(button1, HIGH);          // internal pullup all versions
}

void loop() {
  // Cycle LEDs if button debounced
  if (butndown(digitalRead(button1), &bcount1, &bstate1, 10UL )) {
    allState++; allState &= 3;          // Increment state, constrain to 0 to 3
    if (allState & 1) led1State = HIGH; // LED 1 on if odd state
    else led1State = LOW;
    if (allState & 2) led2State = HIGH; // LED 2 on if 2 or 3
    else led2State = LOW;
  }
  digitalWrite(led1Pin, led1State);
  digitalWrite(led2Pin, led2State);
}

Thanks for the detailed answer. I think it really helped clearing up a few things for me, but there a still a few questions left:

Paul__B:
Then the button will be read on the next cycle through the loop, as it should be. In the process of de-bouncing, there is by definition, no hurry to detect the initial press.

But that's just the case if we're talking about a switch, not a push-button, right? If I'd manage to push and release the button within the cycle of one loop, it'd go unrecognised?

Paul__B:
A common mistake is to fail to conceptualise just how fast the loop() runs. If properly written, it should cycle at least 5000 times per second - at least five times per millisecond - and generally much more than that. Unless your encoder is driven by a power drill, that should be more than fast enough.

It is only the operations that take significant time that you need to manage - such as printing. But actually printing some information generally signifies that you do not need to collect further information at the time of printing. More about that in a moment. :grinning:

We tried reading 5 buttons in a loop, and toggle the corresponding LED. We had the problem that many button clicks were not recognised. But I'm not sure if we had any serial.println calls in it, which might caused the problem.

Later on we'll need to establish a serial connection between the arduino and a PC. On every button click / rotation we'll send a message (using CmdMessenger) to the PC. Will this be fast enough to be executed within the loop?

Paul__B:
The general concept is not too bad, you make a note of when the encoder state changes (on only one pin - you are sensing half steps) and keep checking on every bounce (of that pin) in either direction, waiting for the interval to elapse in which case you act on whether the other encoder pin is high or not.

No, that is not going to work. If you insist in responding to every bounce, you must completely characterise the change of state to determine how to interpret it. 50 ms is a long time (a usual bounce period would be more like 5 ms for an encoder and 10 ms for a pushbutton) and all sorts of things could happen in that interval of which you would be entirely ignorant.

I'll have a look at that as soon as I'm home. Thanks :slight_smile:

You never need to debounce quadrature encoders if you arrange that a single steps worth of
oscillation in the output is ignored (a small deadzone).

If the encoder happens to rest at the point of one contact breaking/making then small vibrations
will also cause oscillations (at longer periods possibly), which you have to deal with anyway.

So I'd say:

Don't debouce the encoder.
Do add a single count of hysteresis in interpreting its output count:

  if (encoder_count > previous_count + 1 || encoder_count < previous_count - 1)
  {
    previous_count = encoder_count ;
    handle_encoder_change () ;
  }

Mazon:
But that's just the case if we're talking about a switch, not a push-button, right? If I'd manage to push and release the button within the cycle of one loop, it'd go unrecognised?

Just how many times can you press the button in one millisecond?

Mazon:
We tried reading 5 buttons in a loop, and toggle the corresponding LED. We had the problem that many button clicks were not recognised. But I'm not sure if we had any serial.println calls in it, which might caused the problem.

I get concerned when you say "in a loop". I am referring to the loop(). There should be no loops within the loop. :grinning:

Mazon:
Later on we'll need to establish a serial connection between the Arduino and a PC. On every button click / rotation we'll send a message (using CmdMessenger) to the PC. Will this be fast enough to be executed within the loop?

I have no idea what CmdMessenger is. Continuing to monitor events while executing slow communications is always going to be a challenge even using interrupts which may poll millis(), but may not execute any function which itself takes any significant time whatsoever.

MarkT:
You never need to debounce quadrature encoders if you arrange that a single steps worth of oscillation in the output is ignored (a small deadzone).

If the encoder happens to rest at the point of one contact breaking/making then small vibrations will also cause oscillations (at longer periods possibly), which you have to deal with anyway.

Interesting point and quite correct. That means you then cannot respond to "quarter" steps and - the problem with the code I was critiquing - you must however at least correctly handle every "half" step. Also, in so handling each "quarter" step with interrupts, you cause a barrage of interrupts with some slight possibility that you may miss an interrupt within the interrupt.


I do wish you would fix (get rid of) your word wrap!

Mazon:
Any ideas on how to fix this?

Give the code a try which I posted here:
https://forum.arduino.cc/index.php?topic=318170.msg2202599#msg2202599

It' already prepared to read more than one encoder.

Your additional "debounce hardware" connected to the A/B pins of the encoder is superfluous and you don't need it.

Proper hardware debouncing requires a low-pass filter (i.e., resistor and capacitor) feeding a Schmitt trigger. The concept being that the new state is integrated such that it must on average, be stable for a certain duration to be considered valid. Note the part of "on average".

Proper software debouncing as I have explained in reply #10, is more strict - the new state is considered valid only once it has been stable for all of the specified duration, not merely for most of it. It does a better job than hardware debouncing.

Hi all,
let me share the solution that worked best for me.
I used 10k pullups and 100n caps to ground for hw debouncing. Initially I used internal pullups but I found a lot of noise on one of the inputs. Probably due to my poor hardware design but I never tried to investigate, 10k pullups fixed the issue. Oscilloscope showed bounce free output even with internal pullups, so they could work for you. The encoder I'm using is a cheap generic one I got from e-bay. It is attached to external interrupts on a attiny2313 running at 8mhz. I'm not using arduino and I'm codding in C.
Interrupts are triggered on the falling edge and what I do inside ISRs is to pull the state of the other pin and if it is low do whatever I want to do(increment/decrement a value, raise a flag, etc.).
In C it looks like this:

void setup()
{
DDRD &= ~(1<<PIND2 | 1<<PIND3);//int0 and 1 inputs
PORTD &= ~(1<<PIND2 | 1<<PIND3);//internal pullups disabled

GIMSK |=  (1<<INT0) | (1<<INT1);//enable external interrupts
MCUCR |= (1<<ISC11)|(1<<ISC01);//falling edge triggers interrupts
}

ISR(INT0_vect)
{
	if (bit_is_clear(PIND, PIND3))//check state of the other pin
	{
               ++value; //or do whatever you want to do
        }
}

ISR(INT1_vect)
{
	if (bit_is_clear(PIND, PIND2))//check state of the other pin
	{
		--value; //or do whatever you want to do
	}
}

The idea is that we have a valid transaction only when both pins are low at the same time and we are using the interrupt on second one that goes down to detect the direction of rotation. If the encoder is properly debounced in hardware this method gives glitch free operation and short code. Even if bounce occurs sometimes it may result only in jumps ahead and never in reverse or totally reversing the direction of the encoder as it happens whit other methods I've tried.
It can easily be adapted for PCIE interrupt.
I couldn't make the code to fail regardless of how fast or slow I rotated the encoder and I'm happy with it.
I hope you got the idea regardless of my bad English :slight_smile: So adapt it for arduino and give it a try.

Paul__B:
Then the button will be read on the next cycle through the loop, as it should be. In the process of de-bouncing, there is by definition, no hurry to detect the initial press.
A common mistake is to fail to conceptualise just how fast the loop() runs. If properly written, it should cycle at least 5000 times per second - at least five times per millisecond - and generally much more than that. Unless your encoder is driven by a power drill, that should be more than fast enough.

It is only the operations that take significant time that you need to manage - such as printing. But actually printing some information generally signifies that you do not need to collect further information at the time of printing. More about that in a moment. :grinning:
Ooh, that is nasty! As best I understand that code - in either version - the interrupt executes a function which calls another function which uses Serial.print. So you are attempting to execute Serial.print within an interrupt. :roll_eyes:
You appear to be saving the time only after Serial.print is called within the interrupt routine. Given how long Serial.print takes to execute, this could be an awful lot longer than 50 ms and all sorts of things might have happened in that interval.

The general concept is not too bad, you make a note of when the encoder state changes (on only one pin - you are sensing half steps) and keep checking on every bounce (of that pin) in either direction, waiting for the interval to elapse in which case you act on whether the other encoder pin is high or not.

No, that is not going to work. If you insist in responding to every bounce, you must completely characterise the change of state to determine how to interpret it. 50 ms is a long time (a usual bounce period would be more like 5 ms for an encoder and 10 ms for a pushbutton) and all sorts of things could happen in that interval of which you would be entirely ignorant.


My method of de-bouncing is to poll the input(s) every time the loop cycles. If a change of state is detected, a decision is made as to whether the change is sustained since the last poll, otherwise it reverts to the previous state. If the change is sustained for every poll over the debounce interval (the timer is only reviewed if a change has been so maintained since the last cycle through the loop), then the corresponding action is taken. In so doing, changes that revert within the debounce interval are completely ignored and the interval is only started again when the next change is sensed. An example of the code is below.

By the way, if by

if(counter > 4){

counter = 0;
    }
    if(counter < 0){
      counter = 4;
    }



you really meant to restrict the range to 0 to 3, it could be abbreviated to 


counter &= 3;




and lastEncoding and currMillis should be unsigned long and 50 should be 50UL


---

Debouncing:


// Binary cycling multiple LEDs

const int led1Pin =  13;    // LED pin number
const int led2Pin =  11;
const int button1 =  4;
char led1State = LOW;        // initialise the LED
char led2State = LOW;
char allState = 0;
char bstate1 = 0;
unsigned long bcount1 = 0; // button debounce timer.  Replicate as necessary.

// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
boolean timeout(unsigned long *marker, unsigned long interval) {
  if (millis() - *marker >= interval) {
    *marker += interval;    // move on ready for next interval
    return true;
  }
  else return false;
}

// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
  switch (*butnstate) {              // Odd states if was pressed, >= 2 if debounce in progress
    case 0: // Button up so far,
      if (button == HIGH) return false; // Nothing happening!
      else {
        *butnstate = 2;                // record that is now pressed
        *marker = millis();            // note when was pressed
        return false;                  // and move on
      }

case 1: // Button down so far,
      if (button == LOW) return false; // Nothing happening!
      else {
        *butnstate = 3;                // record that is now released
        *marker = millis();            // note when was released
        return false;                  // and move on
      }

case 2: // Button was up, now down.
      if (button == HIGH) {
        *butnstate = 0;                // no, not debounced; revert the state
        return false;                  // False alarm!
      }
      else {
        if (millis() - *marker >= interval) {
          *butnstate = 1;              // jackpot!  update the state
          return true;                  // because we have the desired event!
        }
        else
          return false;                // not done yet; just move on
      }

case 3: // Button was down, now up.
      if (button == LOW) {
        *butnstate = 1;                // no, not debounced; revert the state
        return false;                  // False alarm!
      }
      else {
        if (millis() - *marker >= interval) {
          *butnstate = 0;              // Debounced; update the state
          return false;                // but it is not the event we want
        }
        else
          return false;                // not done yet; just move on
      }
    default:                            // Error; recover anyway
      {
        *butnstate = 0;
        return false;                  // Definitely false!
      }
  }
}

void setup() {
  pinMode(led1Pin, OUTPUT);
  pinMode(led2Pin, OUTPUT);
  pinMode(button1, INPUT);
  digitalWrite(button1, HIGH);          // internal pullup all versions
}

void loop() {
  // Cycle LEDs if button debounced
  if (butndown(digitalRead(button1), &bcount1, &bstate1, 10UL )) {
    allState++; allState &= 3;          // Increment state, constrain to 0 to 3
    if (allState & 1) led1State = HIGH; // LED 1 on if odd state
    else led1State = LOW;
    if (allState & 2) led2State = HIGH; // LED 2 on if 2 or 3
    else led2State = LOW;
  }
  digitalWrite(led1Pin, led1State);
  digitalWrite(led2Pin, led2State);
}

Digging up this old thread just to +1 using &= to wrap two's complement integers. I had to test this in the chrome console to be sure, but it's a really elegant solution for something I've written poorly many times.

-2 & 3 = 2
-1 & 3 = 3
0 & 3 = 0
1 & 3 = 1
4 & 3 = 0
5 & 3 = 1