AVR Pin Change Interrupts fire upredicably

Question

So, I am using an ATMega168 with the NerdKits set and have an LCD hooked up to it, using the two interrupts INT0 & INT1. I want to attach a Grove Heart Rate Monitor, which I've attached to PCINT1 and attempted to enable and use

void hrm_init()
{
//clear pin
  DDRB &= ~(1<<DDB0);
  PORTB |= (1<<PORTB0);
//turn on pull up
  PCICR |= (1<<PCIE0);   //enable pcint 1
  PCMSK0 |= (1<<PCINT1); //Trigger on change of PCINT1 (PB1)
  sei();
}

ISR(PCINT0_vect)
{
   uint8_t changedBits;

   changedBits = PINB ^ porthistory;
   porthistory = PINB;

   //pin has changed
   if (changedBits & (1<<PB1)) 
   {     
      beats += ((PINB & _BV(PB1)) ? 1 : 0); //add a beat on the rising edge 
   }
}

(The beat count code stolen from someone online, hence the use of the macro.)

This works, most of the time but can add extra beats here and there, I've seen as much as 10 while I have a timer interrupt as well (which I'll be using to calculate beats per minute).

void clock_init() 
{
  TCCR0A |= (1<<WGM01);
  TCCR0B |= (1<<CS02) | (1<<CS00);
  OCR0A = 143;
  TIMSK0 |= (1<<OCIE0A);
}

SIGNAL(SIG_OUTPUT_COMPARE0A) {
  the_time++;

  if (the_time > 500)
  {
    bpm = beats;
    bpm *= 12; //60 seconds divided by 5

    the_time = 0;
    beats = 0;
  }
}

Answer 1

It is likely that there is noise on the signal from the heart rate monitor. There are a few ways to deal with this. One is a hardware filter, but you can deal with it in software using a "de-bounce" or "de-glitch" mechanism.

Here is one "de-glitch" mechanism. You would not expect there to be two heartbeats within a few milliseconds of each other. So, you can reject (skip the count corresponding to) any rising edge within, say, two milliseconds of the previous rising edge.

If you have a free running timer with millisecond resolution, simply capture the timer at each rising edge. If the difference from the previous edge is more than 2 milliseconds, count the edge, otherwise don't count it. When you count it, save the timer value for the next comparison.

Use unsigned arithmetic to subtract and compare the result to two milliseconds. That way, you don't need to worry about timer wraparound.

ISR(PCINT0_vect)
{
   static uint16_t last_rising_edge = 0;
   uint8_t changedBits;

   changedBits = PINB ^ porthistory;
   porthistory = PINB;

   //pin has changed
   if (changedBits & (1<<PB1)) 
   {
      uint16_t now = get_milliseconds();

      if (0 != (PINB & _BV(PB1) && (now - last_rising_edge) >= 2u)
      {
          beats += 1; //add a beat on the rising edge
          last_rising_edge = now;
      }
   }
}