Millis() function not working

Hello everyone, hope you are doing well. I was making an LED game to test the reaction time of a person. There are two difficulties, and I am having a problem with the second difficulty. What it is supposed to do is switch on one of four LEDs randomly, over and over again. There is also a button, and the person has to press the button when the white LED switches on. I am using the millis() function as I have to switch on and off LEDs and check for a button press. But for some reason all the LEDs are switching on at the same time, and I have no idea what I am doing wrong. The code is as follows:

#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>

#define SCREEN_WIDTH 128 
#define SCREEN_HEIGHT 64 
Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &Wire, -1);

int currentLed = 2;
int triesCounter = 0;
int triesChecker = 0;
int roundNum = 1;
unsigned long interval = 200;
unsigned long interval2 = 500;
unsigned long prevTime1 = 0;
unsigned long prevTime2 = 0;
char* win1 = "You won the LED \n\n game, Difficulty 1.  \n  Congragulations!";
char* win2 = "You won the LED \n\n game, Difficulty 2.  \n  Congragulations!";
char* loss1 = "You lost the LED \n\n  game, Difficulty \n\n  1! Try again!";
char* loss2 = "You lost the LED \n\n  game, Difficulty \n\n  2! Try again!";

void blink(int blinkLed){
  digitalWrite(blinkLed, HIGH);
  delay(200);
  digitalWrite(blinkLed, LOW);
  delay(200);
  digitalWrite(blinkLed, HIGH);
  delay(200);
  digitalWrite(blinkLed, LOW);
  delay(200);
}

void displayRoundandTries(){
  display.clearDisplay();
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(0, 0);
  display.print("Round: ");
  display.print(roundNum);
  display.print("\n\nTries: ");
  display.print(triesCounter);
  display.display();
}

void displaySituation(char* situation){
  display.clearDisplay();
  display.setTextSize(1);
  display.setTextColor(WHITE);
  display.setCursor(10, 10);
  display.println(situation);
  display.display();
}

void setup() {
  Serial.begin(9600);
  if(!display.begin(SSD1306_SWITCHCAPVCC, 0x3C)) {
    Serial.println(F("SSD1306 allocation failed"));
    for(;;);
  }

  pinMode(12, INPUT);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);

  Serial.println("Do you want to play Difficulty 1 or Difficulty 2?");
  displayRoundandTries();
}

void loop(){
  /*
  while (Serial.available() == 0){
  }
  int difficulty = Serial.parseInt();
  switch(difficulty){
    case 1:
      difficulty_1();
    case 2:
      difficulty_2();
  }
  */
  difficulty_2();
}

void difficulty_1() {
  while (triesChecker == 0){
    unsigned long currentTime1 = millis();
    if (digitalRead(12) == 0){
      if (currentLed == 3){
        blink(3);
        interval -= 20;
        triesCounter = 0;
        roundNum += 1;
        if (roundNum > 9){
          displaySituation(win1);
          triesChecker = 2;
          break;
        }else{
          displayRoundandTries();
        }
      } else {
        blink(currentLed);
        triesCounter += 1;
        if (triesCounter > 3){
          displaySituation(loss1);
          triesChecker = 1;
          break;
        }else{
          displayRoundandTries();
        }
      }
    }
    digitalWrite(currentLed, HIGH);
    //Serial.println(currentTime1);
    //Serial.println(prevTime1);
    if (currentTime1 - prevTime1 >= interval){
      digitalWrite(currentLed, LOW);
      currentLed = currentLed + 1;
      if (currentLed > 5){
        currentLed = 2;
      }
      prevTime1 = currentTime1;
    }
  }
}

void difficulty_2(){
  while (triesChecker == 0){
    unsigned long currentTime2 = millis();
    int randomLED = random(2,6);
    if (digitalRead(12) == 0){
      if (randomLED == 3){
        blink(3);
        triesCounter = 0;
        roundNum += 1;
        if (roundNum > 5){
          displaySituation(win2);
          triesChecker = 2;
          break;
        } else {
          displayRoundandTries();
        }
      } else {
        blink(randomLED);
        triesCounter += 1;
        if (triesCounter > 3){
          displaySituation(loss2);
          triesChecker = 1;
          break;
        } else {
          displayRoundandTries();
        }
      }
    }
    digitalWrite(randomLED, HIGH);
    Serial.println(currentTime2);
    Serial.println(prevTime2);
    if (currentTime2 - prevTime2 >= interval2){
      digitalWrite(randomLED, LOW);
      prevTime2 = currentTime2;
    }
    /*
    delay(500);
    digitalWrite(randomLED, LOW);
    delay(500);
    */
  }
}

