Arduino Project

Question

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??

Where am I going wrong?

Please don't mess around with hobbyist boards on real bikes, it's very dangerous and illegal in most countries. The electronics on the bike will have gone through extensive type approval and EMC testing, your hobbyist board has not. — Lundin
– Lundin, Commented Apr 16, 2021 at 13:03
"Where am I going wrong?" You seem to have connected the wire to the wrong pin of the tactile switch... Also you have connected the pull resistor on the wrong side of the switch. — Lundin
– Lundin, Commented Apr 16, 2021 at 13:09
Only if you want the button to work. Use the "beep" on a multimeter to verify your circuit before plugging anything in. Anyway there's no telling from your picture of wires which poles that belong together. — Lundin
– Lundin, Commented Apr 16, 2021 at 13:13
I see you're using pin 13, which is also used internally by the Arduino, so you must use with care. See [Arduino digital pins]("Digital Pins | Arduino" arduino.cc/en/Tutorial/Foundations/DigitalPins). — lurker
– lurker, Commented Apr 16, 2021 at 13:22
I have no idea what "two-wheeler" might refer to or why it is even relevant to this code or circuit. Since it is clearly irrelevant to your question I would omit all reference to it. It does not matter ultimately what your final application is, and it is just distracting. You could also do with a clearer, simpler description of a) what it should do, b) what it actually does - in the question, not in the comments. That is to say, remove the waffle, get to the point, be precise. Don't confuse the issue with 5 minutes vs 10 seconds nonsense - just describe what this test code must do. — Clifford
– Clifford, Commented Apr 16, 2021 at 18:22

Clifford · Accepted Answer · 2021-04-16 21:06:00Z

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

Thanks a lot, @Clifford. I learned so much through this answer. Also, the simulation runs fantastically. I now get your point. delay and millis() don't go together. Thanks a lot once again.
Also, this is exactly what I wanted. I am new to Arduino and don't understand all aspects of its programming. I have done a few projects which did not require anything like this. This is the first time I have done something like this. Thanks a lot once again.

Collectives™ on Stack Overflow

Arduino Project

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related