0

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??enter image description here

Where am I going wrong?

  • 4
    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 Apr 16 '21 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 Apr 16 '21 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 Apr 16 '21 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" https://www.arduino.cc/en/Tutorial/Foundations/DigitalPins). – lurker Apr 16 '21 at 13:22
  • @lurker thanks. But my code is giving me a hard time. Can you please help me with that? – Advait Dandekar Apr 16 '21 at 13:25
  • you mixed the examples Blink and BlinkWithoutDelay – Juraj Apr 16 '21 at 13:27
  • Please read the link I provided. Also explain exactly what it's doing now as opposed to what you intend – lurker Apr 16 '21 at 13:32
  • @lurker I read your notes, they were useful. I have not connected the led to digital pin 8 of Arduino. What the code is currently doing is this: When the push button is pressed, as per the program, the led stays in off condition for 10 seconds, after that, it lights up for half a second, then again stays off for 10 seconds. What I want it to do is this: After staying off for the first 10 seconds, it should start blinking with a delay of 500ms in a loop – Advait Dandekar Apr 16 '21 at 13:42
  • The way your code is written the blink code only runs whist the button state is high. It will probably work if you press and continue to hold the button. Otherwise, you ne d to get the blink code outside of that high button condition. You probably need a state variable that indicates that the button was previously pressed. – lurker Apr 16 '21 at 14:51
  • I do press and continue to hold the button, but it still does not do what I want it to do – Advait Dandekar Apr 16 '21 at 15:03
  • The way the logic is written, when you hold the button, it will also pause 10 seconds before each on/off cycle. My prior comment says what you need to o do to achieve your goal. – lurker Apr 16 '21 at 15:09
  • 1
    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 Apr 16 '21 at 18:22
  • @Lundin The circuit is perfectly working. The resistor is a pull-down. You might not be aware that such micro switches have connected pins, in the picture vertically. When operated, the left and right sides are making contact. – the busybee Apr 16 '21 at 19:12
  • @thebusybee Yeah I realized that the picture doesn't really tell which 2 pins that are connected. It might be upper-left and lower left, or it might be upper-left and upper-right. Perhaps there is a de facto standard that those who are in line with the direction of the legs are the connected ones? – Lundin Apr 19 '21 at 06:39

1 Answers1

2

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 ) ;
    }
}
Clifford
  • 88,407
  • 13
  • 85
  • 165
  • 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. – Advait Dandekar Apr 17 '21 at 04:47
  • 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. – Advait Dandekar Apr 17 '21 at 05:01