Click vs. Hold Button with NeoPixel LEDs

Hello! I took a Robotics class in college a few years ago, that's the full extent of my Arduino knowledge.

I've been working on a program for a button that interacts with NeoPixel RGBWs differently based on whether the button is clicked or held. I've browsed and tried out bits of code from a bunch of different topics here on the forum, and have spliced together something that is 90% working!

The code for the button Click works fine. It does a simple ColorShift, and I'm able to run as many Clicks as I'd like. But once I try out a button Hold, I'm unable to go back to doing just Clicks. I'm guessing the code is getting caught somewhere, but I'm unable to figure out where.

I've tried troubleshooting by adding in several Serial printouts to the monitor, but they indicate that the Click Event is running normally with every button click, but the LEDs don't react if the Hold Event had been previously triggered.

I'm using an Arduino UNO, 3 LEDS from a NeoPixel Digital RGBW strip, and a lil push button.
NeoPixels = GND, PIN 6, and 5V
Button = GND, PIN 2

#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
 #include <avr/power.h>    // Required for 16 MHz Adafruit Trinket
#endif

#define pixelCount 3       // Number of NeoPixels
#define buttonPin 2        // input pin to use as a digital input
#define ledPin1 6          // digital output pin for LED strip

// Declare our NeoPixel strip object:
Adafruit_NeoPixel strip(pixelCount, ledPin1, NEO_GRBW + NEO_KHZ800);

boolean ledVal1 = false; // state of LED strip

void setup() {
   Serial.begin(9600);
   Serial.println("Setup");
   // Set button input pin
   pinMode(buttonPin, INPUT);
   digitalWrite(buttonPin, HIGH );
   // Set LED output pins
   pinMode(ledPin1, OUTPUT);
   digitalWrite(ledPin1, ledVal1);

  strip.begin(); // Initialize NeoPixel strip object (REQUIRED)
  strip.show();  // Initialize all pixels to 'off'
  strip.setBrightness(10);
}

void loop() {
    // Get button event and act accordingly
   int b = checkButton();
   if (b == 1) clickEvent();
   if (b == 2) holdEvent();
}

// Events that can trigger
void clickEvent() {
    Serial.println("Click");
    colorWipe(strip.Color(255, 0, 255), 50);    // Magenta Flash
    delay(100);
    colorWipe(strip.Color(0, 0, 0), 50);    // Black/OFF
    digitalWrite(ledPin1, ledVal1);
}
void holdEvent() {
    Serial.println("Hold");
    brighten(strip.Color(255, 0, 255), 5);   // Magenta Bright
    delay(500);
    brighten(strip.Color(255, 0, 255), 25);   // Magenta Brighter
    delay(500);
    brighten(strip.Color(255, 0, 255), 50);   // Magenta Brightest
    delay(500);
    brighten(strip.Color(0, 0, 0), 0);   // Black/OFF
    digitalWrite(ledPin1, ledVal1);
}

// Commands for LEDs
void brighten(uint32_t color, int intensity){
  for (int i=0; i< strip.numPixels(); i++) {
    strip.setPixelColor(i, color);
    strip.setBrightness(intensity);
    strip.show();
    delay(10);
  }
}
void colorWipe(uint32_t color, int wait) {
  for(int i=0; i<strip.numPixels(); i++) { // For each pixel in strip...
    strip.setPixelColor(i, color);         //  Set pixel's color (in RAM)
    strip.show();                          //  Update strip to match
    delay(wait);                           //  Pause for a moment
  }
}

// SO What Is The Button Doing?? //

int debounce = 20;  // debounce period
int holdTime = 1000; // Hold period, how long to wait

boolean buttonVal = HIGH;   // value read from bottom
boolean buttonLast = HIGH;  // value of button's previous state
long downTime = -1;         // time button has been held
long upTime = -1;           // time button was released
boolean ignoreUp = false;
boolean waitForUp = false;
boolean holdEventPast = false;  // whether Hold occured

int checkButton() {
  int event = 0;
  buttonVal = digitalRead(buttonPin);
  
  // Button is pressed down
  if (buttonVal == LOW && buttonLast == HIGH && (millis() - upTime) > debounce) {
    Serial.println("Pressed");
    downTime = millis();
    ignoreUp = false;
    waitForUp = false;
    holdEventPast = false;
  }
  // Button is released quickly
  else if (buttonVal == HIGH && buttonLast == LOW && (millis() - downTime) > debounce) {
     if (not ignoreUp) {
        upTime = millis();
        Serial.println("Released");
        event = 1;
     }
   }
  
  // Is Button Being Held Down?
  if (buttonVal == LOW && (millis() - downTime) >= holdTime) {
    // Trigger Hold if not already
    if (not holdEventPast) {
      Serial.println("Holding");
      event = 2;
      waitForUp = true;
      ignoreUp = true;
      holdEventPast = true;
    }
  }
  
  buttonLast = buttonVal;
  return event;
}

I'm guessing it's just something totally obvious that I'm overlooking or didn't 100% understand when implementing.

Thank you so much for your time!

I haven't completely surveyed the code but your hold time is 1 second. The execution of your holdEvent() function contains more than 1 second of delays. These delays will mess up your timing because you will miss button transitions.

I would use a state machine for the display patterns in order to eliminate delays and simplify your button code.

Here is a simplification of your button code. I haven't tested it!

No need for fancy debounce just use a short delay. The only delay() permitted! I do it this way a lot:

const unsigned long holdTime = 1000; // Hold period, how long to wait

int checkButton() 
{
  static byte buttonLast = digitalRead(buttonPin);
  static boolean holdEventPast = false;  // whether Hold occured
  static unsigned long downTime;
  
  int event = 0;
  byte buttonVal = digitalRead(buttonPin);

  if (buttonVal != buttonLast)
  {
    delay(50); // debounce (should be only delay in code!!!)
    buttonLast = buttonVal;
    
    if (buttonVal == LOW)
    {
      downTime = millis();
      holdEventPast = false;
    }
    else
    {
      if (!holdEventPast) event = 1;
    }
  }
  
  // Is Button Being Held Down?
  if (buttonVal == LOW && (millis() - downTime) >= holdTime) 
  {
    // Trigger Hold if not already
    if (!holdEventPast) 
    {
      Serial.println("Holding");
      event = 2;
      holdEventPast = true;
    }
  }
  return event;
}

Oh wow, thank you so much! This does work! So it was all the delays I had slapped into the Hold Event that was getting the program stuck :sweat_smile:

If I'd still like for the Hold Event to increase the brightness of the LEDs during the duration of the hold over a certain period of a few seconds, is there a way to accomplish this without delays?

I recommend you have a look at the "OneButton" library.
You can assign a lot of events to your buttons:
click
on hold
double Click
long press ...

have a look at the description: Arduino OneButton Library
you can install that library with the library manager.