monome / norns

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

matron device detection is naive and poorly structured #108

Closed catfact closed 6 years ago

catfact commented 6 years ago

a significant failure case is that it treats all tty devices as monome devices.

catfact commented 6 years ago

in general, device type detection is super hacky and bad:

here's the test where device type is assigned: https://github.com/catfact/norns/blob/dev/matron/src/device/device_monitor.c#L274

these id's just correspond to entries in this table: https://github.com/catfact/norns/blob/dev/matron/src/device/device_monitor.c#L56

device type is the first index in that table with a matching regex for the device path

and then those indices magically correspond to entries in this enum: https://github.com/catfact/norns/blob/dev/matron/src/device/device_common.h#L5

artfwo commented 6 years ago

libmonome will fail to open any non-monome device by checking the serial number on a FTDI chip

artfwo commented 6 years ago

however, the grid available to the lua script doesn't seem to change to actual grid, messages from matron follow:

attaching grid 
>>     serial    m0001754
>>     dev    userdata: 0x5558e5872690
>>     key    function: 0x5558e57d9a20
>>     id    1
>>     name    monome 128
attaching grid 
>>     id    2
>>     dev    userdata: 0x5558e584cd00
>>     key    function: 0x5558e57d9a20
device added:     3    nil
device added:     4    nil
catfact commented 6 years ago

ok, so there are two problems:

1) regex for devices with paths containing tty isn't sufficient to distinguish monome devices from other tty devices (like the norns hardware!) - see above

2) return value for device_monome:dev_monome_init() isn't checked here: https://github.com/catfact/norns/blob/dev/matron/src/device/device.c#L24 so failure to open with libmonome still passes the device up to lua

artfwo commented 6 years ago

closing this, since the issues with grid are now fixed, and we use a single handler for both monitor and scan. let's create more specific issues for further improvements.