Skip to main content
added 730 characters in body
Source Link

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. So If statements are slow, because the processor needs to predict the correct branch (at best) or flush the pipeline (at worst). So I've changed thatremoved 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.

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. So I've changed that.

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.

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.

Source Link

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. So I've changed that.

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.