Counting pulses while carrying out other tasks

[fall-apart-dave]

So following on from this thread where I actually solved my initial query...

https://forum.arduino.cc/index.php?topic=549221.0

I have now moved on (somehow!) to making a more streamlined way of counting pulses without blocking things too much.

I am using an Arduino Mega.

So, I will have this process happening on two pins, with different frequency of pulses.

One will be for engine RPM.

One will be for speed.

RPM will be connected to the ignition coil via a conditioning circuit to give a ground every pulse.
Speed will be through a hall sensor, using the original eddie current speedo reading the rotating magnet.

//The arduino will also be looking at various other parameters at the same time (oil temp, oil pressure, fuel level, turbo pressure, lambda, and a few others). The other parameters are mostly a read analogue signal, do a little maths, crank out a PWM signal proportional to the input based on the response curve of that particular sensor. these are mostly done and working. I plan to have these all as functions to call in a particular order.

So... On to the pulses.

I need to count them, accurately.

Maximum pulses on RPM input will be about 250 Hz (2 pulses per crank revolution, max revs a little over 7000)
Maximum pulses on speed input will be significantly lower, exact frequency not yet calculated but I do have the maths somewhere. But circumference of the wheel, max speed of about 90mph (this car is terrifying at low speeds!), wheel is a 15" rim with a 120/60 tyre, so nothing at all crazy).

So, I can see two ways of doing this:

Set a timer going in loop counting pulses. After an elapsed time, call the function for calculating RPM using pulses per interval.

or

Measure the time between two rising edges, and then call the function for calculating RPM using interval between pulses.

Both methods will mean I have to block to count pulses.

What I would like to do is have something counting time between two falling edges, so that I can call that last recorded value and do some calculations, or have it give me the number of pulses since I last checked, and the time since last check, and then I can do calculations with that.

Alternatively, does there exist a nice little piece of hardware that will count away that I can poll then reset at certain time intervals?

Something like this?

or is there something on board the Arduino Mega that already supports this?

Can anyone give me a little more info about how this works:

http://interface.khm.de/index.php/lab/interfaces-advanced/arduino-frequency-counter-library/

Finally, a HUGE thank you to GoForSmoke for all the help so far given! It is very much appreciated!!

His responses can be seen on the second page of the other thread I linked at the top there^^^

[MorganS]

Yes there is dedicated hardware on the Mega to do this already.

You can use a timer module in "counter" mode but I don't have a link to a good example.

The more common method is to use interrupts and keep your own timer. A quick google for "Arduino RPM measurement" should find some good examples. The frequency counter idea is more complex than you need.

The time between adjacent pulses has a lot of jitter. The calculated RPM will jump around a LOT. Count 50 or 100 pulses and use millis() to get the time between first and last.

[Shuzz]

Use a timer in input capture mode. In this mode, the timer will stop and call an ISR when an external interrupt signal is received.
You'll need to make sure that your timer does not overflow between pulses, especially on low rpm, so select your prescaler accordingly.
(Or you could add a timer overflow isr and simply count the overflows and process those, too. But for car RPM measurement, a slower running timer should do the trick without sacrificing accuracy.)

Write an ISR which will take the value of the timer and store it somewhere, then reset the timer.

You should average over some reading to reduce/remove jitter, but not too many. For storing the data, I'd suggest you use a ring-buffer and always overwrite the oldest reading. Takes a bit of wrangling with pointers/counter variables but it's actually not that much work so it can easily be done from inside the ISR.

[fall-apart-dave]

What do you mean about the time between pulses having jitter? As in the accuracy will be very poor? so best to count pulses for a given time to get a better sample?

There will be instances where the engine is off too - so in those cases I need it to just return a zero.

Best get googling ring buffers...

External IC is very tempting though!

[Robin2]

fall-apart-dave:
Maximum pulses on RPM input will be about 15KHz (2 pulses per crank revolution, max revs a little over 7000)

7000 RPM = approx 117 RPS times 2 gives 234 Hz and not 15khz - or am I missing something?

This code is an extract from a program I use to control the speed of a small DC motor that can run up to 18,000 RPM. It is not complete but it should give you the idea. The same Arduino also listens for wireless messages with the required speed and does PID calculations to adjust the speed.

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

[MorganS]

Yes. Accuravy of adjacent pulses is poor.

Yes, counting for a fixed time also works and can show zero correctly.

[PieterP]

I'm just gonna leave this here:
Nick Gammon: Timers and Counters
It contains multiple examples for frequency counters and more.

Pieter

