[SOLVED] Rotary encoder on ESP32 / Almost there but

Dear All,

As stated in the title, I'm close to get rotary encoder decoding. But not enough as it doesn't work.
So as I'm stuck, I have written below code in order to understand the mechanical behavior of my encoder.
I know there is a lot of threads (here and at other places) about this subject, and I have aleady tried al lot of solutions or approach that can be find over there. But I'm still struggling...

The rotary encoder is a CTS288 (full ref is 288T232R161A2); Rotary encoder Datasheet.
I have 2 of them, and both show the same behavior.
The project is a Sim Racing wheel, using this encoder as +/- knobs for TC and ABS.
This model of encoder is widely used for this purpose.

I've tried with a cheap rotary encoder from a Beginner kit, behavior is identical.

The ESP32 is from AZ Delivery; ESP32 Datasheet

Current pins are 12 and 25, but was previously 18 and 19.
All pins can be used for interrupts.

Software debounce is created without delay() function (because of interrupts).
Simple debounce attempt, don't be shy if it is inapproppriate or very badly coded!

I have manually controlled the encoder (multimeter in continuity mode), both pin acts as it is supposed to do (2 bits Gray code: 00, 01, 11, 10, 00, ....).

But I still have issue with:

  1. Same values randomly read 2 times in a row, despite rotating the shaft even with 1 click (whatever is the direction or the speed).
  2. Values don't follow the correct sequence (00, 01, 11, 10, 00...)

Please, could you have a look on the code hereunder?

  1. I would like to know first if there is an obvious mistake in my code
  2. If so, please could you lead me without providing the solution directly?
// Software Debouncing - Mechanical Rotary Encoder
// Rotary CLK to pin 12
// Rotary DT to pin 25

#define encoder0PinA 12
#define encoder0PinB 25

unsigned long previousMillis = 0;
const long debounce = 80;

int previousStateA;
int previousStateB;
int currentStateA;
int currentStateB;

int flag;

void setup() {
  pinMode(encoder0PinA, INPUT_PULLUP); // Using internal pullup resistor
  pinMode(encoder0PinB, INPUT_PULLUP);

  attachInterrupt(encoder0PinA, rotEncoder, CHANGE); // Need to detect both rising or falling signal
  attachInterrupt(encoder0PinB, rotEncoder, CHANGE);


  previousStateA = digitalRead(encoder0PinA); // Storage of state before the interrupt
  previousStateB = digitalRead(encoder0PinB);


  Serial.begin (115200);
}

void rotEncoder() {
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis >= debounce) {
    previousMillis = currentMillis;
    currentStateA = digitalRead(encoder0PinA); // Simple read of current HIGH or LOW state
    currentStateB = digitalRead(encoder0PinB);
    Serial.print("A: "); Serial.print(currentStateA);
    Serial.print(" "); Serial.print(currentStateB);
    Serial.println(" :B"); // Serial prints only for debugging; won't be used in final code
    if (previousStateA == currentStateA && previousStateB == currentStateB) { // Triggerd only if current state of both pins are equal to previous
      flag = 1;
    }
    previousStateA = currentStateA;
    previousStateB = currentStateB;
  }
}

void loop() {
  if (flag == 1) {
    Serial.println("Same values as previous!");
    flag = 0;
  }
}

