Arduino UNO Low frequency 0-5V square wave measuring

Hello!

I'm working on project with my motorbike and i would like to trigger the arduino to measure RPM frequency and then make action depending on that.

It should look like that: 1. Triggering measurement with button, 2. Measuring frequency for 200ms, 3. Making output LOW or HIGH depending on frequency.

My bike revs up to 12k RPM, and crankshaft sensor wire gives me 0-5V square wave which is double the crankshaft rotation frequency, so frequencies I would like to measure are up to 400Hz.

I don't have much experience with Arduino, so far i've only made some projects like "button press -> action", so I dont know how to make such things like frequency counting in given time.

Can anyone provide me with sketch that will measure frequency? That would be really helpfull

Can anyone provide me with sketch that will measure frequency?

This is done quite often with Arduino. A search of this site or google search for "Arduino measure rpm" will get thousands of hits.

I’ve found RPM counter on google and modified it a bit.

const int hallPin = 12;
const int led = 13;
int hallState = 0;
volatile int rpmcount = 0;
int rpm = 0;
unsigned long lastmillis = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(hallPin, INPUT);
  attachInterrupt(0, rpm_fan, FALLING);//interrupt zero (0) is on pin two(2), there is 0-5V square wave signal wire connected
}

void loop() {
  hallState = digitalRead(hallPin);
    if (hallState == HIGH) {
      
        lastmillis = millis(); //Set last millis to be the same as millis
         rpmcount = 0; // number of nterrupts to 0

         
              while (millis() - lastmillis <= 100){ /* for 100ms counting*/
                rpm = rpmcount * 30; /* Convert frecuency to RPM,  * 30, because two interupts per full rotation.*/
                    }
    
                          if (rpm <= 4000) {}
                          else if (rpm <= 8000)&&(rpm > 4000){
                            digitalWrite(9, HIGH);
                            delay(80);
                            digitalWrite(9, LOW);
                          }
                          else if (rpm <= 10000)&&(rpm > 8000){
                            digitalWrite(9, HIGH);
                            delay(70);
                            digitalWrite(9, LOW);
                          }
                          else if (rpm > 1000){
                            digitalWrite(9, HIGH);
                            delay(60);
                            digitalWrite(9, LOW);
                          }
    else {}
  

}
}
void rpm_fan(){ /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
rpmcount++;
}

Will it work?

Hi,

Welcome to the forum.

Please read the first post in any forum entitled how to use this forum. http://forum.arduino.cc/index.php/topic,148850.0.html then look down to item #7 about how to post your code. It will be formatted in a scrolling window that makes it easier to read.

Thanks... Tom... :)

Your timing code effectively does a delay(100). For that purpose you better replace the while loop by

delay(100);
rpm = rpmcount * 30; /* Convert frecuency to RPM,  * 30, because two interupts per full rotation.*/

Since your code cannot do anything else during that time, it could poll the sensor as well. without using an interrupt.

So it should be lke that:

const int hallPin = 12;
const int led = 13;
int hallState = 0;
volatile int rpmcount = 0;
int rpm = 0;
unsigned long lastmillis = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(hallPin, INPUT);
  attachInterrupt(0, rpm_fan, FALLING);//interrupt zero (0) is on pin two(2), there is 0-5V square wave signal wire connected
}

void loop() {
  hallState = digitalRead(hallPin);
    if (hallState == HIGH) {
      
        lastmillis = millis(); //Set last millis to be the same as millis
         rpmcount = 0; // number of nterrupts to 0

           delay(100);
                rpm = rpmcount * 30; /* Convert frecuency to RPM,  * 30, because two interupts per full rotation.*/
                    }
    
                          if (rpm <= 4000) {}
                          else if (rpm <= 8000)&&(rpm > 4000){
                            digitalWrite(9, HIGH);
                            delay(80);
                            digitalWrite(9, LOW);
                          }
                          else if (rpm <= 10000)&&(rpm > 8000){
                            digitalWrite(9, HIGH);
                            delay(70);
                            digitalWrite(9, LOW);
                          }
                          else if (rpm > 1000){
                            digitalWrite(9, HIGH);
                            delay(60);
                            digitalWrite(9, LOW);
                          }
    else {}

}

void rpm_fan(){ /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
rpmcount++;
}

The other thing is - I’d like Arduino to run only once upon triggering, what I mean is:
HallState == HIGH → count RPM → Do action depending on RPM → Wait for HallState to become LOW to finish one loop → HallState again HIGH → and over and over…

What code could do that?

Try to become more familiar with C, and study some example sketches demonstrating what you want to achieve, like Debounce and StateChangeDetection. The Auto Format IDE command (CTRL+T) will improve the readability of your code, and also will reveal what's wrong with your block structure.

Ok, I’ve done something like this, can you check my understanding at notes?

const int hallPin = 12;
const int led = 13;
volatile int rpmcount = 0;
int rpm = 0;
int hallState = 0;
int lasthallState = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(hallPin, INPUT);
  attachInterrupt(0, rpm_fan, FALLING);//interrupt zero (0) is on pin two(2), there is 0-5V square wave signal wire connected
}

