Whats wrong with this code? Please!

Using Arduino Programming Questions
#1

Hello.

I have 2 Ky-40 encoders and 1 Adafruits neotrellis button matrix on an Arduino uno.

The bug is with encoders. I know its a code bug because both behave exactly in the same way.

If I turn them to the right the give the correct value, but from time to time they give the left turn value.

If I turn the left they always give the correct value.

I think is a timing thing.

Here is a clue, if I put a delay at the end of the Loop, the greater the delay, the more often the wrong

value. If i put a delay of 20 ms or greater, they always give the wrong value.

I try different Baud rates with no luck.

Pleaseeee!!!

#include "Adafruit_NeoTrellis.h"

#define CLK 2
#define DT 3
#define SW 5

#define CLK2 7
#define DT2 10
#define SW2 11



int currentStateCLK;
int lastStateCLK;
String currentDir ="";
unsigned long lastButtonPress = 0;


int currentStateCLK2;
int lastStateCLK2;
String currentDir2 ="";
unsigned long lastButtonPress2 = 0;






Adafruit_NeoTrellis trellis;

TrellisCallback blink(keyEvent evt){
  if (evt.bit.EDGE == SEESAW_KEYPAD_EDGE_RISING) {
     Serial.println(evt.bit.NUM);
    } else if (evt.bit.EDGE == SEESAW_KEYPAD_EDGE_FALLING) {
     // Serial.write(evt.bit.NUM);
  }

  return 0;
}



const byte numChars = 32;
char receivedChars[numChars];
char tempChars[numChars];       
      
char messageFromPC[numChars] = {0};
int a = 0;
int b = 0;
int c = 0;
int d = 0;

boolean newData = false;





void setup() {
  

  pinMode(CLK,INPUT);
  pinMode(DT,INPUT);
  pinMode(SW, INPUT);

  pinMode(CLK2,INPUT);
  pinMode(DT2,INPUT);
  pinMode(SW2, INPUT);

  
  
  Serial.begin(9600);


  lastStateCLK = digitalRead(CLK);
  lastStateCLK2 = digitalRead(CLK2);
  
  
  
  
  !trellis.begin();  

  for(int i=0; i<NEO_TRELLIS_NUM_KEYS; i++){
    trellis.activateKey(i, SEESAW_KEYPAD_EDGE_RISING);
    trellis.activateKey(i, SEESAW_KEYPAD_EDGE_FALLING);
    trellis.registerCallback(i, blink);
  }
}


///////////////////////////////////////////////////////////


void loop1(void) {

  currentStateCLK = digitalRead(CLK);

  if (currentStateCLK != lastStateCLK  && currentStateCLK == 1){

    if (digitalRead(DT) != currentStateCLK) {
  
      currentDir ="1000";
    } else {  
 
      currentDir ="1001";
    }

    Serial.println(currentDir);    
  }

  lastStateCLK = currentStateCLK;

  int btnState = digitalRead(SW);

  if (btnState == LOW) {
      
    if (millis() - lastButtonPress > 50) {
      Serial.println("1002");
    }

    lastButtonPress = millis();
  }

}


//////////////////////////////////////////////////////////////////

void loop2(void) {

 currentStateCLK2 = digitalRead(CLK2);

  if (currentStateCLK2 != lastStateCLK2  && currentStateCLK2 == 1){

    if (digitalRead(DT2) != currentStateCLK2) {
  
      currentDir2 ="1003";
    } else {  
 
      currentDir2 ="1004";
    }

    Serial.println(currentDir2);
    
  }

  lastStateCLK2 = currentStateCLK2;

  int btnState2 = digitalRead(SW2);

  if (btnState2 == LOW) {
      
    if (millis() - lastButtonPress2 > 50) {
      Serial.println("1005");
    }

    lastButtonPress2 = millis();
  }

 
}

  
/////////////////////////////////////////////////////////////////////

void loop(void) {
 
  trellis.read();   
       
       recvWithStartEndMarkers();
    if (newData == true) {
        strcpy(tempChars, receivedChars);
            
        parseData();
        
        newData = false;
    }
  
  
  loop1(); 
  loop2(); 

}





/////////////////////////////////////////////////////////////////////

