Calculating periodic time using micros() on Arduino Nano

[techgzt]

Hi all,

I am doing a supposedly simple Arduino code on the Nano to measure the RPM. In a nutshell, I am using an interrupt service routine which triggers on falling edge. One it triggers, it measured the periodic time by subtracting the time of the last interrupt with the current time. From there, it is converted to frequency and then to RPM. The variables are declared as shown below. Now, when monitoring the values using serial monitoring, I am noticing discrepancies. For instance, for PT of 21896, I am getting FREQ of 49.27 and RPM of 3154.57. What could be causing this to happen please?

volatile unsigned long PT = 0;
float FREQ = 0;                    
float RPM_Reading = 0;                          
volatile unsigned long CurrentTime = 0;                  
volatile unsigned long longTime = 0;

void ISR()  
{
  CurrentTime = micros();
  PT = CurrentTime - lastTime;    // Measure periodic time
  FREQ = 1000000.0 / PT;          // Measure frequency
  RPM_Reading = FREQ * 60;        // Convert frequency to RPM
  lastTime = micros();
}

[UKHeliBob]

Why are you doing the calculations in the ISR rather than in loop()

[jim-p]

@techgzt

What could be causing this to happen please?

FREQ and RPM are not declared as volatile, so who knows what is happening with those variables.
However, as @UKHeliBob pointed out, the only thing you should be doing in the ISR is saving the current time.

[techgzt]

@techgzt

PT = CurrentTime - lastTime;    // Measure periodic time
  FREQ = 1000000.0 / PT;          // Measure frequency
  RPM_Reading = FREQ * 60;        // Convert frequency to RPM

FREQ and RPM are only being changed in the ISR, hence why i did not declare them as volatile.

So I did the following changes, but still the same:

1. Moved the two lines below to the main loop:

 FREQ = 1000000.0 / PT;          // Measure frequency
  RPM_Reading = FREQ * 60;        // Convert frequency to RPM

2. Declared FREQ and RPM_Reading as volatile

[UKHeliBob]

Move everything except saving the current time out of the ISR. Turn off interrupts while you do the calculations in loop() and turn them back on again afterwards

[Railroader]

Please post the entire code. I suspect You do wrong when reading those 2 variables.

[jim-p]

@techgzt

FREQ and RPM are only being changed in the ISR, hence why i did not declare them as volatile.

Then how do you know what the value are?
Are you printing them somewhere.

Need to see your entire code.
You also need to change the ISR as suggested.

[JohnLincoln]

In the Interrupt Service Routine you are checking the time twice, I think that is the problem.

There is going to be a difference between those two times, so you are not measuring the period correctly.

Your period will always be measured too short (by the time taken to do the calculations), and hence your frequency and r.p.m. will be too high.

I think that you need to change:

lastTime = micros();

to:

lastTime = CurrentTime;

[techgzt]

Hi all,

Below is the full code.
@JohnLincoln I am not convinced that this is the problem. I could be causing incorrect readout for the periodic time, but once this is read, the frequency and RPM should be calculated based on that value, irrelevant what the value is.

@jim-p how can i take PT = currentTime - lastTime; out of the ISR and measure the periodic time please?

#define INPUT_PIN_1 2                              

volatile unsigned long PT = 0;
volatile float FREQ = 0;
                     
volatile float RPM_Reading = 0;                          

volatile unsigned long currentTime = 0;                   
volatile unsigned long lastTime = 0;                    
 
void setup() 
{
  Serial.begin(9600);
  
  pinMode(INPUT_PIN_1, INPUT_PULLUP);                                               
  
  attachInterrupt(digitalPinToInterrupt(INPUT_PIN_1), InterruptSR, FALLING);       
}

void loop() 
{
  FREQ = 1000000.0 / PT;             // Calculate frequency in Hz
  RPM_Reading = FREQ * 60;           // Convert frequency to RPM
  
  Serial.print("periodic time in ms: ");
  Serial.println(PT);
  Serial.print("frequency: ");
  Serial.println(FREQ);
  Serial.print("RPM: "); 
  Serial.println(RPM_Reading);       
}

void InterruptSR()  
{
  currentTime = micros();
  PT = currentTime - lastTime;
  lastTime = currentTime;
}

[jim-p]

@techgzt

how can i take PT = currentTime - lastTime; out of the ISR and measure the periodic time please?

Do you actually need to know RPM on a pulse by pulse basis or would once a second be sufficient?

[techgzt]

On pulse by pulse. I already managed to get RPM once a second using micros(). I also achieved reading every 200ms. My issue is that I need very fast response time, hence why I am trying to calculate the RPM based on the periodic time of every pulse.

[jim-p]

Depending on the minimum time between pulses, what you want to do may not be possible. You can try this:

void InterruptSR()  
{
  currentTime = micros();
  flag = true;
}

In loop check when flag is true.
If true then do your calculations like you did in your original ISR
Set flag to false when done

[ec2021]

Hi @techgzt ,

here is your sketch with some changes on Wokwi:

https://wokwi.com/projects/418610438465724417

If I'm not mistaken it should work as intended.

