Something isn't right with my if statements...

Pressing the button for 3sec turns on the LED White after release. Pressing it for 3 more turns it off after release.

Tap the button? Random RGB, then back to white.

Tap and hesitate? It seems to randomly pick RGBW and stick.

What am I missing?

Thanks!

const int buttonPin = 2; //Pin 2
const int redPin = 11;
const int greenPin = 10;
const int bluePin = 9;

int CurrentColor = 0; //White=0,Red=1,Green=2,Blue=3,Yellow=4,Purple=5, Off=6
int threshold_long = 3000; //3000millisec = 3sec
int threshold_short = 500; //500millisec = .5sec
int duration = 0; //How long the button was high
int buttonState = 0; 
int buttonStatus = 0;
int LEDStatus = 0;

unsigned long BUTT_Press = 0;
unsigned long BUTT_Held = 0;
unsigned long NOW = 0;  

void setup()
{
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(bluePin, OUTPUT);  
  pinMode(buttonPin, INPUT);
  analogWrite(redPin, 255);
  analogWrite(greenPin, 255);
  analogWrite(bluePin, 255);}

void loop() 
{
  buttonState = digitalRead(buttonPin);
    if (buttonState == LOW) 
    {
      buttonStatus=0;
      if (duration <=threshold_short && LEDStatus==1)
      {
        CurrentColor=CurrentColor+1; 
        if (CurrentColor>3){CurrentColor=0;}
        if (CurrentColor == 0) {analogWrite(redPin, 0);analogWrite(greenPin, 0);analogWrite(bluePin, 0);}
        if (CurrentColor == 1) {analogWrite(redPin, 0);analogWrite(greenPin, 255);analogWrite(bluePin, 255);}
        if (CurrentColor == 2) {analogWrite(redPin, 255);analogWrite(greenPin, 0);analogWrite(bluePin, 255);}
        if (CurrentColor == 3) {analogWrite(redPin, 255);analogWrite(greenPin, 255);analogWrite(bluePin, 0);}
        duration = 0;
      }
      if (duration >= threshold_long && LEDStatus ==0) 
      {
          // turn LED on WHITE:
          analogWrite(redPin, 0);
          analogWrite(greenPin, 0);
          analogWrite(bluePin, 0);
          LEDStatus = 1;
          duration = 0;
          CurrentColor=0;
      }
      if (duration >= threshold_long && LEDStatus ==1) 
      {
          // turn LED off:
          analogWrite(redPin, 255);
          analogWrite(greenPin, 255);
          analogWrite(bluePin, 255);
          LEDStatus = 0;
          duration = 0;
      }
    }
  
  if (buttonState == HIGH)
  {
    if (buttonStatus==0)
    {
      BUTT_Press = millis();
    }
    else
    {  
      BUTT_Held = millis();
      duration = BUTT_Held - BUTT_Press;
    }
    buttonStatus=1;
  }
}

What am I missing?

Carriage returns, for one thing.

        if (CurrentColor == 1) {analogWrite(redPin, 0);analogWrite(greenPin, 255);analogWrite(bluePin, 255);}

ONE statement per line, not a whole bunch of them.

I don't understand the point of testing duration and THEN assigning a value to it.

If I don’t assign duration to zero, won’t it meet the conditions of the if statement forever?

as for the CRs, I tested the output colors and had them spread out, when I verified that they were correct, I tightened things up for neatness.

const int buttonPin = 2; //Pin 2
const int redPin = 11;
const int greenPin = 10;
const int bluePin = 9;

int CurrentColor = 0; //White=0,Red=1,Green=2,Blue=3,Yellow=4,Purple=5, Off=6
int threshold_long = 3000; //3000millisec = 3sec
int threshold_short = 500; //500millisec = .5sec
int duration = 0; //How long the button was high
int buttonState = 0; 
int buttonStatus = 0;
int LEDStatus = 0;

