rpm sensor

Hello.

Sometime ago I did a sketch for read rpm from an inductive pick up sensor atached to a small motor from a car toy.

The sketch along with the sensor and necessary hardware works fine, except sometimes it gives some strange values.

For example the motor is stabilized at any speed, lets say 3000 rpm and suddenly the value at the serial monitor peaks out to 10000 rpm or sometimes negative values.

I monitored the input signal to arduino (pin 2 wich is an interrupt at Leonardo, the board im working to), and all seems fine, a good square wave every time, so i don t suspect about the hardware.

Anyway i will atach the schematic of the circuit i made and the sketch.

I would like you to give a look at both things and pehraps tell me some improvment or change to avoid this sudden peaks.

Thanks.

Sketch:

////////////////////////variáveis do sensor de rotação\\\\\\\\\\\\\\\\\\\\\\\\\\

volatile int npulse=0;
volatile int  freq=0;
volatile int rpm;
int muqueca =2;

//////////////////////////////////////////////////

void setup()
{
  Serial.begin(9600);
  while(!Serial){;};

  pinMode(muqueca,INPUT);

  digitalWrite(muqueca,HIGH);

  attachInterrupt(1,sense_rpm,FALLING);  
}

//////////////////////////////////////////////////////

void loop()
{
  Serial.println("Frequencia do sinal");
  Serial.println(freq);
  Serial.println();

  Serial.println("Rot por segundo");
  Serial.println(rpm);
  Serial.println();

  Serial.println("Rot por minuto");
  Serial.println(rpm*60);
  Serial.println();
  Serial.println("---------------------------");

  delay(1000);

}
/////////////////////////////////////////////////////////

void sense_rpm()
{
  volatile int n2pulse = micros();
  volatile float ndentes=10;

  /////////////////////////////////////////////////////////  

  freq=1000000/(n2pulse-npulse);

  rpm=1000000/(ndentes*(n2pulse-npulse));

 

  npulse = n2pulse;

}

///////////////////////////////////////////////////////////////

volatile int n2pulse = micros();  <<<--- micros return unsigned long
        volatile float ndentes=10;

  /////////////////////////////////////////////////////////  

  freq=1000000/(n2pulse-npulse);

  rpm=1000000/(ndentes*(n2pulse-npulse));

 

  npulse = n2pulse;

Likely, your mistake is declaring n2pulse as int, it overflows and results getting unpredictable http://arduino.cc/en/Reference/Micros

...and likewise you need to declare npulse as unsigned long too.

From what I've read about interrupts, the interrupt service routine is supposed to do an absolute bare minimum, such as setting a state flag. This isr does a couple of trivial aritmetic calculations, so is it, to the purist, falling foul of that bare minimum thinking? Or is this acceptable practice?

On another note, I'm wondering why pin 2 is named "stew"... 8) (Or at least according to Google Translate, it is....)

JimboZA:
From what I've read about interrupts, the interrupt service routine is supposed to do an absolute bare minimum, such as setting a state flag. This isr does a couple of trivial aritmetic calculations, so is it, to the purist, falling foul of that bare minimum thinking? Or is this acceptable practice?

I'd prefer the ISR to just take the difference between current and previous readings and store it, leaving the lengthy floating point calculations to be done outside the ISR.

This isr does a couple of trivial aritmetic calculations, so is it, to the purist, falling foul of that bare minimum thinking? Or is this acceptable practice?

IMHO, there is nothing written in stone, all depends on application and speed of arriving interrupts. If calculations could be completed before next interrupt happened, why not to do it in ISR? The problem only, if there is more than one interrupt active in the system, than bare minimum approach is mandatory, not an option.

dc42:

JimboZA:
From what I've read about interrupts, the interrupt service routine is supposed to do an absolute bare minimum, such as setting a state flag. This isr does a couple of trivial aritmetic calculations, so is it, to the purist, falling foul of that bare minimum thinking? Or is this acceptable practice?

I'd prefer the ISR to just take the difference between current and previous readings and store it, leaving the lengthy floating point calculations to be done outside the ISR.

Personally, I'd prefer to see the floating math eliminated altogether. It is used in far too many situations that don't even call for it. This looks like one of those places. I feel that it should be discouraged whenever the problem can be solved with simple scaling of integers. Floating point should be a last resort instead of a first pick thing.

Hi Serbar,
also with unsigned long (which you really should use!) you still may get overruns depending on how long you measure. The negative values are a certain indication of overruns.
For 3000rpm you get 180000 interrupts per second. Or more, if I read your coding correctly the number of teeth of some gear ('dentes'?) seem to play a role. Together with the 10 teeth this is one interrupt every 500 nanoseconds or 1800000 interrupts per second. Unsigned long can store 2^32, this is the maximum value micros() can deliver and counting starts I think somewhere before setup() is executed. 2^32 microseconds are 4294 seconds or 71 minutes.
->So after each 71 minutes you will definetely get an overrun.

->500ns interrupt interval: As far as I know this interval should be sufficient for the coding you have in the ISR (but you should probably doublecheck).

If the reading would drop to say below 3000rpm (although motor speed is stable) this may be caused by a full serial buffer or the serial (also using interrupts as far as I know!) blocks your interrupt for RPM. You could also consider only transmitting averages (one average per second or even less), but this depends on your use case.
-> You should IMHO push the data across serial only, if there is a changed RPM or frequency available.

->I´m interested in the sensor you are using, can you pls post a link or something ? I'm currently using an optical fork sensor on the pinion of a small electric motor (which works surprisingly well), but I'd prefer another method...and didn't find any so far (hall sensor was too difficult to position and interpreting the current through the motor basically works but heavily depends on how well the motor is broken in...).

Thanks, Robert

Thanks guys.

Likely, your mistake is declaring n2pulse as int, it overflows and results getting unpredictable http://arduino.cc/en/Reference/Micros

Yes it was. I changed and it works as expected.

From what I've read about interrupts, the interrupt service routine is supposed to do an absolute bare minimum, such as setting a state flag.

I'd prefer the ISR to just take the difference between current and previous readings and store it, leaving the lengthy floating point calculations to be done outside the ISR.

Thats another thing i will care more from now on.

On another note, I'm wondering why pin 2 is named "stew"... smiley-cool

I like sometimes to call the variables fancy names, "Moqueca" is a brazilian fish dish, :wink:

I´m interested in the sensor you are using, can you pls post a link or something ?

Te sensor is a CKP (crankshaft position sensor from a mazda 6 engine), you will find it easy in any car junkyard. Mine is from a japanese brand, DENSO, and honestly i don t have any technical data about it, and googled already.

The Bosch ones are much more easy to find and also the related technical data.

Send me a PM with your mail and i will mail a Bosch catalogue with this kind of data.

Thanks again everyone.