Inconsistency in loop

Hi,

I’m currently working on a Arduino RGB aim game project. I have an Arduino Mega, 5 push buttons and 4 RGB-LED strips.

At random one of the 4 RGB led strips is chosen to display the color red.
The game is to push the corresponding push button.

When the correct button is pushed a different RGB led strip should light up and the game continues until all the RGB led strips show a red color and all the corresponding buttons have been pushed.

The problem i having is the following:

The first loop the code works correct but then some times the random chosen RGB led strip will not show the red color when it needs to (it will just stay white). i think it maybe had to do something with the switch, but i’m not sure.

Here is the code:

int RedLed1 = 2;
int GreenLed1 = 3;
int BlueLed1 = 4;

int RedLed2 = 5;
int GreenLed2 = 6;
int BlueLed2 = 7;

int RedLed3 = 8;
int GreenLed3 = 9;
int BlueLed3 = 10;

int RedLed4 = 11;
int GreenLed4 = 12;
int BlueLed4 = 13 ;

int Button4 = 53;
int Button3 = 51;
int Button2 = 49;
int Button1 = 47;

int RedVal;
int BlueVal;
int GreenVal;

int value, value2, value3;   // value from ,

boolean started;                 // Start
byte currentLed; // the one that is lit up right now

boolean needNewLed = true; // we need a new LED

long time = 0;

int val1, val2, val3, val4;                     // variable for reading the pin status
int buttonState1, buttonState2, buttonState3, buttonState4;             // variable to hold the last button state

boolean Paneel1 = false;
boolean Paneel2 = false;
boolean Paneel3 = false;
boolean Paneel4 = false;


void setup() { //this sets the output pins
  Serial.begin(9600);            //Serial debug

  pinMode(RedLed1, OUTPUT);       //Ledstrip1
  pinMode(GreenLed1, OUTPUT);
  pinMode(BlueLed1, OUTPUT);

  pinMode(RedLed2, OUTPUT);      //Ledstrip2
  pinMode(GreenLed2, OUTPUT);
  pinMode(BlueLed2, OUTPUT);

  pinMode(RedLed3, OUTPUT);       //Ledstrip3
  pinMode(GreenLed3, OUTPUT);
  pinMode(BlueLed3, OUTPUT);

  pinMode(RedLed4, OUTPUT);      //Ledstrip4
  pinMode(GreenLed4, OUTPUT);
  pinMode(BlueLed4, OUTPUT);

  pinMode(Button1, INPUT_PULLUP);  //Knop
  pinMode(Button2, INPUT_PULLUP);  //Knop
  pinMode(Button3, INPUT_PULLUP);  //Knop
  pinMode(Button4, INPUT_PULLUP);  //Knop

  pinMode(45, INPUT_PULLUP);  //Knop

  randomSeed(analogRead(0));


}

void loop() {
  if (started == false) {
    Fade();
    delay(5);
  }
  if (digitalRead(45) == LOW) {
    FadeToRed();
    White();
    started = true; //start the game if any button is pressed
    Serial.println("Start");
  }

  if (started == true) {
    if (needNewLed == true) { //we need another LED
      byte randomLed = random(1, 5); // pick a random LED

      switch (randomLed) {
        case 1:
          digitalWrite(RedLed1, LOW);
          digitalWrite(BlueLed1, HIGH);
          digitalWrite(GreenLed1, HIGH);
          break;

        case 2:
          digitalWrite(RedLed2, LOW);
          digitalWrite(BlueLed2, HIGH);
          digitalWrite(GreenLed2, HIGH);
          break;

        case 3:
          digitalWrite(RedLed3, LOW);
          digitalWrite(BlueLed3, HIGH);
          digitalWrite(GreenLed3, HIGH);
          break;

        case 4:
          digitalWrite(RedLed4, LOW);
          digitalWrite(BlueLed4, HIGH);
          digitalWrite(GreenLed4, HIGH);
          break;
      }

      while ((randomLed == 1 && Paneel1 == true) || (randomLed == 2 && Paneel2 == true) || (randomLed == 3 && Paneel3 == true) || (randomLed == 4 && Paneel4 == true))  { //make sure it's not the same as the last one
        randomLed = random(1, 5);
      }
      currentLed = randomLed;// Currentled is nu de Ledstrip die aanstaat
      Serial.print(currentLed); // geeft currentled weer in serialprint
      delay(10); //wait a little bit
      needNewLed = false;
    }

    val1 = digitalRead(Button1);
    val2 = digitalRead(Button2);
    val3 = digitalRead(Button3);
    val4 = digitalRead(Button4);

    if (val1 != buttonState1 && val1 == LOW || val2 != buttonState2 && val2 == LOW || val3 != buttonState3 && val3 == LOW || val4 != buttonState4 && val4 == LOW) {          // the button state has changed!


      if (currentLed == 1 && val1 == LOW) { // did they hit the right button?
        Serial.println("HIT");
        Paneel1 = true;
        needNewLed = true;
        delay(100);
      }

      else if (currentLed == 2 && val2 == LOW) { // did they hit the right button?
        Serial.println("HIT");
        Paneel2 = true;
        needNewLed = true;
        delay(100);
      }

      else if (currentLed == 3 && val3 == LOW) { // did they hit the right button?
        Serial.println("HIT");
        Paneel3 = true;
        needNewLed = true;
        delay(100);
      }

      else if (currentLed == 4 && val4 == LOW) { // did they hit the right button?
        Serial.println("HIT");
        Paneel4 = true;
        needNewLed = true;
        delay(100);
      }

      else if ((val1 == LOW) || (val2 == LOW) || (val3 == LOW) || (val4 == LOW)) { // did they hit the wrong button?
        Serial.println("MIS");
        needNewLed = true;
      }
    }
    else {
      delay(10);
    }
    buttonState1 = val1;
    buttonState2 = val2;
    buttonState3 = val3;
    buttonState4 = val4;
  }
}

