Tachometer results not stable

system · March 18, 2012, 1:06pm

Hello, I work on a tachometer to a Fuelcontrol system for cars. And I have problem with the stability on the tachometer.
I use the most common strategy for this, using interrupt to count revolutions over time.
I've hooked it up to my lcd to view the RPM and the RPM difference. (Same results as using the basic serial feedback.)

To test the code I use an PWM pin (uptime 1/255) set to about 30 hz (1835 RPM) connected to the interrupt pin with a resistor.
The RPM varies up to about +-150 which is unacceptable for my application. I need like +-10 maximum.

I have searched the internet and forums to see others tachometer code, and I have tested them with no better results.
The PWM frequency is OK, because Ive tested it with the pulseIn() function which gives me super stable RPM values. (+-0), but i have to use interrupt because I need critical timing later on.

So what can be the problem?, I have tested using millis() instead of micros() but no...

#include <Wire.h> 
#include <LiquidCrystal_I2C.h>
LiquidCrystal_I2C lcd(0x27,16,2); //set the LCD address to 0x27 for a 16 chars and 2 line display

//TESTPIN
const int testpin = 3;
//int testvar = 0;

//MAIN INPUTS
const int DispSampleTime = 100;               //[millisec] //= 0.1sec 
const int DiffSampleTime = 2000;               //[millisec] //= 0.1sec 
const int Rpm_Sample_Count = 10;

//--------------------------
//declare functions
void rpm_interrupt();

// variables:
volatile int rpm_count = 0;
int rpm_count_bacup = 0;
int rpm_count_bacup2 = 0;
unsigned long rpm_timeold = 0UL;
unsigned long disp_timeold = 0UL;
unsigned long diff_timeold = 0UL;
long timetemp = 0L;
unsigned long Rpm = 0UL;          // the Rpm sensor value
int RpmMinvalue = 10000;
int RpmMaxvalue = 0;

//SETUP RUN ONLY ONCE @ start
void setup() {

  //DEBUG
  TCCR2B = TCCR2B & 0b11111000 | 0x07 ;  // sets PWM to 30.517578125 Hz on pin 3 and 11 //timer 2
  
  //testpin TESTESTESTEST----------------------<<<<<<<<<<<<>>>>>>>>>>>>>
  pinMode(testpin, OUTPUT);
  analogWrite(testpin, 1);    //sample 30.51757 hz pulse 1/255 uptime.
  
  lcd.init(); // initialize the lcd
  lcd.backlight();
  //set interrupt on pin 2 and call function rpm_interrupt when pin is falling
  attachInterrupt(0, rpm_interrupt, FALLING);
} //end setup


//MAIN LOOP
void loop() {

  //Calculate RPM every 5 turns
  timetemp = micros() - rpm_timeold;
  rpm_count_bacup = rpm_count;
  
  if( timetemp >= 0 ) {   //if micros not resetted (overflow every 70 minutes)
    if(timetemp < 1000000 ){   //check if over 60RPM
      if(rpm_count_bacup >= Rpm_Sample_Count){ //check so count is over Rpm_Sample_Count

        Rpm = 60*1000000*rpm_count_bacup;
        Rpm = (float)Rpm/(timetemp);
        rpm_count_bacup2 = rpm_count_bacup;
        rpm_timeold = micros();
        rpm_count = 0;
      }
    }   
    else {    //under 60RPM reset rpm and time         
      rpm_timeold = micros();
      rpm_count = 0;
      Rpm = 0;
    }	
  }  //end , move on.
  

  //reset min/max values
  timetemp = millis() - diff_timeold;
  if( timetemp >= DiffSampleTime) {   
    diff_timeold = millis();
    RpmMaxvalue = 0;
    RpmMinvalue = 10000;
  }

  //calculate RPM difference
  if(Rpm < RpmMinvalue){
    RpmMinvalue = Rpm;
  }
  if(Rpm > RpmMaxvalue){
    RpmMaxvalue = Rpm;
  }


  //DISPLAYSTUFF
  timetemp = millis() - disp_timeold;
  if( timetemp >= DispSampleTime) {   

    disp_timeold = millis();

    //disp stuff
    lcd.clear();
    lcd.print("Rpm:  ");
    lcd.print(Rpm);
    lcd.setCursor(0,1);
    lcd.print("Diff: ");
    lcd.print( (RpmMaxvalue - RpmMinvalue) );
    lcd.setCursor(11,1);
    lcd.print("C: ");
    lcd.print( rpm_count_bacup2 );


  } //end disp

} //end main loop

//FUNCTIONS________________________________________

void rpm_interrupt() {
  rpm_count++;
  //handle injectors
}

system · March 18, 2012, 2:01pm

  rpm_count_bacup = rpm_count;

rpm_count is an int. Between the low order byte being copied and the high order by being copied, the value in rpm_count can change. This copy should only be done with interrupts disabled. Which means, of course, that it should NOT be done on every pass through loop.

Rpm = 60*1000000*rpm_count_bacup;