[fall-apart-dave]

Robin2:
7000 RPM = approx 117 RPS times 2 gives 234 Hz and not 15khz - or am I missing something?

This code is an extract from a program I use to control the speed of a small DC motor that can run up to 18,000 RPM. It is not complete but it should give you the idea. The same Arduino also listens for wireless messages with the required speed and does PID calculations to adjust the speed.

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

You are right - 15KHz is the PRF of a bit of equipment I'm setting up for work right now... had that number in my head, serves me right for juggling.

PieterP:
I'm just gonna leave this here:
Nick Gammon: Timers and Counters
It contains multiple examples for frequency counters and more.

Pieter

Yes I've read those. but looking for something that will not block as it works. they have been a good resource (I cited them in the other thread) - thank you for the reminder though

[fall-apart-dave]

HEF4020B.pdf

This looks like it will do what I need, without the need to worry about blocking and my code, and return a nice parallel output that I can just read...

Reading up on interrupts is making my head hurt...

[Robin2]

fall-apart-dave:
Yes I've read those. but looking for something that will not block as it works.

My code doesn't

...R

[fall-apart-dave]

Robin2:
My code doesn't

...R

I'm looking at that now...

Slowly understanding... (Normally I pick things up way quicker than this, but coding still feels like a fog even now).

So your code:

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;
}

What does "ISR" stand for?

Is my understanding of this correct? :

Checks to see if newISR is "True"
If not, it does nothing? Just carries on to the void myISR function and sets newISR to "True"
If true, then...
You are saving the last ISR time in microseconds as "previous" by copying over curent value in "Latest"
Then updating "Latest" with the time in microseconds "now"
Calculating microseconds elapsed this period

And I am guessing that period is between any two interrupts? The part I am struggling to understand here is how the arduino knows when to take the new time?

Apologies for being slow... I'm trying to understand it!

[PieterP]

fall-apart-dave:
What does "ISR" stand for?

Interrupt Service Routine

[fall-apart-dave]

Robin2:
My code doesn't

...R

I think the penny has finally dropped...

The code you posted... Is my interpretation of how it works correct?

The loop checks to see if newISR is true. If false, it doesn't do anything.

If true, works out microseconds since last interrupt.

The myISR function is the interrupt... When an interrupt is received, newISR is set to true and time that the interrupt was received is assigned to isrMicros?

Is that correct?

[Robin2]

fall-apart-dave:
The myISR function is the interrupt... When an interrupt is received, newISR is set to true and time that the interrupt was received is assigned to isrMicros?

Is that correct?

Yes.

From Reply #10

If not, it does nothing? Just carries on to the void myISR function and sets newISR to "True"

This is a fundamental misunderstanding of how the code is managed. The sequence of operation NEVER proceeds "down the page" from the end of one function to the start of the next function. When the code gets to the end of a function it goes back to the next line of code after the line that called the function. This means that the order in which you write the functions does not matter. The loop() function is called from a hidden function called main() and when loop() ends the program returns to main() which calls loop() again.

An Interrupt Service Routine operates outside the main program flow and it is triggered by some event to "interrupt" the normal flow, do its thing, and return control into the normal flow as though the ISR had never happened.

...R

[fall-apart-dave]

Robin2:
Yes.

From Reply #10This is a fundamental misunderstanding of how the code is managed. The sequence of operation NEVER proceeds "down the page" from the end of one function to the start of the next function. When the code gets to the end of a function it goes back to the next line of code after the line that called the function. This means that the order in which you write the functions does not matter. The loop() function is called from a hidden function called main() and when loop() ends the program returns to main() which calls loop() again.

An Interrupt Service Routine operates outside the main program flow and it is triggered by some event to "interrupt" the normal flow, do its thing, and return control into the normal flow as though the ISR had never happened.

...R

Ye;, realised that after I had slept and came back with fresh eyes.

So, a question if I may... The myISR() - does this work as you have written it? Or does this need assigning to an interrupt pin? I am reading this attachInterrupt() - Arduino Reference in an effort to understand - and I feel like I am still missing something glaringly obvious.

[Robin2]

fall-apart-dave:
So, a question if I may... The myISR() - does this work as you have written it? Or does this need assigning to an interrupt pin?

You need to use attachInterrupt() in setup() to connect it to the pin. Probably like this

attachInterrupt(digitalPinToInterrupt(2), myISR, RISING);

...R

[fall-apart-dave]

I thought so, I wanted to make sure that I'd not misunderstood myISR() was not something embedded that I'd missed.