void FadeToRed() {

  analogWrite(RedLed1, value);
  analogWrite(BlueLed1, value3);
  analogWrite(GreenLed1, value2);

  analogWrite(RedLed2, value);
  analogWrite(BlueLed2, value3);
  analogWrite(GreenLed2, value2);

  analogWrite(RedLed3, value);
  analogWrite(BlueLed3, value3);
  analogWrite(GreenLed3, value2);

  analogWrite(RedLed4, value);
  analogWrite(BlueLed4, value3);
  analogWrite(GreenLed4, value2);

  int RedVal = 0;
  int BlueVal = 0;
  int GreenVal = 0;
  for ( int i = 0 ; i < 255 ; i += 1 ) {
    GreenVal += 1;
    BlueVal += 1;
    RedVal -= 1;
    analogWrite( GreenLed1, 0 + GreenVal );
    analogWrite( BlueLed1, 0 + BlueVal );
    analogWrite( RedLed1, 0 + RedVal );

    analogWrite( GreenLed2, 0 + GreenVal );
    analogWrite( BlueLed2, 0 + BlueVal );
    analogWrite( RedLed2, 0 + RedVal );

    analogWrite( GreenLed3, 0 + GreenVal );
    analogWrite( BlueLed3, 0 + BlueVal );
    analogWrite( RedLed3, 0 + RedVal );

    analogWrite( GreenLed4, 0 + GreenVal );
    analogWrite( BlueLed4, 0 + BlueVal );
    analogWrite( RedLed4, 0 + RedVal );
    delay(5);
  }
}

void Fade() {
  int periode = 2500;         // fade speed

  int displace = 500;         // offset led intensity
  int displace2 = 1000;       // offset led intensity

  time = millis();
  value = 128 + 127 * sin(2 * PI / periode * time);
  value2 = 128 + 127 * sin(2 * PI / periode * (displace - time));
  value3 = 128 + 127 * sin(2 * PI / periode * (displace2 - time));

  analogWrite(RedLed1, value);
  analogWrite(GreenLed1, value2);
  analogWrite(BlueLed1, value3);

  analogWrite(RedLed2, value);
  analogWrite(GreenLed2, value2);
  analogWrite(BlueLed2, value3);

  analogWrite(RedLed3, value);
  analogWrite(GreenLed3, value2);
  analogWrite(BlueLed3, value3);

  analogWrite(RedLed4, value);
  analogWrite(GreenLed4, value2);
  analogWrite(BlueLed4, value3);
}

void White() {
  digitalWrite(RedLed1, LOW);
  digitalWrite(BlueLed1, LOW);
  digitalWrite(GreenLed1, LOW);

  digitalWrite(RedLed2, LOW);
  digitalWrite(BlueLed2, LOW);
  digitalWrite(GreenLed2, LOW);

  digitalWrite(RedLed3, LOW);
  digitalWrite(BlueLed3, LOW);
  digitalWrite(GreenLed3, LOW);

  digitalWrite(RedLed4, LOW);
  digitalWrite(BlueLed4, LOW);
  digitalWrite(GreenLed4, LOW);
}

I would really appreciate it if someone could help me!
Thanks!

In FadeToRed you have a new set of RedVal, GreenVal and BlueVal. Is that deliberate?

No that's not deliberate. I think a moved it to the top because i was using it somewhere else but that's not the case anymore. I deleted the unnecessary set and the code still works

When you start numbering variables, its time to start thinking arrays, instead.

  }
  if (digitalRead(45) == LOW) {
    FadeToRed();

You gave names to the switch pins. Why are you not using the names? If you are not going to use the names, why did you bother giving the pins names?

    analogWrite( GreenLed1, 0 + GreenVal );
    analogWrite( BlueLed1, 0 + BlueVal );
    analogWrite( RedLed1, 0 + RedVal );

    analogWrite( GreenLed2, 0 + GreenVal );
    analogWrite( BlueLed2, 0 + BlueVal );
    analogWrite( RedLed2, 0 + RedVal );

    analogWrite( GreenLed3, 0 + GreenVal );
    analogWrite( BlueLed3, 0 + BlueVal );
    analogWrite( RedLed3, 0 + RedVal );

    analogWrite( GreenLed4, 0 + GreenVal );
    analogWrite( BlueLed4, 0 + BlueVal );
    analogWrite( RedLed4, 0 + RedVal );

Why is it necessary to add 0 to the 3 values, 4 times?