Need help with LED problem

Hello,

I'm a freshmen ICT student and I have a bit of a problem. For my Embedded Systems class I have to create my own program with the only restriction that it has to use the serial monitor. I decided a whack-a-mole game would be both fun and challenging (for my basic arduino understanding) to make.

Now, the problem: Whenever I start up the program all the LEDs immediately turn on (despite that I even explicitly turn them off in its starting loop). Also when I start the game, it doesn't wait until the timer is done. Instead it immediately interrupts the timer as if the wrong button is pressed (after which they blink 5 times and display the score which they should WHEN the game is actually over).

It might also be note worthy that at first the LEDs didn't work at all because I had forgotten to include the LEDs in the setup. During this time the timer did function as it should; it awaited the correct period of time before ending the game. So I don't think the problem lies within the timer's code.

Anyway, here's the code of my program so that all my rambling makes a bit more sense!

//Variables & Constants
const int led1Pin = 9;
const int led2Pin = 10;
const int led3Pin = 11;
const int led4Pin = 12;
const int button1Pin = 3;
const int button2Pin = 4;
const int button3Pin = 5;
const int button4Pin = 6;
const int buttonStartPin = 7;
int led1ButtonState = digitalRead(led1Pin);
int led2ButtonState = digitalRead(led2Pin);
int led3ButtonState = digitalRead(led3Pin);
int led4ButtonState = digitalRead(led4Pin);
int startButtonState = digitalRead(buttonStartPin);
boolean start = false;
boolean skip = false;
boolean endGame = false;
int lightNumber = 0;
int buttonNumber = 0;
int time = 10000;
int score = 0;

//Functions

//This functions checks if the starting button is pressed. If so: It sets the "start" variable to true when it is released.
void startCheck()
{
  Serial.println("Check2");
  startButtonState = digitalRead(buttonStartPin);
  Serial.println("Check3");
  if(startButtonState == LOW)
  {
    while(startButtonState == LOW)
    {
      Serial.println("Knop word ingehouden");
      startButtonState = digitalRead(buttonStartPin);
    }
    
    start = true;
  }
}

//This function generates a random number (being 9, 10, 11 or 12) and turns the related LED on.
void nextLight()
{
  lightNumber = random(9,12);
  Serial.println("Check6");
  digitalWrite(lightNumber, HIGH);
}

//This function checks if either of the four 'whack' buttons are pressed for a predetermined period of time. If so: it sets the variable "buttonNumber" to the corresponding LED and interrupts the timer prematurely.
void timer()
{
  for(int i = 0; i > 20; i++)
  {
    led1ButtonState = digitalRead(led1Pin);
    led2ButtonState = digitalRead(led2Pin);
    led3ButtonState = digitalRead(led3Pin);
    led4ButtonState = digitalRead(led4Pin);
    
    if(led1ButtonState == HIGH)
    {
      while(led1ButtonState == HIGH)
      {
        led1ButtonState = digitalRead(led1Pin);
      }
      buttonNumber = 9;
      skip = true;
    }
    else
    {
      if(led2ButtonState == HIGH)
      {
        while(led2ButtonState == HIGH)
        {
          led2ButtonState = digitalRead(led2Pin);
        }
        buttonNumber = 10;
        skip = true;
      }
      else
      {
        if(led3ButtonState == HIGH)
        {
          while(led3ButtonState == HIGH)
          {
            led3ButtonState = digitalRead(led3Pin);
          }
          buttonNumber = 11;
          skip = true;
        }
        else    
        {
          if(led4ButtonState == HIGH)
          {
            while(led4ButtonState == HIGH)
            {
              led4ButtonState = digitalRead(led4Pin);
            }
            buttonNumber = 12;
            skip = true;
          }
        }
      }
    }
    
    if(skip = true)
    {
      i = 21;
    }
    
    delay(time); 
  }   
}

//This function checks if the "buttonNumber" (The, if pressed, button) and the "lightNumber" (the burning LED) variable match. 
//If so: It adds 1 to the "score" variable, displays the score on the serial monitor and decreases the given time to react in the next round
//If not: It sets the "endGame" varialble to true
//After this it resets the "buttonNumber" and "lightNumber" variable and turns down all the LEDs
void update()
{
  if(buttonNumber == lightNumber)
  {
    score++;
    Serial.println(score);
    time = time - 50;
  }
  else
  {
    endGame = true;
  }
  
  buttonNumber = 0;
  lightNumber = 0;
  digitalWrite(led1Pin, LOW);
  digitalWrite(led2Pin, LOW);
  digitalWrite(led3Pin, LOW);
  digitalWrite(led4Pin, LOW);
}

//This function simply turns all 4 LEDs on and off with a slight delay in between
void blinking()
{
  digitalWrite(led1Pin, HIGH);
  digitalWrite(led2Pin, HIGH);
  digitalWrite(led3Pin, HIGH);
  digitalWrite(led4Pin, HIGH);
  delay(300);
  digitalWrite(led1Pin, LOW);
  digitalWrite(led2Pin, LOW);
  digitalWrite(led3Pin, LOW);
  digitalWrite(led4Pin, LOW);
  delay(300);
}  

//This function checks if the "endGame" variable is set on true. If so: It displays your final score on the serial monitor, calls the "blinking" function 5 times and resets the "score" and "start" variables  
void gameOver()
{
  if(endGame == true)
  {
    Serial.println("Your final score is " + score);
    blinking();
    blinking();
    blinking();
    blinking();
    blinking();    
    score = 0;
    start = false;
  }
}

