Follow up question to Calling a timer when a button is pushed

Question

I have changed the code with some help here. There are two buttons each one is set to turn on a LED. When the LED turns on it is timed and when the time is up it turns off. The problem I am having is that only one LED can be on at a time even though I am not using the delay() and I am using millis() to keep track of time. So if I press button one the first LED will turn on. Even if I press button two at this time the second LED wont turn on until the first one turns off.

const int ledPin2 = 2;
const int ledPin3 = 3;    
const int buttonPin6 = 6;
const int buttonPin7 = 7;

const unsigned long onTime = 10000;
unsigned long previousMillis = 0;

void timer() {

unsigned long previousMillis = millis();
unsigned long currentMillis = millis();
digitalWrite(ledPin2, HIGH);

while ((currentMillis - previousMillis) < onTime) {

    currentMillis = millis();
}

digitalWrite(ledPin2, LOW);
}

void timer2() {

unsigned long previousMillis = millis();
unsigned long currentMillis = millis();
digitalWrite(ledPin3, HIGH);

while ((currentMillis - previousMillis) < onTime) {

    currentMillis = millis();
}

digitalWrite(ledPin3, LOW);
}


void setup() {
Serial.begin(9600);
pinMode(ledPin2, OUTPUT);
pinMode(ledPin3, OUTPUT);
pinMode(buttonPin6, INPUT);
pinMode(buttonPin7, INPUT);

}

void loop() {

int buttonState2 = digitalRead(buttonPin6);
if (buttonState2 == HIGH) timer2();

int buttonState = digitalRead(buttonPin7);
if (buttonState == HIGH) timer();


}

Answer 1

As jogco says, you are (effectively) using a delay in your timer function, by doing this:

while ((currentMillis - previousMillis) < onTime) {

    currentMillis = millis();
}

What you want is to remember the time each LED was switched on, and each every time through the loop check if the required time has passed. Something like this:

const int ledPin2 = 2;
const int ledPin3 = 3;    
const int buttonPin6 = 6;
const int buttonPin7 = 7;

const unsigned long onTime = 10000;
unsigned long led1_start = 0;
unsigned long led2_start = 0;
boolean led1_on = false;
boolean led2_on = false;

void timerOn(int led) {

    switch(led) {
        case 1:
            led1_start = millis();
            digitalWrite(ledPin2, HIGH);
            led1_on = true;
            break;
        case 2:
            led2_start = millis();
            digitalWrite(ledPin3, HIGH);
            led2_on = true;
            break;
    }
}

void timerOff(int led) {

    switch(led) {
        case 1:
            digitalWrite(ledPin2, LOW);
            led1_on = false;
            break;
        case 2:
            digitalWrite(ledPin3, LOW);
            led2_on = false;
            break;
    }
}

boolean checkTime(unsigned long startTime) {
      return ((startTime + onTime) >= millis());
}

void setup() {
    Serial.begin(9600);
    pinMode(ledPin2, OUTPUT);
    pinMode(ledPin3, OUTPUT);
    pinMode(buttonPin6, INPUT);
    pinMode(buttonPin7, INPUT);
}

void loop() {

    int buttonState2 = digitalRead(buttonPin6);
    if (buttonState2 == HIGH) timerOn(2);

    int buttonState = digitalRead(buttonPin7);
    if (buttonState == HIGH) timerOn(1);

    if (led1_on) {
        if (checkTime(led1_start)) timerOff(1);
    }

    if (led2_on) {
        if (checkTime(led2_start)) timerOff(2);
    }

}

In fact I would look at arrays for the pin numbers and status/time variables, to make it more reusable...

int ledPins[2] = {2, 3};
int ledStart[2] = {0, 0};
int buttonPins[2] = {6, 7};

... and the like

Answer 2

Yes, it is because in timer() and timer2() you busy-loop with while until the condition is not met. Hence, when buttonState2 is HIGH, your code will execute timer2() until the condition of the while loop is not met. You are effectively doing a delay.