Skip to content
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

Finalize syntax for crow as i2c follower #258

Closed
trentgill opened this issue Nov 19, 2019 · 12 comments · Fixed by #354
Closed

Finalize syntax for crow as i2c follower #258

trentgill opened this issue Nov 19, 2019 · 12 comments · Fixed by #354
Labels
design Issue needs design consideration documentation
Projects
Milestone

Comments

@trentgill
Copy link
Collaborator

See #250 for discussion.

Relates strongly to #55

@trentgill trentgill added design Issue needs design consideration documentation labels Nov 19, 2019
@trentgill trentgill added this to the Next Point (1.0.3) milestone Nov 19, 2019
@trentgill trentgill removed this from the 1.0.3 milestone Jun 10, 2020
@trentgill trentgill added this to the Minor milestone Jun 23, 2020
@trentgill trentgill modified the milestones: Minor, 3.0 Jul 15, 2020
@trentgill trentgill added this to Minor in 3.0 Jul 15, 2020
@trentgill
Copy link
Collaborator Author

@tehn
your thoughts would be very welcomed :)

  1. which higher-level ideas should be available?

there's a lot of recent discussion at #5 too!

@simonvanderveldt
Copy link
Member

Ah, I should've probably commented here :|
Do we want to move the discussion from #5 here?

@trentgill
Copy link
Collaborator Author

@simonvanderveldt Let's not worry about reorganization. Perhaps we can start a new thread once we have settled on a path forward?

@tehn
Copy link
Member

tehn commented Jul 16, 2020

wow there is a thick maze of issues and PRs related to this :)

i'm going to start with a dumb list of things a crow in follow mode should be able to do:

  • reset to a clean VM (ie, if to be configured as an i/o satellite)
  • output:
    • volts, slew
    • pulse (action), pulse.time, pulse.volt, pulse.polarity
    • ar (action), ar.a, ar.r
    • lfo (action), lfo.f, lfo.etc...
  • input ??
    • set modes and then later query for data?
  • generic function with args that can be redefined by script

design decision is whether to have packed messages, ie, ar 1 0.2 0.9 ie ouput 1 rise 0.2 fall 0.9. in addition to ar.rise independently. i could see (particularly in TT) firing an ar message repeatedly, while only occasionally tuning the ar.rise value... which is the TT approach in many ways.

but this is about messages.

we're talking about syntax. what's the current leading proposal? (my brain is mush after reading all those issues/etc)

@trentgill
Copy link
Collaborator Author

Here's a proposal for the command list. I've separated crow & TT as I think they should have different interfaces. Crow will support all the commands, but the exposed API should be different owing to the differing concerns of the 2 platforms.

Crow API

--- ACTIONS
-- basics
ii.crow.reset() -- resets the VM (like `crow.reset()` on norns)
ii.crow.volts(chan,val) -- was 'output'
ii.crow.slew(chan,time)

-- ASL actions (fully parameterized)
ii.crow.pulse(chan,time,level,polarity) -- trigger a pulse
ii.crow.ar(chan,attack,release,level,shape) -- trigger an AR envelope
ii.crow.lfo(chan,frequency,level,shape,ramp) -- starts (or resets) the LFO

-- generics (defined on the crow follower)
ii.crow.call1(arg)
ii.crow.call2(a,a2)
ii.crow.call3(a,a2,a3)
ii.crow.call4(a,a2,a3,a4)

--- QUERIES (return exactly 1 value)
-- basics
ii.crow.get('input',chan) -- current input voltage
ii.crow.get('output',chan) -- current output voltage (instantaneous, not destination)

-- generics (defined on the crow follower)
ii.crow.get('query0')
ii.crow.get('query1',a)
ii.crow.get('query2',a,a2)
ii.crow.get('query3',a,a2,a3)

Teletype API

This API is much more longer as TT users are used to setting static variables first, then calling actions separately. I don't think the meta commands are necesary as they would potentially not even fit on a single line. Focus here is on terseness, requiring memorization or a reference companion.

^^ seems like a fun and unambiguous prefix, but obviously it could be CROW or CR or CW, whatever that community wants.

// SET

^^.RST
^^.CV chan val
^^.SLEW chan time

// PULSE
^^.P chan
^^.P.TIME chan time
^^.P.LVL chan volts
^^.P.POL chan polarity

// ATTACK RELEASE ENV
^^.AR chan
^^.AR.A chan time
^^.AR.R chan time
^^.AR.LVL chan volts
^^.AR.SHP chan shape

// LFO
^^.LFO chan
^^.LFO.FREQ chan freq
^^.LFO.LVL chan level
^^.LFO.SHP chan shape
^^.LFO.RAMP chan skew