I added the TimerOne lib to create interrupts for the ISR (and a number of comments ).

/*
  Forum: https://forum.arduino.cc/t/calculating-periodic-time-using-micros-on-arduino-nano/1336853
  Wokwi: https://wokwi.com/projects/418610438465724417

  2024/12/29
  ec2021

*/


// Added to create a digital input for the ISR
// The togglePin switches its state (LOW-> HIGH-> LOW-> ...) every timer1 interrupt.
// As the ISR reacts on a falling edge only
// the measured period equals 2 x timerInterval
#include <TimerOne.h>;
const byte togglePin {3};
const unsigned long timerInterval = 500; //micro seconds !!!

#define INPUT_PIN_1 2

volatile unsigned long PT = 0;

// No need for volatile as FREQ and RPM_Reading are not used in the ISR
float FREQ = 0;
float RPM_Reading = 0;

// Moved to ISR
// volatile unsigned long currentTime = 0;
// volatile unsigned long lastTime = 0;

void setup()
{
  // TimerOne
  Timer1.initialize(timerInterval);
  Timer1.attachInterrupt(toggle);
  pinMode(togglePin, OUTPUT);

  Serial.begin(115200);
  pinMode(INPUT_PIN_1, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(INPUT_PIN_1), InterruptSR, FALLING);
}

void loop()
{
  // Copy PT to the variable copyPT which is "outside" of the ISR
  noInterrupts();
  unsigned long copyPT = PT;
  interrupts();

  FREQ = 1000000.0 / copyPT;             // Calculate frequency in Hz
  RPM_Reading = FREQ * 60;           // Convert frequency to RPM
  // not ms but micro seconds
  //Serial.print("periodic time in ms: ");
  Serial.print("periodic time in µs: ");
  Serial.println(copyPT);
  Serial.print("frequency: ");
  Serial.println(FREQ);
  Serial.print("RPM: ");
  Serial.println(RPM_Reading);
}

void InterruptSR()
{
  static unsigned long lastTime = 0;
  unsigned long currentTime = micros();
  PT = currentTime - lastTime;
  lastTime = currentTime;
}


void toggle() {
  static byte state = LOW;
  digitalWrite(togglePin, state);
  state = !state;
}

Good luck!
ec2021

The simple wiring for the example

[techgzt]

Hi all,

Thanks all for your support. It worked fine with the code provided by @ec2021 . Thanks a lot for taking the time to test.
I tried all the differences between your code and my code to better understand the root cause of the issue. Eventually I ended up with the code below, which is very similar to mine. Then when I removed the last change, which is "unsigned long copyPT = PT;" I started having the same issue again.
I still cannot understand why by just copying the variable to another variable made a difference? What is the issue with using PT directly?

#define INPUT_PIN_1 2

volatile unsigned long PT = 0;

float FREQ = 0;
float RPM_Reading = 0;
volatile unsigned long currentTime = 0;
volatile unsigned long lastTime = 0;

void setup()
{
  Serial.begin(9600);
  pinMode(INPUT_PIN_1, INPUT_PULLUP);
  attachInterrupt(digitalPinToInterrupt(INPUT_PIN_1), InterruptSR, FALLING);
}

void loop()
{
  unsigned long copyPT = PT;

  FREQ = 1000000.0 / copyPT;             // Calculate frequency in Hz
  RPM_Reading = FREQ * 60;           // Convert frequency to RPM
  Serial.print("periodic time in µs: ");
  Serial.println(copyPT);
  Serial.print("frequency: ");
  Serial.println(FREQ);
  Serial.print("RPM: ");
  Serial.println(RPM_Reading);
}

void InterruptSR()
{
  currentTime = micros();
  PT = currentTime - lastTime;
  lastTime = currentTime;
}

[ec2021]

The issue is that the value of the variable PT can (and is) changing several times while your code performs one loop ...

You should keep the noInterrupt() command before and the interrupt() command after copying PT to copyPT!!!!!

Otherwise it might be that the value of PT is changing while your sketch in loop tries to copy it.

Feel free to read this for example:

https://docs.arduino.cc/language-reference/en/variables/variable-scope-qualifiers/volatile/

P.S.: You could use the ATOMIC_BLOCK macro instead ...

Here is another page that may be interesting regarding AVR and interrupts:

https://developerhelp.microchip.com/xwiki/bin/view/products/mcu-mpu/8-bit-avr/peripherals/interrupts/special-considerations/

[Edit:] Although the last link gives a nice explanation I'm afraid there is a mistake in their example:

volatile uint16_t ctr;
 
ISR(TIMER1_OVF_vect)
{
  ctr--;
}
...
int main(void)
{
   uint_16 ctr_copy;
   ...
   ctr = 0x0200;
   start_timer();
   do
   {
     ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
     {
       ctr_copy = ctr;
     }
   } while(ctr != 0);
     // wait
       ;
   ...
}

If I'm not wrong It should read in loop()

   } while(copy_ctr != 0);
     // wait
       ;
   ...

[UKHeliBob]

@techgzt

I ended up with the code below,

There is still more code in the ISR than there needs to be