Frequency / RPM calculation

Hi Everyone,

The intend of this code is to calculate RPM of a an engine using frequency signal from a Halleffect sensor ,display it on 2.42oled and drive Neopixel shift light. At this stage signal is simulated with a square function generator with amplitude of -5V (triggering at LOW state).

When sending 100Hz square wave hardware reads 60-120RPM (jumping between these 2 values)
With 500Hz signal, RPM jumps between 540 and 600 etc... Seems like bunch of wrong calculations on my end...

In my understanding 50Hz (events a second) should equal to 3000RPM (events in a minute) etc. In theory engine signal would never exceed ~8000RPM (~150Hz). Therefore, Arduino Nano Every should easily handle these frequencies with decent accuracy, without necessity of using Low-Pass filters.

Hardware used:
-Arduino Nano Every (Atmega 4809 @ 20MHz)

  • 2.42 oled connected on SPI (using u8g2 library)

  • 8 Bit 2812 RGB (using Adafruit NeoPixel library)

  • [Not a single "delay" used in the code.]

Code bellow (skipped long pixel() function for clarity of reading):

#include <Arduino.h>

//~~~PIXEL DEF~~~//
#include <Adafruit_NeoPixel.h>
#ifdef __AVR__
 #include <avr/power.h> // Required for 16 MHz Adafruit Trinket
#endif
#define PIN        4 // PIN SENDING DATA TO LED STRIP
#define NUMPIXELS 8 // LEDS IN THE ARRAY
Adafruit_NeoPixel pixels(NUMPIXELS, PIN, NEO_GRB + NEO_KHZ800);


//~~~SCREEN DEF~~~//
#include <U8g2lib.h>
#ifdef U8X8_HAVE_HW_SPI
#include <SPI.h>
#endif
#ifdef U8X8_HAVE_HW_I2C
#include <Wire.h>
#endif
U8G2_SSD1309_128X64_NONAME0_F_4W_HW_SPI u8g2(U8G2_R0, /* cs=*/ 10, /* dc=*/ 9, /* reset=*/ 8);

//~~~RPM CALC~~~//
volatile int rpmcount = 0;

int rpm = 0;

unsigned long lastmillis = 0;
unsigned long timenow = millis();
boolean detectState = false;
boolean lastDetectState = false;
int hallPin = 2; //Halleffect sensor for RPM
int led = 0; // for neopixel function

void setup(void) {
  u8g2.begin();   // INITIALIZE 2.42" oled screen
  pixels.begin(); // INITIALIZE NeoPixel strip
  Serial.begin(9600);
  pinMode(hallPin, INPUT_PULLUP); //read pulses from hall sensor

  attachInterrupt(digitalPinToInterrupt(2), rpm_interr, FALLING); //interruprt for rpm calculation
}

void loop(void)
  {
    rpm_calc();
    rpm_display(rpm);
  }

void rpm_calc(){

  //~~~Never executes the statement when calling for millis() instead of previously defined timenow~~~//
if(timenow - lastmillis >=100UL){
  //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~//
    detachInterrupt(digitalPinToInterrupt(2)); //time sensitive calculations
    
    //~~~rpm offlay close to input Hz... constantly oscillates~~~//
    rpm = rpmcount * 60 ; // convert to Rev per Minute
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~//
    
    led = map(rpm, 0, 8000, 0, 8); // convert rpm to 1-8 values to drive LEDS
    pixel(led); // calling function lighting up pixels of the strip (led=1, 1st led lit etc)
  }
rpmcount = 0;
lastmillis = millis();
attachInterrupt(digitalPinToInterrupt(2), rpm_interr, FALLING);

}

//~~~Used with interrupt~~~//
void rpm_interr(){
  rpmcount++;
}

//~~~Test function to display obtained RPM~~~//
void rpm_display(int revs){
  u8g2.clearBuffer();
  u8g2.setFontDirection(0);

  u8g2.setDrawColor(1);
  u8g2.setFont(u8g2_font_10x20_mf);
  u8g2.drawStr(112, 20, "R");
  u8g2.drawStr(112, 40, "P");
  u8g2.drawStr(112, 60, "M");

  u8g2.setCursor(5, 40);
  u8g2.print(revs);

  u8g2.sendBuffer();
}

Code inspired by other codes found on the forum.

I would highly appreciate any help to make this code work correctly and convert input frequency to RPM. I would like to stop it from jumping between values. Perhaps, show RPM in the increments of 100s? Any suggestions for more reliable calculations?

Thanks in advance!

so you do the math every 0.1s

and the math you use is

    rpm = rpmcount * 60 ; // convert to Rev per Minute

so you don't get really Rev per minute but for 60 * 0,1s = 6s

at 100Hz you get 10 pulses in 0.1s but if you are unlucky you'll get 9 or 11 (esp. the way millis counts). if you get 9 ➜ you'll show 54 (9x60), if you get 11, you'll show 66

