Simple software de-bounce in RotaryEncoder::_encoder_ISR()

[martind69]

Hi, i had a problem with bouncy china knob's

maybe you can update your code like this. I copied from RotaryEncoder::_button_ISR() and changed to micros()

void ARDUINO_ISR_ATTR RotaryEncoder::_encoder_ISR()
{
  /**
   * Almost all of this came from a blog post by Garry on GarrysBlog.com:
   * https://garrysblog.com/2021/03/20/reliably-debouncing-rotary-encoders-with-arduino-and-esp32/
   *
   * Read more about how this works here:
   * https://www.best-microcontroller-projects.com/rotary-encoder.html
   */

  static uint8_t _previousAB = 3;
  static int8_t _encoderPosition = 0;
  static unsigned long _lastInterruptTime = 0;
  static long _stepValue;
  bool valueChanged = false;

  // Simple software de-bounce
  if( ( micros() - _lastInterruptTime ) < 100 )
    return;

  _previousAB <<= 2;  // Remember previous state

  if( digitalRead( encoderPinA ) ) _previousAB |= 0x02; // Add current state of pin A
  if( digitalRead( encoderPinB ) ) _previousAB |= 0x01; // Add current state of pin B

  _encoderPosition += encoderStates[( _previousAB & 0x0f )];


  /**
   * Based on how fast the encoder is being turned, we can apply an acceleration factor
   */

  unsigned long speed = micros() - _lastInterruptTime;

  if( speed > 40000 )                              // Greater than 40 milliseconds
    _stepValue = this->stepValue;                  // Increase/decrease by 1 x stepValue

  else if( speed > 20000 )                         // Greater than 20 milliseconds
    _stepValue = ( this->stepValue <= 9 ) ?        // Increase/decrease by 3 x stepValue
      this->stepValue : ( this->stepValue * 3 )    // But only if stepValue > 9
    ;

  else                                             // Faster than 20 milliseconds
    _stepValue = ( this->stepValue <= 100 ) ?      // Increase/decrease by 10 x stepValue
      this->stepValue : ( this->stepValue * 10 )   // But only if stepValue > 100
    ;


  /**
   * Update counter if encoder has rotated a full detent
   * For the following comments, we'll assume it's 4 steps per detent
   * The tripping point is `STEPS - 1` (so, 3 in this example)
   */

  if( _encoderPosition > encoderTripPoint )        // Four steps forward
  {
    this->currentValue += _stepValue;
    valueChanged = true;
  }
  else if( _encoderPosition < -encoderTripPoint )  // Four steps backwards
  {
    this->currentValue -= _stepValue;
    valueChanged = true;
  }

  if( valueChanged )
  {
    encoderChangedFlag = true;

    // Reset our "step counter"
    _encoderPosition = 0;

    // Remember current time so we can calculate speed
    _lastInterruptTime = micros();
  }
}

[MaffooClock]

My apologies for letting this get cold!

So you're proposing the following patch:

--- ESP32RotaryEncoder.cpp	2025-01-23 08:49:43
+++ ESP32RotaryEncoder-changed.cpp	2025-01-23 08:56:51
@@ -328,11 +328,14 @@
   static int8_t _encoderPosition = 0;
   static unsigned long _lastInterruptTime = 0;
   static long _stepValue;
-
   bool valueChanged = false;

-  _previousAB <<=2;  // Remember previous state
+  // Simple software de-bounce
+  if( ( micros() - _lastInterruptTime ) < 100 )
+    return;

+  _previousAB <<= 2;  // Remember previous state
+
   if( digitalRead( encoderPinA ) ) _previousAB |= 0x02; // Add current state of pin A
   if( digitalRead( encoderPinB ) ) _previousAB |= 0x01; // Add current state of pin B

\ No newline at end of file
@@ -343,12 +346,12 @@
    * Based on how fast the encoder is being turned, we can apply an acceleration factor
    */

-  unsigned long speed = millis() - _lastInterruptTime;
+  unsigned long speed = micros() - _lastInterruptTime;

-  if( speed > 40 )                                 // Greater than 40 milliseconds
+  if( speed > 40000 )                              // Greater than 40 milliseconds
     _stepValue = this->stepValue;                  // Increase/decrease by 1 x stepValue

-  else if( speed > 20 )                            // Greater than 20 milliseconds
+  else if( speed > 20000 )                         // Greater than 20 milliseconds
     _stepValue = ( this->stepValue <= 9 ) ?        // Increase/decrease by 3 x stepValue
       this->stepValue : ( this->stepValue * 3 )    // But only if stepValue > 9
     ;
\ No newline at end of file
@@ -384,6 +387,6 @@
     _encoderPosition = 0;

     // Remember current time so we can calculate speed
-    _lastInterruptTime = millis();
+    _lastInterruptTime = micros();
   }
 }
\ No newline at end of file

...appears to be reasonable enough. I'll commit this change after I've tested.

[MaffooClock]

I've looked at this closer and done some testing.

At first, the changes from millis() to micros() didn't seem to have any effect since both of those get a value from esp_timer_get_time(). However, I made that change anyway because it seemed optimal to avoid the math to convert from microseconds to milliseconds.

But then I noticed something... changing to micros() seemed to fix the issue reported in #5. And it was repeatable: changing back to millis() made the dead spot behavior return, then changing back to micros() resolve it. Interesting 🤔

As for adding the "simple software de-bounce" from the _button_ISR(), that is negated by the magic of using a state table in the _encoder_ISR(), which is the most reliable way to filter out noisy encoder signals. Plus, it would interfere with the acceleration factor that causes the value to be incremented exponentially when the encoder is spun quickly.

[martind69]

Perfectly, thanks for reworking