Explain what's happening with this RPM sketch

I'm using the RPM sketch from the arduino PlayGround. Here's the code:

 volatile byte half_revolutions;

 unsigned int rpm;

 unsigned long timeold;

 void setup()
 {
   Serial.begin(9600);
   attachInterrupt(0, rpm_fun, RISING);

   half_revolutions = 0;
   rpm = 0;
   timeold = 0;
 }

 void loop()
 {
   if (half_revolutions >= 20) { 
     //Update RPM every 20 counts, increase this for better RPM resolution,
     //decrease for faster update
     rpm = 30*1000/(millis() - timeold)*half_revolutions;
     timeold = millis();
     half_revolutions = 0;
     Serial.println(rpm,DEC);
   }
 }

 void rpm_fun()
 {
   half_revolutions++;
   //Each rotation, this interrupt function is run twice
 }

The sketch gives correct readings as it is.. However, my RPM meter counter entire revolutions, not half revolutions. To correct for this, I changed to "30 * 1000" to "60 * 1000 " (60 = 30 x 2) . Since the sketch was designed to count half revolutions, I assumed that simply multiplying the value by 2 would be enought to make it work with a "1 interrupt per revolution" design.

As soon as I make the change, the output values become very strang.. I get seemingly random values in the range of 30,000 to about 50,000 but sometimes much lower or higher. I can't understand why this simple edit would cause those values.. Could it be some kind of overflow issue?

max number (16 bits) is 65535. I suspect an overflow Try define the variable as long instead of int

As soon as I make the change, the output values become very strang.. I get seemingly random values in the range of 30,000 to about 50,000 but sometimes much lower or higher. I can't understand why this simple edit would cause those values..

30 * 1000 = 30000, which fits in an int register. 60 * 1000 = 60000 which overflows an int register.

If your encoder is outputting one pulse per revolution instead of 2, the simple solution is to modify the ISR:

 void rpm_fun()
 {
   half_revolutions +=  2; 
 }

60000 which overflows an int register.

I can see why the Op was confused because rpm was defined as an unsigned int and should be good to 65,535.

It is easy to overlook that arithmetic with integer constants default to int. http://arduino.cc/en/Reference/Arithmetic

U & L formatters

By default, an integer constant is treated as an int with the attendant limitations in values. To specify an integer constant with another data type, follow it with: a 'u' or 'U' to force the constant into an unsigned data format. Example: 33u a 'l' or 'L' to force the constant into a long data format. Example: 100000L a 'ul' or 'UL' to force the constant into an unsigned long constant. Example: 32767ul

knut_ny: max number (16 bits) is 65535. I suspect an overflow Try define the variable as long instead of int

I tried that.. but it didn't fix the problem.

cattledog:

60000 which overflows an int register.

I can see why the Op was confused because rpm was defined as an unsigned int and should be good to 65,535.

It is easy to overlook that arithmetic with integer constants default to int. http://arduino.cc/en/Reference/Arithmetic

U & L formatters

By default, an integer constant is treated as an int with the attendant limitations in values. To specify an integer constant with another data type, follow it with: a 'u' or 'U' to force the constant into an unsigned data format. Example: 33u a 'l' or 'L' to force the constant into a long data format. Example: 100000L a 'ul' or 'UL' to force the constant into an unsigned long constant. Example: 32767ul

Could you please clarify your answer a little more?

Here is a numerical example to illustrate the use of U and L formatters with constants.

unsigned int rpm;

void setup() {

Serial.begin(9600);

//assume 10 pulses in 200 milliseconds

rpm = (30*1000/200)*10;//your original equation
Serial.println(rpm,DEC);// rpm = 1500

rpm= (60*1000/200)*10;//your attempt to multiply by two and double the rpm
Serial.println(rpm,DEC);//60*1000 overflows int; rpm = 65,266 

rpm = (30*1000/200)*20;// PaulS's suggestion to double the count
Serial.println(rpm,DEC);// rpm = 3000 

//U and L formatters can be anyplace in the overflowing expression

rpm= (60*1000L/200)*10;//forces multiplication result to long
Serial.println(rpm,DEC);// rpm = 3000 

rpm= (60U*1000/200)*10;//forces multiplication result to unsigned int
Serial.println(rpm,DEC);// rpm = 3000

rpm= (60*1000/200L)*10;//forcing L not in overflowing expression
Serial.println(rpm,DEC);// rpm = 65266

}

void loop() { 
  
}

Thanks for the explanation. I tried changing the dataype of rpm to float (without making any of the changes above) but that didn't work. If the data type of rpm is unsinged long, do we still have to include the U and L formatters?

If the data type of rpm is unsinged long, do we still have to include the U and L formatters?

I think that the answer should be obvious, but just in case it isn't, YES!

OK. It wasn't obvoius to me because I assumed that setting the variable data type to long would tell the compiler that the whole calculation should done using longs. Now I know that that isn't the case.. Thanks for the help.

By the way, I read on the reference that using floats is much slower than using ints. What about longs? Are longs slower than ints too? If so, are they faster than floats?

Are longs slower than ints too?

Yes, but only marginally.

If so, are they faster than floats?

Even turtles are.

So.. what if I want fast, non-integer-value calculations? Is there any way around using floats?

As soon as I make the change, the output values become very strang.. I get seemingly random values in the range of 30,000 to about 50,000 but sometimes much lower or higher. I can't understand why this simple edit would cause those values..

Which speed are you dealing with?

What hardware (wiring) have you got?

(I had quite a similar problem; maybe is the same)

Regards

@vffgaston It turned out to be caused by overflow. Did you read the posts above?

mahela007: @vffgaston It turned out to be caused by overflow. Did you read the posts above?

Actually the Arduino software suppresses compiler warnings by default, otherwise you'd have seen something like:

sketch_aug23a.ino: In function ‘void loop()’:
sketch_aug23a.ino:10: warning: integer overflow in expression

Warnings are useful to have on...

@vffgaston It turned out to be caused by overflow. Did you read the posts above?

I did. Is the problem solved?

Regards