monome / norns

norns is many sound instruments.
http://monome.org
GNU General Public License v3.0
633 stars 147 forks source link

Broken crow APIs #1402

Closed discohead closed 3 years ago

discohead commented 3 years ago

Another report of the same issue here: https://llllllll.co/t/crow-help-norns/25863/174

When setting the input modes from norns, it seems the most recent call to set a mode is applied to both inputs. Also seems like getters for volts, slew, etc... are broken and return tables instead of the values (details at the bottom).

Just got my crow today running v3.0.1 with my fates running norns for fates 210706. It seems to be a norns issue as I don't have the same problem when scripting from druid.

This norns code...

  crow.input[1].mode("stream")
  crow.input[1].stream = function(v) print("stream: "..string.format("%.3f", v)) end
  crow.input[2].mode("change", 2, 0.1, "both")
  crow.input[2].change = function(s) print("change: "..tostring(s)) end

produces this output...

change: true
change: false
change: -0.986633
change: true
change: false
change: 3.76117
change: true
change: false
change: 7.14529

Note that stream is not called, instead change is being called by both inputs, with volts from input[1] and state from input[2].

The same code in druid...

input[1].mode("stream")
input[1].stream = function(v) print("stream: "..string.format("%.3f", v)) end
input[2].mode("change", 2, 0.1, "both")
input[2].change = function(s) print("change: "..tostring(s)) end

produces the expected output...

change: true
change: false
stream: -0.986633
change: true
change: false
stream: 3.76117
change: true
change: false
stream: 7.14529

I didn't manage to track down the bug, but my best guess is that this broke when @trentgill added support for the crow namespace: https://github.com/monome/norns/commit/4e9690297740d8b26c10c8cc57160ef40796820b

It appears there are other things broken as well, most of the getters are returning tables instead of the value and it doesn't seem to matter if the variable even exists or not...

I guess querying values is not supported: https://github.com/monome/norns/blob/main/lua/core/crow.lua#L196

Seems the output[n].receive and output[n].query() functionality shown in the crow-studies no longer exists either.

Is there a right way to do it?

crow.input[1].volts
table: 0x4f4990
crow.output[1].volts
table: 0x480420
crow.output[1].volts[1][1][1][1][1][1][1][1]
table: 0x480af0
crow.foo['bar']
table: 0x4ea400

in druid...

> print(input[1].volts)
0.001770258

> print(output[1].volts)
1.0

> print(foo['bar'])
repl:1: attempt to index a nil value (global 'foo')
stack traceback:
  repl:1: in main chunk
runtime error.
tehn commented 3 years ago

@trentgill for extra eyes

trentgill commented 3 years ago

When setting the input modes from norns, it seems the most recent call to set a mode is applied to both inputs.

Sounds related to the namespacing for sure. My guess is either something is wrong in the norns.crow.register_event function on norns, or here.

For a bit of background, when you assign a crow event handler to a function, we don't actually send that function def to crow. Instead: 1) norns: allocate a new identifier (just a single character) 2) norns: capture the function on norns into norns.crow.events[identifier] table 3) ->crow: assign the crow event to a generic function which will send back to norns the identifier followed by that events data. 4) crow: the event occurs and triggers that generic function 5) ->norns: processes the crow message and runs it as code. 6) norns: the event will be looked up in norns.crow.events table, which passes the returned valueinto the function captured in step 2).

It's a bit roundabout, but the whole system exists to decouple the norns & crow release cycles. With this system, we can freely add new event handlers to crow without any change to the norns API (and scripters can define their own event handlers too). Also, it means norns event handlers can know about nornsy things that would syntax error if we sent them to crow.

I'm not going to have time to look at this for a little while, so if someone wants to dig, I'd start by adding debug prints for the identifier on registration (on both crow & norns) and also on event occurence. The count of events is probably getting messed up somewhere. Perhaps this #norns.crow.events call doesn't work as the events table has a number of named (not indexed) events loaded by default.

Is there a right way to do it?

This block of examples is trying to print the identity of crow entities. This is something the API does not support. The only way to inspect crow's state is to asynchronously request a value, which returns in an event handler.

The reason something like crow.output[1].volts[1][1][1][1][1][1][1][1] doesn't error, is because the return value is just a path to that variable (it doesn't send anything to crow unless you assign or call it). norns doesn't know if that variable exists on crow, but if you tried to call, or assign that variable, crow may return an attempt to index a nil value error.

Seems the output[n].receive and output[n].query() functionality shown in the crow-studies no longer exists either.

This looks to be true. I honestly don't remember removing it, but I think nobody noticed because it's generally not a very useful thing to do? On crow you can use v = output[n].volts, so if you want an asynchronous callback it could be output[n].query = function() _c.tell('receive', n, output[n].volts) end. We can add this back to the Output lib if it's deemed useful (or we could remove it from the studies, as I don't see any specific use case for it).

discohead commented 3 years ago

@trentgill thanks for all the context. I will try to do some more debugging this evening.

discohead commented 3 years ago

@trentgill

Perhaps this #norns.crow.events call doesn't work as the events table has a number of named (not indexed) events loaded by default.

↑ ding, ding, ding!

fix here: https://github.com/monome/norns/pull/1403

cc: @tehn

tehn commented 3 years ago

fixed by #1403