I am also using an OLED display to make things look nice, but I do not think that is the problem. I would greatly appreciate it if someone helps me figure out what is wrong.

The randomLED variable is declared inside the while loop. Every loop iteration, randomLED gets a new random value. Edit it like this:

void difficulty_2() {
  int randomLED = random(2, 6);  // Generate a random LED once
  while (triesChecker == 0) {
    unsigned long currentTime2 = millis();
    
    if (digitalRead(12) == 0) {
      if (randomLED == 3) {  // If the white LED (assuming it's on pin 3) is lit
        blink(3);
        triesCounter = 0;
        roundNum += 1;
        if (roundNum > 5) {
          displaySituation(win2);
          triesChecker = 2;
          break;
        } else {
          displayRoundandTries();
        }
      } else {
        blink(randomLED);
        triesCounter += 1;
        if (triesCounter > 3) {
          displaySituation(loss2);
          triesChecker = 1;
          break;
        } else {
          displayRoundandTries();
        }
      }
    }

    // Control LED timing
    if (currentTime2 - prevTime2 >= interval2) {
      digitalWrite(randomLED, LOW);  // Turn off the previous LED
      randomLED = random(2, 6);  // Get a new random LED
      digitalWrite(randomLED, HIGH); // Turn on the new LED
      prevTime2 = currentTime2;
    }
  }
}

Since blink takes much more time than interval, they will interfere with each other. It is a bit convoluted, but it looks like there is always a gap of 400ms between the prevTime* and millis()

You should re-write things so your code can handle the do-several-things-at-the-same-time problem of:

  1. blinking LEDs
  2. measuring time
  3. checking buttons

Do you have any suggestions on how to start that?

Ok let me try it

It is such a common problem, there are several tutorials and examples. But there's a bit of learning curve before it makes sense as to how it simplifies things.

Thank you, I will check it out

Have you gotten a simple button push sketch to work as intended using your exact hardware the way to have it set up?

Start small. Work your way up. Save all your steps in separate sketches as you work your way up so you can roll back to what worked last and isolate your problems. It's just easier that way.

So this is your button? It's generally agreed to declare you pin assignments with meaningful names but you don't have to.

