Thanks both of you. I used the breakout board just to test it out, I didn’t realise the serial pins were noisy, I’ve moved it to a breadboard now and switched to GPIO18+20.
@PJ_Glasso thank you so much for the explanation! I did find all sorts of types of code out there, including some with state machines, others that were timing the micros() between interrupts, but none of them gave an explanation like you did as to why, so in the end I went with the simplest solution when I was seeing that it was supposed to work like this:
Your reply helps me understand better why sampling on A’s transition isn’t the way to do it, thanks! Also, it’s probably about time I got myself an oscilloscope.
I switched the code (merged with previous to use queues, below) and started getting multiple random readings per detent. Moved it to the breadboard and changed GPIOs, same result. Added 0,1uF caps on the breadboard between the GPIOs and ground and now I’m getting the result correct 95% of the time, although I’m still getting multiple readings per detent (4 per detent showing the correct reading if I twist quickly, 5-6 per detent with some misreadings if I twist slowly).
I guess soldering the caps directly to the encoder board may solve the 5% incorrect results, I’ll try this tomorrow. Should I solder them to SIGA/SIGB as you suggest or directly to the encoder’s SA/SB as per this post - there’s a resistor in between - or doesn’t it matter? [the latter would be easiest as I can solder surface-mount ones directly to pins 1-2 and 2-3 on the encoder].
I’ll compare to the code to other versions on the internet now and look at the libraries you suggest to see why I’m getting multiple readings per detent.
I did note that the product page says that it works at 4.5-5.5V but I assume this is incorrect and the encoder doesn’t care, being mechanical? As I wanted to avoid level shifting.
Current code. I did have to swap A and B to make your state table work in the direction expected. I believe the xQueue* functions are intended to be interrupt safe so I removed the noInterrupts() call.
#include <Arduino.h>
#define PIN_ROTARY_A 18
#define PIN_ROTARY_B 20
QueueHandle_t encoderQueue;
volatile uint8_t encoderLastState = 0;
void setup(void)
{
Serial.begin();
Serial.print("started\n");
pinMode(PIN_ROTARY_A, INPUT_PULLUP); // even if board has pullups, this often improves noise margin
pinMode(PIN_ROTARY_B, INPUT_PULLUP);
if ((encoderQueue = xQueueCreate(500, sizeof(int8_t))) == NULL) {
Serial.println("unable to create queue");
while (1) delay(1000);
}
attachInterrupt(digitalPinToInterrupt(PIN_ROTARY_A), encoderChange, CHANGE);
attachInterrupt(digitalPinToInterrupt(PIN_ROTARY_B), encoderChange, CHANGE);
}
void ARDUINO_ISR_ATTR encoderChange()
{
uint8_t a = digitalRead(PIN_ROTARY_A);
uint8_t b = digitalRead(PIN_ROTARY_B);
uint8_t state = (a << 1) | b;
// transition table (from lastState -> state)
// +1 for CW, -1 for CCW, 0 for invalid/bounce
static const int8_t table[16] = {
0, -1, +1, 0,
+1, 0, 0, -1,
-1, 0, 0, +1,
0, +1, -1, 0
};
int8_t direction = table[(encoderLastState << 2) | state];
encoderLastState = state;
if (direction) {
if (xQueueSendToBackFromISR(encoderQueue, &direction, NULL) != pdPASS) {
Serial.println("queue full");
while (1) delay(1000);
}
}
}
void loop()
{
int8_t direction;
while (xQueueReceive(encoderQueue, &direction, 0) == pdPASS) {
if (direction == -1) {
Serial.println("down");
}
else if (direction == +1) {
Serial.println("up");
}
}
}
also… hilarious
