2

I'm using an Arduino Uno and two (3-pin) rotary encoders for a project. Since the loop takes a bit long and accuracy is important, I'd like to use interrupts to read the values from the rotary encoders. Also, pins 2 and 3 are occupied so I can't use hardware interrupts.

I'm using the Rotary library (github link) for the rotary encoders and the EnableInterrupt library (github link) for external interrupts. The Rotary library includes an interrupt example, but I couldn't manage to modify it to work with pin change interrupts. This is my current state (test code):

#include <EnableInterrupt.h>
#include <Rotary.h>

Rotary r = Rotary(2, 3);

void setup() {
  Serial.begin(9600);

  /* Original interrupt setup:
  PCICR |= (1 << PCIE2);
  PCMSK2 |= (1 << PCINT18) | (1 << PCINT19);
  sei();
  */

  enableInterrupt(2, interruptFunction, CHANGE);
  enableInterrupt(3, interruptFunction, CHANGE);
}

void loop() {

}

void interruptFunction() {

  Serial.println("Int. called");

  unsigned char result = r.process();
  if (result == DIR_NONE) {
    // do nothing
  }
  else if (result == DIR_CW) {
    Serial.println("ClockWise");
  }
  else if (result == DIR_CCW) {
    Serial.println("CounterClockWise");
  }
}

Everytime I turn the encoder for one step, the interruptFunction gets called a bunch of times (about 2~4) but the r.process() call never returns a value.

My current guess is that this has something to do with the rotary encoder outputting gray code, but I'm not quite sure how my approach differs from the Rotary interrupt example using hardware interrupts.

Hope you can help!

  • Avoid Serial in an ISR. Check the online documentation. – Mikael Patel Mar 3 '17 at 17:18
  • I'm aware of that, i just used the provided (and working) example as a base to eliminate possible sources of failure. I will of course change it for my production code. – jfkint Mar 4 '17 at 9:39
  • The function process() of the Rotary Library is not a blocking function and only read the state of the 2 pins and compute an output. How you can say "but the r.process() call never returns a value." ? – J. Piquard Mar 4 '17 at 23:27
  • The best strategy for ISR is do the least possible. You don't need to Serial.print inside the ISR. All you need to do is store result and do the print inside loop. In the ISR change a global variable to signal that you have a new value; reset it in the loop after you do your thing. – user31481 Oct 30 '17 at 13:55
1

I too had this problem. I chased my tail in SW for hours. I was making the ISRs non-reentrant (assumed they accidentally were), I was doing SW debouncing etc. I even looked at the signal on a scope but did not see any significant bounces. I eventually added 0.1uF capacitors to the inputs and that solved all my problems with the ISR. So don't trust the encoder manufacturers, there WILL be signal bounce and your Arduino WILL detect it and throw multiple interrupts. Good luck with your project! LDP

  • 2
    > I too had this problem. it isn't fair to blame a coding issue on this topology. But your point about the beauty of hardware debouncing is still valid, and often ignored by newbies wanting to bet their lives on software debouncing. – dannyf Jun 30 '17 at 15:46
1

Everytime I turn the encoder for one step, the interruptFunction gets called a bunch of times (about 2~4)

This is due to contact bounces. You should either implement a hardware debouncing circuit (a simple R-C filter could be enough. Do not connect instead large capacitors directly in parallel to your contacts, as when the contact closes, the energy stored on the capacitor will damage over time the contacts. Using large capacitances on an input can also lead to problems when power is removed), or better a good software debouncing strategy.

For instance, when the interrupt is called the first time, disable the pin-change interrupts. Then set up a timer, which will trigger an interrupt when it expires (the time should be larger than the bounce time, usually several ms or more). Then in the timer interrupt, first put your code:

  unsigned char result = r.process();
  if (result == DIR_NONE) {
    // do nothing
  }
  else if (result == DIR_CW) {
    Serial.println("ClockWise");
  }
  else if (result == DIR_CCW) {
    Serial.println("CounterClockWise");
  }