unsigned long BUTT_Press = 0;
unsigned long BUTT_Held = 0;
unsigned long NOW = 0;  

void setup()
{
  pinMode(redPin, OUTPUT);
  pinMode(greenPin, OUTPUT);
  pinMode(bluePin, OUTPUT);  
  pinMode(buttonPin, INPUT);
  analogWrite(redPin, 255);
  analogWrite(greenPin, 255);
  analogWrite(bluePin, 255);}

void loop() 
{
  buttonState = digitalRead(buttonPin);
    if (buttonState == LOW) 
    {
      buttonStatus = 0;
      if (duration <=threshold_short && LEDStatus==1)
      {
        CurrentColor=CurrentColor+1; 
        if (CurrentColor>3)
        {
          CurrentColor=0;
        }
        if (CurrentColor == 0)
        {
          analogWrite(redPin, 0);
          analogWrite(greenPin, 0);
          analogWrite(bluePin, 0);
        }
        if (CurrentColor == 1)
        {
          analogWrite(redPin, 0);
          analogWrite(greenPin, 255);
          analogWrite(bluePin, 255);
        }
        if (CurrentColor == 2)
        {
          analogWrite(redPin, 255);
          analogWrite(greenPin, 0);
          analogWrite(bluePin, 255);
        }
        if (CurrentColor == 3)
        {
          analogWrite(redPin, 255);
          analogWrite(greenPin, 255);
          analogWrite(bluePin, 0);
        }
        duration = 0;
      }
      if (duration >= threshold_long && LEDStatus ==0) 
      {
          // turn LED on WHITE:
          analogWrite(redPin, 0);
          analogWrite(greenPin, 0);
          analogWrite(bluePin, 0);
          LEDStatus = 1;
          duration = 0;
          CurrentColor=0;
      }
      if (duration >= threshold_long && LEDStatus ==1) 
      {
          // turn LED off:
          analogWrite(redPin, 255);
          analogWrite(greenPin, 255);
          analogWrite(bluePin, 255);
          LEDStatus = 0;
          duration = 0;
      }
    }
  
  if (buttonState == HIGH)
  {
    if (buttonStatus==0)
    {
      BUTT_Press = millis();
    }
    else
    {  
      BUTT_Held = millis();
      duration = BUTT_Held - BUTT_Press;
    }
    buttonStatus=1;
  }
}

I tightened things up for neatness.

Code that is hard to read is NOT neat.

You can't learn anything about how long the switch is pressed if you don't detect when the switch BECOMES pressed. There is a state change detection example that shows how. Read it, and notice that the current and previous state names have a CLEAR relationship. If you are trying to detect a state change, it is not obvious from the names of your variables.

If the buttonState is high, and I do a millis() check, don't I know when it was pressed?

If the loop comes around, and the buttonState is still high and the buttonStatus has not been reset to 0 (through the ButtonState==LOW if(), can't I just continue to take millis() checks until the difference reaches the threshold value?

If the buttonState is high, and I do a millis() check, don't I know when it was pressed?

You know when it IS pressed, not when it BECAME pressed. It is the time between now and when the switch BECAME pressed that is important (or the time between when the switch BECOMES released and when it BECAME pressed).

Right, so on each loop, check to see if this is the first time the button is HIGH, if it is, mark that time as

BUTT_Press

If it is not the first time, shown by the variable buttonStatus,

Butt_Held

is marked as the time.

Continue to take the difference until the loop shows the buttonState as low, this shows that the button is no longer pressed. With the buttonStatus being 1, we know that in the previous loop it WAS pressed, and we have the "duration" that it was pressed, yes?

Right, so on each loop, check to see if this is the first time the button is HIGH, if it is, mark that time as

To do that, you need to keep track of the previous state, so that you can detect the state change...

if it is, mark that time as

BUTT_Press

If you pressed your butt, yes.

If it is not the first time, shown by the variable buttonStatus,

Butt_Held

is marked as the time.

Why? You do NOT need to keep track of when the switch IS held.