Rotary Encoder Button Not Working?

I'm currently building a project that uses a rotary encoder to control the different colors of an RGB LED. Everything seems to work fine for the most part, except the button seems to not work. Instead of turning off the LED and resetting the colors, it doesn't do anything. I've tried just about everything to get it to work, but no dice. If anybody could offer some help or advice, it would be greatly appreciated :slight_smile:

Here's what I have so far:

#define clkPin 2
#define dtPin 4
#define buttonPin 3
#define redLedPin 9
#define greenLedPin 10
#define blueLedPin 11

int counter = 0;
int ledPins[] = {redLedPin, greenLedPin, blueLedPin};
int buttonState;
int lastButtonState = HIGH;

unsigned long previousRotationTime = 0;
unsigned long previousButtonTime = 0;
unsigned long rotaryInterval = 5;
unsigned long buttonDebounceTime = 50;


void setup() {
  // put your setup code here, to run once:
  attachInterrupt(digitalPinToInterrupt(clkPin), rotationDetect, LOW);
  attachInterrupt(digitalPinToInterrupt(dtPin), ledReset, FALLING);
  pinMode(clkPin, INPUT);
  pinMode(dtPin, INPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(redLedPin, OUTPUT);
  pinMode(greenLedPin, OUTPUT);
  pinMode(blueLedPin, OUTPUT);
}

void loop() {
  // put your main code here, to run repeatedly:
  int ledLoop = abs(counter % 3);
  int buttonReading = digitalRead(buttonPin);
  unsigned long currentTime = millis();

  switch (ledLoop) {
    case 0:
        digitalWrite(ledPins[0], HIGH);
        digitalWrite(ledPins[1], LOW);
        digitalWrite(ledPins[2], LOW);
        break;
    case 1:
        digitalWrite(ledPins[0], LOW);
        digitalWrite(ledPins[1], HIGH);
        digitalWrite(ledPins[2], LOW);
        break;
    case 2:
        digitalWrite(ledPins[0], LOW);
        digitalWrite(ledPins[1], LOW);
        digitalWrite(ledPins[2], HIGH);
        break;
    default:
        digitalWrite(ledPins[0], LOW);
        digitalWrite(ledPins[1], LOW);
        digitalWrite(ledPins[2], LOW);
        break;
  }

}

void rotationDetect() {
  unsigned long currentTime = millis();
  int digitalRotaryReading = digitalRead(dtPin);
  if ((currentTime - previousRotationTime >= rotaryInterval)) {
    previousRotationTime = currentTime;
    if (digitalRotaryReading == HIGH) {
      counter++;
    }
    if (digitalRotaryReading == LOW) {
      counter--;
    }
  }
}

void ledReset() {
  unsigned long currentTime = millis();
  int digitalButtonReading = digitalRead(buttonPin);
  if ((currentTime - previousButtonTime) >= buttonDebounceTime) {
    previousButtonTime = currentTime;
    if (digitalButtonReading == LOW) {
      for (int i = 0; i <= 2; i++) {
        digitalWrite(ledPins[i], LOW);
      }
    }
  }
}

How often do read the button pin? Why do define digitalButtonReading twice?

Whoops, sorry about that. I forgot to mention that I had edited the code before I made this post and had to quickly rewrite it to post it here. Seems like I forgot to remove it.

To answer your first question, I had the code only read the button pin when the interrupt is called.

#define clkPin 2
#define dtPin 4
#define buttonPin 3

attachInterrupt(digitalPinToInterrupt(dtPin), ledReset, FALLING);

What Arduino are you using. On the AT328 devices pin4 is not an interrupt pin.

You really do not need an interrupt to read a button pin, but just use the state change example of the ide.

Arduino UNO. And I've changed the pin to the button pin for the interrupt and still get the same result. The state change example is the debounce code right?

Wrong, it like its name says is a change in state. It has nothing to do with debounce. It allows you to take action on an edge or change of state rather than a level.

Any variable that is used inside and ISR and outside an ISR has to be declared as a 'volatile' type. The button press will only be effective when you check that variable in your code. This will mean that even a short press will be remembered but it will not change how quickly the button press comes into effect.

1 Like

I can't confirm your situation. With the interrupt pin modification to your code and the simplification of the ISR because you know there was a falling trigger I can see the button interrupt. Used a flag to verify entry to the isr.

If the leds are not going low, I would focus on the hardware and not on the interrupt. Have you confirmed that the button pin is wired correctly and you can see it fall with a simple digital read sketch?

#define clkPin 2
#define dtPin 4
#define buttonPin 3
//#define dtPin 3
//#define buttonPin 4
#define redLedPin 9
#define greenLedPin 10
#define blueLedPin 11

int counter = 0;
int ledPins[] = {redLedPin, greenLedPin, blueLedPin};
int buttonState;
int lastButtonState = HIGH;

unsigned long previousRotationTime = 0;
unsigned long previousButtonTime = 0;
unsigned long rotaryInterval = 5;
unsigned long buttonDebounceTime = 50;

volatile boolean flag = false;


void setup() {
  Serial.begin(115200);
  // put your setup code here, to run once:
  attachInterrupt(digitalPinToInterrupt(clkPin), rotationDetect, LOW);
  attachInterrupt(digitalPinToInterrupt(buttonPin), ledReset, FALLING);
  pinMode(clkPin, INPUT_PULLUP);
  pinMode(dtPin, INPUT_PULLUP);
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(redLedPin, OUTPUT);
  pinMode(greenLedPin, OUTPUT);
  pinMode(blueLedPin, OUTPUT);
}

void loop() {
  if (flag)
  {
    flag = false;
    Serial.println("flag");
  }
  // put your main code here, to run repeatedly:
  int ledLoop = abs(counter % 3);
  int buttonReading = digitalRead(buttonPin);
  unsigned long currentTime = millis();

  switch (ledLoop) {
    case 0:
      digitalWrite(ledPins[0], HIGH);
      digitalWrite(ledPins[1], LOW);
      digitalWrite(ledPins[2], LOW);
      break;
    case 1:
      digitalWrite(ledPins[0], LOW);
      digitalWrite(ledPins[1], HIGH);
      digitalWrite(ledPins[2], LOW);
      break;
    case 2:
      digitalWrite(ledPins[0], LOW);
      digitalWrite(ledPins[1], LOW);
      digitalWrite(ledPins[2], HIGH);
      break;
    default:
      digitalWrite(ledPins[0], LOW);
      digitalWrite(ledPins[1], LOW);
      digitalWrite(ledPins[2], LOW);
      break;
  }

}

void rotationDetect() {
  unsigned long currentTime = millis();
  int digitalRotaryReading = digitalRead(dtPin);
  if ((currentTime - previousRotationTime >= rotaryInterval)) {
    previousRotationTime = currentTime;
    if (digitalRotaryReading == HIGH) {
      counter++;
    }
    if (digitalRotaryReading == LOW) {
      counter--;
    }
  }
}

void ledReset() {
  flag = true;
  unsigned long currentTime = millis();
  //int digitalButtonReading = digitalRead(buttonPin);
  if ((currentTime - previousButtonTime) >= buttonDebounceTime) {
    previousButtonTime = currentTime;
    //if (digitalButtonReading == LOW) {
    for (int i = 0; i <= 2; i++) {
      digitalWrite(ledPins[i], LOW);
    }
    //}
  }
}

So it turns out, I'm a complete idiot. I ended up figuring out that the function I wrote wasn't resetting the counter, which the LED states were dependent on. I have since modified my code to implement this as well as included the button state change code for the rotary encoder's button. Thanks to everyone for the help and advice! :slight_smile:

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.