Trying to Count RPM with a sensor and an ISR

Before getting to the RPM I would be glad to get the period right. So I wrote:

unsigned long PreviousMark = 0;
void setup(){
 Serial.begin(115200); 
 
 attachInterrupt(5, Period, FALLING);//interrupt 5 is pin 18 on mega
}

void loop(){

}




void Period ()
{
unsigned long Period (millis()-PreviousMark);
Serial.println (Period);
PreviousMark = millis();
  
  }

Initially it worked fine, but then I see that I get this erroneus readings at random times. My signal is super clean from a pulse generator. Here is what I get:

335
335
335
335
335
334
335
335
335
335
336
157
177
335
335
335
335
Thanks a lot,
Mitch

Don't print in an ISR

Interrupts are disabled when in an ISR
Serial printing uses interrupts

Can you see a problem ?

I'ts not really a good idea to do serial prints from an ISR.

To speed up things a bit make the variable Period as a volitie global. That way Period can be printed from loop() and the variable Peorid is not created and destroyed with each ISR itteration.

OK, Volatile seems to help. I am getting nice numbers,
Still , dont know how to print from the loop and not miss one reading...
Also I dont know how not to destroy the period with each pulse. It is suppose to vary , right?
Sorry, please help...

volatile long PeriodMs;


unsigned long PreviousMark = 0;
void setup(){
 Serial.begin(115200); 
 
 attachInterrupt(5, Period, FALLING);//interrupt 5 is pin 18 on mega
}

void loop(){

}

void Period ()
{
  PeriodMs =(millis()-PreviousMark);
Serial.println (PeriodMs);
PreviousMark = millis();
  
  }

Try...


volatile unsigned long latest = 0;
unsigned long previous = 0;

void setup()
{
  Serial.begin(115200); 
  attachInterrupt(5, Period, FALLING);
}


void loop()
{
  if (latest != previous)
  {
    Serial.println (latest - previous);
    previous = latest;
  }
}


void Period()
{
  latest = millis();  
}
1 Like

Should you be setting pinMode() on Pin 18?

John , Pin 18 is interupt 5 on Mega, right?

Red Car,
Thanks a lot, I got nice readings, I wonder how fast can it keep up? One KHZ?
Should I re-write it in micros for higher freq?

Thanks

Try it... it's a one line change.

  latest = micros();

The limiting factor will likely be the other code you have running in loop.

Thanks a lot, how elegant and simple...

Keep in mind that latest is a multi-byte variable. There is a possibility that while you are accessing this variable in loop() it could get interrupted and you could end up with a corrupted value! You need to disable interrupts while reading latest. Here is one way to accomplish that:

volatile unsigned long latest = 0;
unsigned long previous = 0;

void setup()
{
  Serial.begin(115200); 
  attachInterrupt(5, Period, FALLING);
}


void loop()
{
  noInterrupts();
  unsigned long latest_temp = latest;
  interrupts();
  
  if (latest_temp != previous)
  {
    Serial.println (latest_temp - previous);
    previous = latest_temp;
  }
}


void Period()
{
  latest = millis();  
}
1 Like

thanks for the suggestion. I implemented it.
I also have other buttons using the debouncer library.
This way they wont interfere.
If the debouncer "Bounces" during the noInterrupts(), would I miss it?

Your friend,
Mitch

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.