Add Traffic script #311
Add Traffic script #311
Conversation
|
Rebase from latest |
|
Now that #309 has been merged I've re-sync'd my |
There was a problem hiding this comment.
The code itself here looks pretty good, I just had a few suggestions.
I'm not sure I entirely understand it, as mentioned in a comment, so I'm gonna wait to complete my review until I understand it a little more.
The Jasmine and Olive Trees site says "Imagine having 3 triggers that will instantly set a BIA or Plaits as a kick, snare, and hi-hat." This is in line with a script idea that's been bouncing around in my head for a while now. So I'm excited to understand this more fully.
software/contrib/traffic.py
Outdated
| self.save_state() | ||
|
|
||
| @b2.handler_falling | ||
| def b2_falling(): |
There was a problem hiding this comment.
[optional] This function appears to be the exact same implementation as b1_falling(), why not use the same actual function?
software/contrib/traffic.py
Outdated
| if time.ticks_diff(now, self.last_trigger_at) > TRIGGER_DURATION: | ||
| cv6.off() | ||
| else: | ||
| cv6.voltage(TRIGGER_VOLTAGE) |
There was a problem hiding this comment.
[discussion] Should this instead use cv6.on() to take advantage of the configurable gate voltage being introduced in PR #331?
There was a problem hiding this comment.
Good call. I suspect a few scripts would benefit from that change in the long run.
|
|
||
|
|
||
| class AnalogReaderDigitalWrapper: | ||
| """Wraps an AnalogReader to allow it to simulate a DigitalReader""" |
There was a problem hiding this comment.
[required] I would like to see some text here describing the intended use of this class. Specifically that the state returned by value() is only accurate as of the last call to update().
…stants" This reverts commit 327c3cc.
…nsaver to G&T. Move the screensaver activation & blank timeouts into the Screensaver class itself instead of redefining them externally
…r & screen-blanking. Use this wrapper instead of europi.oled in the 3 scripts that use the screensaver
…nsaver" This reverts commit 0f21705.
…irst and auto_show arguments to centre_text, modify all extant calls to centre_text to use the correct auto_show behaviour
…sociated changes to other scripts
…y by the current knob positions
… it the first time around and clearly didn't do my research properly
There was a problem hiding this comment.
I'm finding the UI pretty confusing, I can kind of tell that the right handle column of values is the voltage - it seems to always be 10x the value of column 1, which makes me wonder why it's there. To me it would seem to make more sense to remove the third column and use the room to add a V, making it clear each 'gain setting' is the voltage that will be output. As I write this I'm realising it could be the calculated output voltage based on the maximum voltage, but now that this is set in europi_config I think it's confusing to have it alongside the gains. I think it would be more intuitive to remove the third column altogether, possibly replaced by A B and C identifiers for each channel to make the grid of numbers more readable.
The other thing I noticed is that you can always change the output voltages of channel 1. The documentation leads me to believe that the outputs switch only on a positive trigger, but they seem to update continuously regardless of input for channel 1.
|
I've simplified the GUI a little and added The values do update continuously, which makes fine-tuning the levels easier. I've updated the documentation to reflect that. The triggers control which set of levels are being continuously applied, but it's not a sample & hold style behaviour; as long as |
…rs -- the limit of human hearing is ~100ms, so half that should be close enough. This lets input 1 properly take priority when both signals trigger at almost the same time
Changes are all great, LGTM now! One suggestion is to increase the samples on knob reads as I find it difficult to not have it flickering now it's at 3 decimal places, and given the potential for accuracy with 3 d.p. and the continuous updating, it would be nice to make sure that fine tuning is really possible. In a test I just did, 1024 samples for each percent() call resulted in much less flickering between values but still a responsive experience. The final thing is that the use of the knob bank class results in behaviour which I'm personally not a huge fan of, which is that if the previous setting was all the way down for example, then you change to adjust a different one to all the way up, when you go back to edit the first you need to swing the knob completely back down to be able to adjust its value. Both of the above are optional, and if you'd prefer not to implement them then this PR is ready to go:) |
|
Good call about the samples. |
New Script: Traffic
Adds a new script based on Traffic by Jasmine & Olive Trees.
AINandDINare treated as gate inputs.CV1-3outputs5V * (din.value() * gain_1 + ain.value() * gain_2)where the gains can be set independently per channel.CV4andCV5output the absolute difference between channel A and either channel B or C.CV6outputs a trigger on every rising edge of eitherAINorDIN.To change the gains of channel B hold
B1while turning K1 and K2. HoldingB2will adjust the gains of channel C.Dependencies
Depends on some changes in
experimentalimplemented in #309.