Strange issue with encoder (Will pay you if you can fix this)

Hey friends,

I’ve got 3 encoders hooked up to a Leonardo running interrupts to press a few buttons and some weird inconsistencies between them although they all run off the same code.

The problem:
Encoder#1 Connected to pins 0 & 1 assigned to buttons 13 & 14

  • Sometimes will trigger buttons 13, 14, 15, 16 and 18 (But never 17). It should only ever trigger 13 or 14 depending on the direction of the rotation

Encoder#2 Connected to pins 2 and 3, assigned to buttons 15 & 16

  • Works perfectly, literally no problems

Encoder#3 connected to pins 4 and 7, assigned to buttons 17 & 18

  • Sometimes triggers the buttons, sometimes doesn’t, very intermittent in when it works.

The encoders being used are SR1230. Identical to the picture below

Here is the code, I’ve also attached it as an .ino file

#include <Joystick.h>

#define NUMROTARIES 3
#define NUM_BUTTONS 18

#define B1 8
#define B2 9
#define B3 10
#define B4 11
#define B5 12
#define B6 13
#define B7 18
#define B8 19
#define B9 20
#define B10 21
#define B11 22
#define B12 23

const byte buttonPins[12] = {B1, B2, B3, B4, B5, B6, B7, B8, B9, B10, B11, B12};

struct rotariesdef {
  byte pin1;
  byte pin2;
  int ccwchar;
  int cwchar;
  volatile unsigned char state;
};

rotariesdef rotaries[NUMROTARIES] {
  {0,1,12,13,0},
  {2,3,14,15,0},
  {4,7,16,17,0},
};

#define DIR_CCW 0x10
#define DIR_CW 0x20
#define R_START 0x0
#define R_CCW_BEGIN 0x1
#define R_CW_BEGIN 0x2
#define R_START_M 0x3
#define R_CW_BEGIN_M 0x4
#define R_CCW_BEGIN_M 0x5
volatile const unsigned char ttable[6][4] = {
  // R_START (00)
  {R_START_M,            R_CW_BEGIN,     R_CCW_BEGIN,  R_START},
  // R_CCW_BEGIN
  {R_START_M | DIR_CCW, R_START,        R_CCW_BEGIN,  R_START},
  // R_CW_BEGIN
  {R_START_M | DIR_CW,  R_CW_BEGIN,     R_START,      R_START},
  // R_START_M (11)
  {R_START_M,            R_CCW_BEGIN_M,  R_CW_BEGIN_M, R_START},
  // R_CW_BEGIN_M
  {R_START_M,            R_START_M,      R_CW_BEGIN_M, R_START | DIR_CW},
  // R_CCW_BEGIN_M
  {R_START_M,            R_CCW_BEGIN_M,  R_START_M,    R_START | DIR_CCW},
};

//Setup joystick functions
Joystick_ Joystick(JOYSTICK_DEFAULT_REPORT_ID,JOYSTICK_TYPE_GAMEPAD,
  NUM_BUTTONS, 0,        // Button Count, Hat Switch Count
  false, false, false,   // X and Y, but no Z Axis
  false, false, false,   // No Rx, Ry, or Rz
  false, false,          // No rudder or throttle
  false, false, false);  // No accelerator, brake, or steering

void setup() {
  Joystick.begin();
  rotary_init();
}

void loop() { 
  delay(100); 
  Joystick.setButton(17, 0);
  Joystick.setButton(16, 0);
  Joystick.setButton(15, 0);
  Joystick.setButton(14, 0);
  Joystick.setButton(13, 0);
  Joystick.setButton(12, 0);
  CheckAllButtons();
}

void CheckEncoders(){
    for (volatile int i=0;i<NUMROTARIES;i++) {
    volatile unsigned char result = rotary_process(i);
    if (result == DIR_CCW) {
      Joystick.setButton(rotaries[i].ccwchar, 1);
    }
    if (result == DIR_CW) {
      Joystick.setButton(rotaries[i].cwchar, 1); 
    }
  }
}

