next topic »

[on:]

Hello, I'm just starting out with this hobby and have tried to familiarize myself with the examples and the forum, but my programming skills are very green.  I have a question regarding using a momentary switch to turn on an LED for a specified time using the "blinkwithoutdelay" method.  I have an Arduino Uno set up with a momentary switch and resistor connected as an input to pin 6 and an LED and resistor connected as an output through a transistor (just wanted to learn how that worked) to pin 3.  What I was attempting from the code below was that upon pressing the momentary switch the LED would turn on for 5 seconds and then turn off.  What is actually happening is that the LED will blink on and off.  Once I've pressed the button and the LED has blinked I can press the button again with no blink.  I then have to wait approximately five seconds until a button press will trigger a blink.  I've also noticed that if I press and hold the momentary switch what I get is a blink then approximately a 5 second wait and then another blink.  May be coincidental?  I've tried many different variations with if, if/else, and while, but I'm stumped. Any help would be appreciated!

Code:

/* Turns on an LED for 5 seconds when a momentary pushbutton is pressed.
 
 The circuit:
 *pin 3 connected to LED through transistor
 *pin 6 connected to pushbutton/resistor */

const int buttonPin = 6;        //the number of the pushbutton pin
const int LEDPin =  3;          //the number of the alarm pin
int buttonState;            //variable for the pushbutton status
unsigned long ontime = 5000;    //variable for the alarm on-time
unsigned long currentMillis;
long previousMillis = 0;

void setup() {
  pinMode(LEDPin, OUTPUT);       //initialize the LED pin as an output
  pinMode(buttonPin, INPUT);     //initialize the pushbutton pin as an input
  Serial.begin(9600);            //initialize serial communication for debugging
}

void loop() {
    
    buttonState = digitalRead(buttonPin);   //check if the pushbutton is pressed
    Serial.println(buttonState);            //print pushbutton condition to serial monitor
    currentMillis = millis();
    if (buttonState == HIGH) {                            //if pushbutton has been pressed:      
       if (currentMillis - previousMillis > ontime) {     //check if difference between current time        
         digitalWrite(LEDPin, HIGH);                      //and previous time is larger than the alarm time
         previousMillis = currentMillis;        
       }
    }
    digitalWrite(LEDPin, LOW);              //turn LED off
}

Eventually I would like to add an on/off switch that could be used to arm/disarm the system and also turn off the LED prior to the 5 second time limit if desired, but we must crawl before we walk. If anyone knows of any examples of how to do this (or something similar to this) I would gladly welcome a link.

[Reply #1 on:]

Code:

  pinMode(buttonPin, INPUT);     //initialize the pushbutton pin as an input

But, you don't enable the pullup resistor. So, you have an external resistor, right? How is the switch wired?

Code:

    buttonState = digitalRead(buttonPin);   //check if the pushbutton is pressed
    Serial.println(buttonState);            //print pushbutton condition to serial monitor
    currentMillis = millis();
    if (buttonState == HIGH) {                            //if pushbutton has been pressed:     
       if (currentMillis - previousMillis > ontime) {     //check if difference between current time       
         digitalWrite(LEDPin, HIGH);                      //and previous time is larger than the alarm time
         previousMillis = currentMillis;       
       }
    }
    digitalWrite(LEDPin, LOW);              //turn LED off

Apparently, you didn't fully understand the blink without delay example.

If the switch is HIGH (or LOW if you wire the switch the easy way), turn the LED and and set a variable (with a meaningful name, like startTime; currentMillis is not such a name).

Then, periodically see if the LED is still on (startTime is not zero) and enough time has passed (now - startTime is greater than the on time you want), turn the LED off and set startTime to 0.

[Reply #2 on:]

This results in turning the LED off immediately after turning it on:

Code:

    buttonState = digitalRead(buttonPin);   //check if the pushbutton is pressed
    Serial.println(buttonState);            //print pushbutton condition to serial monitor
    currentMillis = millis();
    if (buttonState == HIGH) {                            //if pushbutton has been pressed:     
       if (currentMillis - previousMillis > ontime) {     //check if difference between current time       
         digitalWrite(LEDPin, HIGH);                      //and previous time is larger than the alarm time
         previousMillis = currentMillis;       
       }
    }
    digitalWrite(LEDPin, LOW);              //turn LED off

Note that you say "if button is high, set led pin high;" followed by "always set led pin low".  That "always" part (digitalWrite(LEDPin, LOW)) happens on every single loop.  Consider the following instead:

Code:

unsigned int debounceTime = 100;
unsigned long          onTime = 5000;


...


void loop() {

  static bool                     ledOn = false;
  static unsigned long buttonTm = 0;
  static unsigned long      ledTm = 0;

    // always debounce your inputs, but debouncing uses a different time counter
    // than your LED on, necessarily, as you want to be able to press the button
    // more than once every on period

  if( digitalRead(buttonPin) == HIGH && millis() - buttonTm > debounceTime ) {

    buttonTm = millis();

    if( ledOn ) {
        // LED is already on, turn it off
      digitalWrite(LEDPin, LOW);
      ledOn = false;
    }
    else {
       // LED is not on, turn it on
      digitalWrite(LEDPin, HIGH);
      ledOn = true;
      ledTm = millis();
   }
 }

 if( ledOn == true && millis() - ledTm >= onTime ) {
     // LED has been on long enough to turn off automatically
   digitalWrite(LEDPin, LOW);
   ledOn = false;
 }
}

Hope that helps.

 
   
 

[Reply #3 on:]

Code:

    if( ledOn ) {
        // LED is already on, turn it off
      digitalWrite(LEDPin, LOW);
      ledOn = false;
    }
    else {
       // LED is not on, turn it on
      digitalWrite(LEDPin, HIGH);
      ledOn = true;
      ledTm = millis();
   }

@drone
I have a problem with this code. The ledOn variable should, in my opinion, be of type int, and it should be assigned/compared to HIGH and LOW. Why? Because then you can do:

Code:

    ledOn = HIGH;
    digitalWrite(LEDPin, lodOn);

This assures that the state of the LED and the value of ledOn never get out of sync.

I know that you think it won't happen, but it does. A month later, you make a change to the digitalWrite statement and forget to change the following statement, and you spend a long time trying to figure out what went wrong with such a simple change.

[Reply #4 on:]

Easy:

1) at the time you trigger that output, start a timer to count time.
2) in the timer isr, compare elapsed time vs. desired time.
3) when the desired time has elapsed, turn off the output.

[Reply #5 on:]

1) at the time you trigger that output, start a timer to count time.

That is hardly easier than calling millis().

2) in the timer isr, compare elapsed time vs. desired time.

