Optical tachometer

Hello, I’m working on optical tachometer that measures a delay between two pulses and calculates rpm. However, there must be some hidden problem in my sketch and I’m not able to find it. Arduino doesn’t calculate rpm correctly. It basically does but the delay doesn’t correspond to the incoming signal. rpm keeps fluctuating between cca 2000 to several hundreds of thousands (correct value is about 2800). I built up a square signal generator to simulate sensor signal to test if there’s a problem with the hardware but it isn’t. So I’m fairly sure it’s a software issue. I tried everything that came to my mind, even to disable interrupts in many places in the code, but that didn’t work either. Names of variables are in Czech

prvni_rising = first_rising
druhy_rising = second_rising
otacky = rpm
rozdil = delay
staryCas = oldTime
soucasnyCas = currentTime

#include<LiquidCrystal.h>
LiquidCrystal lcd(12, 11, 6, 5, 4, 3);

volatile unsigned long prvni_rising = 0;
volatile unsigned long druhy_rising = 0;
unsigned long otacky = 0;
unsigned long rozdil = 1000;

volatile bool i = 1;

unsigned long staryCas = 0;
unsigned long soucasnyCas;

void setup() {
  detachInterrupt(digitalPinToInterrupt(2));
  lcd.begin(16, 2);
}

void loop() {
  soucasnyCas = millis();
  if (i == 1) {
    if (soucasnyCas - staryCas >= 500) {

      otacky = (1000000.00 / rozdil) * 60;

      if (rozdil >= 450000) {
        otacky = 0;
      }

      if (druhy_rising == 0) {
        otacky = 0;
      }

      lcd.clear();
      lcd.setCursor(0, 0);
      lcd.print(otacky);
      //  lcd.setCursor(0, 1);
      // lcd.print(druhy_rising);
      //  lcd.print(" RPM");
      //  lcd.print("   ");

      staryCas = millis();
      attachInterrupt(digitalPinToInterrupt(2), mereniOtacek, RISING);

    }
  }
}

void mereniOtacek() {
  i = !i;

  if (i == 0) {
    prvni_rising = micros();
  }

  if (i == 1) {
    druhy_rising = micros();
    detachInterrupt(digitalPinToInterrupt(2));
    rozdil = (druhy_rising - prvni_rising);
  }
}

I have not looked closely at your code but why are you detaching the interrupt? The normal usage is to attach it in setup and leave it attached. Use noInterrupts() followed quickly by interrupts() for the briefest period needed to read the variable that is updated by the ISR.

How many pulses per revolution does your sensor detect?

The following is extracted from a program I use to control the speed of a small DC motor. As you can see it measures the number of microsecs for each revolution (one pulse per rev). It is not complete but should give you the idea.

volatile unsigned long isrMicros;
unsigned long latestIsrMicros;
unsigned long previousIsrMicros;
volatile boolean newISR = false;


void loop() {
  if (newISR == true) {
    previousIsrMicros = latestIsrMicros; // save the old value
    noInterrupts(); // pause interrupts while we get the new value
       latestIsrMicros = isrMicros;
       newISR = false;
    interrupts();
    microsThisRev = latestIsrMicros - previousIsrMicos;
}

void myISR() {
  isrMicros = micros();
  newISR = true;
}

...R

My works similarly. The sensor makes 1 rising per revolution. I put detach in the setup() to be sure no interrupts are executed before loop() proceeds. In the beginning, interrupts are detached, so the main program is not disturbed by them. Then in loop() after 500 millis rpm (otacky) are calculated (they are 0 at this point) and interrupt is attached. When first rising occurs, the current time is written to the prvni_rising variable. Then it exits mereniOtacek() function and does nothing until the second interrupt comes. druhy_rising is set to micros(), interrupt is detached not to mess up those variables and since the delay between two pulses (two interrupts) is now known, we can calculate it. The result is saved into rozdil variable. Then the program jumps back into the loop() and waits until the rest of 500 milliseconds is over. otacky aka rpm is calculated and displayed. Interrupt is attached again and the whole process repeats. This code uses only two necessary interrupts instead of tens of them to measure rpm.

There is no need to worry about other parts of your program being affected by a very short ISR. As well as measuring the time per revolution (for every revolution) my program uses PID to keep the motor speed steady and receives data by wireless to tell it what RPM is required.

...R

If I don't find a solution to my problem, I'll probably go for your version. Even though my code would be more accurate if it worked.

kubajz22: Even though my code would be more accurate if it worked.

Please explain why and by how much.

...R

I tried your code with some modifications and for some reason, microsThisRev changes a bit. I assume in my code this would be suppressed because writing micros into the variable is done in fact immediately after the interrupt has occurred, so their delay should be almost perfectly accurate. But it isn't...

EDIT: For some reason, your code stopped fluctuate so much. :D I'm ok with it.

kubajz22: I tried your code with some modifications and for some reason, microsThisRev changes a bit.

That can happen. But it is not the fault of the program - it is due to variations in the signal that is generating the interrupt.

...R

Signal was good, I don't know why that happened. But now it works great, thanks.