// GENERIC CALL
^^.C1 a
^^.C2 a a2
^^.C3 a a2 a3
^^.C4 a a2 a3 a4

// GET

^^.CV chan
^^.SLEW chan

^^.P.TIME chan
^^.P.LVL chan
^^.P.POL chan

^^.AR.A chan
^^.AR.R chan
^^.AR.LVL chan
^^.AR.SHP chan

^^.LFO.FREQ chan
^^.LFO.LVL chan
^^.LFO.SHP chan
^^.LFO.RAMP chan

// GENERIC QUERY
^^.Q0
^^.Q1 a
^^.Q2 a a2
^^.Q3 a a2 a3

Inputs

I think we should only allow basic voltage query of the inputs. It would be too weird having functions to activate the crow input modes but then need to query the values. Pretty much the whole point of the input libs is that they are event based - the processing they do is very basic - what makes them special is the event-detection logic.

crow <-> crow input events makes more sense, but it would change the mental model of the ii.self.event callback. This would no longer just be a near-instant response to query, but instead become a generic event handler which could switch on input types:

ii.self.event = function(e,val)
  if e.name == 'input' then
    -- process a direct input query
  elseif e.name == 'change' then
    -- process a change event detected on a remote device
  end
end

I could be convinced to implement this, but I can't see it being very exciting / enabling many more things.

nb: This does not make sense on TT, so I would argue it be crow only.

Crow syntax

