4
\$\begingroup\$

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 still feel that copy-pasting so many things is something that should usually be avoided in programming.

Furthermore, I'm wondering whether there is any faster way of doing my step() function; since it's an interrupt service routine that I expect to be called a lot, I'd like it to be as fast as possible.

\$\endgroup\$
4
\$\begingroup\$

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<int const pinIndex, int const pinIndex2>
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<int const pinIndex, int const pinIndex2>
void registerInterrupt(void)
{
    pinMode(pinIndex, INPUT);
    pinMode(pinIndex2, INPUT);
    attachInterrupt(digitalPinToInterrupt(pinIndex), &interrupt<pinIndex, pinIndex2>, CHANGE);
}
void registerInterrupts(void)
{
    registerInterrupt<18, 22>(void);
    registerInterrupt<19, 23>(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<int const pinIndex, int const pinIndex2>
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.

\$\endgroup\$
  • \$\begingroup\$ Thanks. Quite a clever approach to start with. However, I'm still not clear on how it holds state - how do I even access distance? Something like interrupt<19,23>.distance doesn't work. \$\endgroup\$ – Sanchises Sep 24 '16 at 7:45
  • \$\begingroup\$ Static variables are local variables that the compiler guarantees will be initialized exactly once, in a thread-safe, reentrant manner. See en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables So basically I'm hardcoding the members of the StepCounter object into the interrupt. \$\endgroup\$ – Jacob Manaker Sep 25 '16 at 23:57
  • \$\begingroup\$ I didn't realize you'll need access to distance outside the interrupt; you can't do it as written, but if you use the "static member of a struct" alternative I mentioned, you can. \$\endgroup\$ – Jacob Manaker Sep 25 '16 at 23:57

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.