Search code examples
carduinointerruptatmegaled

c arduino button interrupts (isr), make multiple leds go on and off


I've been trying to make my LEDs on my Arduino go on and off with the corresponding button press.

I'm using interrupts to make it happen and the button press does get registered, but for some reason it doesn't change the global variable's value(int button_pressed1,...);

What's supposed to happen is that when I press button 1, Led 1 is supposed to go on and off, same with button 2 and button 3.

I really appreciate you taking a look, interrupts are pretty new to me so it might be a minor overlook. <3

*I left out the code for button2 and 3. If I can make the LEDs turn on on button 1, I'll be able to make them turn on for the others.

#include <util/delay.h>
#include <avr/io.h>
#include <avr/interrupt.h>
#include "usart.h"

#define LED_DDR DDRB
#define LED_PORT PORTB

#define BUTTON_DDR DDRC
#define BUTTON_PORT PORTC
#define BUTTON_PIN PINC

int button_pressed1 = 0; //globale variabele to turn on functions

ISR(PCINT1_vect)
{
    if (bit_is_clear(BUTTON_PIN, PC1))
    {                
        _delay_us(500); //debounce
        if (bit_is_clear(BUTTON_PIN, PC1))
        {

            button_pressed1 = 1;
            printf("button 1 pressed\n");
        }
    }
}

int main()
{
    LED_DDR |= _BV(PB2) | _BV(PB3) | _BV(PB4); //registrer pins  output(bit = 1)
    LED_PORT |= _BV(PB2) | _BV(PB3) | _BV(PB4);

    BUTTON_DDR &= ~_BV(PC1) & ~_BV(PC2) & ~_BV(PC3); //registrer inputs(bit = 0)
    BUTTON_PORT |= _BV(PC1) | _BV(PC2) | _BV(PC3);   // pull up ( bit =1 )

    PCICR |= _BV(PCIE1);                      //type pins doorgeven
    PCMSK1 |= _BV(PC1) | _BV(PC2) | _BV(PC3); //pin/button doorgeven aan change mask

    initUSART(); 

    sei();

    while (1)
    { //infinte loop
        if (button_pressed1 == 1)
        {
            LED_PORT &= ~_BV(PB2); //turn led on
            _delay_ms(500);
            LED_PORT |= _BV(PB2); //turn led off
            _delay_ms(500);
        }
    }

    return 0;
}

Solution

  • A couple of fundamental problems:

    • All variables shared with an ISR should be declared volatile and have protection against race conditions. See this for details.
    • You should not have busy-delays inside an ISR. Instead you should setup the timer interrupt to trigger again, within a certain time period. Generally, it is hard to use GPIO interrupts for buttons specifically, polling from a cyclic timer interrupt is preferable. You can use interrupts, but it is rather intricate, details here.
    • 500us isn't likely sufficient time for a de-bounce. Mechanical signal bounces are relatively slow. It is more common to wait ~10ms or so. You can quite easily measure the bounce characteristics with a scope, by adding supply to the button, then press it and capture the edge. It will look like a sinusoidal and you easily measure for how long it will last.
    • The 500ms delays in your main loop will get experienced as "lag" by the user. They might start to suspect a broken button. You might want to at least skip the turn off delay.