real-time reading of a quadrature encoder at full resolution with only one interrupt on ATmega328

Question

I want to read a quadrature rotary encoders at full resolution with only one interrupt on Arduino Nano (ATmega328). So I found out that we can use XOR to reach a full resolution:

Where pin 3 is interruptable but 4 and 5 are not.

Our requirements are:

1. Full resolution: for example, the solution offered in this answer works at a half-resolution only, not detecting all the edges.
2. Real-time: being as fast as possible up to the level that this hardware allows and safe in terms of not missing any of the signal edges.
3. using only one interrupt. We need the other one for other purposes.

So far, I have developed two different codes, one aiming for safety and the other for performance:

Preamble:

#include "digitalWriteFast.h"

#define pinT 3 // trigger pin comming from XOR

#define pinA 4 // channel A
#define pinB 5 // channel B

volatile long counter = 0;
long counter_ = counter;

where the digitalWriteFast library can be downloaded from here.

Performant:

volatile short int increment = 0;
volatile bool tmpA;
volatile bool tmpB;
volatile bool lastA;

ISR(trigger) {
  tmpA = digitalReadFast(pinA);
  tmpB = digitalReadFast(pinB);
  counter += (1 - 2 * tmpA) * (1 - 2 * tmpB) * (1 - 2 * (lastA != tmpA));
  lastA = tmpA;
}

void setup() {
  pinMode(pinA, INPUT_PULLUP);
  pinMode(pinB, INPUT_PULLUP);
  pinMode(pinT, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(pinT), trigger, CHANGE);
  lastA = digitalReadFast(pinA);
  Serial.begin(9600);
}

void loop() {
  if (counter != counter_) {
    Serial.println(counter);
    counter_ = counter;
  }
}

Safe:

volatile unsigned long error = 0;
unsigned long error_ = 0;

volatile bool inputA;
volatile bool inputB;

volatile short state; // 100 110 101 111
volatile short state_;

void detectState() {
  inputA = digitalReadFast(pinA);
  inputB = digitalReadFast(pinB);

  if (inputA) {
    if (inputB) {
      state = 111;
    } else {
      state = 110;
    }
  } else {
    if (inputB) {
      state = 101;
    } else {
      state = 100;
    }
  }
}

ISR(trigger) {

  detectState();

  if (state_ != state) {
    switch (state_) {
      case 100:
        if (state == 110) {
          counter++;
        } else if (state == 101) {
          counter--;
        } else {
          error++;
        }
        break;
      case 110:
        if (state == 100) {
          counter--;
        } else if (state == 111) {
          counter++;
        } else {
          error++;
        }
        break;
      case 101:
        if (state == 100) {
          counter++;
        } else if (state == 111) {
          counter--;
        } else {
          error++;
        }
        break;
      case 111:
        if (state == 110) {
          counter--;
        } else if (state == 101) {
          counter++;
        } else {
          error++;
        }
        break;
      default:
        error++;
        break;
    }

    state_ = state;
  } else {
    error++;
  }
}

void setup() {
  pinMode(pinT, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(pinT), trigger, CHANGE);

  pinMode(pinA, INPUT_PULLUP);
  pinMode(pinB, INPUT_PULLUP);

  detectState();
  state_ = state;

  Serial.begin(9600);
}

void loop() {
  if (counter != counter_) {
    Serial.print("counter: ");
    Serial.println(counter);
    counter_ = counter;
  }

  if (error != error_) {
    Serial.print(error);
    Serial.println(" errors");
    error_ = error;
  }
}

Now my questions are:

1. Is there any way to improve the speed/safety of the performant/safe code? Any way to improve this code is appreciated.
2. Is it possible to have maximum speed and safety together or some trade-off in between?
3. What is the maximum frequency we can get out of this configuration?

Thanks for your help in advance.

Answer 1

This is just a partial, rushed answer.

You should be able to get a significant speed-up if you define your own ISR for handling the interrupt, as in:

ISR(INT1_vect) { ... }

The Arduino way of using interrupts is instead

void trigger() { ...}

void setup() {
    attachInterrupt(int_number, trigger, CHANGE);
    ...
}

but this is slow, as it involves the ISR provided by the Arduino core, which has to locate and call your interrupt handler. The way you wrote the handler is wrong: either you define a regular function, without the ISR macro, and go the Arduino way, or you go the plain AVR path and define the ISR yourself, which should be named INTERRUPT_NAME_vect, in which case you don't use attachInterrupt(). The way you did it is probably the slowest you can get, as the compiler will needlessly add an ISR-type prologue and epilogue to your handler.

As for the maximum frequency, that's hard to assess without testing, but you may get some idea by reading this answer on interrupt latency.

---

Edit: As I stated in my first comment to the question, a pin change interrupt can be used instead of an external logic gate. Here is how to do it.

First, one should look at the Arduino Uno pinout and see that:

• digital 4 = PD4 = PCINT20
• digital 5 = PD5 = PCINT21

Both pins are tied to the pin change interrupt request 2 (PCINT2_vect). This interrupt can be configured in setup() as follows:

PCMSK2 = _BV(PCINT20)   // enable interrupt on pin PCINT20 = PD4
       | _BV(PCINT21);  // enable interrupt on pin PCINT21 = PD5
PCIFR  = _BV(PCIF2);    // clear interrupt flag
PCICR  = _BV(PCIE2);    // enable pin change interrupt request 2

Now, the interrupt will fire on every change of the logic levels of inputs 4 and 5.

Then, in the ISR, the whole port D can be read in a single cycle by just evaluating the corresponding pin input register, i.e. PIND. With some shifting and masking, one can get a phase between 0 and 3 (the count is in Gray code: 0, 1, 3, 2, 0...). From the previous and the current phase, one can compute the change that needs to be applied to the counter:

// Handle pin change interrupt request 2.
ISR(PCINT2_vect) {
    static uint8_t previous_phase;
    uint8_t phase = (PIND >> 4) & 0x03;  // read input pins
    counter += encoder_delta(previous_phase, phase);
    previous_phase = phase;
}

The implementation of encoder_delta() is left as an exercise to the reader. ;-)