Fast Pulse Counter - High Speed Counter

Hello,

I have some code which was derived from another member some time ago, which I am looking at again and wanting to improve.

I have a PCB with a 328P on it, which is dedicated to counting the time between two consecutive pulses and estimating the number of pulses there would be in a second. The answer is then requested from another uC every 300ms or so, transmitted over I2C.

The code I have uses Timer0 with a prescaler of 64.
From what I understand of it, each pulse that comes in, it takes a record of Timer0 in the ISR, and then compares the time to the next pulse that comes in. It then does a calculation and determines how many pulses there would be in a second.

I am wanting this code to count as fast and as accurately as possible, and I need some assistance in doing that.

Should I change this over to use Timer1 instead of Timer0, to get a higher resolution, and change the Prescaler over to /8 instead of /64 too?
Do you think this will help? I was thinking about putting on a 20Mhz crystal instead of the 16Mhz one, to help things along too.
The code handles 2 inputs, attached to INT0 and INT1.

If you can please assist with improvements to the counting code (not the I2C stuff), I would appreciate it. The I2C stuff is still being worked on.

Attached to a function generator, I can read up to 43kHZ or there abouts, however the resolution up there is extremely poor, it is +/-1000Hz territory. When under 1000Hz, the accuracy is rather good, and is generally within 1Hz.

Help would be appreciated.

Code next post (this one is getting too long)

Regards
James

//This Micro handles the counting of the pulses

#include <Wire.h>
#include <EEPROM.h>

extern volatile unsigned long timer0_overflow_count;  // Record the most recent timer ticks
volatile boolean INPUT1_ticks_valid;                  // Indicates a valid reading
volatile unsigned long INPUT1_ticks_per_rev;          // Number of ticks counted in the current revolution
volatile unsigned long INPUT1_ticks_now;
float lastINPUT1;                            // Most recent INPUT1 reading

volatile boolean INPUT2_ticks_valid;                  // Indicates a valid reading
volatile unsigned long INPUT2_ticks_per_rev;          // Number of ticks counted in the current revolution
volatile unsigned long INPUT2_ticks_now;
float lastINPUT2;                            // Most recent INPUT2 reading

const float TICKS_TO_PPS = 250000;                    // Convert clock ticks to PPS by dividng ticks into this number
                                                      // 16000000Mhz / 64 Prescaler = 4uS (Timer0 resolution)
                                                      // 1000*1000/4us = 250000 

volatile byte DataForMaster[8];
volatile int Address;   

byte* INPUT1FloatPtr;
byte* INPUT2FloatPtr;

//Command from Master
volatile byte CMD;

//Board information for Master to read (Trial)
volatile int mySerial = 12347;
volatile byte myModel[20] = "High Speed - Rev 01";

void setup()
{
  attachInterrupt(0, INPUT1_ISR, RISING);             // INPUT1 sense will cause an interrupt on pin2 
  attachInterrupt(1, INPUT2_ISR, RISING);             // INPUT2 sense will cause an interrupt on pin3  
  lastINPUT1 = 0;                                     // Current INPUT1 to zero
  lastINPUT2 = 0;                                     // Current INPUT2 to zero
    
  Address = 119; //Address that the Master automatically assigns a new address too (like DHCP);  
  if((int)EEPROM.read(0) < 119)
  {
    Address = (int)EEPROM.read(0); 
  }
  
  Wire.begin(Address);          // Address
  TWAR = (Address << 1) | 1;    // enable broadcasts to be received 
  Wire.onRequest(requestEvent); // register event
  Wire.onReceive(receiveEvent); // received event
}                                                   

