So, I am making a project wherein I will be using an Arduino Uno. What I want to do is, whenever the switch is on, the Arduino will make the led blink. But there is a twist. The LED will start blinking after 10 seconds of the switch status becoming high. But what is happening is that the led is switched off for 10 seconds and then turns on for 0.5 seconds, then again turns off for 10 seconds. What I want it to do is, after remaining in Off condition for 10 seconds, it will keep blinking continuously.
Here's the code
const int upperSwitch=2;
int buttonState;
const int ledPin=13;
unsigned long startMillis;
unsigned long currentMillis;
const unsigned long period = 10000;
void setup()
{
pinMode(upperSwitch,INPUT);
pinMode(ledPin,OUTPUT);
startMillis=millis();
}
void loop()
{
buttonState=digitalRead(upperSwitch);
if(buttonState==HIGH)
{
delay(10000);
currentMillis = millis();
if (currentMillis - startMillis >= period)
{
digitalWrite(ledPin,HIGH);
delay(500);
digitalWrite(ledPin,LOW);
delay(500);
}
}
}
Where am I going wrong?
Your logic is flawed, and the code exhibits a number design flaws. If you press the button and release it it will enter the if(buttonState==HIGH)
section once, so flash once. If you press and hold the button (or use a toggle switch), it will re-enter the if(buttonState==HIGH)
section, but repeat the delay before flashing.
Your code lacks "state" - a means of remembering what you were doing in the previous loop()
iteration to inform what to do in the next.
The design flaws include:
Unnecessary use of global variables - it is a common bad-habit in Arduino code encouraged by numerous examples on the Arduino website. You only need globals for those variables shared between setup()
and loop()
- and even then that is not strictly necessary, but I'll let that go - Arduino code seldom gets complicated enough for that to be an issue. In general allow variables the narrowest scope necessary.
Use of delay()
in loop()
. This makes your code unresponsive and is a waste of CPU resource - doing nothing. In this case once you have started the delay, there is no means of cancelling it or doing other work until it is complete. You had the right idea with if (currentMillis - startMillis >= period)
, but rendered it pointless by preceding it with a delay and initialising startMillis
at the start of the program, rather then when the button was pressed. After the delay, currentMillis - startMillis >= period
will certainly be true
so the test serves no purpose.
The following code implements press-on/press-off toggle semantics for the button with necessary debouncing. The button state can be toggled on/off at any time (no delays where the button will not be read).
When toggled-on, the delay starts by timestamping the event, and testing time passed since the timestamp. When the delay expires, it starts flashing - timestamping each LED state toggle to effect the flash timing.
When toggled-off, none of the delay/flash code is executed so you can cancel the delay and flashing at any time. The LED is forced off in this state. It seems likely that this is the semantics you intended.
You can run a simulation of this here. I have included debug output - click the "Code" button and expand the "Serial Monitor" to see the debug output, then click "Start Simulation", and click the simulated tact switch. The simulation timing is not accurate, about 50% longer than nominal - YMMV. Debug is output when the button state is toggled and while the LED is flashing. The voltmeter I added to verify the button was wired correctly.
const int UPPER_SWITCH = 2 ;
const int LED_PIN = 13 ;
void setup()
{
pinMode( UPPER_SWITCH, INPUT ) ;
pinMode( LED_PIN, OUTPUT ) ;
}
void loop()
{
// Get initial button state
static int button_toggle_state = digitalRead( UPPER_SWITCH ) ;
// Indicator timing
static unsigned long delay_start_timestamp = 0 ;
// Get current time
unsigned long current_millis = millis();
// Toggle button state on press (press-on/press-off) with debounce
static const unsigned long DEBOUNCE_MILLIS = 20 ;
static unsigned long debounce_timestamp = 0 ;
static int previous_button_state = digitalRead( UPPER_SWITCH ) ;
int current_button_state = digitalRead( UPPER_SWITCH ) ;
if( current_millis - debounce_timestamp > DEBOUNCE_MILLIS &&
current_button_state == HIGH && previous_button_state == LOW )
{
debounce_timestamp = current_millis ;
if( button_toggle_state == LOW )
{
button_toggle_state = HIGH ;
delay_start_timestamp = current_millis ;
}
else
{
button_toggle_state = LOW ;
}
}
previous_button_state = current_button_state ;
// If button toggle state has remained HIGH for DELAY_PERIOD...
static const unsigned long DELAY_PERIOD = 10000 ;
if( button_toggle_state == HIGH &&
current_millis - delay_start_timestamp > DELAY_PERIOD)
{
// ... start flashing
static const int FLASH_PERIOD = 500 ;
static int led_state = LOW ;
static unsigned long flash_toggle_timestamp = 0 ;
if( current_millis - flash_toggle_timestamp > FLASH_PERIOD )
{
flash_toggle_timestamp = current_millis ;
led_state = led_state == HIGH ? LOW : HIGH ;
}
digitalWrite( LED_PIN, led_state ) ;
}
else
{
// LED off
digitalWrite( LED_PIN, LOW ) ;
}
}