A nice int value...

  timetemp = micros() - rpm_timeold;
  if( timetemp >= 0 ) {   //if micros not resetted (overflow every 70 minutes)

Your understanding of how subtraction involving unsigned values seems to be lacking. The value in timetemp can never be negative, so this test is unnecessary.

You are calculating Rpm far too often.

robtillaart · March 18, 2012, 2:12pm

The fluctuations come from a big part of the math, the faster you want to measure the more inprecise the measurements will be

if you display per second and you have a measurement of R pulses in one second you multiply by 60, if there is one pulse more or less that means on the RPM scale you get + or - 60 !! that is quite a difference

Here a minimized version of your code that shows this effect

#include <Wire.h> 
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27,16,2); //set the LCD address to 0x27 for a 16 chars and 2 line display

//TESTPIN
const int testpin = 3;

//MAIN INPUTS
const int DispSampleTime = 1000;               //[millisec] //= 0.1sec 

// variables:
volatile unsigned long rpm_count = 0;
unsigned long prev_rpm_count = 0;

unsigned long disp_timeold = 0UL;

unsigned long timetemp = 0L;
unsigned long Rpm = 0UL;     


//SETUP RUN ONLY ONCE @ start
void setup() 
{
  //DEBUG
  TCCR2B = TCCR2B & 0b11111000 | 0x07 ;  // sets PWM to 30.517578125 Hz on pin 3 and 11 //timer 2

  //testpin TESTESTESTEST----------------------<<<<<<<<<<<<>>>>>>>>>>>>>
  pinMode(testpin, OUTPUT);
  analogWrite(testpin, 1);    //sample 30.51757 hz pulse 1/255 uptime.

  lcd.init(); // initialize the lcd
  lcd.backlight();
  
  //set interrupt on pin 2 and call function rpm_interrupt when pin is falling
  attachInterrupt(0, rpm_interrupt, FALLING);
} 

void loop() 
{
  // DISPLAY RPM
  timetemp = millis() - disp_timeold;
  if( timetemp >= DispSampleTime) 
  {   
    disp_timeold += timetemp;
    
    unsigned long t = rpm_count;
    Rpm = t - prev_rpm_count;      // rounds made since last display
    prev_rpm_count = t;            // remember previous counter
    
    Rpm = Rpm * 60000UL / timetemp; // convert to rounds per minute

    //disp stuff
    lcd.clear();
    lcd.print("Rpm:  ");
    lcd.print(Rpm);
  }
}

//FUNCTIONS________________________________________

void rpm_interrupt() 
{
  rpm_count++;
}

another version is coming soon

robtillaart · March 18, 2012, 2:28pm

This variation does measure the time it takes to do one rotation and measures the RPM from the very last rotation. It is definitely a different approach

#include <Wire.h> 
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27,16,2); //set the LCD address to 0x27 for a 16 chars and 2 line display

//TESTPIN
const int testpin = 3;

//MAIN INPUTS
const int DispSampleTime = 1000;               //[millisec] //= 0.1sec 

// variables:
volatile unsigned long lastTime;
volatile unsigned long thisTime;
volatile unsigned long duration;

unsigned long disp_timeold = 0UL;
unsigned long timetemp = 0L;   


//SETUP RUN ONLY ONCE @ start
void setup() 
{
  //DEBUG
  TCCR2B = TCCR2B & 0b11111000 | 0x07 ;  // sets PWM to 30.517578125 Hz on pin 3 and 11 //timer 2

  //testpin TESTESTESTEST----------------------<<<<<<<<<<<<>>>>>>>>>>>>>
  pinMode(testpin, OUTPUT);
  analogWrite(testpin, 1);    //sample 30.51757 hz pulse 1/255 uptime.

  lcd.init(); // initialize the lcd
  lcd.backlight();
  
  //set interrupt on pin 2 and call function rpm_interrupt when pin is falling
  attachInterrupt(0, rpm_interrupt, FALLING);
} 

void loop() 
{
  // DISPLAY RPM
  timetemp = millis() - disp_timeold;
  if( timetemp >= DispSampleTime) 
  {   
    disp_timeold += timetemp;
    
    float RPM = 60e6/duration;  // 60.000.000 micros / duration in micros

    //disp stuff
    lcd.clear();
    lcd.print("Rpm:  ");
    lcd.print(RPM);
  }
}

void rpm_interrupt() 
{
  lastTime = thisTime;
  thisTime = micros();
  duration = thisTime-lastTime;
}

robtillaart · March 18, 2012, 2:50pm

and yet another version, this one keeping the rounds per second in a circular buffer and has a var RPM that reflects the rounds made in the last minute.

This one shows a memory effect.

#include <Wire.h> 
#include <LiquidCrystal_I2C.h>

LiquidCrystal_I2C lcd(0x27,16,2); //set the LCD address to 0x27 for a 16 chars and 2 line display

//TESTPIN
const int testpin = 3;