void loop()
{
  // Calculate INPUT1
  if (INPUT1_ticks_valid)                               // Only come here if we have a valid INPUT1 reading
  {                            
    unsigned long INPUT1_thisTicks;
    
    noInterrupts();                             	// Disable interrupts so the next calculation isnt changed half way through
    INPUT1_thisTicks = INPUT1_ticks_per_rev;
    INPUT1_ticks_valid = false;
    interrupts();                               	// Enable interrupts again
    
    lastINPUT1 = TICKS_TO_PPS / INPUT1_thisTicks;       // Convert ticks to PPS
    INPUT1_ticks_valid = false;                         // Reset the flag.
  } 
 
  // Calculate INPUT2
  if (INPUT2_ticks_valid)                               // Only come here if we have a valid INPUT1 reading
  {                            
    unsigned long INPUT2_thisTicks;
    
    noInterrupts();                             	// Disable interrupts so the next calculation isnt changed half way through
    INPUT2_thisTicks = INPUT2_ticks_per_rev;
    INPUT2_ticks_valid = false;
    interrupts();                               	// Enable interrupts again
    
    lastINPUT2 = TICKS_TO_PPS / INPUT2_thisTicks;       // Convert ticks to PPS
    INPUT2_ticks_valid = false;                         // Reset the flag.
  }
  
  //Convert floats into bytes to send over Wire
  INPUT1FloatPtr = (byte*) &lastINPUT1;
  INPUT2FloatPtr = (byte*) &lastINPUT2;
  DataForMaster[0] = INPUT1FloatPtr[0]; 
  DataForMaster[1] = INPUT1FloatPtr[1]; 
  DataForMaster[2] = INPUT1FloatPtr[2]; 
  DataForMaster[3] = INPUT1FloatPtr[3]; 
  DataForMaster[4] = INPUT2FloatPtr[0];
  DataForMaster[5] = INPUT2FloatPtr[1];
  DataForMaster[6] = INPUT2FloatPtr[2];
  DataForMaster[7] = INPUT2FloatPtr[3];  
}

void INPUT1_ISR()
{
  static unsigned long INPUT1_pre_count;                   // Last timer0_overflow_count value
  INPUT1_ticks_now = timer0_overflow_count;                // Read the timer over count (number of times timer0 has overflowed since startup)

  byte INPUT1_t = TCNT0;                                   // Copy the current value of Timer0
  if ((TIFR0 & _BV(TOV0)) && (INPUT1_t<255))               // If the Timer has overflowed
    INPUT1_ticks_now++;                                    // Increment the number of Ticks (1024uS resolution)
  INPUT1_ticks_now = (INPUT1_ticks_now << 8) + INPUT1_t;         // Bit shift Ticks by 8 bits and add Timer0 current value, to make ticks_now 4uS resolution now

  if (INPUT1_pre_count == 0)                               // If this was the first time around the loop
  {                         
    INPUT1_pre_count = INPUT1_ticks_now;                   // Set the precount to the current count, don't use this number.
  } 
  else 
  {
    INPUT1_ticks_per_rev = INPUT1_ticks_now - INPUT1_pre_count;  // Calculate the difference to get the actual number of ticks...
    INPUT1_pre_count = INPUT1_ticks_now;                      // reset the counter with the current count value
    INPUT1_ticks_valid = true;                             // and flag the change up
  }
}

void INPUT2_ISR()
{
  static unsigned long INPUT2_pre_count;            // Last timer0_overflow_count value
  INPUT2_ticks_now = timer0_overflow_count;         // Read the timer over count (number of times timer0 has overflowed since startup)

  byte INPUT2_t = TCNT0;                                 // Copy the current value of Timer0
  if ((TIFR0 & _BV(TOV0)) && (INPUT2_t<255))             // If the Timer has overflowed
    INPUT2_ticks_now++;                                  // Increment the number of Ticks (1024uS resolution)
  INPUT2_ticks_now = (INPUT2_ticks_now << 8) + INPUT2_t;     // Bit shift Ticks by 8 bits and add Timer0 current value, to make ticks_now 4uS resolution now

  if (INPUT2_pre_count == 0)                             // If this was the first time around the loop
  {                         
    INPUT2_pre_count = INPUT2_ticks_now;                   // Set the precount to the current count, don't use this number.
  } 
  else 
  {
    INPUT2_ticks_per_rev = INPUT2_ticks_now - INPUT2_pre_count;     // Calculate the difference to get the actual number of ticks...
    INPUT2_pre_count = INPUT2_ticks_now;                          // reset the counter with the current count value
    INPUT2_ticks_valid = true;                                  // and flag the change up
  }
}

// function that executes whenever data is requested by master
// this function is registered as an event, see setup()
void requestEvent()
{
  //Send Serial Number to Processor Board (Does not occur during normal operation)
  if(CMD == 0x01)
  {
    byte data[22];
    data[0] = highByte(mySerial);
    data[1] = lowByte(mySerial);
    for(int i=0; i<20; i++)
    {
      data[i+2] = myModel[i];
    }
    Wire.send(data,22);
  }
  //Send Data to Processor Board (Requests come from Master every 300ms)
  if(CMD == 0x05)
  {
    Wire.send(DataForMaster,8);
  }
}

