Closed beserge closed 3 years ago
The looper class should be pulled out and added to daisysp before merging. It's pretty spartan for now but it works well. There's a short list of TODO items for features I'd eventually want to add. Feel free to pitch in any other ideas for features to add there as well.
Seems like we're most of the way there. These are looking pretty good. Just a few notes for each one.
patch
instead of hw
../
to reach the root directory. we added an -l
flag to specify the path to the root folder where libs are. (not sure if it works with tcv
he update command or not, though)..f
syntax for floats in these examples. There may be enough new concepts here that we don't want to add another layer of confusion like, "why do you write numbers like that?" Let's use 96kHz instead of 16kHz (bit more common of a change)
In the explanation of the AudioCallback, might be nice to add a note below the array explanation about the OUT_N
and IN_N
macros.
Otherwise, looks fine to me.
Table in README is broken
Let's further simplify the led setting:
if (value > 0.5)
patch.SetLed(true);
else
patch.SetLed(false);
For the CV Input example lets use one of the bipolar inputs on the patch.Init() (i.e. CV_5 - CV_8) to demonstrate, using 0.0 as the threshold, and for the Pot example use the pin we're using now, but use 0.5 as the threshold and mention that the pots are wired between 0-5V on the hardware.
When grabbing the value with GetAdcValue
we should encourage the use of the CV_1
, etc. enum values that are made for that. It also will make it more clear to users with a patch.Init() board what is going on.
let's simplify this a little bit, and do something like:
patch.WriteCvOut(CV_OUT_2, 0);
patch.Delay(1000);
patch.WriteCvOut(CV_OUT_2, 1);
patch.Delay(1000);
. . .
etc. up to 5V. with comments saying that it'll be approximately that voltage, etc.
Again, we should use the enum values for the DAC channel instead of just an int.
Also just to make it a bit easier to verify on the patch.Init() hardware, let's change it to CV OUT_2 so that it doesn't require patching a jack to something else to test.
Solid.
Table in README is broken
Also, this is so ugly compared to the other examples because of the c-style GPIO still. We'll fix it when that PR finally gets merged. Otherwise looks fine.
button
instead of momentary
System::GetNow()
for the timing purposes. This is obv. a breaking change. So kind of annoying because we'll have to go fix it elsewhere, but I think its a good change to make.With that change we can simplify the init in these examples, and remove the delay since that may be misleading when people start moving stuff into audio callback instead of the main loop.
You can move this example back out into the main patch_sm
list. It is very similar to the Audio Settings, but I see it more as a generic "application example" rather than one of the Getting Started examples
Small note on the CV OUT. The led only actually lights at > 3V. So the stepping through doesn't step up in brightness in the satisfying way you might hope. Maybe we could just write 0V, 5V instead or something.
This now depends on https://github.com/electro-smith/libDaisy/pull/423
Alright, just a few little things left over. And I should have the Switch update rate PR merged shortly. So you can just sync libdaisy up to the master branch for that.
/** otherwise . . . */
comment placement makes the code look awkward.update all CVs
comment stands out as strange since we're not using CVs. Maybe mention that it updates "gates and CVs" or something that makes it clear its relevant to the example.Last thing, mostly just a question: Does the CV Input example cause the LED to flicker with the CVs (I assume it's not reading in exactly 0V). I'd like to avoid anything that makes the code feel more difficult than it has to here, but I'd also like for the example to work consistently across various units.
Oh, and for this PR let's take out the Looper example for now. Since we're going to be adding a module to DaisySP, etc. we can handle that in a separate PR, and that'll help wrap these examples up sooner anyway.
@stephenhensley Yeah there's definitely some flickering at GND on the CV examples. If we go with the Led
object and do the PWM brightness we can avoid this and still make a pretty concise example
@beserge cool. These are more for reading, than doing. So I think it's fine.
Just a few minor cleanup notes, and then I think we're good to go on all of these:
patch.Init()
in the Audio Settings example.CV_OUT_N
) -- you may need to fix the ADSR, and the RandomVoltage examples as well.vs/
folders don't have valid files (i.e. paths through GettingStarted aren't correct, let's just remove all of those files from the GettingStarted Examples (i.e. Project.sln
, and vs/*
)Oh, and one other thing, in the README of each example, can we add a note of where to find the typical application in the datasheet?
For example, for the CV input example you can point to table 1.3 in the datasheet, etc.
Here's the libdaisy PR for the Patch SM cv out enum namespace thingie
Can we move these new, simple examples to a nested
GettingStarted
folder within thepatch_sm
hierarchy, and add that to the build examples scripts, etc.Let's change the pot example to just control the brightness of the LED via the CV output. No need to set up/use the audio callback, just use
patch.Delay(1)
between calls.For the button/toggle stuff, I don't think
Switch::Type::TYPE_TOGGLE
, etc. actually has a functional purpose. So, as mentioned below you can just use the defaults.and just a few additional notes across all examples:
1000
and theDelay(1)
should be made clear in comments. (i.e. 1ms == 1000Hz)