-
Notifications
You must be signed in to change notification settings - Fork 76
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
turing machine #114
turing machine #114
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly nits and optional feedback. Amazing work, I love the approach and all the tests! I want to give this a try on my EuroPi before I give the final LGTM.
Updated with these recommendations. |
software/contrib/turing_machine.py
Outdated
cv6.voltage(self.tm.get_voltage()) | ||
|
||
def flip_probability(self): | ||
return clamp(int((round(1 - k1.percent() - ain.percent(), 2)) * 100), 0, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting, I think my EuroPi is calibrated differently, because when I have k1 all the way CCW it would only reach 99, so I never got the "looped" state. Digging in a bit, it looks like my analog input at unpatched state hovers around 0.00932
percent. So this is what flip_probability()
looks like for me:
>>> t.flip_probability()
99
>>> 1 - k1.percent()
0.9999847
>>> 1 - ain.percent()
0.9907246
>>> 1 - k1.percent() - ain.percent()
0.9903508
>>> round(_, 2) * 100
99.00001
Now if I remove the analog input, I'll get a probability of 100 as expected:
>>> 1 - k1.percent()
0.9999847
>>> round(_, 2) * 100
100.0
I think using the percent()
methods is more logical and best-practice, but switching over to using ranges ends up with the correct expected values for me:
>>> clamp(100 - k1.range(101) - ain.range(101), 0, 100)
100
This might be symptomatic of a bug in ain.percent()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oof. I had a lot of trouble getting my module to work correctly with both ain
voltage and without. It's unfortunate to hear that it wasn't good enough to get it to work on all modules. I'll look into this range suggestion.
0c09c6f
to
1c8b2a4
Compare
I've update this PR to use the new (in PR #155) classes LockableKnob and KnobBank. I think it is ready to be reviewed again. I have two more things that I'd like to change about this script, but have elected to leave out for now in the interest of getting a functional script merged. I will tackle these in separate PRs, probably soon, as they will surely annoy me. The first is the onscreen UI is terrible. I have some ideas on how to make it more fun and useful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I'm looking forward to playing around with this script.
software/contrib/turing_machine.py
Outdated
cv2 - pulse 2 | ||
cv3 - pulse 4 | ||
cv4 - pulse 1 & 2 | ||
cv5 - pulse 2 & 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[discussion] How do you feel about making this one "1 & 2 & 4" so it triggers less frequently than cv4? That would make cv5 less frequent than cv4 and good for using cv4 as a kick drum and cv5 as snare trigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fine to me. I expect that users might tweak these to their liking (I have an idea for that), but I'd like the defaults to be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
a6d15ef
to
33c3c51
Compare
also adds the experimental dir for code that we're not sure of yet adds the pin_id field to AnalogueReader so that we can wrap knobs more easily.
a collection of LockableKnobs to simulate several virtual knobs on the same physical knob
not intended to be merged into main
also: - add some more tests - cleanup threshold - name the Disabled Knob
add experimental.knobs to the api docs also add a missing parameter and remove an unused parameter
- lambdas in test patching configurable EuroPiTuringMachine class
also update MockHardware to add some nicer methods for setting percents
update KnobBank to know the name of the current knob.
updated get_bit_and() to take varargs added test
33c3c51
to
a406667
Compare
A script meant to recreate the Music Thing Modular Turning Machine Random Sequencer as faithfully as possible on the
EuroPi hardware using bit shift operations to mimic the analog shift register.
I've abstracted the TM itself away from the code that it uses to interface with the EuroPi hardware. This way it is possible to use the TM class to back other scripts that want that functionality. I didn't go so far as to put the TM class into the firmware directory, but i think we should if a second script uses it.
I've also started exploring the ability to test by mocking the hardware inputs a bit, and I think it shows promise. Check it out and let me know what you think.