Problem programming pushbutton to select LED and brightness

Hello,

I am new to this forum and Arduino programming. I apologize for any mistakes on my side in advance.

I am trying to program a pushbutton to select one of three LEDs (cycling through them - red - yellow - green - again red - ...) on a short press (<1000ms). Once I press the button for a longer period of time (>1000ms), I want the current LED to be either getting more or less bright, depending on the previous brightness. I programmed this to happen in increments of 5 everytime the loop repeats and the button is pressed.
To use the same brightness on the current LED, no matter which one is selected, I write the brightness into a variable named brightnessState. This way I can either set the LED to 0 or said variable.

Unfortunatly there seems to be a fault in my code as only the red LED is lit up and my button press is not doing anything.
I already tried to look at some integers, that should be changing (as you can see in the code, where my Serial.println is defined).

I hope someone can help me point out where the problem in the code is.

Components used:

  • Arduino Uno
  • red, yellow, green LEDs
  • pushbutton
/* red LED in 3, yellow LED in 5, green LED in 6, button in 8
 * 
 */

const int ledRed = 3;  //Red LED in pin A3
const int ledYel = 5;  //Yellow LED in pin A4
const int ledGre = 6;  //Green LED in pin A5
const int button1 = 8;  //Pushbutton for activity

//variables for the debounce function
int buttonState;  //the current reading from the input pin
int lastButtonState = LOW;  //the previous reading from the input pin

long lastDebounceTime = 0;  //last time the input pin is toggled
long debounceDelay = 50;  //the debounce time

//variales for the function of the switch
long lastButtonPressTime = 0; //last time the input pin is toggled
const int buttonPressTime = 1000; //the time until the second function is activated

//states of the LEDs
long brightnessState = 255;
long lastBrightnessState = 25;
int ledRedState = brightnessState;
int ledYelState = 0;
int ledGreState = 0;



void setup() {
  //debug serial monitor 
 Serial.begin(9600);
  //LEDs as output
 pinMode(ledRed, OUTPUT);
 pinMode(ledYel, OUTPUT);
 pinMode(ledGre, OUTPUT);
 
 //Buttons as input
 pinMode(button1, INPUT);
 
  //LED initial state
 digitalWrite(ledRed, ledRedState);
 digitalWrite(ledYel, ledYelState);
 digitalWrite(ledGre, ledGreState);
}
 
void loop() {
  int reading = digitalRead(button1); //read the state of the switch into variable "reading"
  if(reading != lastButtonState) { //if the switch changed due to noise or pressing
    lastDebounceTime = millis();  //reset the debounce timer
  }
    if((millis() - lastDebounceTime) > debounceDelay) { 
      //whatever the reading is, it's been there for longer than the debounce delay
      //so take it as the actual current state
        if(reading != buttonState){
            buttonState = reading; //the setButtonState shall be now set to the initial read state
        }
     }
  if(buttonState != lastButtonState) { //if the switch changed due to letting it go
    if(lastButtonState == 1){
      lastButtonPressTime = millis(); //reset the button-press timer    
    
      if((millis() - lastButtonPressTime) > buttonPressTime) {  //if the button is pressed for longer than 1000ms do the function nr.2
        //action for second function of the button
        if(brightnessState == 255){
          lastBrightnessState = brightnessState;
          brightnessState = brightnessState - 5;
        }
        if(brightnessState < lastBrightnessState) {
          if(brightnessState >= 30){
          lastBrightnessState = brightnessState;
          brightnessState = brightnessState - 5;
          }
          else{
          lastBrightnessState = brightnessState;
          brightnessState = brightnessState  + 5;
          }
        }
        if(brightnessState > lastBrightnessState){
          lastBrightnessState = brightnessState;
          brightnessState = brightnessState + 5;
        }
      }
    }
  }
    //debug Serial Monitor 
    //Serial.println(millis() - lastButtonPressTime);
    Serial.println(buttonState);

    
    //the action if the button is pressed shortly
    if((millis() - lastButtonPressTime) <= buttonPressTime) { //if the button is pressed shorter or equal to 250ms do the function nr.1
      //option 1: The red LED is on while the button is pressed
      if(ledRedState >= 1){
        ledRedState = 0;
        ledYelState = brightnessState;
      }
      //option 2: The yellow LED is on while the button is pressed
      if(ledYelState >= 1){
        ledYelState = 0;
        ledGreState = brightnessState;
      }
      //option 3: The green LED is on while the button is pressed
      if(ledGreState >= 1){
        ledGreState = 0;
        ledRedState = brightnessState;
      }
    }
    digitalWrite(ledRed, ledRedState);
    digitalWrite(ledYel, ledYelState);
    digitalWrite(ledGre, ledGreState);
  lastButtonState = reading;  //reset the last button state
}

Thank you for everyone taking the time to help me.

Custos

I thought that buttonState would be LOW or HIGH, and then I tripped across

if (buttonState == 1)

which could work but is not documented.

from Arduino.h:

#define HIGH 0x1
#define LOW  0x0

Thus, these are exactly equivalent:

if (buttonState == 1)
if (buttonState == HIGH)

But HIGH is preferred because the definition might change at some future date (in theory!).

Thanks! I am currently separated from my Arduino software and could not look this up.

A schematic or wiring diagram might help.

vaj4088:
A schematic or wiring diagram might help.

The pin mode statement in the code say:

 //Buttons as input
 pinMode(button1, INPUT);

This implies that you are using an external pullup or pulldown resistor.

    if(lastButtonState == 1){

This implies that the resistor is a pulldown resistor. That is how the switch is wired, isn't it?

If so, why? Use the internal pullup resistor, with INPUT_PULLUP as the mode, and connect one leg to ground and the other leg to a digital pin. Far simpler. Then, LOW means pressed.

It is NOT necessary to use three variables to determine if the state of the pin changed from not pressed to pressed or from pressed to not pressed. Rewrite your code to use two variables - the current state and the previous state.