void CheckAllButtons(void) {
  bool buttonVals[NUM_BUTTONS];  
  for (int index =0; index < NUM_BUTTONS; index++){ 
      buttonVals[index] = !digitalRead(buttonPins[index]); 
  } //This
    for(int i = 0; i < NUM_BUTTONS; i++) { 
      Joystick.setButton(i, buttonVals[i]); 
    }  
}

void rotary_init() {
  for (int i=0;i<NUMROTARIES;i++) {
    pinMode(rotaries[i].pin1, INPUT_PULLUP);
    pinMode(rotaries[i].pin2, INPUT_PULLUP);
    attachInterrupt(digitalPinToInterrupt(0), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(1), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(2), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(3), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(7), CheckEncoders, CHANGE);
    digitalWrite(rotaries[i].pin1, HIGH);
    digitalWrite(rotaries[i].pin2, HIGH);
      }
  pinMode(B1, INPUT_PULLUP);
  pinMode(B2, INPUT_PULLUP);
  pinMode(B3, INPUT_PULLUP);
  pinMode(B4, INPUT_PULLUP);
  pinMode(B5, INPUT_PULLUP);
  pinMode(B6, INPUT_PULLUP);
  pinMode(B7, INPUT_PULLUP);
  pinMode(B8, INPUT_PULLUP);
  pinMode(B9, INPUT_PULLUP);
  pinMode(B10, INPUT_PULLUP);
  pinMode(B11, INPUT_PULLUP);
  pinMode(B12, INPUT_PULLUP);
}

volatile unsigned char rotary_process(volatile int _i) {
   volatile unsigned char pinstate = (digitalRead(rotaries[_i].pin2) << 1) | digitalRead(rotaries[_i].pin1);
  rotaries[_i].state = ttable[rotaries[_i].state & 0xf][pinstate];
  return (rotaries[_i].state & 0x30);
}

I’ve been poring over this for hours if you can fix this I’ll PayPal you $20 straight up. Please noblemen/women of the Arduino forums I beg you for you help! If you need any more information please let me know.

Button_box_V10.3.ino (3.76 KB)

You lists of pins are rather inconsistent, some of them aren’t set to INPUT_PULLUP so will
be floating. 14,15,16,17 all seem to be floating.

MarkT:
You lists of pins are rather inconsistent, some of them aren’t set to INPUT_PULLUP so will
be floating. 14,15,16,17 all seem to be floating.

Where did you distinguish between pins 13 and 18? As far as I can see they are the same as the others? All set by the code below?

But I “think” (not 100% certain) the existing bit should have enabled it for all pins declared in the “rotaries” variable

struct rotariesdef {
  byte pin1;
  byte pin2;
  int ccwchar;
  int cwchar;
  volatile unsigned char state;
};

rotariesdef rotaries[NUMROTARIES] {
  {0,1,12,13,0},
  {2,3,14,15,0},
  {4,7,16,17,0},
};

