-
Notifications
You must be signed in to change notification settings - Fork 190
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
Fix frequency offset for AVR #202
Conversation
I have not done any testing. I suppose this makes some sense, though: Timer1.initializeCPUCycles() actually just seems to set the timer compare register to that value. So if I understand that correct (which I am not sure about), a value of 0 means to trigger every timer tick, 1 every other timer tick, and so on. What I find a bit unsettling, is that the formulas in AVRs own application note do not include the -1. At any rate, for better readability, I suggest adding braces as --- Reference: |
Hi, Thanks @tfry-git !
Done. For the rate, I do not really understand why the -1 is needed, but maybe the timer is losing one clock cycle somewhere? In any case, it seems to be quite closer to the ideal case (there is always a rounding), but I just want to be sure it is not a local defect on the board I am testing on. |
Any comment/test for this? I have drafted a new release which could (or not) include this. More importantly it is adding two new boards (Giga and Uno R4). It is one click away from being published, if you do not have concerns I'll do it in a few days! |
My first approach was to check what the CPU itself has to say about it: I added debug code inside the timer 1 ISR to measure the time spend for 10*AUDIO_RATE samples (based on micros()). This yields 9994212 micros (with the patch) vs. 10014696 micros (without). Those numbers are fairly stable (with an occasional deviation of 4 micros), and are identical at AUDIO_RATE 16384 and 32768. -> Slightly better with the patch. I then used a simple updateAudio like this:
For a scope-friendly rectangle wave at theoretically 32 Hz. On my crap-scope that yielded a stable 32.02 Hz with the patch and 31.95 Hz without. -> Again slightly better with the patch. Results were again remarkably similar for AUDIO_RATE 16384 or 32768 (adjusting the divisor, of course). Tested using STANDARD_PLUS, only. Also all testing done on a low quality Uno clone, but in particular the experiment using on-board timing with micros() seems to provide fairly solid backing that this is indeed the correct thing to do. Please do add a comment pointing to the issue / MR near that |
Ok, great, thanks for testing! I do not have a digital scope at home, but listing to the "beating" with respect to a calibrated source also yielded to a slight improvement, so it sounds like the way to do it. I'll add some comments to these tests and merge soon then. |
Added documentation for tweak on AVR timer frequency
Added some comments and again made a mess with git trying to squash my last two commits… Sorry for that. Will merge once tests are completed. |
Hi,
Fixes #201 by fine tuning the timer frequency. Basically, we get closer to the "ideal" timer frequency by this. Output frequency detuning shifts from 0.25% to 0.06%. To test this, I generated sinusoidal tones in Audacity and listen to the beating when superimposing them to the same tone generated by Mozzi. HiFi is 0.06% off in the native version so I did not touch that.
Untested with external audio, but that should behave the same. Of course, this changes the behavior of all sketches on AVR (but to be closer to reality which I think is fine!)
Would love to have confirmation from someone else owning an AVR to be sure I am not looking to fix problems with the particular board I am testing with!