void loop() {
  hallState = digitalRead(hallPin); //Reads current hall state

  if (hallState != lasthallState) { //if current hall state is different than last hall state then:
    if (hallState == HIGH) { //check if current hall state is HIGH, if yes then:

      rpmcount = 0; // set number of interrupts to 0, to begin counting

      delay(100); //wait 100ms, it will run rpm_fan during that time and count LOWs on pin 2
      rpm = rpmcount * 30; // Convert frecuency to RPM,  * 30, because two interupts per full rotation.
    }

    if (rpm <= 4000) {} //if rpm is lower than 4000 then do nothing
    else if ((rpm <= 8000) && (rpm > 4000)) { // if 4000<rpm <= 8000 then at pin 9 - HIGH -> 80ms -> LOW
      digitalWrite(9, HIGH);
      delay(80);
      digitalWrite(9, LOW);
    }
    else if ((rpm <= 10000) && (rpm > 8000)) {
      digitalWrite(9, HIGH);
      delay(70);
      digitalWrite(9, LOW);
    }
    else if (rpm > 10000) {
      digitalWrite(9, HIGH);
      delay(60);
      digitalWrite(9, LOW);
    }
    else {}
    delay(50);

  }
  lasthallState = hallState;
}


void rpm_fan() { /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
  rpmcount++;
}

The other thing is - Is Arduino with that code quick enough to read up tu 40 LOWs during that 100ms?

Now the code does something close to your goal, but not perfectly.

How often do you want to update rpm? How often do you want to output the pulses?

Check the ranges of your if-statements. Eventually a flow chart of your code will help you matching your expectations and what really happens.

I would like to update RPM only when i trigger it with hall sensor, the most frequently I will do that is once per 2-3 seconds.
II want to output signal same as RPM update, so once per 2-3 seconds.

Ahh, I see… I made one } to early, namely imediately after RPM conversion. Now I have moved all RPM-dependant if-statements inside “hallState == HIGH”. I have also changed RPM conversion multiplier to 300:

const int hallPin = 12;
const int led = 13;
volatile int rpmcount = 0;
int rpm = 0;
int hallState = 0;
int lasthallState = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(hallPin, INPUT);
  attachInterrupt(0, rpm_fan, FALLING);//interrupt zero (0) is on pin two(2), there is 0-5V square wave signal wire connected
}

void loop() {
  hallState = digitalRead(hallPin); //Reads current hall state

  if (hallState != lasthallState) { //if current hall state is different than last hall state then:
    if (hallState == HIGH) { //check if current hall state is HIGH, if yes then:

      rpmcount = 0; // set number of interrupts to 0, to begin counting

      delay(100); //wait 100ms, it will run rpm_fan during that time and count LOWs on pin 2
      rpm = rpmcount * 300; // Convert frecuency to RPM,  * 300 = *10 * 30,, *10 - to make it into 1sec, *30, because two interupts per full rotation.


      if (rpm <= 4000) {} //if rpm is lower than 4000 then do nothing
      else if ((rpm <= 8000) && (rpm > 4000)) { // if 4000<rpm <= 8000 then at pin 9 - HIGH -> 80ms -> LOW
        digitalWrite(9, HIGH);
        delay(80);
        digitalWrite(9, LOW);
      }
      else if ((rpm <= 10000) && (rpm > 8000)) {
        digitalWrite(9, HIGH);
        delay(70);
        digitalWrite(9, LOW);
      }
      else if (rpm > 10000) {
        digitalWrite(9, HIGH);
        delay(60);
        digitalWrite(9, LOW);
      }
      else {}

    }
    delay(50);
  }
  lasthallState = hallState;
  else {}
}


void rpm_fan() { /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
  rpmcount++;
}

Now the flow chart is:
Read hall state → check if hall state has changed, if yes → check if hall state is now HIGH, if yes → set rpm count to 0 → delay 100ms to count how many LOWs has occured during that time → calculate RPM bu multiplying rpm count by *300 → send OUTPUT signal depending on RPM → delay 50ms for debounce → set current hall state as last hall state.

Now I’m also thinking if i should detach interrupt after that delay(100); and then attach it again after RPM dependant action? I mean if it should look like that:

const int hallPin = 12;
const int led = 13;
volatile int rpmcount = 0;
int rpm = 0;
int hallState = 0;
int lasthallState = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(hallPin, INPUT);
  attachInterrupt(0, rpm_fan, FALLING);//interrupt zero (0) is on pin two(2), there is 0-5V square wave signal wire connected
}

