Encoder not incrementing correctly.

Hello. I have written a simple sketch using 2 rotary encoders with a switch.

const byte numEnc = 2;
const byte ck[numEnc] = {22, 24};
const byte dt[numEnc] = {23, 25};
const byte sw[numEnc] = {26, 27}; 


volatile boolean count[numEnc];
volatile boolean lastCount[numEnc];
volatile boolean dir[numEnc];
volatile boolean lastDir[numEnc];
volatile boolean encButt[numEnc];
volatile int ticks[numEnc];
volatile int encPos[numEnc];




void setup()
{
  Serial.begin(9600);

  DDRA = B00000000; // Set port A to input.

}

void loop()
{
  handleEncoder();  
}

void handleEncoder()
{
  for (int i = 0; i < numEnc; i++)
  {
    encButt[i] = digitalRead(sw[i]);
    count[i] = digitalRead(ck[i]);
    dir[i] = digitalRead(dt[i]);

// If encoder switch is pressed. Do high x4 resolution for encoder cycle.
// If encoder switch is not pressed. Do low resolution x1 for encoder cycle.

    if(encButt[i] != true) 
    {
      ticks[i] += lowResEnc();
    }
    else
    {
      ticks[i] += highResEnc();
    }   

    lastCount[i] = count[i];
    lastDir[i] = dir[i];
  }
}

// Low reolution....

int lowResEnc()
{
  for (int i = 0; i < numEnc; i++)
  {
    if (count[i] != lastCount[i])
    {
      if ((count[i] == LOW) && (dir[i] == LOW))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count up ");
        Serial.println(i);
      }
      else if ((count[i] == LOW) && (dir[i] == HIGH))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
    }
  }
}

// High resolution

int highResEnc()
{
  for (int i = 0; i < numEnc; i++)
  {
    if (count[i] != lastCount[i])
    {
      if ((count[i] == LOW) && (dir[i] == LOW))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count up ");
        Serial.println(i);
      }
      else if ((count[i] == LOW) && (dir[i] == HIGH))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == HIGH))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Up ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == LOW))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
    }
  }
  for (int i = 0; i < numEnc; i++)
  {
    if (dir[i] != lastDir[i])
    {
      if ((count[i] == LOW) && (dir[i] == LOW))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
      else if ((count[i] == LOW) && (dir[i] == HIGH))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Up ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == HIGH))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == LOW))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Up ");
        Serial.println(i);
      }
    }
  }
}

This works s expected. I turn the encoders without the button pressed they move by one increment each rotary step. If I turn them while the switch is pressed they move by 4 increments per rotary step. I get very sporadic miss reads when I turn the encoders fast so I got rid of the digital reads just to test and wrote this.

const byte numEnc = 2;
const byte ck[numEnc] = {22, 24};
const byte dt[numEnc] = {23, 25};
const byte sw[numEnc] = {26, 27}; 


volatile boolean count[numEnc];
volatile boolean lastCount[numEnc];
volatile boolean dir[numEnc];
volatile boolean lastDir[numEnc];
volatile boolean encButt[numEnc];
volatile int ticks[numEnc];
volatile int encPos[numEnc];


void setup()
{
  Serial.begin(9600);

  DDRA = B00000000; // Set port A to input.
}

void loop()
{ 
  handleEncoder();  
}

void handleEncoder()
{
  int _ReadPort = PINA; // digital read port A.
  
  for (int i = 0; i < numEnc; i++)
  {
    encButt[0] = _ReadPort & B00010000; // read encoder1 switch (pin 26)
    encButt[1] = _ReadPort & B00100000; // read encoder2 switch  (pin 27)
    count[0] = _ReadPort & B00000001; // read encoder1 ck (pin 22)
    count[1] = _ReadPort & B00000100;// read encoder2 ck  (pin 24)
    dir[0] = _ReadPort & B00000010; // read encoder 1 dt  (pin 23)
    dir[1] = _ReadPort & B00001000; // read encoder 2 dt  (pin25)

 // If encoder switch is pressed. Do high x4 resolution for encoder cycle.
// If encoder switch is not pressed. Do low resolution x1 for encoder cycle.

    if(encButt[i] != true)
    {
      ticks[i] += lowResEnc();
    }
    else
    {
      ticks[i] += highResEnc();
    }
    
    lastCount[i] = count[i];
    lastDir[i] = dir[i];
  }
}

int lowResEnc()
{
  for (int i = 0; i < numEnc; i++)
  {
    if (count[i] != lastCount[i])
    {
      if ((count[i] == LOW) && (dir[i] == LOW))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count up ");
        Serial.println(i);
      }
      else if ((count[i] == LOW) && (dir[i] == HIGH))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
    }
  }
}

