Use constants for PWM and CPU frequency

[awonak]

In order to get more stable dc voltage out of the PWM output, the PWM frequency needs to be set to a higher value.

This PR also removes the machine freq settings from all user contribution scripts and moves that command into the EuroPi firmware to ensure consistent cpu performance across all scripts.

Note: this is a continuation of PR #166 and includes changes in that branch too.

Define constants for CPU Freq and PWM Freq. Centralize the machine.freq() call to overclock the pico.

This PR and #154 contain lots of good discussion of our efforts to make better sense of how to best use PWM to get a stable control voltage for the EuroPi.

[mjaskula]

The changes here look good, just one minor change requested. However I have two notes:

• I'd rather this be two PRs than packed together as they are separate topics.
• I'm not entirely sure how to test this with my current setup. So while I approve of the code changes, I'm not necessarily sure that the change in the PWM frequency is an improvement.

[roryjamesallen]

I haven't had a chance to test this yet, but I would like to make very sure that this has no chance of introducing an issue that used to happen with high PWM frequencies where the effective bit depth is reduced massively, to the point where large audible steps occur where there should be a smooth output voltage.

There is a good chance this was caused by some kind of interference elsewhere in the firmware that has since been fixed, or even in MicroPython itself as this loss of bit depth wasn't documented but had been identified by a few other people I've talked to who have used PWM on the Pico to generate an analogue voltage.
But, either way I would like to make very sure that we don't merge this and push a release only to find some people saying 'why does my pitch output jump in steps of 2V!?'

[roryjamesallen]

I haven't had a chance to test this yet, but I would like to make very sure that this has no chance of introducing an issue that used to happen with high PWM frequencies where the effective bit depth is reduced massively,

I have just tested the PWM change alone, with no other alterations, to 250_000_000, and set the machine.freq to the same in a test .py file, and I can't even get it to output voltage.
If I set the machine freq to 250_000_000 and the PWM to 10_000_000 I get very roughly 0.5V steps in the outgoing CV, so it seems to be the exact same issue as it used to be. Have you definitely had it working with these parameters @awonak ?
I have only changed the PWM rate so if there is another change in this PR that fixes it then maybe that's it!

[awonak]

If I set the machine freq to 250_000_000 and the PWM to 10_000_000 I get very roughly 0.5V steps in the outgoing CV, so it seems to be the exact same issue as it used to be. Have you definitely had it working with these parameters @awonak ?

Good catch! I was focusing on the dc output noise/stability and neglected to check the step depth.

The TinyGo interface for PWM gives us different levers for configuration that make your observations more explicitly clear. The configuration for pwm.Set(ch, top) allows us to set the top variable, which is essentially that step resolution (higher pwm frequency in MicroPython == lower top step resolution in TinyGo).

With higher top step resolution, we get noisy dc voltage output. Conversely, if we want less noisy dc voltage output, we need lower top step resolution (higher pwm frequency). This is the equation we need to perfect here!

https://tinygo.org/docs/tutorials/pwm/#explanation

[pcurry]

This looks totally solid to me, for what that's worth.

[awonak]

Okay, I've conducted a few more tests of the step resolution using the exponential envelope on the smooth random voltages script to observe the audible differences with a vco and visualized with the Mordax Data. In my previous test, I was only going off the visual differences (https://imgur.com/a/C5h0YYn), but after listening to the difference I can confirm that even though the 100khz freq looks wavy, it sounds better than 1mhz. So I've reverted the PWM freq back to 100khz.

I also moved the machine.freq(CPU_FREQ) call out of the frimware and into the menu.py script (along with the call to reset_state()) following the observations in #174 that the firmware should only declare classes and variables, and should not be executing code.

This PR should be good to go if anyone else would like to give the final LGTM and merge.

[mjaskula]

[discussion] These most recent changes ignore the case where the user does not use the menu at all, and instead copies their favorite script to main.py. I'm not sure how many of those users there are (not me usually, sometimes for testing), but it is a supported mode.

So If we move all of the overclocking out of the scripts and only do it in the menu, then we are robbing those scripts of the speed change in the non-menu configuration. Same with the call to reset_state(). I don't think that this is what we want.

[awonak]

So If we move all of the overclocking out of the scripts and only do it in the menu, then we are robbing those scripts of the speed change in the non-menu configuration. Same with the call to reset_state(). I don't think that this is what we want.

Yeah, I thought about that. I was weighing the good points raised in #174 that we shouldn't be executing code when we import the firmware, vs the universal benefit those two calls would provide. I was thinking that it really should be up to the script author to make that call. menu.py is opinionated and makes that decision for all EuroPiScript subclassed scripts. For individual script authors who just want firmware + their script, it's up to them to make that decision in their script. Personally, I think I agree more with the point raised in #174 and that it's better to let individual script authors decide to add those calls to their scripts.

[awonak]

I have reverted the changes moving calls to reset_state() and freq() from the firmware to menu.py to be addressed in another PR.

[mjaskula]

This looks good to me now.

[mjaskula]

It's probably worth updating the main description of this PR as it no longer makes a change to the PWM freq value.

[awonak]

Hmm, should we perhaps just close this PR since it doesn't change PWM and only makes unrelated changes, and address those all together in a separate issue/PR?

[mjaskula]

Hmm, should we perhaps just close this PR since it doesn't change PWM and only makes unrelated changes, and address those all together in a separate issue/PR?

I would just assume update this PR. It has all of the history right here and you have the approvals. Just add to the description to include the new direction. And readers can look at the conversation if they want to know more.

But if you want to open a new PR, I think that's fine too. IT's just more work for you.