void loop() {
  hallState = digitalRead(hallPin); //Reads current hall state

  if (hallState != lasthallState) { //if current hall state is different than last hall state then:
    if (hallState == HIGH) { //check if current hall state is HIGH, if yes then:

      rpmcount = 0; // set number of interrupts to 0, to begin counting

      delay(100); //wait 100ms, it will run rpm_fan during that time and count LOWs on pin 2
      detachInterrupt(0); //detach interrupt for calculating

      rpm = rpmcount * 300; // Convert frecuency to RPM,  * 300 = *10 * 30,, *10 - to make it into 1sec, *30, because two interupts per full rotation.


      if (rpm <= 4000) {} //if rpm is lower than 4000 then do nothing
      else if ((rpm <= 8000) && (rpm > 4000)) { // if 4000<rpm <= 8000 then at pin 9 - HIGH -> 80ms -> LOW
        digitalWrite(9, HIGH);
        delay(80);
        digitalWrite(9, LOW);
      }
      else if ((rpm <= 10000) && (rpm > 8000)) {
        digitalWrite(9, HIGH);
        delay(70);
        digitalWrite(9, LOW);
      }
      else if (rpm > 10000) {
        digitalWrite(9, HIGH);
        delay(60);
        digitalWrite(9, LOW);
      }
      else {}
      attachInterrupt(0, rpm_fan, FALLING); //enable interrupt again
    }
    else {}
    delay(50);
  }
  lasthallState = hallState;

}


void rpm_fan() { /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
  rpmcount++;
}

You're learning quickly :-)

When you compile your code, the compiler should warn about the misplaced "lastHallState=hallState" statement before the last else. That else can be removed, but it does no harm. Your latest suggestion fixed that bug already. Nonetheless this statement should be moved inside the if block, it's required only in case that lastHallState was really different from hallState.

Similarly for the attach/detachInterrupt. I'd attach the interrupt only if required, i.e.

rpmcount = 0;
attachInterrupt(...);
delay(100);
detachInterrupt(...);

The detachInterrupt() or other means (noInterrupts...) is required for reading rpmcount properly, in an uninterruptable (atomic) sequence. Else the interrupt may occur between reading the low and high byte of rpmcount, so that one byte could hold the old value (before the interrupt) and the other one the new value (after the interrupt). Such precaution is always required when reading volatile multi-byte variables, the volatile modifier alone is not sufficient.

Another note on if-else. The rpm tests can be squeezed a bit, if you consider that an else part is executed only if the preceding test failed (was false). Then you can rewrite that part like this:

if (rpm > 10000) ...
else if (rpm > 8000) ...
else if (rpm > 4000) ...

So if I understand You right then i should finnish up with something like that:

const int hallPin = 12;
const int led = 13;
volatile int rpmcount = 0;
int rpm = 0;
int hallState = 0;
int lasthallState = 0;

void setup() {
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  pinMode(9, OUTPUT);
  pinMode(hallPin, INPUT);
}

void loop() {
  hallState = digitalRead(hallPin); //Reads current hall state

  if (hallState != lasthallState) { //if current hall state is different than last hall state then:
    if (hallState == HIGH) { //check if current hall state is HIGH, if yes then:

      rpmcount = 0; // set number of interrupts to 0, to begin counting

      attachInterrupt(0, rpm_fan, FALLING);//interrupt zero (0) is on pin two(2), there is 0-5V square wave signal wire connected
      delay(100); //wait 100ms, it will run rpm_fan during that time and count LOWs on pin 2
      detachInterrupt(0); //detach interrupt for calculating

      rpm = rpmcount * 300; // Convert frecuency to RPM,  * 300 = *10 * 30,, *10 - to make it into 1sec, *30, because two interupts per full rotation.

      if (rpm > 10000) {
        digitalWrite(9, HIGH);
        delay(60);
        digitalWrite(9, LOW);
      }
      else if (rpm > 8000) {
        digitalWrite(9, HIGH);
        delay(70);
        digitalWrite(9, LOW);
      }
      else if (rpm > 4000) {
        digitalWrite(9, HIGH);
        delay(80);
        digitalWrite(9, LOW);
      }
      else {}
    }
    else {}
    delay(50);
  }
  lasthallState = hallState;

}


void rpm_fan() { /* this code will be executed every time the interrupt 0 (pin2) gets low.*/
  rpmcount++;
}

The RPM ifs - program will only run till it finds one that is true, right? So if one with rpm >8000 will be true, then it wont proceed to check if rpm >4000 is also true?

I put lastHallstate = hallState after } closing the if(hallState !=lastHallState) like it was in example sketch. So is that right or wrong?

Kamool: The RPM ifs - program will only run till it finds one that is true, right? So if one with rpm >8000 will be true, then it wont proceed to check if rpm >4000 is also true?

Right.

I put lastHallstate = hallState after } closing the if(hallState !=lastHallState) like it was in example sketch. So is that right or wrong?

It's not wrong, but executed on every iteration of loop(). I'd move it to the begin of the if-statement, where It's know that an update is required, and lastHallState will not be used afterwards. Then it becomes obvious that the state is updated as required, and cannot get lost or misplaced on later edits of the code.

I've got one more question.

Will "delay(100)" stop both loop and rpm_fan functions or just loop one, and during these 100ms it will still run rpm_fan counting FALLINGs?

A delay() stops all code, except interrupts. The effects depend on the implementation of the libraries.

So if it stops everything then it will stop rpm_fan function as well? Then it won't count falling signals, so it will not work. Or I understand it wrong?

rpm_fan seems to be an interrupt handler, which will continue running.

Ok, I will try that, thanks for help