Attaching an interrupt to a member function

I am making my own optical encoders. Each of them follows the exact same structure, so I figured I'd lump all the encoder processing in a class, StepCounter. However, when I directly attach an interrupt to a member function, I get

cannot convert 'StepCounter::step' from type 'void (StepCounter::)()' to type 'void (*)()'

Fair enough, a member method is different than a global method (however, maybe I can somehow cast it to a void(*)() by providing this?). So, I made the following solution (note: 18, 19 and 22, 23 are pin numbers, so assume that the naming convention makes any sense there):

class StepCounter {
  public:
    StepCounter(int pinInterrupt, int pinSecond, void (*wrapperFcn)(void)) :
      pinInterrupt(pinInterrupt),
      pinSecond(pinSecond) {
      pinMode(pinInterrupt, INPUT);
      pinMode(pinSecond, INPUT);
      stateSecond = digitalRead(pinSecond);
      attachInterrupt(digitalPinToInterrupt(pinInterrupt), wrapperFcn, CHANGE);
    };

    volatile int distance = 0;

    void step() {
      int newSecond = digitalRead(pinSecond);
      int stateXOR = digitalRead(pinInterrupt);
      // Direction is stateXOR * (stateSecond==newSecond)
      // if we take (-1,1) rather than (0,1) for (true,false)
      int  multiplier = (stateXOR == HIGH) ? -1 : 1;
      if (stateSecond == newSecond) {
        distance -= multiplier;
      } else {
        distance += multiplier;
      }
      stateSecond = newSecond;
    }

  private:
    const int pinInterrupt;
    const int pinSecond;
    volatile int stateSecond;

};

void interrupt1822(); // Forward declaration
void interrupt1923();

StepCounter stepCounter1822(18,22,interrupt1822),
            stepCounter1923(19,23,interrupt1923);

void interrupt1822() {
  stepCounter1822.step();
};
void interrupt1923() {
  stepCounter1923.step();
};

But, in all honesty, this looks rather clumsy with a bunch of forward declarations. And, although there is only so many interrupt pins on an Arduino, I sti

Yes; the key issue here is that you are dynamically determining values that can be determined statically.

Luckily, you are working in a language with extensive static analysis capabilities (template metaprogramming), so let's use that! (This sounds more complicated than it is.)

Also: I'm not sure why you determine the sign of multiplier separately from whether you should add or subtract it from distance. If statements are slow, because the processor needs to predict the correct branch (at best) or flush the pipeline (at worst). So I've removed that; hopefully it will get optimized into a setcc or similar. (I should also mention I'm not familiar with Arduino processors; it is entirely possible these changes are irrelevant.)

Finally, since you're working in C++, you really should use constructors or uniform initializers, rather than assignments. Yes, a decent compiler will optimize away the temporary, but it is always better to be easy on your poor compiler.

template
void interrupt(void)
{
    volatile static int pinState(digitalRead(pinIndex2)), distance(0);
    int const newState(digitalRead(pinIndex2)), multiplier(((HIGH == digitalRead(pinIndex)) ^ (newState == pinState)) ? -1 : 1);
    distance += multiplier;
    pinState = newState;
}

template
void registerInterrupt(void)
{
    pinMode(pinIndex, INPUT);
    pinMode(pinIndex2, INPUT);
    attachInterrupt(digitalPinToInterrupt(pinIndex), &interrupt, CHANGE);
}
void registerInterrupts(void)
{
    registerInterrupt(void);
    registerInterrupt(void);
}

If you're confused how this holds state, look at the static variables. These also might slow the interrupt down; if so, make them static members of a templated struct, like so:

template
struct interruptData
{
    volatile static int pinState(digitalRead(pinIndex2)), distance(0);
}

Then change the references inside the interrupt.

Beyond this, I don't see a way to improve your interrupt. It's pretty bare-bones to begin with.

StackExchange Code Review Q#142246, answer score: 4