//MAIN INPUTS
const int DispSampleTime = 1000;               //[millisec] //= 0.1sec 

// variables:
volatile unsigned long rounds;
unsigned long lastrounds;

unsigned long rps[60];
uint8_t idx = 0;
unsigned long RPM = 0;

unsigned long disp_timeold = 0UL;
unsigned long timeold = 0UL;
unsigned long timetemp = 0L;   


//SETUP RUN ONLY ONCE @ start
void setup() 
{
  //DEBUG
  TCCR2B = TCCR2B & 0b11111000 | 0x07 ;  // sets PWM to 30.517578125 Hz on pin 3 and 11 //timer 2

  //testpin TESTESTESTEST----------------------<<<<<<<<<<<<>>>>>>>>>>>>>
  pinMode(testpin, OUTPUT);
  analogWrite(testpin, 1);    //sample 30.51757 hz pulse 1/255 uptime.

  lcd.init(); // initialize the lcd
  lcd.backlight();
  
  //set interrupt on pin 2 and call function rpm_interrupt when pin is falling
  attachInterrupt(0, rpm_interrupt, FALLING);
} 

void loop() 
{
  // get rounds in last second
  // place it in a circular buffer
  // and keep the RPM up to date (after a minute it will stabilize)
  timetemp = millis() - timeold;
  if( timetemp >= 1000) 
  {
    timeold += timetemp;
    unsigned long t = rounds;
     
    RPM -= rps[idx];
    rps[idx] = t - lastrounds;
    RPM += rps[idx];
    idx++;
    if (idx == 60) idx = 0;
   
    lastrounds = t;
  }
    
  // DISPLAY RPM
  timetemp = millis() - disp_timeold;
  if( timetemp >= DispSampleTime) 
  {   
    disp_timeold += timetemp;
    
    //disp stuff
    lcd.clear();
    lcd.print("Rpm:  ");
    lcd.print(RPM);
  }
}

void rpm_interrupt() 
{
  rounds++;
}

So in the end there are several ways to calculate the RPM bute every method has its own side effects.

if you count pulses in last second and multiply *60 (1st) , one pulse more or less give you a delta of 120
if you count all pulses in a minute (above) you get a memory effect when accelerating. It takes a minute to become stable
if you measure time of the last round (2nd) the accuracy depends on the duration (especially when using integer math)

IMHO The advantage of measuring time of the last round is that it is very reactive.

system · March 18, 2012, 2:58pm

Thanx for the replies, Tested the code, and it is very stable, have to go through the code and your comments so I learn exactly what i did wrong and how to approach this type of timing in the code.

robtillaart · March 18, 2012, 3:04pm

and it is very stable

which version ...

Remember, first get the basics right and then add the bells and whistless (like min and max etc)

system · March 18, 2012, 4:14pm

Tested both now and the first one varies with +- 0.12 rpm which is fantastic and the second one are absolutely still as its a long, (no decimals).
I know what you mean with the integrating math and I will have some small integration, so it wont affect the RPM acceleration.

So a overflow test isn't necessary when checking passed time?
what if the micros reseted between assigning rpm_timeold and the difference test, wouldn't that be negative or 0(unsigned)?

timetemp = micros() - rpm_timeold;
if( timetemp >= 0 ) { //if micros not resetted (overflow every 70 minutes)

robtillaart · March 18, 2012, 8:13pm

as the datatype is unsigned it cannot be < 0 . By definition.

It can however overflow/underflow but by using subtraction you can compensate the overflow of the micros() with the underflow of the subtraction.

let ths prog run until overflow and you will see the deltas a

void setup()
{
  Serial.begin(115200);
}

void loop()
{
  Serial.print(micros(), DEC);
  Serial.print("\t ");
  Serial.print(lasttime, DEC);
  Serial.print("\t ");
  Serial.println(micros() - lastTime, DEC);
  delay(600000UL);
  lasttime = micros();
}

The micros is only reset after the program itself is reset. And then all counters are reset

Tested both now and the first one varies with +- 0.12 rpm which is fantastic and the second one are absolutely still as its a long,

OK sounds very good, probablyonle one or two pulses variation. Now the real life tests need to be done, its getting serious

W'll hear from you in this thread.

GoForSmoke · March 19, 2012, 12:21am

robtillaart:

if you measure time of the last round (2nd) the accuracy depends on the duration (especially when using integer math)

By reducing the scale you measure, like using micros instead of millis, you can increase the accuracy.
It is like measuring your height as integer, meters will not be as accurate as millimeters.

Topic		Replies	Views
Tachometer Accuracy using millis ? Programming	54	18112	May 5, 2021
In search of a better RPM readout... Troubleshooting	17	14456	May 6, 2021
Arduino Tachometer Problem Programming	59	11714	May 5, 2021
Need math help with tachometer that calculates rpm w/1 cycle. General Guidance	29	3438	May 5, 2021
Help with tachometer code Troubleshooting	15	31152	May 6, 2021

Tachometer results not stable

Related topics