precise RPM counter - micros()/interrupt/serial problem?

Hey guys,
I’m building an RPM counter for counting RPM of a fan at work. I'm using Arduino Duemillanove(Atmega328) and an optical sensor. The sensor senses light reflecting of a reflective sticker on one of the fan blades, it outputs a logical high(5V), and each time it shines on the sticker a logic low(0V). So my way of approaching the problem was to use interrupts to catch the logic low from the sensor. I found a lot of stuff on rpm counters using Arduino, but all of which seem to originate from this post: http://playground.arduino.cc/Main/ReadingRPM
I don’t like the idea to measure 20 or so revolutions and average them out to get a smooth number flow. I would rather measure a period of one cycle as precise as possible. So my way of tackling the problem is to measure time in interrupt routine using micros(). Each time interrupt occurs time is measured and previous time subtracted from it to get a period of one cycle. The problem is that the period time jumps from 20000us to 19500us and back at 3000RPM. This then relates to around 70RPM error. The code I wrote is below:

#define baudrate 115200

char PERCommand[] = "PER?";
char SerialCommand[10];
int SerialDelay;
byte cnt = 0;
unsigned long time[2] = {0,0};
volatile unsigned long per = 0;

void setup() {
	Serial.begin(115200);
        SerialDelay = (10000000/baudrate)+50;
	attachInterrupt(0, handler, FALLING);
}

void loop() {
  if(Serial.available() > 0) {
//    detachInterrupt(0);
    GetCommand();
    CompareAndReturn();
//    attachInterrupt(0, handler, FALLING);
  }
}

void GetCommand() {
  while(Serial.available() > 0 && cnt < 9) {
    char c = Serial.read();
    SerialCommand[cnt] = c;
    SerialCommand[cnt+1] = '\0';
    cnt++;
    delayMicroseconds(SerialDelay);
  }
  cnt = 0;
}

int CompareAndReturn() {
  if(strcmp(PERCommand, SerialCommand) == 0) {
    Serial.print(per);
  } else {  
      error();
  }
}

int error() {
  Serial.print(-1);
  while(Serial.available() > 0){
    Serial.read();
    delayMicroseconds(SerialDelay);
  }
  return(0);
}

// ISR
void handler() {
	time[1] = micros();
	per = time[0] - time[1];
	time[0] = time[1];
}

I tried to comment out GetCommand() and CompareAndReturn() functions and just putting there a Serial.print() function to return period(I was wondering if the serial communication could be the problem), I also tried detachInterrupt(), but it's just not working properly.
Is there another way to tackle the problem?
If anyone knows what the problem could be please do help.

Cheers,
Jure

The problem with the way that you do it is that it doesn't account for overhead from other ISRs (especially Timer0) or "malware" spending large amounts of time in an ISR or running for long periods with interrupts disabled. Fortunately, there is an easy out for you. Instead of calling micros(), try letting the hardware insure that the number is correct and sampled at precisely the right time. This is done using the Input Capture Facility of Timer1.

Check this out:

#include "Arduino.h"

volatile unsigned t1captured = 0;
volatile unsigned t1capval = 0;
volatile unsigned t1ovfcnt = 0;
volatile unsigned long t1time;
volatile unsigned long t1last = 0;

#define BUFFER_SIZE 32

volatile unsigned long int buffer[BUFFER_SIZE];
volatile byte head = 0;
volatile byte tail = 0;

void setup() {

  Serial.begin(9600);  

  TCCR1A = 0x0;    // put timer1 in normal mode
  TCCR1B = 0x2;    // change prescaler to divide clock by 8

  // clear any pending capture or overflow interrupts
  TIFR1 = (1<<ICF1) | (1<<TOV1);
  // Enable input capture and overflow interrupts
  TIMSK1 |= (1<<ICIE1) | (1<<TOIE1);
  
  pinMode(8, INPUT);   //  Feed the signal in here
}

void loop() {

  byte newhead;

  if(head != tail) {
    newhead = (head + 1) % BUFFER_SIZE;
    Serial.println(buffer[newhead]);
    head = newhead;
  }
}

ISR(TIMER1_OVF_vect) { 
   t1ovfcnt++;              // keep track of overflows
}

ISR(TIMER1_CAPT_vect) {
  
  unsigned long t1temp;

  // combine overflow count with capture value to create 32 bit count
  //  calculate how long it has been since the last capture
  //   stick the result in the global variable t1time in 1uS precision
  t1capval = ICR1;
  t1temp = ((unsigned long)t1ovfcnt << 16) | t1capval;
  t1time = (t1temp - t1last) >> 1;
  t1last = t1temp;
  
  tail = (tail + 1) % BUFFER_SIZE;
  buffer[tail] = t1time;
}

EDIT: Changed head and tail to bytes and changed update of head until after accessing the new time-stamp. Feed the input signal into Pin 8.

Hi JurC,
I ran into similar problems when trying to submit the data through Serial, that´s why I switched to logging to SD and transmitting the data later on after the measurement (I do not need it transmitted live).
I´m measuring about 18000 rpm x 16 (200µs period). Works ok for my purpose, but I need to try @afremont s approach, thanks for the hint!
Best, Robert

You should be able to get minimum period measurements in the vicinity of 20uS or better. I had added the circular queue to allow for a rapid succession of interrupts when I was trying to measure switch contact bounce. Since the hardware does it all, your results should be as consistent and jitter free as your signal is. Since it hijacks Timer1 and changes the prescaler, you won't be able to do analogWrite() to produce PWM on Pin 9 or Pin 10. The other PWM pins will be unaffected. If there is an overflow in the circular queue, a queue full of data (BUFFER_SIZE samples) will be dropped at a time.

Hi,

I finally got around in testing afremonts code. It worked just fine, but I still had the same issue - those cca. 500us delay from time to time. I then hooked my Arduino to function generator to test it out and the code works like a charm. So the problem was not the code but the sensor. I found in the datasheet that the sensors response time is <=0.5ms, so this is what is causing all the problems. It would not be that big of a deal if the sensor delay was always the same, but it’s not. So sometimes it takes 460us to respond, sometimes less, sometimes more, and this is making the period jump up and down. There are two solutions to the problem that I could think off. The first is to buy a sensor that is much faster (30us or less – 5RPM error at 3000RPM), the second is to count the number of rotations in a given interval – say 10s and dividing the time by number of counts – but this would only work well for “higher” frequencies, lower the frequency bigger the error.

Afremond,
correct me if I'm wrong, but I think you lose 0.5us in line where you divide t1time with two if the number stored in counter is odd. t1time = (t1temp - t1last) >> 1; You shift all the bits in t1time to the right one bit, so what happens to one (1) stored in LSB, if the number in t1time is odd?

I set the timer up to count in .5uS increments Fosc/8. I do the right shift (divide by two) to convert to pure uS because that's what I needed. You can change it if you want the extra precision.