Encoder reading with interrupts

Projects Programming
1

Hi, I have problem with my 2 encoders connected to arduino micro (32u4). One encoder - for controlling volume, second for controlling analog input selector.

Vol control is in count0
Sel control is in count1, so first I need to go to count1, then I can control up or down channels

Encoders works ok, they are full step, 20-24pulses. But if I will rotate a little volume encoder, I have "1" on controller , and encoder freezes in this position, then if I rotate second encoder (selector), controller think that it is in count 0 (volume control), and if I rotate selector encoder, then controller are jumping between count0 and count1. When I again rotate a little, then I heard "click" from encoder - full step, encoder hangs up and then all is working ok.

Same situation if I will rotate a little selector encoder , "half step", without hearing click, again controller see "1" from encoeder selector, again rotating volume encoder make changes for selector input.

I have no problem with reading value, it works perfect until encoder go to halfstep / freeze state (move between 0 state and full step, before "click" sound)

I have opened serial monitor and here what I see:
Rotate to down, freeze state
rSelL 1
rSelR 1

Rotate to up, freeze state
rSelL 0
rSelR 1

Selector down
rSelL 1
rSelR 1

Selector up
rSelL 1
rSelR 0

rotate to down, freeze state
rVolL 1
rVolR 0

rotate to up, freeze state
rVolL 1
rVolR 1

Vol down:
rVolL 1
rVolR 1

Vol up:
rVoLL 1
rVolR 0

#define rotaryVolL 13
#define rotaryVolR MISO //(PCINT3)

#define rotarySelL 8 //(PCINT4) (pcint disabled)
#define rotarySelR SS //(PCINT0)

boolean rVolL;
boolean rVolR;
boolean rSelL;
boolean rSelR;

ISR(PCINT0_vect) {  


  rVolL = digitalRead(rotaryVolL);
  rVolR = digitalRead(rotaryVolR);
  rSelL = digitalRead(rotarySelL);
  rSelR = digitalRead(rotarySelR);


//volume encoder
    if (rVolL) {
      if (!rVolR) {
        switch (count) {
          case 0:
              vol = vol - 1;
            break;
        }
      } else {
        switch (count) {
          case 0:
              vol = vol + 1;
              break;
            }
        }
      }
    }
  