void receiveEvent(int HowMany)
{
  //Receive Command
  if(Wire.available() > 0)
  {
    CMD = Wire.receive();
  }
  
  //Assign Specific Address from User (via Broadcast) (Does not occur during normal operation)
  if(CMD == 0x02)
  {
    uint8_t serial[2];
    int testSerial;
    serial[0] = Wire.receive(); 
    serial[1] = Wire.receive(); 
    testSerial = (serial[0] * 256) + serial[1];
    int newAddress = Wire.receive(); 
    if(testSerial == mySerial)
    {
      Address = newAddress;
      EEPROM.write(0, Address);     // Store address in EEPROM
      delay(5);
      Wire.begin(Address);
      TWAR = (Address << 1) | 1;    // enable broadcasts to be received
    }
  }
  
  //Assign Auto Address from Master (Does not occur during normal operation)
  if(CMD == 0x03)
  {
    Address = Wire.receive();     // Change board to specified address
    EEPROM.write(0, Address);     // Store address in EEPROM
    delay(5);
    Wire.begin(Address);          // Start as new address
    TWAR = (Address << 1) | 1;    // enable broadcasts to be received
  }
  
  //Clear Address (via Broadcast) (Does not occur during normal operation)
  if(CMD == 0x04)
  {
    Address = 119;                // Reset Address to be 'DHCP' address
    EEPROM.write(0, (Address));   // Clear EEPROM Address space for next restart
    delay(5);
    Wire.begin(Address);          // Start as new address
    TWAR = (Address << 1) | 1;    // enable broadcasts to be received
  }  
}

I have made a bit of a start, but I am unsure if what I have done is correct or how to proceed without the variable "Timer1_overflow_count" - as this is not defined like timer0's was.

These are the bits I have changed, old bits commented out:

//const float TICKS_TO_PPS = 25000;                    // Convert clock ticks to PPS by dividng ticks into this number
                                                      // 16000000Mhz / 64 Prescaler = 4uS (Timer0 resolution)
                                                      // 1000*1000/4us = 250000 
													  
const float TICKS_TO_PPS = 250000;                    // If using 20Mhz crystal and Timer1
                                                       // 20000000Mhz / 8 Prescaler = 0.4uS (Timer1 resolution)
                                                      // 1000*1000/0.4uS = 2500000
void INPUT1_ISR()
{
  static unsigned long INPUT1_pre_count;                   // Last timer0_overflow_count value
  //INPUT1_ticks_now = timer0_overflow_count;                // Read the timer over count (number of times timer0 has overflowed since startup)
  INPUT1_ticks_now = timer1_overflow_count;
  
  //byte INPUT1_t = TCNT0;                                   // Copy the current value of Timer0
  int INPUT1_t = (TCNT1H << 8) + TCNT1L;
  //if ((TIFR0 & _BV(TOV0)) && (INPUT1_t<255))               // If the Timer has overflowed
  if ((TIFR1 & _BV(TOV1)) && (INPUT1_t<65535))
    INPUT1_ticks_now++;                                    // Increment the number of Ticks (1024uS resolution)
  //INPUT1_ticks_now = (INPUT1_ticks_now << 8) + INPUT1_t;         // Bit shift Ticks by 8 bits and add Timer0 current value, to make ticks_now 4uS resolution now
  INPUT1_ticks_now = (INPUT1_ticks_now << 16) + INPUT1_t; 
  
  if (INPUT1_pre_count == 0)                               // If this was the first time around the loop
  {                         
    INPUT1_pre_count = INPUT1_ticks_now;                   // Set the precount to the current count, don't use this number.
  } 
  else 
  {
    INPUT1_ticks_per_rev = INPUT1_ticks_now - INPUT1_pre_count;  // Calculate the difference to get the actual number of ticks...
    INPUT1_pre_count = INPUT1_ticks_now;                      // reset the counter with the current count value
    INPUT1_ticks_valid = true;                             // and flag the change up
  }
}

Thanks in advance