Exemple of Serial monitor (rotating the encoder in one direction only:
19:39:36.132 -> A: 1 1 :B
19:39:36.132 -> Same values as previous!
19:39:36.822 -> A: 0 1 :B
19:39:37.469 -> A: 1 0 :B
19:39:38.158 -> A: 1 0 :B
19:39:38.158 -> Same values as previous!
19:39:38.942 -> A: 0 1 :B
19:39:39.726 -> A: 0 1 :B
19:39:39.726 -> Same values as previous!
19:39:40.369 -> A: 0 0 :B
19:39:40.968 -> A: 1 0 :B
19:39:41.614 -> A: 1 1 :B
19:39:42.213 -> A: 0 1 :B
19:39:42.812 -> A: 0 0 :B
19:39:43.504 -> A: 1 0 :B
19:39:44.195 -> A: 1 1 :B
19:39:44.792 -> A: 0 0 :B
19:39:45.390 -> A: 0 0 :B
19:39:45.390 -> Same values as previous!
19:39:46.036 -> A: 1 0 :B
19:39:46.681 -> A: 0 1 :B

Thank you in advance for your time!

Never attempt serial I/O within an interrupt routine. I'm surprised that the code you posted doesn't crash, but then, the ESP32 is not an Arduino.

Variables shared with interrupt routines must be declared "volatile" and when accessed by the main code, must be protected from corruption by the interrupt routine. The usual way to do this is something like the following:

volatile int count = 0;  //global, incremented in ISR
...

noInterrupts();
int count_copy = count;
interrupts();
Serial.println(count_copy);
1 Like

The reply from @jremington is the heaviest.
The spec of the encoder says max 5 mS contact bounce time. You use 80 mS in the code. Are You loosing steps?

have you had a look at esp32encoder?
you can use the Arduino IDE Library Manager to load it (in fact if you search for ESP32 encoder there are several libraries)
the Examples>ESP32Encoder>Encoder works OK with a low cost QEI encoder which I use for menu selection
fora high speed QEI encoder I would look at a micro with a hardware Quadrature Decoder such as the Arduino Due or Microchip dsPIC

Dear @jremington ,

I'm aware serial output should not be used into interrupt, the goal was only debugging.
But I have removed them as they shouldn't be there.

Dear @Railroader ,

The bounce time in datasheet is for the optional switch. The encoder itself doesn't have such specification written. The 80ms are just (gross?) safety.
I'm not really loosing step: each detent is detected, but the new states can be identical from the previous.

Dear @horace ,

Yes I already have had a look, and did nothing ( :sweat_smile: shame on me!)...
Using the example code without interrupts (and using encoder.attachFullQuad(12, 25) instead of encoder.attachHalfQuad(12, 25)) gives me what I want. :star_struck:

So for now, the original problem is not solved but I have an alternative solution.
I still have to figure out how to use this alternative into the full code, if interrupts will still be a problem.

Not only is that a problem in code execution, it is quite obviously just stupid from a planning point of view!

The only reason (aside from waking from sleep, in case someone get picky) to use an interrupt is because an event requires immediate response in the particular context of microcontroller time, which is microseconds.

But a printout to serial (or however it might be done) is clearly not immediate, so if that is required, then by definition an interrupt is not needed. :roll_eyes: Further, the time taken to do the printout - even if it were practical in code - would prevent response to further interrupts.

Try "volatile".

Yes I did, no improvement except the serial-gate is now cleared (I hope).

Show the modified code

Just put things back in context.

The ESP32 will do nothing 99% of the time, just waiting for a bunch of buttons on a steering wheel to be pushed from time to time.
EPS32 is communicating with computer through BLE and is powered by a battery so no task is planned in the Loop section.
The interrupts are choosen because I need an immediate response (shifting gears, i.e.) and will just send "pressed state", and I don't need to check every single button forever.

Does the choice of interrupts make sense to you, or did I miss something?

Here it is:

// Software Debouncing - Mechanical Rotary Encoder
// Rotary CLK to pin 12
// Rotary DT to pin 25

#define encoder0PinA 12
#define encoder0PinB 25

unsigned long previousMillis = 0;
const long debounce = 80;

int previousStateA;
int previousStateB;
int currentStateA;
int currentStateB;

volatile int flagStates;
volatile int flag;

void setup() {
  pinMode(encoder0PinA, INPUT_PULLUP); // Using internal pullup resistor
  pinMode(encoder0PinB, INPUT_PULLUP);

  attachInterrupt(encoder0PinA, rotEncoder, CHANGE); // Need to detect both rising or falling signal
  attachInterrupt(encoder0PinB, rotEncoder, CHANGE);


  previousStateA = digitalRead(encoder0PinA); // Storage of state before the interrupt
  previousStateB = digitalRead(encoder0PinB);


  Serial.begin (115200);
}

void rotEncoder() {
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis >= debounce) {
    previousMillis = currentMillis;
    currentStateA = digitalRead(encoder0PinA); // Simple read of current HIGH or LOW state
    currentStateB = digitalRead(encoder0PinB);
    flagStates = 1;
    if (previousStateA == currentStateA && previousStateB == currentStateB) { // Triggerd only if current state of both pins are equal to previous
      flag = 1;
    }
    previousStateA = currentStateA;
    previousStateB = currentStateB;
  }
}

void loop() {
  if (flagStates == 1) {
    noInterrupts()
    Serial.print("A: "); Serial.print(currentStateA);
    Serial.print(" "); Serial.print(currentStateB); Serial.println(" :B");
    flagStates = 0;
    interrupts();
  }
  if (flag == 1) {
    noInterrupts()
    Serial.println("Same values as previous!");
    flag = 0;
    interrupts();
  }
}

const long debounce = 80;

const unsigned long debounce = 80UL;


if (previousStateA == currentStateA && previousStateB == currentStateB)
I'm not sure, maybe more parentheses are needed, depending on your intent and how the compiler figures that.

cf.
if ((previousStateA == currentStateA) && (previousStateB == currentStateB))

I remain confused about what you mean.

