calling a timer when a button is pushed

Question

I'm trying to call a function to keep an LED on for a certain amount of time. The function is called when the button is pushed but the LED stays on. I used the same code in the void loop before moving it into a function and it worked.

const int ledPin2 = 2;
const int buttonPin7 = 7;
const unsigned long onTime = 1000;
unsigned long previousMillis = 0;


// 
void timer(){
    unsigned long currentMillis = millis();
    digitalWrite(ledPin2, HIGH);

    if (currentMillis - previousMillis >= onTime){
        digitalWrite(ledPin2, LOW);
        previousMillis = millis();
    }
}


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

void loop(){
    int buttonState = digitalRead(buttonPin7);
    if (buttonState == HIGH) timer();
}

Answer 1

When your code:

if (currentMillis - previousMillis >= onTime){
    digitalWrite(ledPin2, LOW);
    previousMillis = millis();
}

was in the loop, it was executed multiple times, so when if (currentMillis - previousMillis >= onTime) eventually became true, it would set ledPin2 LOW again. Now that you moved it to the timer funtion, it only gets executed once (and if (currentMillis - previousMillis >= onTime) never gets a chance to become true).

You need to introduce a loop in the timer function. It also looks like you have your currentMillis / previousMillis messed up. I think you are looking for something like this:

void timer(){

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

    while((currentMillis - previousMillis) < onTime) {
        // wait for a bit
        sleep(10);
        // update currentMillis
        currentMillis = millis();
    }

    digitalWrite(ledPin2, LOW);
}

Answer 2

Your code says

if (buttonState == HIGH) timer();

So... what will it do if buttonstate is LOW? Sorry, but the answer is "nothing". You don't have any code to tell it what to do when the button is in the opposite state.

What you should do is check the button, and if it's pressed (whether that's HIGH or LOW depends on your circuit), then light the LED and start the timer (assign to previousMillis).

Then, regardless of the state of the button, check to see if currentMillis - previousMillis >= onTime. If it is, then turn off the LED.