end re-enable the pin-change interrupts.

  enableInterrupt(2, interruptFunction, CHANGE);
  enableInterrupt(3, interruptFunction, CHANGE);

but the r.process() call never returns a value.

r.process() will ALWAYS returns a value. Maybe you did mean that it returns DIR_NONE ?

1

Code Correction 7th October - Tidy code as per comments Code Correction 7th October - proper reading of rotary encoder

Uses the Pin Change interrupts to allow increment and decrement of the 2 rotary encoders within an interrupt service routine using arduino (in my case nano) with ATMEGA328P chip. Need to add 2 x 104 (0.1uF caps) at the rotary encoder between each pin and the centre ground pin to stop the debounce, doesnt work without the capacitors added. Does not need any library. Watched the Tutorial: Pin Change Interrupts on the Arduino by makecourse which helped understand the Pin change interrupts, will help to watch this if have a different version of arduino so know what digital pins relate to what interrupts.

          //*******************************************  rotary encoder ************************************************************
          #define cbi(sfr, bit) (_SFR_BYTE(sfr) &= ~_BV(bit))    //shortcut way to clear a bit in a register
          #define sbi(sfr, bit) (_SFR_BYTE(sfr) |= _BV(bit))     //shortcut way to set a bit in a register
          
          
          #define Encoder1pin 3 //corresponds to PCINT19
          #define Encoder2pin 4 //corresponds to PCINT20
          #define Encoder3pin 5 //corresponds to PCINT21
          #define Encoder4pin 6 //corresponds to PCINT22
          #define Encoder5pin 7 //corresponds to PCINT23

          unsigned long encoderPos1 = 100;
          unsigned long oldEncPos1 = 0; 
          unsigned long encoderPos2 = 100; //this variable stores our current value of encoder position. Change to int or uin16_t instead of byte if you want to record a larger range than 0-255
          unsigned long oldEncPos2 = 0; //stores the last encoder position value so we can compare to the current reading and see if it has changed (so we know when to print to the serial monitor)
          
          volatile byte portDstatus;
          
          volatile bool readingEncoder1pin = 0;
          volatile bool readingEncoder2pin = 0;
          volatile bool readingEncoder3pin = 0;
          volatile bool readingEncoder4pin = 0;
          volatile bool lastreadingEncoder1pin = 0;
          volatile bool lastreadingEncoder2pin = 0;
          volatile bool lastreadingEncoder3pin = 0;
          volatile bool lastreadingEncoder4pin = 0;
          
          void setup() {
          
            Serial.begin(9600);
          
          
          //*************************************************** rotary encoder ******************************************************************************
            pinMode(Encoder1pin, INPUT);//define pin as INPUT  
            digitalWrite(Encoder1pin, HIGH);//connect the internal pull-up resistor - this will yield a HIGH readout if the switch is open.
            pinMode(Encoder2pin, INPUT);
            digitalWrite(Encoder2pin, HIGH);
            pinMode(Encoder3pin, INPUT);
            digitalWrite(Encoder3pin, HIGH);
            pinMode(Encoder4pin, INPUT);
            digitalWrite(Encoder4pin, HIGH);
            pinMode(Encoder5pin, INPUT);
            digitalWrite(Encoder5pin, HIGH);
          
          
            //setting up the interrupt pins:   //I needed to set all of these otherwise the serial monitor does not read the byte correctly -
                                               // and it doesnt work.
          
            sbi (PCICR, PCIE2); // enable interrupt PCI2);
            sbi (PCMSK2,PCINT23); // set the interrupt control bits for pins 7-3. These bits control whether the pins can trigger the interrupt or not. 
            sbi (PCMSK2, PCINT22);
            sbi (PCMSK2, PCINT21);
            sbi (PCMSK2, PCINT20);
            sbi (PCMSK2, PCINT19); // we do not want to set interrupt control for pins 0 or 1 since this is serial reading .
          
          //******************************************************************* LED *********************************************************************
          }
          
          void loop() 
          
              {
               }
          
           
          
          
          ISR(PCINT2_vect) // Interrupt Service Routine. This code is executed when a state change occurs on a pin 
          {
          
            
          
                portDstatus=PIND;
               readingEncoder1pin = bitRead(portDstatus,3);
               readingEncoder2pin = bitRead(portDstatus,4);
             if(lastreadingEncoder1pin == 1 && lastreadingEncoder2pin == 0 && readingEncoder1pin == 1 && readingEncoder2pin == 1)
              {
                  encoderPos1 ++;
                   lastreadingEncoder1pin = readingEncoder1pin;
                   lastreadingEncoder2pin = readingEncoder2pin;
              }
          
              else if(lastreadingEncoder1pin == 0 && lastreadingEncoder2pin == 1 && readingEncoder1pin == 1 && readingEncoder2pin == 1)
              {
                encoderPos1 --;
                  lastreadingEncoder1pin = readingEncoder1pin;
                  lastreadingEncoder2pin = readingEncoder2pin;;
              }
              else
                {
                   lastreadingEncoder1pin = readingEncoder1pin;
                   lastreadingEncoder2pin = readingEncoder2pin;
                }
          
          
    //  reading the second encoder:       
          
          
          
               readingEncoder3pin = bitRead(portDstatus,5);
               readingEncoder4pin = bitRead(portDstatus,6);
          
             if(lastreadingEncoder3pin == 1 && lastreadingEncoder4pin == 0 && readingEncoder3pin == 1 && readingEncoder4pin == 1)
              {
                  
                  encoderPos2 ++;
                   lastreadingEncoder3pin = readingEncoder3pin;
                   lastreadingEncoder4pin = readingEncoder4pin;
              }
          
              else if(lastreadingEncoder3pin == 0 && lastreadingEncoder4pin == 1  && readingEncoder3pin == 1 && readingEncoder4pin == 1)
              {
                encoderPos2 --;
                  lastreadingEncoder3pin = readingEncoder3pin;
                  lastreadingEncoder4pin = readingEncoder4pin;;
              }
              else
                {
          
                   lastreadingEncoder3pin = readingEncoder3pin;
                   lastreadingEncoder4pin = readingEncoder4pin;
                }
             // serial print for testing only then remove from ISR  
          Serial.print(encoderPos1);
          Serial.println(encoderPos2);
        
             }
  • 1
    This code has a few issues: 1. portDstatus is set but never used. For consistency, it would be better to read PIND only once, then get the individual pin readings out of portDstatus. 2. lastportDstatus is never set nor used. 3. encoderPos and oldEncPos may overflow, they should thus be unsigned in order to avoid undefined behavior. 4. (last)readingEncoder[12]pin need not be long, byte should suffice. 5. Only encoderPos needs to be volatile, assuming it is going to be used in loop(). – Edgar Bonet Oct 6 '20 at 10:59
  • 1
    (continued...) 6. The pull-ups can be enabled with pinMode(pin, INPUT_PULLUP);, which is clearer that using digitalWrite(). 7. There is no point in enabling PCINT on pins 5, 6 and 7 if the encoder is only connected to pins 3 and 4. 8. The decoding logic looks flawed, e.g. lastreadingEncoder1pin should not be set to zero unless the last reading of the encoder 1 pin was indeed zero. 9. Using Serial.println() within an ISR is asking for trouble. – Edgar Bonet Oct 6 '20 at 10:59
  • You have fixed issues 1–4. Great! Still a small issue with the encoder logic: 8′. If the encoder oscillates between readingEncoder(1,2)pin = (1,0) and (1,1), the position will continually increment. And it will decrement if it oscillates as (0,1) ↔ (1,1). This can be fixed by decrementing on (1,1) → (1,0) instead of (0,1) → (1,1). And then: 7′. Only PCINT{20,22} need to be enabled, which would avoid superfluous interrupts. – Edgar Bonet Oct 7 '20 at 7:53
0

check the library to see if it debounces the input.

if not, write your own code. usually state machine-based approaches are very resistant to switch bounces.

-1

The correct solution is to use a debouncing circuit. The MC14490 is a hex debouncing circuit and is perfect for this type of application. No need to block interrupts or set timers. It's available as either SMD or thru hole.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.