Toggling Blink Led Program on and off

Question

My aim: To make a button that turns on and off a "Blink LED" Program. PortD only has output LED. PORTBO has input button. Microcontroller: Atmega328p

Problem: Currently when I press button case1 of switch runs so the "Blink LED" Program starts. But I cannot turn off LED with button push.

Note: I am a beginner, I think I may have to use an interrupt somehow, but not sure

#include <avr/io.h> // header file file for input output pins
#include <util/delay.h> // header file for delay.
#define DEBOUNCE_TIME 25 // time to wait while "de-bouncing" button

void init_ports_mcu()
{ /* set pin 5 of PORTB for output*/
    DDRD = 0xFF;
    PORTD = 0x00;
    PORTB |= (1 << DDB0);
}

unsigned char button_state()
{
    if( !(PINB & 0x01) )
    {
        _delay_ms( DEBOUNCE_TIME );
        if( (PINB & 0x01) ) return 1;
    }
    return 0;
}

int main( void )
{
    init_ports_mcu();
    unsigned char n_led = 1;

    while( 1 )
    {
        if( button_state() )
        {
            switch( n_led )
            {
                case 1:
                    while( 2 )
                    {
                        PORTD |= 0xFF;
                        _delay_ms( 1000 ); //delay 1 second

                        PORTD &= ~0xFF;
                        _delay_ms( 1000 );

                        if( button_state() ) // If the button is pressed break while loop
                        {
                            break;
                        }
                    }
                    break;
                case 2:
                    PORTD |= 0x00;
                    _delay_ms( 5000 ); //delay 1 second
                    n_led--; // swtiches back to case1 LED number
                    break;
            }
            //n_led ++;
        }
    }
    return 0;
}

Can't say I follow the logic, and the code formatting doesn't help, either. One thing, though, main starts with unsigned char n_led = 1; and the only time n_led changes afterwards is n_led--;. I don't see how switch(n_led) could ever reach case 2:. — dxiv
The button_state() function is an edge detector. In order to get that function to return 1, you need to press the button within the 25msec delay time. When the code is executing the while(1) loop, most of the time is spent in the _delay(DEBOUNCE_TIME) call, so button presses are recognized. But in the while(2) loop, most of the time is spent in the _delay(1000) calls, so you need to time the button press down to the millisecond. Good luck with that! The odds are 40 to 1 against you. — user3386109
Here's a fun thing to try. Remove the second _delay_ms(1000). Change DEBOUNCE_TIME to 1000. Now when you press the button, the LED will start blinking. If you press the button when the LED is on, it will keep blinking. But if you press the button when the LED is off, it will stop blinking. — user3386109

Clifford Clifford · Accepted Answer · 2020-12-01T20:12:48

There are a number of issue. Your debounce logic seems flawed, it says:

if the button is not pressed, wait 20ms and then see if it is pressed.

Better to say:

If the button state has changed, wait 25ms and then see if remains changed

unsigned char button_state()
{
    static unsigned last_state = (PINB & 0x01) ;

    unsigned new_state = (PINB & 0x01) ;
    if( last_state != new_state )
    {
        _delay_ms( DEBOUNCE_TIME ) ;

        if( (PINB & 0x01) == new_state ) last_state = new_state ;
    }

    return last_state ;
}

It is still a rather crude debounce method, but probably adequate. Using edge interrupts and a timer interrupt is a more robust solution (example pseudocode).

In your main body your biggest problem is that while you are delaying, you are processing nothing else - the delays are "busy-waits". There are no doubt other flaws in the logic, but to be frank enumerating them all is unproductive. A redesign is called for.

Without introducing library code or hardware resources not in your existing code, I would suggest a state-machine looping on a delay of 1 with a counter to keep track of time. The states are flashing/not flashing and a sub-state of LED on/LED off:

int main( void )
{
    #define FLASH_INTERVAL 1000 ;

    init_ports_mcu();
    unsigned char flashing = 0 ;
    unsigned char led_state = 0 ;
    unsigned long tick = 0 ;
    unsigned flash_time = FLASH_INTERVAL ;
    unsigned char button_down = 0 ;

    for(;;)
    {
        // Toggle indication state on button-down event
        unsigned char btn = button_state() ;
        if( !button_down && btn )
        {
            button_down = 1 ;

            // Toggle LED mode
            flashing = flashing == 0 ? 1 : 0 ;

            // Initialise the flashing starting with LED on
            flash_time = FLASH_INTERVAL ;
            led_state = flashing ;
        }
        else if( !btn )
        {
            // Button released
            button_down = 0 ;
        }

        // If flashing "ON"...
        if( flashing )
        {
            // Set the LED state
            if( led_state ) 
            {
                PORTD |= 0xFF ;
            }
            else
            {
                PORTD &= 0xFF ;
            }

            // Toggle the LED state when the flash interval has elapsed
            flash_time-- ;
            if( flash_time == 0 )
            {
                flash_time = FLASH_INTERVAL ;
                led_state = led_state ? 0 : 1 ;
            }
        }
        else // flashing off
        {
            PORTD &= ~0xFF;
        }

        // Increment tick every ms
        _delay_ms(1) ;
        tick++ ;
    }

    return 0;
}

One problem wit the above is the tick count interval will be extended by however long the loop body takes to execute. In the case of a button press or release, that will include the debounce delay, but in this case that is not an issue in particular. In real-time / time critical applications it needs to be considered.

Improvements and simplifications are possible with perhaps a timer interrupt incrementing the tick count rather than relying on the delay and the loop body execution time. Or you could use a timer interrupt to toggle the LED directly, and simply switch the timer on/off or set a flag that blocks it from toggling the LED on the button press event.

So yes you could use interrupts and timer hardware extensively to implement this, and there are many ways you might do that, but it is by no means necessary.

Toggling Blink Led Program on and off

1 Answers