void recvWithStartEndMarkers() {
    static boolean recvInProgress = false;
    static byte ndx = 0;
    char startMarker = '<';
    char endMarker = '>';
    char rc;

    while (Serial.available() > 0 && newData == false) {
        rc = Serial.read();

        if (recvInProgress == true) {
            if (rc != endMarker) {
                receivedChars[ndx] = rc;
                ndx++;
                if (ndx >= numChars) {
                    ndx = numChars - 1;
                }
            }
            else {
                receivedChars[ndx] = '\0';
                recvInProgress = false;
                ndx = 0;
                newData = true;
            }
        }

        else if (rc == startMarker) {
            recvInProgress = true;
        }
    }
}

///////////////////////////////////////////////////////////////////

void parseData() {

     
    
    
    char * strtokIndx; 

    strtokIndx = strtok(tempChars," ");     
    strcpy(messageFromPC, strtokIndx); 
 
    strtokIndx = strtok(NULL, " "); 
    a = atoi(strtokIndx);     

    strtokIndx = strtok(NULL, " ");
    b = atoi(strtokIndx);     


    strtokIndx = strtok(NULL, " ");
    c = atoi(strtokIndx);    

    strtokIndx = strtok(NULL, " ");
    d = atoi(strtokIndx);    

    trellis.pixels.setPixelColor(a,b,c,d);
    trellis.pixels.show();

  
}
#2

You're using a keypad matrix library to read an encoder? I'm not sure that's a sound approach

I don't even use a library for reading an encoder... but IIRC you need to use the history of the readings on BOTH pins, and you only change the reading after both lines have transitioned... and the order they transition in determines which direction it was in (you get two transitions per "notch" of the encoder if it's one of the usual encoders, which I'm pretty sure it is, because Keyes always used the cheapest most abundant parts they could), they might well have jitter when the wheel is right along a tooth, but your code is supposed to filter that out.

The names clk and dt are terrible names for them, they're normally referred to as A and B outputs, as individually they both contain exactly half of the information.

#3

The KY-40 encoders have many possible issues.

First, try and remove any switch bounce with a capacitor in the .1uf to .470uf range between what you are calling the CLK and DT pins and ground.

Second, your code is based on one pin’s Rising transition, but you do not reach the detent position (both pins with digitalRead == 1) with this transition in both directions. That is why one of the directions is flaky and the other is solid. Try and modify your code to catch for Rising transitions on both pins. You may need interrupts to do so.

#4

Oh shiiiii I forgot I had code to do EXACTLY what you want, and it's not wedged into an intensely confusing piece of code for a new person (like I thought it was)

Just need to add the global variables and this will work no problem, with no hardware debounce (in fact, I would never put hardware debounce on an encoder.

Works on 2 encoders, you just need to declare the the globals it uses.

Oh, wait - there is one other catch - you need to give it a max value for each one, too (it's meant to scroll through two menus, with a limited number of items each), or adapt that part of the ISR.

And one other - no, you can't change the pins without a lot of work that I don't think you're ready to do since you sound newish to Arduino

All the logic is in the ISR there. It's a dubious way to do it, but I had no choice due to limiting decisions elsewhere in the sketch.

I'll improve the ISR next time I work on it (which I hope will be soonish, because I really want to get it running on some new hardware that I get in 2 weeks; I think I can provide an ISR that works on 328p/pb and is incrementally better by just doing that part before the stuff specific to Dx/tinyAVR 0/1/2-series/megaAVR 0-series parts. Running the thing I want on a DA-series part will make things so much better it's not even funny (not for this, but for the other functionality of the sketch - 2:1 clock speed increase if I overclock to 32MHz (some functionality was clock speed limited), and it has oodles of RAM (some stuff was RAM limited), and I can use the DAC to replace the contrast potentiometer - not only was that pot an obnoxious part to put on because it made the assembly highly path dependent (it wound up blocking something else), and that thing nearly blocked access to it's pins (hmm, actually until I get my other soldering iron working again (just needs tips, but rather exotic ones), and the tips for my other are a shape that might not let me reach between the two boards to solder the pot in place), adjusting it required a screwdriver, whereas this will just require software. So yeah, getting rid of that will be awesome.... AAAAAND I was able to find just enough extra pins to control ones with an RGB backlight, so they're gonna look AWESOME. And best of all, for now at least, I have a guy who I can pay hourly to do assembly for me, and he's picking up soldering faster than I'd dared hope for! He's already assembled $700ish of inventory for my store that I had kicking around as parts but no time to assemble...

#5