Calculation error for RPM calculation using PinChangeInt

Hi all, I have worked through numerous examples to put together the following code.

#include <PinChangeInt.h>
#include <PinChangeIntConfig.h>
#define PIN 12

volatile int elapsed = 0;
volatile int previous = 0; 
volatile int current = 0;
volatile byte count = 0;
float RPM = 0;
float average = 0;

void setup()
{
  Serial.begin(9600);
  pinMode(PIN, INPUT);
  digitalWrite(PIN, HIGH);
  PCintPort::attachInterrupt(PIN, clock, FALLING);
}

void loop()
{
  if((millis() - previous) > 500 && count > 200){
   elapsed = millis() - previous;
   previous = millis();
   RPM = 60000/(elapsed/count);
   count = 0;
   //Serial.print(elapsed);
   //Serial.print(", ");
   //Serial.print(count);
   //Serial.print(", ");
   Serial.println(RPM, 2);
 }
}

void clock()
{
  count++;
}

The good news is that i’ve taught myself to use the PinChangeInt library tonight. If i can do it, anyone can!

However, there is an error in the RPM calculation. It settles on 5000 rpm, but if i slow it by hand enough, it will drop to around 4650 but will jump straight back to 5000 when i release the pressure. I do know that the elapsed time is being captured correctly. I’ve been checking the average time interval between falling interrupts (with my oscilloscope) and the time period (elapsed) is correct.

When ‘average’ time is calculated however, there are no decimal places (problem number 1).

The biggest problem crops up however when the RPM calculation is done. Given the average time for a full revolution is around 12.3 to 12.5 ms, the RPM should be around 4800 (60000/12.5).

Can anyone suggest what is going wrong in the calculation process?

Chatty

void clock()
{
  count++;
}

Well, it's certainly obvious why this function name was chosen, over, say d8fGSzX(). NOT!

When 'average' time is calculated however, there are no decimal places (problem number 1).

There is no "average" time calculated. There is ONE time calculated. And, of course, that time is an integer value, and, of course, integers do not have decimal places.

You need to read up on the differences between ints and floats, and on integer arithmetic (that you are doing) vs. floating point arithmetic.

PaulS, took your advice and read up on floating point calculations and modified the code as such;

int elapsed = 0;
int previous = 0; 
float current = 0;
volatile byte count = 0;
float RPM = 0;
float average = 0;

Modified also the calculation to;

RPM = 60000/(elapsed/count);

From what I can tell, that should have addressed the issue but hasn't.

The values being used in the calculation are;

elapsed = 1266
count = 101

But RPM is being calculated as 5000 - in other words, 60000/12.

Should there be an additional step in the calculation to divide 1266 by 101 (as a floating point calculation) before dividing that number in 60000?

Chatty

RPM = 60000.0/((float)elapsed/(float)count);

A huge thankyou AWOL.

One small question though, i'd modified the variable declarations so that the variables involved were floats, but the calculation was performed as an integer. I read in the reference section that if one of the variables involved in a calculation is a float (as RPM is) then the calculation is treated as floating point.

Is the fact that RPM is a float irrelevent as it's only elapsed and count that matter in the formula above?

Thanks again,
Chatty

I read in the reference section that if one of the variables involved in a calculation is a float (as RPM is) then the calculation is treated as floating point.

That's quite correct - in this case, RPM isn't part of the calculation, it is simply assigned the result of the calculation, all components of which are integers.

Is the fact that RPM is a float irrelevent as it's only elapsed and count that matter in the formula above?

Yes. The type of argument to store the result in has no bearing on the type of result that will be obtained. Only the types involved on the right side of the equal sign matter.

Bear in mind that millis() is not very accurate, it jumps by 2 sometimes. Also by measuring over 500ms you are limiting the resolution to 120rpm. For measuring rpm, it's usually better to time the interval between successive interrupts (by calling micros() in the ISR), then have the main loop calculate the rpm from that interval.