Did you use a pulldown resistor on that button circuit? You can (and it's preferable) to use to built in pullup resistors on the Arduino (I'm assuming an Uno R3 here, you didn't say) like so:

pinMode(12, INPUT_PULLUP);

and note that the logic for that button press will now be reversed throughout your code (LOW is HIGH and vice versa).

Your suggestion is working!! Thank you so much!

Yes I have an R3, and yeah I think I should use the '#define' function for that. And yes I have resistors for each circuit element. And sure I can use the input_pullup, but I think its working out fine without. What is the difference between using input and input_pullup, apart from the fact that one starts it HIGH and the other LOW?

I did what you suggested. This is the final code:

#include <Wire.h>
#include <Adafruit_GFX.h>
#include <Adafruit_SSD1306.h>

#define buttonPin 12
#define SCREEN_WIDTH 128 
#define SCREEN_HEIGHT 64 
Adafruit_SSD1306 display(SCREEN_WIDTH, SCREEN_HEIGHT, &Wire, -1);

int currentLed = 2;
int triesCounter = 0;
int triesChecker = 0;
int roundNum = 1;
unsigned long interval = 200;
unsigned long interval2 = 500;
unsigned long prevTime1 = 0;
unsigned long prevTime2 = 0;
char* win1 = "You won the LED \n\n game, Difficulty 1.  \n  Congragulations!";
char* win2 = "You won the LED \n\n game, Difficulty 2.  \n  Congragulations!";
char* loss1 = "You lost the LED \n\n  game, Difficulty \n\n  1! Try again!";
char* loss2 = "You lost the LED \n\n  game, Difficulty \n\n  2! Try again!";

void blink(int blinkLed){
  digitalWrite(blinkLed, HIGH);
  delay(200);
  digitalWrite(blinkLed, LOW);
  delay(200);
  digitalWrite(blinkLed, HIGH);
  delay(200);
  digitalWrite(blinkLed, LOW);
  delay(200);
}

void displayRoundandTries(){
  display.clearDisplay();
  display.setTextSize(2);
  display.setTextColor(WHITE);
  display.setCursor(0, 0);
  display.print("Round: ");
  display.print(roundNum);
  display.print("\n\nTries: ");
  display.print(triesCounter);
  display.display();
}

void displaySituation(char* situation){
  display.clearDisplay();
  display.setTextSize(1);
  display.setTextColor(WHITE);
  display.setCursor(10, 10);
  display.println(situation);
  display.display();
}

void setup() {
  Serial.begin(9600);
  if(!display.begin(SSD1306_SWITCHCAPVCC, 0x3C)) {
    Serial.println(F("SSD1306 allocation failed"));
    for(;;);
  }

  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(2, OUTPUT);
  pinMode(3, OUTPUT);
  pinMode(4, OUTPUT);
  pinMode(5, OUTPUT);

  Serial.println("Do you want to play Difficulty 1 or Difficulty 2?");
  displayRoundandTries();
}

void loop(){
  
  while (Serial.available() == 0){
  }
  int difficulty = Serial.parseInt();
  switch(difficulty){
    case 1:
      difficulty_1();
    case 2:
      difficulty_2();
  }
}

void difficulty_1() {
  while (triesChecker == 0){
    unsigned long currentTime1 = millis();
    if (digitalRead(buttonPin) == 1){
      if (currentLed == 3){
        blink(3);
        interval -= 20;
        triesCounter = 0;
        roundNum += 1;
        if (roundNum > 9){
          displaySituation(win1);
          triesChecker = 2;
          break;
        }else{
          displayRoundandTries();
        }
      } else {
        blink(currentLed);
        triesCounter += 1;
        if (triesCounter > 3){
          displaySituation(loss1);
          triesChecker = 1;
          break;
        }else{
          displayRoundandTries();
        }
      }
    }
    digitalWrite(currentLed, HIGH);
    if (currentTime1 - prevTime1 >= interval){
      digitalWrite(currentLed, LOW);
      currentLed = currentLed + 1;
      if (currentLed > 5){
        currentLed = 2;
      }
      prevTime1 = currentTime1;
    }
  }
}

void difficulty_2(){
  int randomLED = random(2,6);
  while (triesChecker == 0){
    unsigned long currentTime2 = millis();
    if (digitalRead(buttonPin) == 1){
      if (randomLED == 3){
        blink(3);
        triesCounter = 0;
        roundNum += 1;
        if (roundNum > 5){
          displaySituation(win2);
          triesChecker = 2;
          break;
        } else {
          displayRoundandTries();
        }
      } else {
        blink(randomLED);
        triesCounter += 1;
        if (triesCounter > 3){
          displaySituation(loss2);
          triesChecker = 1;
          break;
        } else {
          displayRoundandTries();
        }
      }
    }
    if (currentTime2 - prevTime2 >= interval2) {
      digitalWrite(randomLED, LOW);  // Turn off the previous LED
      randomLED = random(2, 6);  // Get a new random LED
      digitalWrite(randomLED, HIGH); // Turn on the new LED
      prevTime2 = currentTime2;
    }
  }
}

See here for the use of INPUT_PULLUP

https://roboticsbackend.com/arduino-input_pullup-pinmode/

ok, thanks!

Great, if it works it works.

If you forget the external resistor in your button using INPUT, you're creating a short to the Arduino pin and could damage it.
The other reason is that using the built in pullup resistors is just easier than wiring up the external resistor, but if you're learning about all these concepts (shorting pins, pullup/pulldown resistors and so forth), adding your own is great practice for learning, too.

It really comes down to personal preference.

Thanks for helping me! I really appreciate it.

1 Like

No hazard to an input pin exists, INPUT or INPUT_PULLUP, regardless. You can wire an input to VCC or GND without fear. Just don't make that pin into an output, because THEN you have a situation where your output won't survive long.

EDIT - note the correction. To be safe for CPUs of any voltage, you must keep the input between VCC and GND. Exceeding VCC, whether 3.3 or 5V, will 'probably' cause damage.

Ah, it seems I was mistaken then, thanks.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.