Decrementing causes variable corruption when used with interrupts

[M-Reimer]

Your library suffers from this issue: https://www.arduino.cc/reference/en/language/variables/variable-scope--qualifiers/volatile/

You can easily reproduce this if you enhance your interrupt example with code for resetting the value after printing it by doing "setPosition(0)" after printing.

The easiest and not hardware dependent solution to fix this would be to make this variable an signed char. I could create a pull request for this but I want to wait for your feedback, first.

[M-Reimer]

As your internal variable is 4 times the external and both would need to be unsigned chars, this would basically limit the range to +-32 No problem in theory as you could always sum this up to a long in the main loop. It's just about getting some safe variable type for manipulation in both, interrupts and main loop.

[mathertel]

Maybe you create a pull request - thanks in advance.

[M-Reimer]

In the meantime, I've created my own library which works with interrupts and also supports full- and half-step encoders: https://github.com/M-Reimer/EncoderStepCounter If I find the time, I could still create a pull request, but the limitations, I've pointed out in https://github.com/mathertel/RotaryEncoder/issues/11#issuecomment-533506532 still apply. In my library, I do the whole decoding in a different way which gives me more "room" for the counting.

[eudoxos]

Just as a note, I think I bumped into the same issue (as far as I understand) even without interrupts when implementing acceleration along the lines of

long newPos=enc.getPosition();
if(re.getMillisBetweenRotations()<100) newPos+=(newPos-pos)*10;
enc.setPosition(newPos);

Would you perhaps suggest how this could be done in a better way? (and BTW thanks for this very useful lib!)

[dzalf]

Hey guys

Interestingly, I recently ran into a similar problem using this encoder and driven by an Arduino Nano (ATMEGA328P) using Pin Change Interupts (PCINT2).

The pins A and B from the rotary were connected to pins 3 and 2 from the MCU, respectively. The Push Button from the encoder was attached to Pin 5

When using the example code "InterruptRotator" I experienced erratic jumps on the value from the "pos" variable. In addition, the tick() method does NOT work inside the ISR if I check what pin from the vector D[0:7] triggered the interrupt. After exploring my options I found that any variable being affected by an interrupt must be declared as a volatile, according to this amazing tutorial from Nick Gammon.

volatile bool rotSwPressed = false;
volatile bool rotTicked = false;

volatile int prevRotA;
volatile int prevRotB;
volatile int prevPos;
int newPos = 0;

As a result I changed the approach to read the encoder: got rid of the "pos" variable and checked the status of the Rotary based on a volatile boolean flag. I do not check for specific pins that started the ISR and I tick() as soon as the interrupt is called

The only way that I could get a stable and smooth response with no spurious results was to read any change on the encoder through polling, as the following portion of code shows:

 unsigned long now = millis();

 if (now - previousPollingRotary >= delayRotary){    // My delayRotary is 5 or 10 ms
    previousPollingRotary = now;

    newPos = encoder.getPosition();

    if (prevPos != newPos){
      cli();    // Perhaps I am being too cautious, but just in case ;)

      rotaryVal = (newPos / 10.0) * mult;  

      Serial.print("Encoder Position = ");
      Serial.println(newPos);

      prevPos = newPos;

      sei();
    }
  }

 if (rotTicked){
    rotTicked = false;
    Serial.println("Ticked!");
  }

Where the ISR that WORKS looks like this..

ISR(PCINT2_vect){ 
  cli();
  encoder.tick();    // Registered without problems
 rotTicked = true;

// Based on this macro: #define ROT_SW_PRESSED !(PIND & (1 << PIND5))
 if (ROT_SW_PRESSED) { // Check if the Interrupt was triggered by the Rotary Button
    rotSwPressed = true;
  }
sei();
}

The reason why I check where the interrupt came from is because I am using the same vector for the Rotary Switch.

However, if I try this, the ticks aren't registered until I rotate the encoder perhaps more than 10 turns

ISR(PCINT2_vect){
  cli();

  // Apart from encoder.tick() everything else works fine
  if ((digitalRead(rotA) != prevRotA) || (digitalRead(rotB) == prevRotB)) {
    prevRotA = digitalRead(rotA);
    prevRotB = digitalRead(rotB);

    encoder.tick();    // Ticks DO NOT occur, HOWEVER:

    rotTicked = true; // this flag DOES change, 
                               // meaning that this routine actually works
  }
  sei();
}

I hope my experience helps to polish this great library.

Thank you Matthias for the great work!

Cheers