More Timing with interrupts

This is a follow-on to the thread on timing with a external interrupt and a square wave. Here's what I've got:

INPUT - IR pulse from Andromeda Space Rocker -> simple detector -> pin 2

DESIRED OUTPUT:

  • Three pluse trains, using three out pins (Atm, 11-13), simple LED out.
  • A - same rate as input
  • B - twice the rate
  • C - three times the rate.

I used the example from a previous discussion, where the ISR sets
a flag, and the main loop does the work.
Current plan-

  • Interrupt sets a flag
  • if (flag) get time from millis(), subtract from previous
  • keep running average for smoothness

This code more-or-less works, but I'd really like to smarten it up a bit.
Also, I get a compile error unless CheckAndClearFlag is defined before being called, which I don't understand..
Submitted for your disapproval....

// sketch to receive an input clock signal on pin 2
// determine average clock rate
// blink LEDs on 1/3, 1/2 note 
byte CheckAndClearFlag( volatile byte& Flag )
{
  byte Result;
  
  uint8_t SaveSREG = SREG;
  cli();
  Result = Flag;
  Flag = false;
  SREG = SaveSREG;
  
  return( Result );
}
volatile byte clockFlag = false;  //Interrupt flag
unsigned clockCount = 0; //interrupt counter

long prevbase=0;
long prev2 = 0;
long prev4 = 0;
long curtime = 0;
int arprate = 300;
long mtime = 0;
long lastime = 0;
long deltatime = 0; 
long count = 0;
long tottime = 1;

int sensePin = 2;


void setup() {
  pinMode(13,OUTPUT);
  pinMode(12,OUTPUT);
  pinMode(11,OUTPUT);
  
  pinMode(sensePin, INPUT);
  attachInterrupt(0,clock,RISING);
}

void loop() {
   curtime = millis();
   if (curtime - prevbase >= arprate ) {   //everybody trips on the beat
    prevbase = curtime;   
    digitalWrite(13, HIGH);  
    digitalWrite(12, HIGH);  
    digitalWrite(11, HIGH);  
    delay(5);
    digitalWrite(13, LOW);
    digitalWrite(12, LOW);
    digitalWrite(11, LOW);    
    
  } else if (curtime - prev2 >= (arprate/2) ) { // 2x the beat
    prev2 = curtime;   
    digitalWrite(12, HIGH);  
    delay(5);
    digitalWrite(12, LOW);
    
  } else if (curtime - prev4 >= (arprate/3) ) {
    prev4 = curtime;   
    digitalWrite(11, HIGH);  
    delay(5);
    digitalWrite(11, LOW);
  }
  
  // Figure out the average time. 
  // Only redo our clock if we have an interrupt
  if ( CheckAndClearFlag( clockFlag )){  //Only need to go through if a new interrupt has been triggered
    delay(2);  //Debounce interrupt input
    uint8_t SaveSREG = SREG;
    cli();
    clockFlag = false;
    SREG = SaveSREG;
    // the delta between now, and the last time we hit interrupt + loop time.
    mtime = millis();
    deltatime = mtime - lastime;
    lastime = mtime;
    
     // Do the average
     tottime = tottime + deltatime;
     count++;
      arprate = tottime/count;
  
      constrain(arprate, 200, 2000);  // 200 = 300 bpm, 2000 = 30 bpm
      // Clear our totals so tottime does not overflow
      if (count > 3) {
        count = 0;
        tottime = arprate;
        }  
 }
}
void clock(){
  clockFlag = true; //Interrupt routine
}

Also, I get a compile error unless CheckAndClearFlag is defined before being called, which I don't understand..

It's probably the reference (&) in the declaration. Using a pointer (*) instead would probably get the problem to go away.

However, because of the way clockFlag is used and because it is a single byte, it is safe to remove CheckAndClearFlag. There are some details here...
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1269212782/16#16

If you really do need to debounce the input, clearing the flag should be done at the bottom of the if...

  if ( clockFlag )  /* remove ( CheckAndClearFlag( clockFlag )){  //Only need to go through if a new interrupt has been triggered */
  {
    delay(2);  //Debounce interrupt input
/* remove
    uint8_t SaveSREG = SREG;
    cli();
    clockFlag = false;
    SREG = SaveSREG;
*/
    // the rest of the stuff in the then-clause

    /* Put this at the bottom of the then-clause */
    clockFlag = false;
  }