So, now you need another function, instead of the same simple if statement in loop().

3) when the desired time has elapsed, turn off the output.

Which, of course, you could do in loop() with sacrificing a timer.

[Reply #6 on:]

Code:

    ledOn = HIGH;
    digitalWrite(LEDPin, lodOn);

This assures that the state of the LED and the value of ledOn never get out of sync.

I know that you think it won't happen, but it does. A month later, you make a change to the digitalWrite statement and forget to change the following statement, and you spend a long time trying to figure out what went wrong with such a simple change.

Stranger things have happened!  A good point.  In general, I would isolate the LED code out of the loop too, unless I didn't expect that I'd do much more in my sketch.  I hate seeing long loop/main functions.

[Reply #7 on:]

This is something that I would do:

Code:

unsigned long _timer1_period=0;  //timer period, in seconds
unsigned long _timer1_counter=0;  //timer1 counter
//timer1 isr
ISR(TIMER1_OVF_vect) {
  _timer1_counter+=0x10000ul;   //increment timer_counter
  if (_timer1_counter >=F_CPU) {
    _timer1_counter -= F_CPU;    //reset timer_counter
    _timer1_period -= 1;            //decrement duration_sec
    if (!_timer1_period) {        //elapsed time has been reached
      //turn off the led
      digitalWrite(LEDPin, LOW);
      TIMSK1 &=~(1<<TOIE1);      //turn off timer1 interrupt
    }
  }
}

//initialize the timer
//to expire in user-specified period of time
void tmr1_init(unsigned long duration_sec) {
  _timer1_counter = 0;            //reset timer1 counter
  _timer1_period=duration_sec;    //set timer period
  TCCR1B = 0x00;                //turn off the timer
  TCCR1A = 0x00;                //normal portion operation
  TCNT1 = 0;                    //reset the timer
  TIFR1 |= (1<<TOV1);            //clear the timer flag
  TCCR1B = 0x01;                //turn on the timer, 1:1 prescaler
  TIMSK1 |= (1<<TOIE1);        //enable the timer interrupt
  sei();                      //enable global interrupt
}

The isr keeps track of time elapsed and at the end, turns itself off. The other routine just initialize the timer. Once it is set, it goes on by itself.

Here would be  your user code:

Code:

    if (buttonState==HIGH) {
      digitalWrite(LEDPin, HIGH);          //turn on the led pin
      tmr1_init(5);                       //initialize the timer to elapse in 5 seconds
    }

As the routines are set-and-forget type, your user code is greatly simplified: all it does is to set up the timer and then it moves on.

[Reply #8 on:]

This is something that I would do:

Maybe that's what you'd do, but IMO using a hardware timer for such a simple problem is overkill, excessively complex and unscalable.

The 'blink without delay' shows how to implement this sort of thing in a very simple and scalable way that also fits in well with handling other events asynchronously.