void rotary_init() {
  for (int i=0;i<NUMROTARIES;i++) {
    pinMode(rotaries[i].pin1, INPUT_PULLUP);
    pinMode(rotaries[i].pin2, INPUT_PULLUP);
    attachInterrupt(digitalPinToInterrupt(0), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(1), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(2), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(3), CheckEncoders, CHANGE);
    attachInterrupt(digitalPinToInterrupt(7), CheckEncoders, CHANGE);
    #ifdef ENABLE_PULLUPS
      digitalWrite(rotaries[i].pin1, HIGH);
      digitalWrite(rotaries[i].pin2, HIGH);
    #endif

That said, I changed the code here and i’m still having the same issue :frowning:

    pinMode(rotaries[i].pin1, INPUT_PULLUP);
    pinMode(rotaries[i].pin2, INPUT_PULLUP);

it's not obvious what the 1st argument to setButton() is.

Not clear if it's an arbitrary value presumably starting at 0 such as used i CheckAllButtons(). should the 1st argument be buttonPins ?
but in loop() values seem to be pins #s, there are 2 cases for pin 12 and the range seems to be 12-17 but your comments refer to pins 13-18.
*isn't it unnecessary to configure pins B1-B12 using pinMode() for each iteration of i in rotary_init()? likewise for attachInterrupt(). *
you set buttonPins to the #defines that range from 8-13 and 18-23 which doesn't include 14-17. buttonPins [] is used in CheckAllButtons(). seems inconsistent with the range specified in your comments either 12-17 or 13-18
*your use of volatile is unnecessary. *

gcjr:
it's not obvious what the 1st argument to setButton() is.

Not clear if it's an arbitrary value presumably starting at 0 such as used i CheckAllButtons(). should the 1st argument be buttonPins ?
[/quote]
So for this particular example below. 17 is the number of the button 0 is the state. The two states being 1 and 0.
Joystick.setButton(17, 0);
> but in loop() values seem to be pins #s, there are 2 cases for pin 12 and the range seems to be 12-17 but your comments refer to pins 13-18.
So in the code the buttons at 0-17 with 0 being the first button and 17 being the last, for 18 totally including 0. So button 0 actually appears in windows as being button #1. I also picked up on the double #12 and fixed that up, unfortunately didn't fix anything :(.
> isn't it unnecessary to configure pins B1-B12 using pinMode() for each iteration of i in rotary_init()? likewise for attachInterrupt().
Sorry, could you clarify this a little further? I don't quite follow, my apologies.
> you set buttonPins to the #defines that range from 8-13 and 18-23 which doesn't include 14-17. buttonPins [] is used in CheckAllButtons(). seems inconsistent with the range specified in your comments either 12-17 or 13-18
On the Leonardo pins 14, 15 and 16 are the MISO, MOSI, SCK (I wasn't sure if these could be used as digital inputs so I skipped them).
Pin 17 is the RX LED
So with buttonPins, above that I've mapped an alias for each of the pins to a relevant buttons then put them into the array so I can call them easier when I read them. The order looks a little janky but when you see the pin layout on the board it makes sense. Imgur: The magic of the Internet Here's a diagram I found of the ping layout.
```
*#define B1 8
#define B2 9
#define B3 10
#define B4 11
#define B5 12
#define B6 13
#define B7 18
#define B8 19
#define B9 20
#define B10 21
#define B11 22
#define B12 23

const byte buttonPins[12] = {B1, B2, B3, B4, B5, B6, B7, B8, B9, B10, B11, B12};*

```
> your use of volatile is unnecessary.
I just read when using interrupts any variables that get passed into it should be marked as volatile? Should I undo this? I got the info from here. attachInterrupt() - Arduino Reference

On the Leonardo pins 14, 15 and 16 are the MISO, MOSI, SCK (I wasn't sure if these could be used as digital inputs so I skipped them).

why in loop() do you reference pins 14, 15 and 16?

i can understand the mapping, but if you're going to have a #define for the button value (e.g. B7) you should be using the #define throughout your code instead of the hardcoded values (e.g. 14).

i believe you're use of volatile is harmless, but confusing

you didn't answer

in CheckAllButtons(), should the 1st argument [to setButton()] be buttonPins ?

why in loop() do you reference pins 14, 15 and 16?

I think you're referring to this bit? I could be mistaken but I'm fairly certain the first number refers to the number of the button not the pin.

void loop() { 
  delay(100); 
  Joystick.setButton(17, 0);
  Joystick.setButton(16, 0);
  Joystick.setButton(15, 0);
  Joystick.setButton(14, 0);
  Joystick.setButton(13, 0);
  Joystick.setButton(12, 0);

i can understand the mapping, but if you're going to have a #define for the button value (e.g. B7) you should be using the #define throughout your code instead of the hardcoded values (e.g. 14).

Everything that's defined is only ever referred to by the definition any other time in the code. As above, to the best of my knowledge (which could be wrong) in "setButton(14, 0);" the 14 refers to button#14 not pin 14.

i believe you're use of volatile is harmless, but confusing

Alright, cheers, I'll tidy it up then.

this use of setButtons.

void CheckEncoders(){
    for (volatile int i=0;i<NUMROTARIES;i++) {
        volatile unsigned char result = rotary_process(i);
        if (result == DIR_CCW) {
            Joystick.setButton(rotaries[i].ccwchar, 1);
        }

        if (result == DIR_CW) {
            Joystick.setButton(rotaries[i].cwchar, 1);
        }
    }
}