Instead of worrying about these issues together, we can consider adding a special crow-ii library which enables some shortcuts, or alternative syntax. This is particularly aimed at crow<->crow communication (or controlled via norns). Rather than change how ii is implemented globally, it can be made as a translation layer in the user-space (means we don't break every other module in the process).

-- This is what we'd write on on crow#2 directly

output[1].volts = 5
output[1].slew = 2
output[2]( lfo(0.1, 4) )

--- Now let's call those same functions via ii (from another crow)
-- These would be prefixed with `crow.` on norns

-- above proposed version (ie. same style as other ii devices)
ii.crow.volts( 1, 5 )
ii.crow.slew( 1, 2 )
ii.crow.lfo( 2, 0.1, 4 )

-- and we could add some magical indexing for pointing at hardware like tables
-- pro: feels more like normal crow indexing
-- con: we're indexing the *action* rather than the *hardware*
ii.crow.volts[1]( 5 )
ii.crow.slew[1]( 2 )
ii.crow.lfo[2]( 0.1, 4 )

-- the con above could be improved with explicit 'output' names
-- pro: more like normal crow
-- con: more verbose
ii.crow.output[1].volts( 5 )
ii.crow.output[1].slew( 2 )
ii.crow.output[2].lfo( 0.1, 4 )

-- the con above could be dealt with by the magical output indexing
-- pro: more terse
-- pro: 'feels like' the outputs are an extension of crow#1
-- con: opaque, not explicit, syntax sugar is too sweet
-- con: suggests that *all* of the normal output behaviour is available (it's not, and won't be)
output[5].volts( 5 )
output[5].slew( 2 )
output[6].lfo( 0.1, 4 )

I'm sure there's many other permutations that are better than these. My main point is we can separate the issues, and that the 'fancy syntax' issue is clearly secondary.

@simonvanderveldt
Copy link
Member

simonvanderveldt commented Jul 17, 2020

+1 for getting some basics working first before adding some wrappers/syntactic sugar to make it look different/better integrated.
Using ii.crow.action(channel, x, y, etc) I think is totally fine to start with and since it's the same way as other ii devices it might even make sense to just keep it as this without any additions. This might be something where some user feedback after they've used it for a bit would be good.

Regarding the crow syntax as list at the top of your post: 👍
I've got three questions:

  • ii.crow.volts(chan, val) Do we want to call this action volts or cv? I know now cv was partially used because it indicated certain hardware ports, but still it's the most (only?) used command for setting a cv/voltage output from the devices that are currently supported (https://github.com/monome/norns/blob/b1395713cf7d81fd727fae57f68a81792657c798/lua/core/crow/ii_actions.lua).
  • ii.crow.slew(chan, time) Isn't this missing the value? What does it slew towards?
  • ii.crow.get('output', chan) -- current output voltage (instantaneous, not destination) What do you mean by this? That it just doesn't check the real output value of that port?

And I don't really understand the suggestion(s) mentioned in the "Input" section :/

@simonvanderveldt simonvanderveldt mentioned this issue Jul 17, 2020
4 tasks
@tehn
Copy link
Member

tehn commented Jul 17, 2020

@simonvanderveldt

  • volts makes sense for crow because the output is generalized. TR/CV split on other devices is because their outputs can only do one or the other.
  • slew is set independently of the target volt (like on TT)
  • instantaneous means at this moment, versus destination, in the case that it is mid-slew to another value. so yes, real output of the port.

@tehn
Copy link
Member

tehn commented Jul 17, 2020

@trentgill looks excellent

i'd vote for this, despite its verbosity:

-- the con above could be improved with explicit 'output' names
-- pro: more like normal crow
-- con: more verbose
ii.crow.output[1].volts( 5 )
ii.crow.output[1].slew( 2 )
ii.crow.output[2].lfo( 0.1, 4 )

least confusing and weird. most continuity with normal output[1].volts = x. i think it's fine that you need to be explicit about ii.crow. as a prefix.

main question is (ugghh) multiple crow remotes. i guess at this point the outputs could "magic index" ie ii.crow.output[5].volts is the first output of crow remote #2 --- unless you had a different idea, which is welcome

@simonvanderveldt
Copy link
Member

@tehn Thanks for the clarifications.

* `slew` is set independently of the target `volt` (like on TT)

How would this work then? The output action is currently instantaneous, I assume the same applies to the ii.crow.volts() action. So then there's nothing to slew towards?

ii.crow.output[1].volts( 5 )

Will the same convention then also be applied to all the other ii devices instead of the current ii.crow.output(1, 5)?

main question is (ugghh) multiple crow remotes. i guess at this point the outputs could "magic index" ie ii.crow.output[5].volts is the first output of crow remote #2 --- unless you had a different idea, which is welcome

Why not just do the simple thing by indexing the crows? So ii.crow[x].output()?
Definitely not a fan of automagic numbering, also keep in mind use cases where people mount their crows spread out in their case identifying them by crow 1, 2, 3, etc. Using that index directly is much clearer, I don't think they'd have much fun with an automatically increasing index for the outputs.

@trentgill
Copy link
Collaborator Author

Why not just do the simple thing by indexing the crows? So ii.crow[x].output()?
Yes exactly. That's the way it will work.

The ii.crow.output we've been talking about is assuming the user hasn't changed their i2c address (ie. both crow's are on the same ii address). if you have changed the address on the 2nd crow, it would need to be ii.crow[2].output.

No magic numbering. In fact the crow[n] indexing comes 'for free' with the exiting ii infrastructure. The output indexing will come later.

Will the same convention [output[n].action] then also be applied to all the other ii devices instead of the current ii.crow.output(1, 5)?

I don't think so. If we do this it'll need to be a per-module helper library. We'd have to drastically change the way the ii files were generated if we wanted to have a special handling for an 'index' parameter. I don't think it makes sense to break the existing API for other modules. We're talking about doing this for crow in particular in order to keep things feeling more consistent within the crow ecosystem. Imagine a user who's only use of ii is to communicate between multiple crow modules - they don't care about the ii 'conventions'.

I don't really understand the suggestion(s) mentioned in the "Input" section
I just mean, we should only provide a basic input getter. ie. "what is the value on the input jack right now?". Crow has many other input modes that create events when a change in their value is detected, but I don't think it translates well over the ii bus. Could be extended in the future if we feel the need.

/////////////////////////////////////

Ok. Seems like we're on the same page about what the initial implementation should be. Feels like a great start and if we make it clear that it's experimental support we should be able to get some good feedback on the interface.

@simonvanderveldt
Copy link
Member

I don't think so. If we do this it'll need to be a per-module helper library. We'd have to drastically change the way the ii files were generated if we wanted to have a special handling for an 'index' parameter. I don't think it makes sense to break the existing API for other modules. We're talking about doing this for crow in particular in order to keep things feeling more consistent within the crow ecosystem. Imagine a user who's only use of ii is to communicate between multiple crow modules - they don't care about the ii 'conventions'.

Isn't the group of people with crow + some other ii device much larger, though?
Of course it's early days for crow to crow ii, so it's a bit of a chicken and egg problem, but even assuming people having multiple crows connected over ii wouldn't most of them also have one or more other ii modules? For example currently I'd assume most people using ii have a Just Friends.
Wouldn't they benefit more from a consistent ii interface between different modules than from a consistent crow interface?

@trentgill
Copy link
Collaborator Author

You are probably right which I think reinforces our "let's make it work in the existing model first" approach. crow to crow communication feels like an easy opportunity to explore any syntax changes because we can control both sides of the equation with a single firmware.

Even if we do extend the syntax possibilities, I don't imagine we would disable the current style. This discussion can be treated as added functionality, rather than a breaking change, and as such should be discussed in its own Issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Issue needs design consideration documentation
Projects
3.0
Minor
Development

Successfully merging a pull request may close this issue.

3 participants