Tachometer Accuracy using millis ?

i have a shaft that is turning at a constant rpm

connected to the shaft are 4 magnets, the magnets tigger a square wave signal to the arduino using a active sensor

the rpm transmitted is quite unstable - it jumps about quite a bit

code here

volatile int tachCount = 0;
long time;
long rpm = 0;

void setup()
{
Serial.begin(115200);
attachInterrupt(0, tachPulse, RISING);
time = millis();
}

void loop()
{
//Runs every 1/4 of a sec
if (millis() - time >= 250)
{
// rpm = number of interrupts counted in 250ms
// *4 to make per second
// 60 to make per minute
// /4 for 4 pulses per rotation (4 cylinder)
// /10 for some reason???
rpm = (((tachCount
4)*60)/4)/10;
if ( rpm > 0 )
{
Serial.println(rpm,4) ;
}
//Do things with the RPM value here (update display etc)...
//
tachCount = 0;
time = millis();
}
}

void tachPulse()
{
++tachCount;
}

has anyone got any suggestions why it would be so unstable at a constant rpm ?

After over 100 posts, do you think you could start posting code properly, please?

how do you do that ?

Using the Code (#) icon on the editor's toolbar.

volatile int tachCount = 0;
long time;
long rpm = 0;

void setup()
{
Serial.begin(115200);
attachInterrupt(0, tachPulse, RISING);
time = millis();
}

void loop()
{
//Runs every 10th of a sec
if (millis() - time >= 250)
{
// rpm = number of interrupts counted in 250ms
//  *4 to make per second
//  *60 to make per minute
//  /4 for 4 pulses per rotation (4 cylinder)
//  /10 for some reason???
rpm = (((tachCount*4)*60)/4)/10;
if ( rpm > 0 )
{
Serial.println(rpm) ;
}
//Do things with the RPM value here (update display etc)...
//
tachCount = 0;
time = millis();
}
}

void tachPulse()
{
++tachCount;
}

cheers - that should do it

any idea's why the rpm is so unstable ?

(you can also retrospectively post code correctly, by clicking on "modify", highlighting the code, clicking on the # icon, then clicking "save")

What sort of range of RPM are you measuring?
Is this a Hall-effect sensor or a reed relay?

rpm is under 12,000 rpm

sensor is hall effect with square wave output

What sort of range of RPM are you measuring?
What does the sensor output look like on a 'scope?

thanks for the reply,

the rpm range is approx 60 to 12000 rpm

the test i just did is approx 1000 rpm

i do not have a scope but have used this type of sensor before without problems

its output will be a 0-5v positive square wave

(i could use a magnetic analogue hall sensor, but this type lends itself to the digital input nicely)

What is the range of RPMs you're getting in your test?

Also, this looks like it might be a potential issue:

(((tachCount*4)*60)

At 12,000 rpm, multiplying 150460 = 36,000, which won't fit in a signed int.

i could use a magnetic analogue hall sensor

There's another kind?

@wildbill: Yes, I wondered about the if ( rpm > 0 ) too.

AWOL:

i could use a magnetic analogue hall sensor

There's another kind?

some output an analogue voltage - some output a square wave

here is some data I just collected from the serial port at approx 400 rpm

402
402
396
384
408
402
450
402
456
432
426
444
432
438
420
414
414
420
426
462
450
468
426
480
432
414
426
456
486
450
468
426
438
462
426
456
432
426
474
432
450
396
444
504
438
432
414
438
414
414
402
462
486
390
462
432
450
420
456
414
414
414
450
450
450
420
408
456
432
420
390
426
474
474
396
456
420
450
420
456
450
402
486
450
408
462
426
420
456
432
438
480
384
516
384
528
390
486
414
462
414
462
462
444
468
408
420
420
390

i will get rid of the 'if rpm > 0 ' and try again

My guess is that whatever is causing the mysterious factor-of-ten error is causing the instability. Perhaps the sensor is too close to the motor and commutator noise is (usually) triggering the sensor. Could the commutator have 10 segments? Try putting the magnets and sensors further from the motor.

As John says - the factor of ten has to go. Try printing the value of tachCount too. Possibly running it at lower revs might shed some light on what's happening. With regard to the instability though, at 400 revs you're only going to get 6 or 7 pulses in 250ms, so depending on the orientation of the shaft when you start counting, the number may (apparently) vary between 6 and 8, at least according to your data. That little variation in pulses amounts to a large variation in rpm when multiplied up. Try a longer sample period & see if it becomes more stable.

As wildbill said, if you are using this method to determine speed, your major source of error comes from a "mismatch" between sampling time and the number of pulses you expect to see during that time at a given speed. Another way to put it is that at most speeds you are expecting a non-integer amount of pulses, the fractional part accumulates and you get differing readings. As a result you have the variable number of pulses (6 to 8 in wildbill's example) which leads to fluctuating readings even while the speed is constant and everything is working as expected (systematic error).

Probably way more trouble that it is worth but one way to deal with this is vary your sampling period to "match" your speed such that you expect an integer value of encoder counts every sampling period. Then when you arrive at steady state there should be less error in your rpm value without making the sampling time excessive. Only go this route if you have some time to kill.

Could the commutator have 10 segments? Try putting the magnets and sensors further from the motor.

out of the box: Or put the sensors nearer, as this seems to generate 10 perfect pulses .... no need for those magnets ...

There is a serious logical error in the code:

-   if (millis() - time >= 250)
{
// rpm = number of interrupts counted in 250ms
//  *4 to make per second

if millis() -time is 500 or 1000 or 314 it is still multiplied by 4 and that is wrong ...

try something like this ...

void loop()
{
if (millis() - time >= 250)
{
// get the exact millis;
long t = millis() - time;

// calc the rotations per second
long rps = (250 * tachCount)/t;   // 1000 millis per second and the 4 pulse /rotation => constant 250

rpm = rps * 60;
Serial.println(rpm) ;

//Do things with the RPM value here (update display etc)...
//
tachCount = 0;
time = millis();
}
}

try something like this ...

if (millis() - time >= 250)
{
// get the exact millis;
long t = millis() - time;

// calc the rotations per second
long rps = (250 * tachCount)/t;

time = millis();

I'd prefer to see millis() called just once.

unsigned long now = millis();
if(now - then >= 250)
{
unsigned long interval = now - then;
long rps = ...

then = now;

Also, I'd like to see you get rid of the magic numbers.
#define TIMING 250
if(now - then >= TIMING)
This makes it easier to change the timing period to 100 milliseconds, or 500 milliseconds.

You are right Paul,

As there are two magic 250's in the code that have different semantics I like to change the code to make thoughts clear. One 250 now dissapears as RPS is not used anymore.
Also the interval only needs to be calculated once.

#define TIMING 250
#define PPR 4

...setup...

void loop()
{
unsigned long interval = millis() - time;
if ( interval >= TIMING )
{
// calc the rotations per minute

// RPMS = rotations per millisec = tachCount/PPR/interval,
// RPS = rotations per sec = 1000 * RPMS
// RPM = rotations per minute = 60 * RPS
// to keep the type unsigend long, UL is added to the consts

unsigned long rpm = 60UL * 1000UL * tachCount/PPR/interval ;    // compiler can optimize this

//Do things with the RPM value here (update display etc)...
Serial.print("RPM:  ");
Serial.println(rpm) ;

tachCount = 0;
time = millis();
}

...IRQ...
}

It is still possible that there will be an IRQ between the calculation of the interval and the usage of tachCount in the formula.

Hi Guys,

thanks for taking a look at the code and making the suggestions. I will try the modified code later and post up the results.

I guess the problem with using a time based approach is you are always going to have a senario where you have just missed a pulse at the end and start of the period. A solution here would be more pulses per rev to increase the resolution.

Another approach would be to time the period between pulses - this means a pulse will not be missed and the resolution comes down to the timing. I will try this also and post up the code and results

thanks again

How about this: have the IRQ note the time in microseconds of every 4th pulse and store the last two. Then if the interrupt count is low (below 100?) you could use the time between samples to get the time of he last revolution and calculate RPM from that.