-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add pulseio.FrequencyIn #1144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add pulseio.FrequencyIn #1144
Conversation
…QM periph to get the DFLL frequency
… to point to the proper handler
This reverts commit 5ddeb67.
…s created at the common-hal level
This is relevant to my interests. I have a metro m4 express and a GPS referenced oscillator, is there some useful testing I can do? |
Test? Yes, please! That would be much appreciated. Usage is pretty easy:
In current state, it will not detect over ~512kHz. It should pause detection above that. I'll take the DMA leap eventually... |
I gave this a whirl with an Adafruit Metro M4 Express together with the this GPS referenced RF generator.. In one terminal I connect to the M4, and in the other I connect to the RF generator and send frequency commands. With the frequency set to 1kHz,
With the frequency set to 2kHz,
The two frequency averages are in a ratio of 1.99936:1, which is close to the true 2:1 to within 1 part in 1000, which is fine. However, things are systematically about 5% (5000ppm) low, probably owing to the low quality of the frequency reference on the M4 Express. There's also a pretty high sample to sample variance of around 1%; I don't know whether this is expected or unexpected. When I reprogram the frequency source to 200kHz, it still works. But at 300kHz -- even without actually querying the frequency -- the board hangs.
For values below 1kHz, the results don't seem very good either. For example,
(those last two values, differing by a factor of 9x, were both counted when the input signal is a 1Hz square wave!) |
The TC overflow flag has been shaky throughout my debugging; never could get it to work out sub-1kHz reliably.
I expected this. The usage of FREQM to check the frequency of DFLL is inside the call to get the value; not the interrupt handler. So, the DFLL has likely changed frequencies enough to make a difference when the value is calculated. When I had the frequency check inside the interrupt handler, I got consistent HardFaults. Overall, I am not surprised by your results. Hoped for better, but confirmation some of the issues I encountered is good. Thank you @jepler for running these tests! |
EDIT: This applies to SAMD51 only... Through additional brainstorming, and discussion over in I could use DPLL1, set below 100MHz to ensure that TC4-7 don't exceed that limit. Then, all the kludgeyness of using the FREQM to adjust for DFLL drift wouldn't be necessary. In my quick-looks, DPLL1 isn't used, but I still need to roll through the I lose the "easy button" of using the established timer/clock functions in samd-peripherals, but that's not a biggie in my book. |
Thank you for your continued work on this. I think it should only be added on express boards with space available. We don't have room for new things in the non-express builds. What is the lower bound of what you are trying to measure? What's the high bound? Interrupting on every pulse seems very limiting to the top end. Why not interrupt at a fixed frequency based on the best calibrated clock and count the number of clock cycles over that duration? The TC event action would then be COUNT instead of PPW. The upper bound will be then based on the TC limits (100mhz) and range. |
👍 I'll work out the least intrusive way I can do this (and hopefully not set a nasty precedent).
My very lazy answer to this is "whatever is capable". I think sub-1kHz would be a nice lower bound, but would be easily convinced to raise that when balancing usability and overall firmware impacts. A lot of that depends on implementation (ability to capture count wraparound...). High bound...I invoke Scotty: "I'm giving her all she's got, Captain". 😄
I've spent the last day pondering on your meaning of this. I can't bridge "fixed frequency interrupt and counting cycles" with knowing the state of the pin. Here are a few things I've walked through while thinking about this:
|
The first is closer to what I'm thinking. I should have been more explicit that the approach will use two TCs. One will count cycles of the input frequency and the second will measure the fixed duration (say 1ms). So, the first won't interrupt at all. The second will on overflow to generate the reference period and an interrupt. The second can also be shared if there are multiple frequencies in. In the interrupt handler you read and reset the first TC count and then compute the frequency based on the count and sample period. It'll end up averaging over a number of source cycles but that should help. |
Ahh, two TCs makes sense. Tried switching simply the INTENSET to OVF, and keeping PPW during the shows tonight. Didn't think that through; counter resets automagically when a capture is taken so overflow wouldn't trigger the interrupt until there was no signal. 😄 Working the dual TC approach now...then will work the clock source. |
Seeing as how this PR is REALLY stale, would it be better to close this and open a new PR for my current iteration? I'm close (hopefully this weekend), but have been thinking about this... |
Sure, no need to ask permission. |
Thanks @dhalbert. Only concern was obfuscating the history. I was planning to reference this PR in the new one at any rate. Will close once the new one is up. |
Closing in favor of #1614. |
Additions
pulseio.FrequencyIn
PWMOut
program. EDIT: Accuracy is more like ±1%...Changes
shared_timer_handler()
out of samd-peripherals and back into CircuitPython..../atmel-samd/timer_handler.c
.shared_timer_handler()
incommon-hal
modules.Issues
Size. pIRkey will fail Travis for the French translation. But, even beyond that,
FrequencyIn
eats up ~1.5K of the flash, which is what I consider a large problem on non-Express builds. During a discussion with @dhalbert on Discord, decreasing the-finline-limit
was mentioned with a caveat that I have no interest in defeating. I'm kind of hoping this review will point out some obvious waste I'm conducting in my code... If that doesn't bare fruit though, I figure excludingFrequencyIn
from non-Express builds will be the best option.