How to properly code rotary encoder

[strawberry]

Don't know why this code in Arduino works fine but not in plain c/c++
Inputs are clean.

Code: [Select]

char rotary_enc()
{
   unsigned char State1;
   unsigned char State2;
   unsigned char State1_prev;
   unsigned char State2_prev;
   State1=(PINB & (1 << Enc1));
   State2=(PINB & (1 << Enc2));
  if(State1 == State2 )
  {
  State1_prev = State1;
  State2_prev = State2;
  return 0;
  }
  else{
     if(State1_prev == 1 )
     {
      if (State2 == 0){
        return -1;
      }
      else {
        return 1;
      }
     }
    if (State1_prev == 0 )
    {
    if (State2 == 1){
        return 1;
      }
      else {
        return -1;
      }
    }
  }

[andersm]

What do you mean by not working?

[SiliconWizard]

Don't know why this code in Arduino works fine

Me neither. State1_prev and State2_prev are declared as plain (non-static) local variables, and won't retain their value across calls of this function. I have no clue what would make it appear to work in "Arduino", but it's certainly not a behavior you can count on.

[GromBeestje]

How do you initialise your pins in arduino vs normal code? Are they configured as input?

[Siwastaja]

Add -Wall to the compiler command line (assuming GCC) to emit a warning of reading an uninitialized variable, and possibly other helpful warnings to catch mistakes.

[Renate]

Erm, this code is pretty bad.

Starting with the smallest problem, you aren't sampling the two phase at the same time, i.e. the same instruction.
Okay, on a gazillion Hertz processor that hardly makes a difference, but it is sloppy.
Oh, and you're only getting half the transitions, half the resolution.

Code: [Select]

state=(PINB>>whatever)&3; // get two bits, presuming that they are adjacent.
state=(state>>1)^state; // convert from Gray code to binary code so we can do useful things
delta=(state-oldstate)&3; // we can do simple math since it's all binary
oldstate=state; // update, since we got what we wanted, delta
switch(delta)
{
   case 0: break; // no change
   case 1: break; // move forward one
   case 3: break; // move backward one
   case 2: break; // Hmm, we seemed to have missed polling fast enough.
                  // We either moved forward or backwards two.
                  // If we had kept track of which direction we were going before we could probably guess which one.Of course, we also want to add a bit of backlash so that a single transition (or contact bounce) won't change anything.

[bolting.gnome]

Couple of things

1. Lack of static on XXXX_prev variables
2. XXXX_prev variables not initialised (probably should be 0)
3. I suspect the Arduino was using bool rather than unsigned char so reading State1/2 should be something like this
  State1=(PINB & (1 << Enc1)) != 0;
  State2=(PINB & (1 << Enc2)) != 0;
  otherwise State1 will never be equal to State2 unless Enc1 and Enc2 are both 0 which makes no sense for the obvious reason you are trying two read to separate inputs.

[Jan Audio]

Of course, we also want to add a bit of backlash so that a single transition (or contact bounce) won't change anything.

Just code it in the loop instead of interrupts, if it is slow enough you dont need this.

[sokoloff]

Quote from: Renate on December 27, 2019, 12:07:27 pm

Of course, we also want to add a bit of backlash so that a single transition (or contact bounce) won't change anything.

For a quadrature encoder, you don't need to worry about contact bounce.
Any contact bounce between two logical states will just toggle the logical position between those two states.



In the stream of bits above, you want to change the counter as follows on the state transitions:

A	B	Change
High	Rises	+1
Low	Falls	+1
Falls	High	+1
Rises	Low	+1
High	Falls	-1
Low	Rises	-1
Rises	High	-1
Falls	Low	-1

Imagine the user is turning the encoder clockwise, A and B both start High. The first transition that can happen is a falling edge on A. If A bounces during this transition, each bounce down will +1 the counter and each bounce up will -1 the counter. When A finally settles Low, the counter will be increased by 1.

[Renate]

For a quadrature encoder, you don't need to worry about contact bounce.

Well, ok, if you don't want to.

If we are talking about the standard twiddle knob...
They have 24 detents per rotation, 15 degrees a detent.
For each click you have 4 transitions.

Yes, of course +1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1-1+1=1
The problem comes if you react to each transition.
Do you really want to update your OLED from "Apples" to "Bananas" and back in a flurry of updates as a user slowly turns a (gratchy) knob between two indents?
I add hysterisis (or backlash) of two transitions, that is, a fraction of a detent.
It works fine for me.

[bingo600]

Peter D made som nice routines for avr  (Use google xlate - German)

https://www.mikrocontroller.net/articles/Drehgeber#Beispielcode_in_C

Additional
https://www.mikrocontroller.net/topic/112603#new

https://www.mikrocontroller.net/topic/drehgeber-auslesen#new


/Bingo

[chris_leyson]

If you want to use pin change interrupts then there is a good article by Oleg Mazurov, here:-
https://chome.nerpa.tech/mcu/reading-rotary-encoder-on-arduino/
And a follow up article here:-
https://chome.nerpa.tech/mcu/rotary-encoder-interrupt-service-routine-for-avr-micros/
I've used it on PIC and AVR microcontrollers without any problems.

[ehughes]

https://www.mkesc.co.uk/ise.pdf


These is the cleanest way to read an encoder.  Simple, effective and tolerant to about anything you can throw at it.

[newbrain]

I normally use, for "full step" encoders, an FSM based approach.

It's not - if you look at it squinting a bit - very different from the one ehughes suggested...

It works very well, both in polling and interrupt mode - even without any HW debouncing.
I generally use it polling, wrapped by code for velocity implementation.

Code: [Select]

// Read a full step encoder with negative logic
// Based on a simple state machine
// Licence: WTFPL - http://www.wtfpl.net/

typedef enum
{
  ENCIDLE = 0,
  CW__INI,
  CW__MID,
  CW__END,
  CCW_INI,
  CCW_MID,
  CCW_END,
  ENC_ERR,
  CW__OUT = 0x10u, /* The actual state is IDLE, action encoded in high nibble */
  CCW_OUT = 0x20u, /* The actual state is IDLE, action encoded in high nibble */
} State;

#define ENC_STATE_MASK 0x0Fu

/** As the new state for the encoder FSM depends only on the phase inputs and
 *  the previous state, a simple state table is enough.
 *    Clockwise step input sequence: 11, 10, 00, 01, 11, 11 is rest position
 *    CounterCW step input sequence: 11, 01, 00, 10, 11
 *  Unexpected transitions goto an error state, ENC_ERR, then to idle, ENC_IDLE.
 *  The error state persists until a 11 is read (return to rest position).
 *  If the unexpected transition is to the rest position, ENC_ERR is skipped.
 **/
unsigned char const nextState[4][8] = {
    /* Previous state
     ENCIDLE  CW__INI  CW__MID  CW__END  CCW_INI  CCW_MID  CCW_END  ENC_ERR */
    {ENCIDLE, ENCIDLE, ENCIDLE, CW__OUT, ENCIDLE, ENCIDLE, CCW_OUT, ENCIDLE}, // Input 11
    {CW__INI, CW__INI, CW__INI, ENC_ERR, ENCIDLE, CCW_END, CCW_END, ENC_ERR}, // Input 10
    {CCW_INI, ENC_ERR, CW__END, CW__END, CCW_INI, CCW_INI, ENC_ERR, ENC_ERR}, // Input 01
    {ENC_ERR, CW__MID, CW__MID, CW__MID, CCW_MID, CCW_MID, CCW_MID, ENC_ERR}, // Input 00
};

typedef struct
{
  GPIO_Type *encGPIO;
  const uint32_t ph1Pin;
  const uint32_t ph2Pin;
  const int16_t kInc;
  const int16_t kDec;
  State FSMstate;
} Encoder;

#define ENCODERS 1
static Encoder enc[ENCODERS] = {
    {.encGPIO = ENC_PH1_GPIO,
     .ph1Pin = ENC_PH1_PIN,
     .ph2Pin = ENC_PH2_PIN,
     .kInc = +1,
     .kDec = -1,
     .FSMstate = ENCIDLE},
};

// Read full step encoder
int PollEncoder(Encoder *enc)
{
  int key = 0;
  bool ph1 = GPIO_PinRead(enc->encGPIO, enc->ph1Pin);
  bool ph2 = GPIO_PinRead(enc->encGPIO, enc->ph2Pin);
  uint32_t FSMinput = (ph1 | (ph2 << 1)) ^ 0x03u; /* Remove the XOR if rest position is 00 */

  /* Advance the state machine */
  State newState = nextState[FSMinput][enc->FSMstate];

  /* Check if we have a full step */
  if (newState == CW__OUT)
    key = enc->kInc;
  else if (newState == CCW_OUT)
    key = enc->kDec;

  /* Clean up the state from the ouput indication */
  enc->FSMstate = newState & ENC_STATE_MASK;

  return key;
}


[chris_leyson]

Using the FSM approach, either outlined by Oleg Mazurov or the paper linked by ehughs, they're both the same, it's easy to add a second encoder channel.

In the PIC24 example below, encoder_1 is wired to port pins B0 and B1 and encoder_2 is wired to port pins B2 and B3. Pin change interrupt is set to detect a change on B0, B1, B2 and B3.

When a pin change interrupt occurs

For encoder_1 the two LSBs in old_state_1 are left shifted and then ORed with port pins B0 and B1 to generate the 4-bit index for the enc_state lookup table. The value fetched from the enc_state table is then added to encval_1, this is then checked for over and underflow >3 or <-3 respectively and increments or decrememts encoder count_1.

For encoder_2, bits 2 and 3 of old_state_2 are right shifted and then ORed with port pins B2 and B3 to generate the 4-bit index for the enc_state lookup table. However, because we've swapped around the two pairs of index bits for the enc_state table -1 becomes +1 and similarly +1 becomes -1, so instead of adding the table value we now have to subtract it from encval_2 because of the sign change. encval_2 is now checked for over and underflow the same way as encval_1.

Code: [Select]

// Change interrupt:- Incremental encoders
void __attribute__((__interrupt__, no_auto_psv)) _CNInterrupt(void)
{
   static int enc_state[] = {0,1,-1,0,-1,0,0,1,1,0,0,-1,0,-1,1,0};
   old_state_1 = old_state_1 << 2;
   old_state_1 |= (PORTB & 0x0003);
   encval_1 = encval_1 + (enc_state[old_state_1 & 0x0f]);
   if(encval_1 > 3){ count_1++; encval_1 = 0;}
   else if(encval_1 < -3){ count_1--; encval_1 = 0;}
   // because old_state_2 bit positions are swapped
   // enc_state has opposite sign
   old_state_2 = old_state_2 >> 2;
   old_state_2 |= (PORTB & 0x000c);
   encval_2 = encval_2 - (enc_state[old_state_2 & 0x0f]);
   if(encval_2 > 3){ count_2++; encval_2 = 0;}
   else if(encval_2 < -3){ count_2--; encval_2 = 0;}
   IFS1bits.CNIF = 0;
}

I think if encval were unsigned then that would speed up things a bit. 

[Jan Audio]

Yo, you dont have accelleration.

[free_electron]

Yo, you dont have accelleration.

the question is : do you want that ?

when used as a motor/ position encoder you absolutely do not want acceleration !
when used as a scroller in a user interface you may want that.

Rohde & Schwarz came up with the , in my opinion, best solution. they simply added a large flywheel to the back of their encoders. you could just whip that thing and it would keep running for a while. that combined with some clever software allowed very fast but very smooth ( the flywheel dampened the finger jiggle) scrolling.

[Jan Audio]

Do you have a part for that at mouser ?
thanks

[Bud]

Rohde & Schwarz came up with the , in my opinion, best solution. they simply added a large flywheel to the back of their encoders. you could just whip that thing and it would keep running for a while. that combined with some clever software allowed very fast but very smooth ( the flywheel dampened the finger jiggle) scrolling.

Perhaps this was a novel thing in test equipment but this has been employed in Ham radio for decades , for frequency dials. The fast scroll wheel at the back is called Jog Dial or Shuttle Dial.

[edigi]

Yet another rotary encoder (ugly code from me; not using const, sparse commenting etc.). It's FSM and interrupt based, debouncing is purely SW, and it works very well even with crappy KY-040 (that I use a lot for my hobby projects) and has velocity (speed) control as option. Originally designed for UI (for frequency dial).
Code example is for Arduino.

Code: [Select]

/*
This code is free software; you can redistribute it and/or
modify it under the terms of the CC BY-NC-SA 3.0 license.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND...
*/

#define ROTE_CLK    GPIO_NUM_xx
#define ROTE_DT     GPIO_NUM_xy

#define ROTE_SPCTM      50000   // speed control time limit, not defined no velocity control

volatile int32_t  rotval = 0;
void IRAM_ATTR isrrot() {
  volatile static uint8_t  pinsta = 0x3, cwi = 0, ccwi = 0;
  volatile static uint8_t  cwexp[] = {0xD, 0x4, 0x2, 0xB};
  volatile static uint8_t  ccwexp[] = {0xE, 0x8, 0x1, 0x7};
  int32_t  rvchg;
#ifdef ROTE_SPCTM
  volatile static uint32_t tc = 0, tm = 0;
  uint32_t  ctm, td;
#endif

  pinsta = (pinsta << 2) & 0xf;
  if (digitalRead(ROTE_DT)) pinsta |= 0x2;
  if (digitalRead(ROTE_CLK)) pinsta |= 0x1;

  if (pinsta == cwexp[cwi]) cwi++;
  else if (pinsta == ccwexp[ccwi]) ccwi++;
  if (cwi == 0x4 || ccwi == 0x4)
  {
    if (cwi == 4) rvchg = 1;
    else rvchg = -1;
    pinsta = 0x3; cwi = 0; ccwi = 0;
#ifdef ROTE_SPCTM
    ctm = micros();
    td = ctm - tm;
    tm = ctm;
    if (td < ROTE_SPCTM / 2) rvchg *= 7;
    else if (td < (ROTE_SPCTM * 2) / 3) rvchg *= 4;
    else if (td < ROTE_SPCTM) rvchg *= 2;
#endif
    rotval += rvchg;
  }
} // isrrot

int16_t getrotv() {
  static int32_t lval = 0;
  int32_t cval = rotval;
  int16_t rotc = 0;
  if (lval != cval) {
    rotc = cval - lval;
    lval = cval;
  }
  return (rotc);
} // getrotv

void inirotein(gpio_num_t clk, gpio_num_t dt) {
  pinMode(clk, INPUT);
  pinMode(dt, INPUT);
  attachInterrupt(digitalPinToInterrupt(clk), isrrot, CHANGE);
  attachInterrupt(digitalPinToInterrupt(dt), isrrot, CHANGE);
} // inirotein

...
  inirotein(ROTE_CLK, ROTE_DT);
...


[Jan Audio]

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND...

No and i can see why.
When i post my code i give full warranty and guarantee.
Need to remember to bring the code at the right moment.

[Jan Audio]

This code has full warranty.
It works for PIC16 @ 32MHz = 8MHz instruction clock.

Code: [Select]

bit lastC0;

// accelleration timers
unsigned short encodertime = 0;
unsigned short temptime;

// init main loop
if( PORTCbits.RC0 != lastC0 )lastC0 = PORTCbits.RC0;

// main loop
   if( PORTCbits.RC0 != lastC0 )
     {
     lastC0 = PORTCbits.RC0;
     
     changeparam = curpage * 4 + 3;
     
     if( PORTCbits.RC0 )
       {
       temptime = encodertime ? 2 + encodertime >> 7 : 1;
       
       if( !PORTCbits.RC1 )
         {
         SetValue( parametervalue[ changeparam ] + temptime );
         }else
         {
         SetValue( parametervalue[ changeparam ] - temptime );
         }
       
       encodertime = 1023;
       }
     }else
   if( encodertime )
     encodertime--;

Please leave a comment.
You have to take out some line of code and make your own.
I left it in for your inspiration.