Counting the number of pulses in a second

Question

For my speedometer application, I want to count the number of pulses occurring every second and convert it to speed. So for that, I need to count the number of pulses occurring at a given input pin for one second. This count is used to convert to speed. The count should go to zero and start over after every second. But I don't get how to set a limit of one second and start over every second. So far I have come up to this:

int count = 0;
float lasttime = 0;
float currenttime = 0;
const int speedpin = 52;
void setup()
{
 Serial.begin(115200);
 pinMode(speedpin, INPUT);
}
void loop()
{
 currenttime = millis();
 if (currenttime - lasttime <= 1000)
 {
 if (digitalRead(speedpin == HIGH))
 {
 count = count + 1;
 }
 else
 {
 count = count;
 }
 }
 else
 {
 count = 0;
 }
 Serial.println(count);
}

• chrisl: You may want to have a look at attachInterrupt(), which is often used for these things, because with this you will not miss a pulse. Then you should also read more about interrupts, so that you understand, what you are coding with it.
• KIIV: @chrisl It starts missing pulses much faster than using external clock source to the counter. Depends on how many mcu cycles takes to handle whole ISR.
• DataFiddler: @KIIV: if the pulses do not bounce (as mechanical switches do), attachInterrupt() is rather convenient and the counting itself is very fast. If there are less than 256 pulses/sec to expect, handling of this byte counter in the main loop is super easy as well.
• VE7JRO: How are you planning to display the output from your speedometer application? Will you use the serial monitor, a 7 segment display module, or maybe a 16 x 2 LCD?
• Jackie: Right now am seeing in the serial monitor. But in the application, I will display it in an LCD screen

Answer 1

As already explained in Michel Keijzers' answer, instead of counting how many times you find the input HIGH, you should count the LOW → HIGH transitions. This requires saving the previous input state and counting a transition only when the input is HIGH while it was LOW last time you saw it:

static int last_pin_state = LOW;
int pin_state = digitalRead(SPEED_PIN);
if (pin_state == HIGH && last_pin_state == LOW) { // rising edge
 count++;
}
last_pin_state = pin_state;

For displaying the count every second, use the technique explained in the Blink without delay Arduino tutorial:

static uint32_t last_time = 0;
if (millis() - last_time >= PERIOD) {
 Serial.println(count);
 count = 0;
 last_time += PERIOD;
}

Note that you have to reset the count every time you print it.

VE7JRO suggests using interrupts. Interrupts save you the pain of writing the edge detection. On the other hand, they also make things slightly more complicated, because you now have to worry about atomicity, race conditions, volatile variables and such. There is no need to use interrupts unless the pulses are so fast (or your loop() is so slow) that you may miss a pulse. If your loop() gets bigger and slower, and you risk missing pulses, then interrupts can really help. But the proper way to use them is to count the pulses in the ISR. If you use the ISR only to rise a flag, you loose the speed advantage of the interrupts, which is the whole point of this approach.

The interrupt service routine would then be like this:

volatile unsigned int count;
void count_pulse()
{
 count++;
}

Then, the loop only has to print and reset the count:

void loop()
{
 // Periodically print and reset the count.
 static uint32_t last_time = 0;
 if (millis() - last_time >= PERIOD) {
 noInterrupts();
 unsigned int count_copy = count;
 count = 0;
 interrupts();
 Serial.println(count_copy);
 last_time += PERIOD;
 }
}

Note that the variable count is shared between the ISR and the main loop. That's why it has to be declared volatile. Also, its access within loop() has to be protected within a critical section delimited by a noInterrupts()/interrupts() pair, otherwise you would have a race condition. Also note that, in order to keep the critical section as short as possible, the value of count is only copied within the critical section, and the actual printing is deferred to after the critical section is finished.

Answer 2

First align your code:

int count = 0;
float lasttime = 0;
float currenttime = 0;
const int speedpin = 52;
void setup()
{
 Serial.begin(115200);
 // put your setup code here, to run once:
 pinMode(speedpin, INPUT);
}
void loop() {
 // put your main code here, to run repeatedly:
 currenttime = millis();
 if(currenttime - lasttime <= 1000)
 {
 if (digitalRead(speedpin == HIGH))
 {
 count = count + 1;
 }
 else
 {
 count=count;
 }
 }
 else 
 {
 count=0;
 }
 Serial.println(count);
}

Than the statement count = count assigns a variable to itself which is not needed, and for increment you can use count++, so you get:

int count = 0;
float lasttime = 0;
float currenttime = 0;
const int speedpin = 52;
void setup()
{
 Serial.begin(115200);
 // put your setup code here, to run once:
 pinMode(speedpin, INPUT);
}
void loop() {
 // put your main code here, to run repeatedly:
 currenttime = millis();
 if(currenttime - lasttime <= 1000)
 {
 if (digitalRead(speedpin == HIGH))
 {
 count++;
 }
 }
 else 
 {
 count = 0;
 }
 Serial.println(count);
}

In the loop, you do not count pulses, but only the measurements where a pin is HIGH and when the pin is LOW anywhere within the second, it is set to 0.

What you instead want to do is (assuming you want to count the LOW -> HIGH transients: Make a global variable called e.g. previousState. Now, if the previousState == LOW and the current state of the GPIO is HIGH, only than increase count with one. Also, within the loop, each time update the previousState with the current state.

I leave the implementation to yourself.

• Jackie: But for one second I have to count the pulse. And after that one second is over count should go to zero and start counting again. Thus I need the number of pulses for each second.
• Michel Keijzers: Than you store after one second the number of measured pulses (count) in a separate variable to be processed in the next second (while count already measures the new pulses for that second).

Answer 3

If 'count' comes from a counter or device register, or is kept by an interrupt service routine, you don't ever want to zero the count. If a pulse arrives between the time you read the variable and the time you clear it, you will have failed to count that pulse.

Instead, make sure 'count' is of a suitable length to contain the largest number of pulses you could ever receive in one interval. Then, at each interval, save count to a temporary, say, 'tempCount'. Subtract the current 'tempCount' from the previous count, 'prevCount', in which you stored 'count' the previous time through. The difference will be the number of pulses counted in this interval. Use it or save it or whatever you need to do with it. Then store 'tempCount' into 'prevCount', and you're done.

No missed pulses, no overflow errors.

• Edgar Bonet: If count comes from an ISR, you can safely read it and clear it in the same critical section. Unless it's only 8 bits, you need that critical section just to read it anyway. If count comes from a hardware counter, then you are right, it should never be cleared.
• JRobert: Yes, if done in a critical section. But if background (ISR) code is collecting pulses and foreground code is calculating the counts, there would still be an opportunity for errors. Using a continuous counter (and it's only 1 byte wide), you could avoid the need to turn off interrupts again.