user20714: 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();
}


user22924: 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.

user24169: 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);
}


20714: <p>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. </p>

<pre><code>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 &gt;= 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();
}
</code></pre>
 22924: <p>Your code says</p>

<pre><code>if (buttonState == HIGH) timer();
</code></pre>

<p>So... what will it do if <code>buttonstate</code> is <code>LOW</code>? 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.</p>

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

<p>Then, <em>regardless of the state of the button</em>, check to see if <code>currentMillis - previousMillis &gt;= onTime</code>. If it is, then turn off the LED.</p>
 24169: <p>When your code:</p>

<pre><code>if (currentMillis - previousMillis &gt;= onTime){
    digitalWrite(ledPin2, LOW);
    previousMillis = millis();
}
</code></pre>

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

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

<pre><code>void timer(){

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

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

    digitalWrite(ledPin2, LOW);
}
</code></pre>
