ky040 encoder test sketch on i2c lcd

Using Arduino Programming Questions
#1

Hello have been playing around with encoder and 20-04 i2c fed LCD trying to get both to work I am almost there have read a lot of old posts on the subject with my small amount of learning that i have ( best way to learn is put it together)
the problem i cannot see is the encoder counts OK counter clockwise but clockwise it shows only 0-1-0 and so on
i think the problem lies with incrementing decrementing the count ?? I think, could someone take a look please and enlighten me thanks

#include <LiquidCrystal_I2C.h>

// LCD defaults
#define I2C_ADDR    0x27  // Define I2C Address where the PCF8574A is
#define BACKLIGHT_PIN 3
#define En_pin  2
#define Rw_pin  1
#define Rs_pin  0
#define D4_pin  4
#define D5_pin  5
#define D6_pin  6
#define D7_pin  7

LiquidCrystal_I2C lcd(I2C_ADDR,En_pin,Rw_pin,Rs_pin,D4_pin,D5_pin,D6_pin,D7_pin);

const int PinCLK = 2;                   // Used for generating interrupts using CLK signal
const int PinDT = 3;                    // Used for reading DT signal
const int PinSW = 4;                    // Used for the push button switch
const int PinLED = 13;

int SW;
int oldSW;

volatile boolean encChanged;
volatile long encPosition = 0;
volatile boolean up;

void isr()  {                    // Interrupt service routine is executed when a HIGH to LOW transition is detected on CLK
  volatile boolean CLK = digitalRead(PinCLK);
  volatile boolean DT = digitalRead(PinDT);
  up = ((!CLK && DT) || (CLK && !DT));
  if (!up)
    encPosition++;
  else
    encPosition--;
  if (encPosition < 0) encPosition = 0;
  encChanged = true;
  delay(10);
}


void setup()  {
  pinMode(PinCLK, INPUT);
  pinMode(PinDT, INPUT);
  pinMode(PinSW, INPUT_PULLUP);
  pinMode(PinLED, OUTPUT);
  SW = HIGH;
  oldSW = HIGH;
  attachInterrupt(0, isr, FALLING);   // interrupt 0 is always connected to pin 2 on Arduino UNO
 
  Serial.begin(9600);
  Serial.println("Start");

 lcd.setBacklightPin(BACKLIGHT_PIN,POSITIVE);
  lcd.setBacklight(HIGH); 
  lcd.begin(20,4);      // Cuzz we have a 20x4 display
  lcd.clear();           // Get rid of any garbage that might appear on startup
  lcd.print("KY040 encoder test ");
  lcd.setCursor(0,1);
  lcd.print("Rotated");

}
void loop()  {
  
 
  
  SW = digitalRead(PinSW);
  if ( SW == LOW && oldSW == HIGH) {      // check if pushbutton is pressed
    encPosition = 0;              // if YES, then reset counter to ZERO
    Serial.print("Reset");
    encChanged = true;
  }
  oldSW = SW;

  if (encChanged)  {        // do this only if rotation was detected
    encChanged = false;          // do NOT repeat IF loop until new rotation detected
     Serial.print("Count = ");
     Serial.println(encPosition);
     analogWrite(PinLED, encPosition);
     lcd.setCursor(12,1);
     lcd.print(encPosition);
    
  }
}
#2

I don't know that this is your problem but an ISR should NEVER have a delay(...) statement in it.

  1. delay(...) depends upon interrupts that are disabled while you are in an ISR.
  2. An ISR should execute as quickly as possible. delay(..) slows down the ISR and prevents it from executing as quickly as possible.

ditto for Serial.print(...) and its cousins.

#3

Try making these changes :slight_smile: I put lots of notes in the code to help

void isr()  {                    // Interrupt service routine is executed when a HIGH to LOW transition is detected on CLK
  // so you know at this moment in time that the CLK pin #2 went low  >>>  attachInterrupt(0, isr, FALLING);   // interrupt 0 is always connected to pin 2 on Arduino UNO
 // volatile boolean CLK = digitalRead(PinCLK);// We don't need this
 boolean DT;
 DT = digitalRead(PinDT);// lets read this as fast as we can it will be changeing shortly if it keeps turning we keep turning it
 // if you need debounce code we will put it here
 // something like this:
 /*
  static unsigned long DebounceDelay;
  if ((unsigned long)(micros() - DebounceDelay) < 10000) return; // 10 microseconds wait till we can read again
   DebounceDelay = micros();
 */
  if (DT)// DT is High Lets add
    encPosition++;
  else// DT is Low  Lets Subtract
    encPosition--;
 // if (encPosition < 0) encPosition = 0; /// this is a problem and isnt what yo are looking for and would cause the "clockwise it shows only 0-1-0 and so on"
  encChanged = true;
  
 // delay(10); // Lets not use delays here!!!
}

Lets avoid delays in interrupts as vaj4088 pointed out :slight_smile: they are bad

#4 
// if you need debounce code we will put it here
 // something like this:
 /*
  static unsigned long DebounceDelay;
  if ((unsigned long)(micros() - DebounceDelay) < 10000) return; // 10 microseconds wait till we can read again
   DebounceDelay = micros();
 */

You are far better off hardware debouncing these devices with a capacitor in the .1uf to .470uf range between what you are calling the CLK and DT pins and ground.

Software debounce within ISR’s is tricky, and you can easily queue an additional interrupt to get processed.

If you are turning this encoder by hand, you may do better with polling instead of interrupts.

#5

I agree with cattledog if you need to debounce you should use a hardware denounce. if you have an optical encoder you shouldn't need any debounce code/circuit. As for polling vs interrupts, if you have enough other things going on I find interrupts to be great and simple to implement. I used an optical rotary encoder with 400 steps per rotation and was able to measure the RPM of a motor up to 2000RPM using an arduino uno and interrupts. So just don't do a lot of stuff inside the interrupt function and you will find it useful. putting too much inside the interrupt function and you may interrupt your interrupt function. or miss needed interrupts all together. depending on how you code it.

#6