//selector encoder
  if (rSelL) {
    if (!rSelR) {
      switch (count) {
        case 0:
          count = 1;

          break;
        case 1:
            Analog++;
          break;
      }
    }

    else {
      switch (count) {
        case 0:
          count = 1;
          break;
        case 1:
            Analog--;
          break;
      }


void setup() {
  pinMode(rotaryVolL, INPUT_PULLUP);
  pinMode(rotaryVolR, INPUT_PULLUP);
  pinMode(rotarySelL, INPUT_PULLUP);
  pinMode(rotarySelR, INPUT_PULLUP);

 PCICR = 1;
  PCMSK0 |= (1 << PCINT0) //SS
  PCMSK0 |= (1 << PCINT3) //MISO
}

If I understand well, I need function which detect if pulses/interrupts are from encoder volume or encoder selector. Then "freezes state" (half step from encoder which occurs "1" on controller) will not disturb work of second encoder?

2

Your code is incomplete. You should post the entire program.

You have way too much code in your ISR. ISRs should be very short. Variables used in an ISR should be declared volitile.

Why not use the Encoder library? It makes things much easier and robust. Use 1 hardware interrupt pin and 1 non-interrupt pin for each encoder to get good response.

3

there should be separate interrupt routine for each encoder. you can use attachInterrupt ()

make things simple to start and only detect the rising edge of one of the encoder outputs. the ISR then inc/decrements the count depending on the state of the other encoder output

print both counts in loop() until satisfied

4

Hi! Thank you for your reply.

I have tried with this code -> https://electronoobs.com/eng_arduino_tut125.php , "Code Example II (with interruptions)"

#define rotaryVolL 13
#define rotaryVolR MISO

#define rotarySelL 8
#define rotarySelR SS

int counter = 0;                    //Use this variable to store "steps"
int previous_counter = 0;           //Use this variable to store previous "steps" value
int currentStateClock;              //Store the status of the clock pin (HIGH or LOW)
int StateData;                      //Store the status of the data pin (HIGH or LOW)
int lastStateClock;                 //Store the PREVIOUS status of the clock pin (HIGH or LOW)
String currentDir ="";              //Use this to print text 

int counter1 = 0;                    //Use this variable to store "steps"
int previous_counter1 = 0;           //Use this variable to store previous "steps" value
int currentStateClock1;              //Store the status of the clock pin (HIGH or LOW)
int StateData1;                      //Store the status of the data pin (HIGH or LOW)
int lastStateClock1;                 //Store the PREVIOUS status of the clock pin (HIGH or LOW)


ISR(PCINT0_vect){  
  cli(); //We pause interrupts happening before we read pin values
  currentStateClock =   digitalRead(rotaryVolR);       //Check pin D9 state? Clock
  StateData  =   digitalRead(rotaryVolL);            //Check pin D8 state? Data
  if (currentStateClock != lastStateClock){
    // If "clock" state is different "data" state, the encoder is rotating clockwise
    if (StateData != currentStateClock){ 
      counter  ++;                                // We increment   
      lastStateClock = currentStateClock;         // Updates the previous state of the clock with the current state
      sei(); //restart interrupts
    }
    //Else, the encoder is rotating counter-clockwise 
    else {
      counter --;                                 // We decrement
      lastStateClock = currentStateClock;         // Updates  previous state of the clock with the current state    
      sei(); //restart interrupts
    } 
  }  

 currentStateClock1 =   digitalRead(rotarySelR);       //Check pin D9 state? Clock
  StateData1  =   digitalRead(rotarySelL);            //Check pin D8 state? Data
  if (currentStateClock1 != lastStateClock1){
    // If "clock" state is different "data" state, the encoder is rotating clockwise
    if (StateData1 != currentStateClock1){ 
      counter1  ++;                                // We increment   
      lastStateClock1 = currentStateClock1;         // Updates the previous state of the clock with the current state
      sei(); //restart interrupts
    }
    //Else, the encoder is rotating counter-clockwise 
    else {
      counter1 --;                                 // We decrement
      lastStateClock1 = currentStateClock1;         // Updates  previous state of the clock with the current state    
      sei(); //restart interrupts
    } 
  }  

void setup() {
  pinMode(rotaryVolL, INPUT_PULLUP);
  pinMode(rotaryVolR, INPUT_PULLUP);
  pinMode(menuVol, INPUT);
  pinMode(rotarySelL, INPUT_PULLUP);
  pinMode(rotarySelR, INPUT_PULLUP);
  pinMode(menuSel, INPUT);
  PCICR = 1;
  PCMSK0 |= (1 << PCINT0); //SS
  PCMSK0 |= (1 << PCINT3); //MISO
  PCMSK0 |= (0 << PCINT4);

  Serial.begin(115200);
  lastStateClock = digitalRead(rotaryVolR);
   lastStateClock = digitalRead(rotarySelR);

}

void loop() {

  if(counter != previous_counter)
  {
    Serial.print("Counter: ");
    Serial.println(counter);
  }

    if(counter1 != previous_counter1)
  {
    Serial.print("Counter1: ");
    Serial.println(counter1);
  }
}

and iit reads encoder half step before "click" as normal step. so one correctly rotate = 2 step. I dont know how to change in this code to read only full step, after "click" sound from encoder. Also it doesnt show "0" count, just number from 1 to up and from -1 to down.

5

lotta code in the ISR

i believe you need to recognize both RISING and FALLING and inc/dec based on both signals.

code i'm using to monitor 2 encoders on an ESP32 turned by hand. separate interrupts that call a common routine

#include <Arduino.h>

#include "encoder.h"
#include "pins.h"

// -----------------------------------------------------------------------------
//  table containing action indexed by inputs BA and state

typedef enum { ___, For, Rev, Skp, ActSize } Act_t;

const char* actStr [] = { "___", "For", "Rev", "Skp", "ActSize" };

Act_t  encAct [ActSize][ActSize] = {
// state  00   01   10   11      inputs
       { ___, Rev, For, Skp },  // 00
       { For, ___, Skp, Rev },  // 01
       { Rev, Skp, ___, For },  // 10
       { Skp, For, Rev, ___ },  // 11
};

int encAst  = 0;
int encBst  = 0;

// ------------------------------------------------
// read brake
inline void readEncoder (
    byte  dt,
    byte  clk,
    int & brkPos,
    int & encSt )
{
    byte  val = digitalRead (dt) << 1 | digitalRead (clk);

    switch (encAct [val][encSt])  {
    case For:
        brkPos++;
        break;

    case Rev:
        brkPos--;
        break;

    default:
        break;
    }

    encSt = val;

 // digitalWrite (LED, ! digitalRead (LED));
}

// -------------------------------------
void IRAM_ATTR isrEncA (void)
{
    readEncoder (Enc_A_Dt, Enc_A_Clk, encApos, encAst);
}

// -------------------------------------
void IRAM_ATTR isrEncB (void)
{
    readEncoder (Enc_B_Dt, Enc_B_Clk, encBpos, encBst);
}

// -------------------------------------
void
encoderInit (void)
{
    // encoders
    pinMode (Enc_A_Clk, INPUT_PULLUP);
    pinMode (Enc_A_Dt,  INPUT_PULLUP);

    attachInterrupt (Enc_A_Clk, isrEncA, CHANGE);
    attachInterrupt (Enc_A_Dt,  isrEncA, CHANGE);

    pinMode (Enc_B_Clk, INPUT_PULLUP);
    pinMode (Enc_B_Dt,  INPUT_PULLUP);

    attachInterrupt (Enc_B_Clk, isrEncB, CHANGE);
    attachInterrupt (Enc_B_Dt,  isrEncB, CHANGE);
}
6

Ok, I have connected output from encoder to oscilloscope and in "halft step", before "click" output go high, then, after click (like full step), go low. So as you already wrote, I need to detect if its rising or falling signal. But I dont know how to make this.

encoder.h - which library are you using?

7

You should study the State Table Approach to reading encoders. Here's an implementation of that technique that I wrote: https://github.com/gfvalvo/NewEncoder

8

@gfvalvo

your NewEncoder-library requires an interrupt-capable pin for both channels.
Would it be possible to use the state-table-approach with only one channel on a interrupt-capable IO-pin?

If yes. Have you ever thought about expanding your NewEncoder-code to work this way?

Do you happen to know a encoder-library that combines state-table-approach with a timer-interrupt?

best regards Stefan

9

CHANGE means RISING or FALLING
look that code over to understand "what" it does and hopefully why

this is code i wrote, no library

encoder.h

#ifndef ENCODER_H
# define ENCODER_H

extern int encApos;
extern int encBpos;

extern int encAst;
extern int encBst;

void encoderInit (void);

#endif
10

Probably possible. But I've never had a need to do so since my projects use processors that support interrupts on nearly all pins (ARM and ESP-based). The PJRC Encoder Library is capable of working off a single interrupt-capable pin.

Sorry, I haven't looked at the source code of any encoder libraries that are based on timer interrupts. There's no fundamental reason they couldn't use state tables.

11

OK. I looked up the Rotary-library from Github-user Buxtronix

I took parts of his poll encoder-example and combined it with a timer-interrupt using timer2 which is called at a frequency of 10 kHz.

I tested the demo-code with a cheap mechanical rotary-encoder which surely has some bouncing.
Counting up/down works reliably.

/* Demo-Code that uses the Rotary-library from GitHub-User https://github.com/buxtronix
 * using his library https://github.com/buxtronix/arduino/tree/master/libraries/Rotary  
 * in combination with a timer-interrupt executed 10000 times per second
 * Copyright 2023 StefanL38. Licenced under the GNU GPL Version 3.
 * A T T E N T I O N ! 
 * this demo-code uses Timer2 which is used by other libraries too. 
 * This means using this code can interfere with other libraries
 * causing malfunction of both
 * 
 * As timer-interrupts are very hardware-sepcific
 * this demo-code works with Arduino-Uno R3 based on the chip AtMega328P
 * if you want to use it on other microcontrollers you will have to
 * modify setting up the timer-interrupt according to the hardware
 * of this other microcontroller
 * 
 * The demo-code simply prints the value of variable myCounter 
 * each time the value of the variable changes
 */
#include <Rotary.h>

// Rotary encoder is wired with the common to ground and the two
// outputs to pins 5 and 6.
const byte channel_A_Pin = 5;
const byte channel_B_Pin = 6;
Rotary rotary = Rotary(channel_A_Pin, channel_B_Pin);

unsigned long myISR_TimerFrequency = 10000;
// myCounter that will be incremented or decremented by rotation.
// as this variable is changed in an interrupt-service-routine
// this variable MUST !! be declared volatile to make sure 
// that it works properly !
volatile long  myCounter = 0;
long last_myCounter = 0;


void setupTimerInterrupt(unsigned long ISR_call_frequency) {
  long OCR2A_value;

  const byte Prescaler___8 = (1 << CS21);
  const byte Prescaler__32 = (1 << CS21) + (1 << CS20);
  const byte Prescaler__64 = (1 << CS22);
  const byte Prescaler_128 = (1 << CS22) + (1 << CS20);
  const byte Prescaler_256 = (1 << CS22) + (1 << CS21);
  const byte Prescaler1024 = (1 << CS22) + (1 << CS21) + (1 << CS20);

  const unsigned long CPU_Clock = 16000000;

  cli();//stop interrupts

  TCCR2A = 0;// set entire TCCR2A register to 0
  TCCR2B = 0;// same for TCCR2B
  TCNT2  = 0;//initialize counter value to 0

  TCCR2A |= (1 << WGM21); // turn on CTC mode
  TIMSK2 |= (1 << OCIE2A); // enable timer compare interrupt

  TCCR2B = Prescaler__32; 
  // OCR2A_value must be smaller than 256
  // use a different prescaler if OCR2A_value is > 256
  OCR2A_value = (CPU_Clock / ( 32 * ISR_call_frequency) )  - 1;

  OCR2A = OCR2A_value; // set the value of OCR2A

  sei();//allow interrupts
}


// timer-interrupt-service-function for AtMega328P
ISR(TIMER2_COMPA_vect) {
  unsigned char result = rotary.process();

  // depending on having detected rotation
  if (result == DIR_CW) {
    myCounter++;
  }
  else if (result == DIR_CCW) {
    myCounter--;
  }
}


void PrintFileNameDateTime() {
  Serial.println( F("Code running comes from file ") );
  Serial.println( F(__FILE__) );
  Serial.print( F("  compiled ") );
  Serial.print( F(__DATE__) );
  Serial.print( F(" ") );
  Serial.println( F(__TIME__) );
}


// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

unsigned long MyTestTimer = 0;  // Timer-variables MUST be of type unsigned long
const byte    OnBoard_LED = 13;


void BlinkHeartBeatLED(int IO_Pin, int BlinkPeriod) {
  static unsigned long MyBlinkTimer;
  pinMode(IO_Pin, OUTPUT);

  if ( TimePeriodIsOver(MyBlinkTimer, BlinkPeriod) ) {
    digitalWrite(IO_Pin, !digitalRead(IO_Pin) );
  }
}


void setup() {
  Serial.begin(115200);
  Serial.println("Setup-Start");
  PrintFileNameDateTime();
  setupTimerInterrupt(myISR_TimerFrequency);
}


void loop() {

  BlinkHeartBeatLED(OnBoard_LED, 250);

  // check if value has changed
  if (last_myCounter != myCounter) {
    // only if value REALLY HAS changed
    last_myCounter = myCounter; // update last_myCounter
    Serial.print("myCounter=");
    Serial.println(myCounter);
  }
}

best regards Stefan

12

That statement does not access the volatile long 'myCounter' atomically. So it is not interrupt-safe and can be corrupted when the variable is modified during a timer interrupt.

13

Aha. And how would a code-version look like that is interrupt-safe?

14

make them bytes

15

in the timer-isr or in the if-statement?
and as the value might count up / down beyond +-127 using multiple bytes would be even worse or am I wrong?

16

Turn off interrupts, make a copy, turn on interrupts, use the copy.

void loop() {

  BlinkHeartBeatLED(OnBoard_LED, 250);

   //  Copy encoder variable
  noInterrupts();
  long counterCopy = myCounter;
  Interrupts();
  // check if value has changed
  if (last_myCounter != counterCopy) {
    // only if value REALLY HAS changed
    last_myCounter = counterCopy; // update last_myCounter
    Serial.print("myCounter=");
    Serial.println(counterCopy);
  }
}
17

i suggested making the variables bytes, but you're right, not if their values can change by +/- 127 between interrupts

but aren't these used as hand adjusted controls? not likely to change much. just track the deltas

18

most likely and then -+127 or 0-255 is enough

This is a good idea

19

Using a Critical Section (Post #16) is more general and flexible.

1 Like
20

why is it more flexible?