you should measure over 1 full second to get the math right but might still be +/- 1 on the number of pulses

if(timenow - lastmillis >=1000UL){

That implies your code is detecting only 1 or 2 interrupts over a 100ms period. But with a 100Hz signal you would expect to get 10 or maybe 9 or 11 interrupts in a 100ms period.

Another technique would be to measure the time period over 10 or 20 interrupts. Use micros() rather than millis().

Measuring the period for 20 interrupts would give you an update every 1.2s at 1000RPM and every 0.15s at 8000RPM.

Thanks for the reply! I changed millis() to micros() and also tried sampling over a full second, sadly with same poor results. What's interesting, if I change "timenow" in the if statement directly to millis() / micros(), function never executes... In my understanding calling directly for millis() in the function would give me the most accurate value of current time, rather than the value assigned to "timenow" before that loop executed... I can't understand why if statement never gets satisfied...

Thanks for the reply! I believe there might be something wrong with this whole if statement. RPM displayed stays exactly the same for a particular signal input regardless if calling for >=100 or >=100000. No matter what 100Hz = ~120RPM, and not 6000RPM. When trying to call directly for time as follows

if(millis() - lastmillis >=1000UL){

statement never executes... I really don't understand what might be the reason for it. Am I calling out lastmillis = millis() at the wrong place? Wouldn't calling for millis() directly be more accurate than timenow? Maybe something wrong with the interrupt itself?

You could use if(millis() - lastmillis >=1000UL){ or you could to update timenow=millis() often, like once per loop(), and move the lastmillis = millis(); or a lastmillis = timenow; inside of the conditional so it records the last time you did the action.

Folks prefer the timenow=millis(); form, as in https://www.arduino.cc/en/Tutorial/BuiltInExamples/BlinkWithoutDelay so they avoid extra calls to millis() and also do all the time calculations in sync.

1 Like

Yes you did not update timenow + @DaveX comments

1 Like

You are missing my point.

Picked up all of your comments and my code still seems to be detecting only a few interrupts over the defined time period. How could I modify the code to detect 20 interrupts as you mentioned? I tried using micros() and that didn't seem to do much difference. As you probably can tell, coding is not a strong side of mine. I'd highly appreciate slightly more detailed tips and instructions.

considered adding print statements to see what is going on?

void rpm_calc(){

  //~~~Never executes the statement when calling for millis() instead of previously defined timenow~~~//
Serial.print( "timenow - lasmillis");
Serial.println( timenow - lastmillis );
if(timenow - lastmillis >=100UL){
  //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~//
    detachInterrupt(digitalPinToInterrupt(2)); //time sensitive calculations
    
    //~~~rpm offlay close to input Hz... constantly oscillates~~~//
    rpm = rpmcount * 60 ; // convert to Rev per Minute
    //~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~//
    
    led = map(rpm, 0, 8000, 0, 8); // convert rpm to 1-8 values to drive LEDS
    pixel(led); // calling function lighting up pixels of the strip (led=1, 1st led lit etc)
  }
rpmcount = 0;
lastmillis = millis();<<<<<< this will be why the ifie thinge not workies.
attachInterrupt(digitalPinToInterrupt(2), rpm_interr, FALLING);

}

Perhaps seeing the things might make a bit more sense why it no works.

1 Like

You are absolutely right! I modified the code as follow:

if(millis() - lastmillis >=1000UL){

    detachInterrupt(digitalPinToInterrupt(2)); //time sensitive calculations
    rpm = rpmcount * 60 ; // convert to Rev per Minute

    led = map(rpm, 0, 8000, 0, 8); // convert rpm to 1-8 values to drive LEDS
    pixel(led); // calling function lighting up pixels of the strip (led=1, 1st led lit etc)
    lastmillis =millis();
  }
attachInterrupt(digitalPinToInterrupt(2), rpm_interr, FALLING);
rpmcount = 0;
}

The if statement works now. Sadly over the defined period of time when sending 100Hz wave, code detects only 2 interrupts (reading 120RPM). When sending 500Hz wave, it detects 9-10 interrupts (reading 540-600Hz). It's far away the the intent, 10Hz signal should be detected as 600RPM , 50Hz as 3000RPM etc. Trying to convert events in second to approximate events in minute. I truly wish I was better at this coding thingy...

1 Like

I think I might change the interrupt code:

//volatile int rpmcount = 0;
volatile unsigned long period = 0;

void rpm_interr(){
  static byte rpmcount = 0;
  static unsigned long lastTime = 0;
  rpmcount++;
  if (rpmcount == 20) {
    rpmcount = 0;
    unsigned long timeNow = micros();
    period = timeNow - lastTime;
    lastTime = timeNow;
  }
}

Your code can then use period to calculate the rpm:

rpm = 60000000UL * 20 / period;
1 Like

It takes time. You need to learn and mostly master simple things first, so you have a solid foundation. In that regard, it's like math. Chapter 12 looks like nonsense unless you've done 1,2,3...

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