//Setup: This starts the serial monitor, sets all the Buttons to input and all the LEDs to output
void setup()
{
  Serial.begin(9600);
  pinMode(3,INPUT);
  pinMode(4,INPUT);
  pinMode(5,INPUT);
  pinMode(6,INPUT);
  pinMode(7,INPUT);
  pinMode(9,OUTPUT);
  pinMode(10,OUTPUT);
  pinMode(11,OUTPUT);
  pinMode(12,OUTPUT);
}

//Programma: First the program keeps looping until the start button is pressed. When that happens it enters a new loop where it turns on a random LED, waits (and checks for button presses) for a period of time and then checks if the crrect button was pressed
//If so: it adds 1 to the score, displays it and speeds up the game
//If not: the final score is displayed, the LEDs blink 5 times and the program re-enters it's first loop
void loop()
{
  while(start == false)
  {
    Serial.println("Check1");
    startCheck();
    digitalWrite(led1Pin, LOW);
    digitalWrite(led2Pin, LOW);
    digitalWrite(led3Pin, LOW);
    digitalWrite(led4Pin, LOW);
    Serial.println("Check4");
  }
  Serial.println("Check5");  
  while(start == true)
  {
    nextLight();
    timer();
    update();
    gameOver();
  }
}

Note: most of the serial.println you see are just checks to see if the code 'arrives' at certain points.

If anyone could explain my LEDs' weird behaviour, I would really appreciate it! And if I need to clarify anything in my code, or whatever, don't hesitate to ask!

Many thanks in advance,

-Elusive

int led1ButtonState = digitalRead(led1Pin);

At this point in the program, the pin is, by default, an input, but in setup, you change it to be an output. You need to get rid of all the magic pin numbers in setup, and think about what it is you want to achieve.

AWOL: int led1ButtonState = digitalRead(led1Pin);

At this point in the program, the pin is, by default, an input, but in setup, you change it to be an output. You need to get rid of all the magic pin numbers in setup, and think about what it is you want to achieve.

By that, do you mean throw them out of setup entirely? Because I don't really get what you mean by "magic pin numbers"

  pinMode(3,INPUT);
  pinMode(4,INPUT);
  pinMode(5,INPUT);
  pinMode(6,INPUT);
  pinMode(7,INPUT);
  pinMode(9,OUTPUT);
  pinMode(10,OUTPUT);
  pinMode(11,OUTPUT);
  pinMode(12,OUTPUT);

Magic numbers. They bear no relationship to the nice names you've defined.

pinMode(button1Pin,INPUT);
  pinMode(button2Pin,INPUT);
  pinMode(button3Pin,INPUT);
  pinMode(button4Pin,INPUT);
  pinMode(buttonStartPin,INPUT);
  pinMode(led1Pin,OUTPUT);
  pinMode(led2Pin,OUTPUT);
  pinMode(led3Pin,OUTPUT);
  pinMode(led4Pin,OUTPUT);

Better? Did not influence the problem, however. Or did I miss the point?

Or did I miss the point?

Well, you're still reading "led1pin".

int led1ButtonState = digitalRead(led1Pin);

I don't know why you'd want to do that.

Maybe cut down your sketch and simplify; start simple and work up to what it is you want to do.

AWOL: int led1ButtonState = digitalRead(led1Pin);

I don't know why you'd want to do that.

Oh wow... Didn't even see that when you pointed it out the first time. Fixed it now!

int led1ButtonState = digitalRead(button1Pin);
int led2ButtonState = digitalRead(button2Pin);
int led3ButtonState = digitalRead(button3Pin);
int led4ButtonState = digitalRead(button4Pin);
int startButtonState = digitalRead(buttonStartPin);

Program still behaves the exact same way though...

Program still behaves the exact same way though...

Well, you've got some debug prints in there. You could start looking at what they tell you, and possibly even expand on them. You're an ICT student - invent and expand!

Interesting development...

while(start == false)
  {
    Serial.println("Check1");
    startCheck();
    digitalWrite(led1Pin, LOW);
    digitalWrite(led2Pin, LOW);
    digitalWrite(led3Pin, LOW);
    digitalWrite(led4Pin, LOW);
    Serial.println("Check4");
  }

The digitalWrites should make sure the LEDs were off, yet they stayed on. So for the sake of trying everything, I changed LOW into HIGH. And against all logic they in fact went off this time...

Anyway, thanks a lot for your help and time! I think I can get a bit further now with your advice and this 'revelation'

And against all logic

No, not really. It depends on how they're wired, but you haven't told us that bit.

... and in your 'timer' function.

    led1ButtonState = digitalRead(led1Pin);
    led2ButtonState = digitalRead(led2Pin);
    led3ButtonState = digitalRead(led3Pin);
    led4ButtonState = digitalRead(led4Pin);

AWOL:

And against all logic

No, not really. It depends on how they're wired, but you haven't told us that bit.

That so? In that case I'll have to have a look at this with my partner, who did the wiring

... and in your 'timer' function.

Code: led1ButtonState = digitalRead(led1Pin); led2ButtonState = digitalRead(led2Pin); led3ButtonState = digitalRead(led3Pin); led4ButtonState = digitalRead(led4Pin);

Thank you!

... and I think these, also in 'timer', may be wrong, but you're the only one that "may" know for sure.

      buttonNumber = 9;
        buttonNumber = 10;
          buttonNumber = 11;
            buttonNumber = 12;