atmega328p button press via interrupt not working

Question

I'm trying to control a servo using two buttons connected to a atmega328p. The two buttons place the servo horn in two different positions. The servo is a SG90 micro servo.

I'm trying to use interrupts to detect the button press, but I can't get it working:

#include <stddef.h>
#include <avr/io.h>
#include <avr/interrupt.h>
#define PWM_MIN 1200
#define PWM_MID 3000
#define PWM_MAX 5000
#define SERVO_PIN PB1
#define LOCK_BTN PD6
#define UNLOCK_BTN PD7
static inline void servo_init(void)
{
 DDRB |= 1 << SERVO_PIN;
 TCCR1A |= (1 << WGM11) | (1 << COM1A1);
 TCCR1B |= (1 << WGM12) | (1 << WGM13) | (1 << CS11);
 ICR1 = 40000;
 DDRD &= ~((1 << LOCK_BTN) | (1 << UNLOCK_BTN));
 PORTD |= (1 << LOCK_BTN) | (1 << UNLOCK_BTN);
}
int main(void) 
{
 servo_init();
 EICRA |= (1 << ISC00);
 EIMSK |= (1 << INT0);
 sei();
 for(;;)
 ;
 return 0;
}
ISR (INT0_vect)
{
 if (!((PIND >> LOCK_BTN) & 0x01))
 OCR1A = PWM_MID;
 if (!((PIND >> UNLOCK_BTN) & 0x01))
 OCR1A = PWM_MIN;
}

If I change the main function to do this without interrupts, it's working fine. So, something like the following works:

#include <stddef.h>
#include <avr/io.h>
#include <util/delay.h>
#define PWM_MIN 1200
#define PWM_MID 3000
#define PWM_MAX 5000
#define SERVO_PIN PB1
#define LOCK_BTN PD6
#define UNLOCK_BTN PD7
static inline void servo_init(void)
{
 DDRB |= 1 << SERVO_PIN;
 TCCR1A |= (1 << WGM11) | (1 << COM1A1);
 TCCR1B |= (1 << WGM12) | (1 << WGM13) | (1 << CS11);
 ICR1 = 40000;
 DDRD &= ~((1 << LOCK_BTN) | (1 << UNLOCK_BTN));
 PORTD |= (1 << LOCK_BTN) | (1 << UNLOCK_BTN);
}
static inline uint8_t is_btn_pressed(uint8_t btn)
{
 return !((PIND >> btn) & 0x01);
}
int main(void) 
{
 servo_init();
 for(;;) {
 if (is_btn_pressed(LOCK_BTN))
 OCR1A = PWM_MID;
 if (is_btn_pressed(UNLOCK_BTN))
 OCR1A = PWM_MIN;
 }
 return 0;
}

• Lundin: I don't see how either of it could possibly work as there is no "debouncing" algorithm present. Which is embedded systems 101. Generally we always avoid GPIO interrupts for buttons but instead have a cyclic timer polling the GPIO port every n ms.
• Lundin: Also how does PORTD |= (1 << LOCK_BTN) | (1 << UNLOCK_BTN); make sense for an input port?
• kovac: The PORTD |= (1 << LOCK_BTN) | (1 << UNLOCK_BTN); is for pull-ups. The buttons connect the GPIO lines to GND.
• Velvet: You could use the external interrupt to wake up - and evaluate your button states in the main context.
• G36: Isn't INT0 on pin PD2?

Answer 1

You have buttons on PD6 and PD7.

You enable INT0 which is on PD2.

So neither button will generate INT0.

Answer 2

I couldn't get it working with int0 interrupt. But managed to get it working with pin change interrupt. Both interrupts seem to be operational for the sleep modes I'm interested in, so this works for me. Still, I'd be interested in learning why my initial approach didn't work.

#include <stddef.h>
#include <avr/io.h>
#include <avr/interrupt.h>
#define PWM_MIN 1200
#define PWM_MID 3000
#define PWM_MAX 5000
#define SERVO_PIN PB1
#define LOCK_BTN PD6
#define UNLOCK_BTN PD7
static inline void servo_init(void)
{
 DDRB |= 1 << SERVO_PIN;
 TCCR1A |= (1 << WGM11) | (1 << COM1A1);
 TCCR1B |= (1 << WGM12) | (1 << WGM13) | (1 << CS11);
 ICR1 = 40000;
 DDRD &= ~((1 << LOCK_BTN) | (1 << UNLOCK_BTN));
 PORTD |= (1 << LOCK_BTN) | (1 << UNLOCK_BTN);
}
static inline void pcint2_init(void)
{
 PCICR |= (1 << PCIE2);
 PCMSK2 |= ((1 << PCINT22) | (1 << PCINT23));
}
static inline uint8_t is_btn_pressed(uint8_t btn)
{
 return !((PIND >> btn) & 0x01);
}
int main(void) 
{
 servo_init();
 pcint2_init();
 sei();
 for(;;)
 ;
 return 0;
}
ISR(PCINT2_vect)
{
 if (is_btn_pressed(LOCK_BTN))
 OCR1A = PWM_MID;
 if (is_btn_pressed(UNLOCK_BTN))
 OCR1A = PWM_MIN;
}