Added poly_square oscillator contrib script. #141
Conversation
|
not part of the review but just wanted to say how awesome this is, just such a cool proof that boundaries i thought were set in stone (no audio without extra hardware) can be broken by clever enough programmers! |
There was a problem hiding this comment.
Wow this is awesome. I hope that you are proud of this work. Aside from pushing the boundaries of what the EuroPi is capable of, but you are doing it in a very clean and well factored and well thought out script. 👏
I've commented using our proposed contributing guidelines, give it a read if you haven't already: https://github.com/Allen-Synthesis/EuroPi/blob/contributing-updates/contributing.md
I've added some minor suggestions and questions. I'd like to understand the assembly code a little bit better, at least how it interfaces with the python, before I click the approve button. But I wanted to hand these comments over to you while I did that.
There was a problem hiding this comment.
Amazing work, thank you for the thorough and detailed script helping pave the way for more PIO scripts!
I'm not sure if it was intentional, but if you want to have this script included in the Menu, there's one more step; you'll need to import and add PolySquare to the list of scripts in contrib/menu.py as described at the end of the Menu Inclusion instructions.
Use EuroPi clamp instead of individual min & max calls. Co-authored-by: Adam Wonak <adam.wonak@gmail.com>
|
New commit incoming with all of the above changes - plus a couple other small things I've noticed while reviewing the script. |
…ax detune available; better tuning UI; refactored methods called in update_settings() so that oscillator_index is the parameter instead of the oscillator itself.
…to poly_square
I do want this to happen, but I need to figure out a way to prevent the "tuning" mode menu from confusingly interfering every time you try to navigate back to the main script menu by holding both buttons down. |
This is as far as I know just something that we accept with the double button press; that the single press handlers will also trigger unless you specifically write in something to prevent that (which would come at the cost of single presses being delayed as it works out whether the other button is also pressed, hence leaving it up to the user rather than making this delay default) Yours certainly wouldn't be the first to have something pop up as you try to exit to menu with a double press, so I wouldn't worry too much about this, especially given the alternative is to potentially add an artificial lag to the usage of the tuning mode which will be accessed much more often than the main script menu |
|
I forgot to mention this in my description of the previous commit, but I did also include some new functionality (just for fun): button 2 now toggles the maximum detune between a half step and a major 9th for those of us that enjoy a bit of chaos. This also required some changes in the code to save settings. |
|
One potential way to avoid button press confusion is to move all handler behavior off of Personally, I prefer the snappy behavior of a |
That's a good point - and the script doesn't save the tuning changes until the button's falling handler is called, which is intercepted by the return to the menu anyways, so maybe it won't actually be much of an issue. |
[Optional] I think that button 2 needs some kind of on screen indication of what mode we are in. I found it confusing to not know for sure what mode i was in. |
|
[Optional] I feel like the tuning would be more useful with a larger range (probably on both knobs). My first impulse was to set up a nice fat drone, but the tuning controls didn't get me to where I wanted. At first, I thought that something might be wrong with the code or my module. Once I connected up a 1 v/oct source it behaved better. I guess I expected the tuning functionality to behave like the frequency knobs on my oscillators, giving me access to its entire range. To elaborate, without a v/oct signal, the output is too low to really show off the module, even at the highest tuning. All of the chords sounded muddy to my ears, and stepping through the features wasn't that inspiring. Once I brought the pitch up with an external voltage it really came alive. I think it would be nice to have that experience more immediately. Plus: better drones. |
There's actually gonna be two, because there are some unmocked micropython classes being used. I can work on fixing this if you want to have the menu working in this PR. |
That's a good idea. I'll probably remove the "Poly Square" title, shift the current polyphony mode text upward, and use the space below to clarify the detune mode. |
That's a good idea. I originally wrote the script only to interact with an external V/oct, but now that we have the internal tuning it probably should have the full frequency range available. |
Sounds good - I'll include it in the menu in my next commit. |
…des; tuning settings no longer change until the knobs have been moved; coarse tune range is now 8 octaves; fine tune range is now an octave; get_offset() now uses a modulo to prevent issues when there are less voltage offsets available than the number of oscillators; tuning UI has been updated to reflect the new coarse tuning range; the main UI now includes the max detune setting; tuning & detuning knob values are no longer quantized when read; doc updates; PolySquare has been added to the menu; added display name.
|
Hm, not sure why the menu imports test failed on my latest commit. 😢 |
Oh, it's because that PR with the fix hasn't been merged yet - got it. |
Ok, the fix has been merged into the main branch upstream. You'll have to perform some git work to get the changes into this branch. Depending on your git philosophy you can either rebase your branch on the current main, or you can merge the upstream changes into your branch. I'm not sure how comfortable you are with git, but if you need help I'm glad to. |
|
Ok, I've merged the upstream changes, and everything looks good to go now! |
This PR adds a new poly square oscillator script to the contrib directory.