int highResEnc()
{
  for (int i = 0; i < numEnc; i++)
  {
    if (count[i] != lastCount[i])
    {
      if ((count[i] == LOW) && (dir[i] == LOW))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count up ");
        Serial.println(i);
      }
      else if ((count[i] == LOW) && (dir[i] == HIGH))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == HIGH))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Up ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == LOW))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
    }
  }
  for (int i = 0; i < numEnc; i++)
  {
    if (dir[i] != lastDir[i])
    {
      if ((count[i] == LOW) && (dir[i] == LOW))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
      else if ((count[i] == LOW) && (dir[i] == HIGH))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Up ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == HIGH))
      {
        encPos[i]--;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Down ");
        Serial.println(i);
      }
      else if ((count[i] == HIGH) && (dir[i] == LOW))
      {
        encPos[i]++;
        Serial.print("encPos ");
        Serial.print(encPos[i]);
        Serial.print("   ");
        Serial.print("count Up ");
        Serial.println(i);
      }
    }
  }
}

Now when I turn the 2nd encoder without pressing button it increments by 2 and not 1. The rest works as expected. Why is it doing this? Can somebody help me understand and possibly make this whole thing better.

Thank you in advance.

In your second example, you fill in all your data but you have it wrapped inside your for() loop so you do everything twice.

Also, when you call lowResEnc() or highResInc() you have a for loop inside those functions but you should only be applying that to the current value.

const byte numEnc = 2;
const byte ck[numEnc] = {22, 24};
const byte dt[numEnc] = {23, 25};
const byte sw[numEnc] = {26, 27};


volatile boolean count[numEnc];
volatile boolean lastCount[numEnc];
volatile boolean dir[numEnc];
volatile boolean lastDir[numEnc];
volatile boolean encButt[numEnc];
volatile int ticks[numEnc];
volatile int encPos[numEnc];


void setup()
{
  Serial.begin(9600);

  DDRA = B00000000; // Set port A to input.
}

void loop()
{
  handleEncoder();
}

void handleEncoder()
{
  int _ReadPort = PINA; // digital read port A.

  encButt[0] = _ReadPort & B00010000; // read encoder1 switch (pin 26)
  encButt[1] = _ReadPort & B00100000; // read encoder2 switch  (pin 27)
  count[0]   = _ReadPort & B00000001; // read encoder1 ck (pin 22)
  count[1]   = _ReadPort & B00000100;// read encoder2 ck  (pin 24)
  dir[0]     = _ReadPort & B00000010; // read encoder 1 dt  (pin 23)
  dir[1]     = _ReadPort & B00001000; // read encoder 2 dt  (pin25)

  // If encoder switch is pressed. Do high x4 resolution for encoder cycle.
  // If encoder switch is not pressed. Do low resolution x1 for encoder cycle.

  for (int i = 0; i < numEnc; i++)
  {
    if (encButt[i] != true)
    {
      ticks[i] += lowResEnc(i);
    }
    else
    {
      ticks[i] += highResEnc(i);
    }

    lastCount[i] = count[i];
    lastDir[i] = dir[i];
  }
}

int lowResEnc(int i)
{
  if (count[i] != lastCount[i])
  {
    if ((count[i] == LOW) && (dir[i] == LOW))
    {
      encPos[i]++;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count up ");
      Serial.println(i);
    }
    else if ((count[i] == LOW) && (dir[i] == HIGH))
    {
      encPos[i]--;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Down ");
      Serial.println(i);
    }
  }
}

int highResEnc(int i)
{
  if (count[i] != lastCount[i])
  {
    if ((count[i] == LOW) && (dir[i] == LOW))
    {
      encPos[i]++;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count up ");
      Serial.println(i);
    }
    else if ((count[i] == LOW) && (dir[i] == HIGH))
    {
      encPos[i]--;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Down ");
      Serial.println(i);
    }
    else if ((count[i] == HIGH) && (dir[i] == HIGH))
    {
      encPos[i]++;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Up ");
      Serial.println(i);
    }
    else if ((count[i] == HIGH) && (dir[i] == LOW))
    {
      encPos[i]--;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Down ");
      Serial.println(i);
    }
  }
  if (dir[i] != lastDir[i])
  {
    if ((count[i] == LOW) && (dir[i] == LOW))
    {
      encPos[i]--;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Down ");
      Serial.println(i);
    }
    else if ((count[i] == LOW) && (dir[i] == HIGH))
    {
      encPos[i]++;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Up ");
      Serial.println(i);
    }
    else if ((count[i] == HIGH) && (dir[i] == HIGH))
    {
      encPos[i]--;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Down ");
      Serial.println(i);
    }
    else if ((count[i] == HIGH) && (dir[i] == LOW))
    {
      encPos[i]++;
      Serial.print("encPos ");
      Serial.print(encPos[i]);
      Serial.print("   ");
      Serial.print("count Up ");
      Serial.println(i);
    }
  }
}