Thank you very, very much! I think I am just about there with this! It's just a damn shame that I have nothing to test it with except a PPS pulse from these here GPS box... Might go see if the engine room has a function generator (I'm on a ship right now).

[fall-apart-dave]

taken from my other thread, as it is relevant here and both these threads are about different parts of the same sketch so there is some inevitable crossover. This is what I have come up with, using the code example you gave me (I've not assigned interrupts yet).

volatile int PulsesSinceRefresh; //counting number of interrupts since display was last refreshed
volatile unsigned long rollingaverageRPM; //variable to give me a rolling average of revs, to refresh the display with
volatile unsigned long isrMicros;
unsigned long microsThisRev;
unsigned long latestIsrMicros;
unsigned long previousIsrMicros;
volatile boolean newISR = false;
int RPM; // product of time between interrupts (falling edge)
byte writeToPort; // this is for displaying the result in binary, but not in the way you normally would (I need each digit displayed as a 4 bit binary, rather than all four digits displayed as one binary string)
int UpdateRatePin = A0; // Potentiometer on here to give 0-5v to allow manual on the fly adjustment of display refreshes
int UpdateRate; //Stores value of A0 to use as a time (0-1024ms)
unsigned long startMillis; //Starts counting
unsigned long currentMillis;  //current count


void setup() {

  attachInterrupt(digitalPinToInterrupt(2), myISR, FALLING);
  startMillis = millis();

}

void loop() {

  UpdateRate = analogRead(UpdateRatePin); //assign value to the update rate

  //looks to see if time equal to the value of the reading on A0. If no, skip, if yes, call DisplayRefresh()

  currentMillis = millis();
  if (currentMillis - startMillis >= UpdateRate);
  {
    DisplayRefresh();
    startMillis = currentMillis;
  }

  //Does the measuring of RPM

  {
    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 - previousIsrMicros;
    }

    //Averaging of the value for RPM to store until DisplayRefresh() is called

    RPM = 30000000 / microsThisRev; //to give me RPM
    rollingaverageRPM = rollingaverageRPM + RPM / PulsesSinceRefresh; // this is to average the RPM count ready for display




  }
}

void DisplayRefresh() {

  //This takes each individual digit of RPM and outputs them seperately as individual binary numbers on the ports. I.E 3294 would be displayed as 0011 , 0010 , 1001 , 0100   NOT 110011011110

  writeToPort = (RPM / 1000 % 10) << 4;
  writeToPort = writeToPort + (RPM / 100 % 10);
  PORTE = writeToPort + (RPM / 100 % 10);
  writeToPort = (RPM / 10 % 10) << 4;
  writeToPort = writeToPort + (RPM % 10);
  PORTD = writeToPort;
  rollingaverageRPM = 0; //resets the rolling average
  PulsesSinceRefresh = 0; //resets the pulse since last update count

}
void myISR() {
  isrMicros = micros();
  newISR = true;
  PulsesSinceRefresh = PulsesSinceRefresh + 1; //increments the interrupt count by 1 each time
}

I still have no way to test it yet... I'd very much value your opinion though.

EDIT Updated the code to make it more complete

[Robin2]

I am not good at commenting on code without info about how it actually behaves when tested.

In this piece of code

      noInterrupts(); // pause interrupts while we get the new value,
      latestIsrMicros = isrMicros;
      newISR = false;
      interrupts();

you should also save the value of PulsesSinceRefresh. The purpose of temporarily pausing interrupts is to prevent the possibility that the variable can be changed by the interrupt while you are reading the value.

Also I would make PulsesSinceRefresh an unsigned long and I would never bother to set it back to 0. Just subtract the previous version from the current version to get the difference.

I'm not sure if this will work

RPM = 30000000 / microsThisRev; //to give me RPM

At the very least you should change it to

RPM = 30000000UL / microsThisRev; //to give me RPM

to ensure the calculation uses unsigned long values

Do you really need the RPM value? Division is very slow and I always try to avoid it.

In C/C++ programming style things that start with Uppercase letters are usually Constants and I would use pulsesSinceRefresh as the name for that variable. Obviously the compiler won't care.

...R

[fall-apart-dave]

Ok, thank you.

But, if interrupts are blocked, then pulsesSinceRefresh won't update anyway while the value is being read, or have I misunderstood?

What is the reason for not resetting it? Or is it just a case of different folks, different strokes?

RPM, yes I need that, as this will output to the nixie tubes to display engine RPM. Is there a more slick way to derive engine RPM from the pulses from the ignition coil?