My improvements for the FreqCount library

[hackdog]

In the past I've built a simple Clapp oscillator to measure the inductance of unknown coils, by measuring the oscillation frequency with a frequency counter and doing some math. Recently I realized that a Teensy or Arduino can do that just as well and can additionally print the correct uH value directly. Your FreqCount library was ideally suited for this task.

While studying how FreqCount actually worked, I noticed a few areas for improvement. One thing led to another and before I knew it, I was knee-deep in the timer hardware specs and AVR assembly!

The original code uses the difference between two ever-increasing counters (previous total and current total) to calculate the actual count during the current gate interval. But given enough time and a high enough frequency, count_msw, count or count_prev can overflow, because count_msw is never decreased. This could cause subtle bugs which only occur after a certain amount of time. To avoid that scenario, I've added a "counter_reset" instruction to the hardware abstraction layer. FreqCount now simply measures the frequency, reports it and resets for the next round -- no need to deal with the previous count anymore. I don't have a Teensy 3 so I haven't been able to test my counter_reset() implementation for the Freescale MCU. From the reference manual I gather that it suffices to toggle the Timer Enable bit, but you'd have to test it in practice.

I've also added some documentation in a few places and eliminated variables "length" (redundant, equal to gate_length), "index" (gate_index is sufficient; it's incremented before the if so the effect is the same) and "count" (can directly use count_output).

The new code is smaller, more reliable and a bit easier to understand. Accuracy is identical: for a frequency of 87420 Hz, as measured by my frequency counter, the Arduino reads only about 3 to 4 Hz less for either version. Not bad at all!

 

[PaulStoffregen]

I've added a link to this forum thread on the FreqCount web page. Hopefully that will help people find this code.

I'm a little puzzled by the counter reset function. Why is this needed? The 32 bit overflow should not be a problem, since 2 uint32_t are subtracted, which automatically gives the correct elapsed count even when there's overflow.

Writing to the counter seems risky. It's possible to introduce tiny timing errors. By always allowing it to free-run and subtracting 2 reads from each other, you can never "lose" any edges. The worst case scenario is one or more extra counts in one reading and the next one short by that amount. But if you write to the counter, the risks of timing errors are completely different.

Maybe resetting the counter is really better?

I'd really like to hear some feedback from people who've tried this version.

 

[hackdog]

I fully agree that more extensive testing is needed, especially with higher frequencies.

My reasoning about the overflow situation was as follows. Let's say f = 5MHz. If count_msw overflows then count_output also overflows. This will happen when FreqCount has been running for (2^32) / (5 * 10^6) = 859 seconds (14m 19s). It's reasonable to assume some code would use FreqCount for this time or longer. What would be the impact? count would overflow, becoming a small uint32 again. count_prev would remain a large uint32. At this point, I'm not sure that count_output = count - count_prev yields the intended answer. Unfortunately I don't have a benchtop function generator to test this scenario. To avoid the overflow altogether, possibly trading accuracy for long-term stability, I wrote counter_reset().

I did a diff on the generated assembly for my version and v1.1. The changes in the ISR did not seem to have a negative impact on timing, and a before/after check on an ATmega 328 yielded the same accuracy for 87 kHz, so I had good faith in it. Nevertheless you may be right about the risk of tiny timing errors; more testing will hopefully shed some light on this.

 

[PaulStoffregen]

What would be the impact? count would overflow, becoming a small uint32 again. count_prev would remain a large uint32. At this point, I'm not sure that count_output = count - count_prev yields the intended answer.

This does indeed work as intended. Subtracting 2 unsigned integers does give the correct answer, even when there's overflow. You can easily verify this with any number of test cases. At long as the difference is less than 2^31, subtracting after overflow still works.

The exact same issue occurs with subtracting 2 numbers from millis() and micros(). It's been discussed over and over on the Arduino forums and developer mail list.

 

[robsoles]

I've added a link to this forum thread on the FreqCount web page. Hopefully that will help people find this code.

I'm a little puzzled by the counter reset function. Why is this needed? The 32 bit overflow should not be a problem, since 2 uint32_t are subtracted, which automatically gives the correct elapsed count even when there's overflow.

Writing to the counter seems risky. It's possible to introduce tiny timing errors. By always allowing it to free-run and subtracting 2 reads from each other, you can never "lose" any edges. The worst case scenario is one or more extra counts in one reading and the next one short by that amount. But if you write to the counter, the risks of timing errors are completely different.

Maybe resetting the counter is really better?

I'd really like to hear some feedback from people who've tried this version.

Resetting the counter is definitely not better.

I've not looked at any version of freqcount. Work made me figure out how to make a fast and accurate (guitar) tuner using an attiny84, since then I've written a(n improved) simile of it for Teensy 3.x - I cannot share the methodology which actually made it fast at detecting the pitch being played but I feel qualified to comment on resetting counters;

Any iteration of the code in which I modified the contents of the counter I was relying on to determine (base) frequency was simply not accurate.

 

[hackdog]

Good to know that the original algorithm works fine after all, and that resetting a running counter should be avoided. That pretty much settles this discussion.

 

[NPlayer]

I have a Teensy 3.1, and I'm using the Serial_Output code from this library to measure a 4MHz clock signal. It doesn't seem to work (probably my fault). I've used the same code to measure a 60Hz signal with consistents results. The same with another one with 9 kHz aprox. But with the 4MHz I can't get good results. Right now I'm reading 351327. I also pass the signal though the circuit Paul has on the FreqCount webpage, but the data is wrong. I should be reading something near 4000000, right?

Thank you!

 

[Nantonos]

Given that the major change between this library and Paul's version is that this library resets a counter; and that discussion here reveals that the counter does not need to be reset and resetting it may be harmful to accuracy, should this still be linked from FreqCount as an improved version?

 

[PaulStoffregen]

Oh, opps, I completely forgot the web page had been changed. I'd taken that link off.

If anyone does post another improved version, I'll probably still be willing to give it the "benefit of doubt" and link first, and figure out later if it really is an improvement.