Thank you. That works now. Can you explain the (int I) in the resolution functions please? Just so I understand. I am still learning...

I am still getting mis reads when I turn fast. The encPos occasionally alternates back and forth. Even tho I'm going in one direction. Is there a way to clean this up?

you may need to use an interrupt. use the following on an esp32

#define Brk_A_Clk    32
#define Brk_A_Dt     33

#define Brk_B
#ifdef Brk_B
#define Brk_B_Clk    35
#define Brk_B_Dt     26
#endif



//  table containing action indexed by inputs BA and state

typedef enum { ___, For, Rev, Skp, ActSize } Act_t;

const char* actStr [] = { "___", "For", "Rev", "Skp", "ActSize" };

Act_t  encAct [ActSize][ActSize] = {
// state  00   01   10   11      inputs
       { ___, Rev, For, Skp },  // 00
       { For, ___, Skp, Rev },  // 01
       { Rev, Skp, ___, For },  // 10
       { Skp, For, Rev, ___ },  // 11
};

int encAst = 0;
int encBst = 0;

// ------------------------------------------------
// read brake
inline void readEncoder (
    byte  dt,
    byte  clk,
    int & brkPos,
    int & encSt )
{
    byte  val = digitalRead (dt) << 1 | digitalRead (clk);

    switch (encAct [val][encSt])  {
    case For:
        brkPos++;
        break;

    case Rev:
        brkPos--;
        break;

    default:
        break;
    }

    encSt = val;
}

// -------------------------------------
void IRAM_ATTR isrEncA (void)
{
    readEncoder (Brk_A_Dt, Brk_A_Clk, brkApos, encAst);
}

// -------------------------------------
#ifdef Brk_B
void IRAM_ATTR isrEncB (void)
{
    readEncoder (Brk_B_Dt, Brk_B_Clk, brkBpos, encBst);
}
#endif

add following in setup()

   // encoders
    pinMode (Brk_A_Clk, INPUT_PULLUP);
    pinMode (Brk_A_Dt,  INPUT_PULLUP);

    attachInterrupt (Brk_A_Clk, isrEncA, CHANGE);
    attachInterrupt (Brk_A_Dt,  isrEncA, CHANGE);

#ifdef Brk_B
    pinMode (Brk_B_Clk, INPUT_PULLUP);
    pinMode (Brk_B_Dt,  INPUT_PULLUP);

    attachInterrupt (Brk_B_Clk, isrEncB, CHANGE);
    attachInterrupt (Brk_B_Dt,  isrEncB, CHANGE);
#endif
1 Like

Wow. Thanks gcjr. I don't fully understand what is happening here. I assume I change the #define CK dt numbers to my corresponding ones? Do I use he external interrupt? I can't try now until tomorrow.

Can I try the on Arduino? I don't have an esp32.

dionnaki:
Wow. Thanks gcjr. I don't fully understand what is happening here. I assume I change the #define CK dt numbers to my corresponding ones? Do I use he external interrupt? I can't try now until tomorrow.

i prefer to post code that I either use or have tested. so i happen to be using this.

it supports 2 encoders and any gpio on an esp32 can be interrupt. there happens to be an ifdef for the 2nd encoder that you should disabled -- delete/comment out the define of Brk_B. you'll need to set the pins to 2 & 3 on the arduino. this code originally came from an arduino project.

the code uses a small state machine to track encoder movements that can deal with bounces. you should look over and understand the code and integrate it into your code. probably delete the code for Brk_b and just make readEncoder() the ISR.

i had written a simpler non-interrupt version for the esp32 that had the same problem you mentioned, that it didn't track well when spun quickly, so i got the state machine interrupt version of the code and made it work for 2 encoders

dionnaki:
Now when I turn the 2nd encoder without pressing button it increments by 2 and not 1. The rest works as expected. Why is it doing this? Can somebody help me understand and possibly make this whole thing better.

Thank you in advance.

One thing I noticed is you set your variables to bit masks, and later compare them to HIGH. You
need to be consistent.

count and dir are poor names for A and B encoder outputs.

MarkT...how should it be written?

GCjr of DT an error message when compiling your code. It reads
Expected initialiser before isr EncA?

delete the IRAM_ATTR.

this is apparently needed on the esp32 to keep the ISR in ram, otherwise there is too much latency moving the code from external memory

BrkApos was not declared in this scope?

add

int brkApos = 0;