You only use interrupts for a rotary encoder when it is spinning so fast that it might miss steps if polled, as it is generating more than a thousand steps per second. To what is your rotary encoder connected, presumably the crankshaft? :face_with_raised_eyebrow:

https://en.cppreference.com/w/cpp/language/operator_precedence

Hi,

What is your application?
Monitoring a gear selector?

A schematic of you project would be a great help to?
An image of a hand drawn circuit will be fine.
Please include power supplies, component names and pin labels.

Have you got 0.1uF capacitors between the two outputs of the encoder and the controller gnd?

How many positions do you need to detect?

Some pictures of your project would also help with component positioning.

Thanks.. Tom.. :smiley: :+1: :coffee: :australia:

Hi @TomGeorge

As explained in the 1st post, "The project is a Sim Racing wheel, using this encoder as +/- knobs for TC and ABS."
We can simply call this project: a gamepad with rotating knob. This knob must act as 2 buttons (button A pressed when rotating CW, button B when rotating CCW).

Nope, as nobody using the CTS288 rotary encoder has to deal with mechanical debounce solution. Software debounce

Not the amount of position but the direction of following position (CW or CCW for each click).
No need to count them, just 1 click = 1 button press (this part is already done and works fine).

RotaryEncoderKY-040-1
Powered by USB cable, used pins are 19 and 18 (which can use internal pullup resistor).
The board is an ESP32 NodeMCU.
As you can see, nothing fancy, just a straight connection.

wired low cost rotary encoder as diagram in post #17
NodeMcu ESP32 WROOM-32 used ESP32encoder library to run

// ESP32encoder https://www.arduino.cc/reference/en/libraries/esp32encoder/
// documentation https://madhephaestus.github.io/ESP32Encoder/classESP32Encoder.html

#include <ESP32Encoder.h>

ESP32Encoder encoder;

void setup(){
	Serial.begin(115200);
	// Enable the weak pull up resistors
	ESP32Encoder::useInternalWeakPullResistors=UP;
	encoder.attachHalfQuad(19, 18);
	// clear the encoder's raw count and set the tracked count to zero
	encoder.clearCount();
}

void loop(){
  static int oldEncoder=-32000;
  int encoder1=encoder.getCount();
  if(encoder1 != oldEncoder) {
         oldEncoder=encoder1;
	  Serial.println("Encoder count = "  + String((int32_t)encoder1));
	  //delay(100);
  }
}

serial monitor output

Encoder count = 0
Encoder count = 1
Encoder count = 2
Encoder count = 3
Encoder count = 4
Encoder count = 5
Encoder count = 6
Encoder count = 5
Encoder count = 4
Encoder count = 3
Encoder count = 2
Encoder count = 1
Encoder count = 0
Encoder count = -1
Encoder count = -2
Encoder count = -3
Encoder count = -4
Encoder count = -5
Encoder count = -6

the rotary encoder I used incremented/decremented by 2 for each click of rotation

Dear @horace ,

Thank you for you input
Here is my take:

#include <ESP32Encoder.h>

ESP32Encoder encoder;

volatile int lastCount;
volatile int currentCount;

unsigned long previousMillis = 0;
const long debounce = 50;

void setup() {

  Serial.begin(115200);
  ESP32Encoder::useInternalWeakPullResistors = UP;
  encoder.attachFullQuad(18, 19);

  encoder.setCount(37);
  lastCount = encoder.getCount();

}

void loop() {
  // Loop and read the count

  readEncoder();

}

void readEncoder() {
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis >= debounce) {
    previousMillis = currentMillis;
    currentCount = encoder.getCount();
    if (currentCount != lastCount) {
      Serial.println("Encoder count = " + String((int32_t)encoder.getCount()));
    }
    lastCount = encoder.getCount();
  }
}

Result:

3:07:37.716 -> Encoder count = -6
23:07:37.762 -> Encoder count = -5
23:07:37.808 -> Encoder count = -4
23:07:37.900 -> Encoder count = -3
23:07:37.993 -> Encoder count = -2
23:07:38.039 -> Encoder count = -1
23:07:38.178 -> Encoder count = 0
**23:07:38.824 -> Encoder count = 1**
**23:07:38.870 -> Encoder count = 3**
23:07:38.917 -> Encoder count = 4
23:07:39.008 -> Encoder count = 5
23:07:39.147 -> Encoder count = 6
23:07:39.193 -> Encoder count = 7
23:07:39.284 -> Encoder count = 8
23:07:39.376 -> Encoder count = 9

Well, I'm still losing steps... I have 2 pieces of CTS288, same behavior.
I will try another EPS32, just in case.

50 milliseconds seems quite long for debounce, try halving it perhaps.

Have you tried example provided by @horace ?