Used the original code but changed the crystal and prescale to be 20Mhz and /8, and it is running better.
At 7.545kHz on the Function Generator, my Multimeter says its 7.543KHz, and the Arduino flicks between 2 values - 7.530Khz and 7.552KHz, but every 7 seconds or so it jumps to another value, like 7.8Khz or 10.1kHz and flicks back.

But overall its improved.

If anyone is able to assist with changing it over to Timer1 though, I would appreciate the assistance.

Thanks

Have you seen this? - http://interface.khm.de/index.php/lab/experiments/arduino-frequency-counter-library/ -

might be helpfull

Hi - No I had not seen that....

Looking at it now, it looks like it is what I need to implement.

Sadly it uses an input pin I dont use on this board, but I will just hack the board to test it as I need to make a new revision anyway.

I had no idea you could even use a timer like this - rather impressive.

Will post back with the results, if/when I can get it working.

Ok I have got this running with my board running 20Mhz.

It seems to work, however in order to get the frequency it only updates once a second.
It seems to actually count all the pulses in a full second, rather than measuring the difference between pulses and estimating how many there will be in the second - like my first code example does.

I have the frequency generator outputting 943.265 kHz and Arduino is reading 933.073 kHz, My multimeter couldnt keep up.
At 850kHz, Multimeter and Arduino both agree.

So it does work, its just the updating is slower than I would like.

I dont fully understand what the library is doing in terms of the value you put in start(x). I need to put in 1000 in order to get the correct number of digits. If I put in 100 for example, then I am out on an order of 10, and if I put in 1, then I am an order of 100 out.
ie 1234Hz on the freq generator, when start(10) is selected comes out to be 12 showing on the Arduino. Start(100) will show 123, and Start(1000) will show 1234.

Currently have 850.456 kHz on the generator, and arduino is Start(10) and is displaying 8504.

Its probably just my understanding of the library, or lack of, however it seems weird how it would clip the number of digits shown.

Also, if I have it on Start(10), then I no longer have the ability to read frequencies under 100Hz - it just shows 1.

So its not really ideal.

So still very appreciative of someone assisting to get my first example of code working better, as that works more to my needs.

Regards
James

I hope I haven't got my numbers out by an order of magnitude here :slight_smile:

Just looking at the three numbers you've got above

7.543KHz
7.530Khz
7.552KHz

The difference in period between those three is < 1uS.

The "ISR"s you have aren't really ISRs at all, they are functions called by the the real INTx ISR, however as the real ISR doesn't re enable interrupts they are effectively the same thing but with a higher overhead.

Given that these two interrupts are asynchronous with the program and each other I reckon there are two potential issues.

a) The interrupt latency is variable, and with tolerances as tight as you have this could easily make a difference, ie the difference between getting a count of 7.543KHz and 7.530Khz 228nS or about 3-4 processor cycles.

b) If you are in INT0 and get INT1 (or VV) then the second interrupt will be ignored until you finish the first, meanwhile the timer is happily ticking away. Likewise if you are in the millis() interrupt.

I'm not convinced this can be totally overcome, but changing the "ISR" to something that did almost nothing like

volatile byte INPUT1_t;
volatile byte new_count_1;
void INPUT1_ISR()
{
  INPUT1_t = TCNT0;     
  new_count_1 = true; // set flag for external processing to occur
}

and doing all the other work in the main loop would have to help. At least then there's less chance of interrupts clashing. But there's still a chance and you have a double whammy, the higher the frequency the more likely to clash and the more affect it has on the count.

Interrupts are fine but they do introduce these sort of issues.

I think for high-speed counts using pulseIn() may be a better option. It is blocking but it sounds like you have known times, say directly after servicing the i2c protocol with the master, where you can afford to concentrate on this.

As pulseIn() does nothing but look at the state of a pin it is very deterministic, I would also kill interrupts around the pulsIn() call, this will affect millis() and maybe other things but you don't need them by the look of the program.

If the gadget is supposed to measure a large range of periods then a two-pronged approach may be the idea. PulseIn() for high speed and the other for low speed. Of course with pulseIn() you would have to measure both mark and space periods and add them together.


Rob

Bringing this back up again,

I just came across this:
http://www.pjrc.com/teensy/td_libs_FreqMeasure.html

Which looks to be a rewritten and improved version of the link supplied already in this post (http://interface.khm.de/index.php/lab/experiments/arduino-frequency-counter-library/)

Havent tested it yet, but sounds promising. Thought I would share.

James