device monitor - unnecessary checks on usb subsystem (?)

[TheTechnobear]

because norns uses alsa raw open (snd_rawmidi_open) the dependency on monitor is to /dev/snd/midiCxDx.

there is no requirement in monitor to check that this is a usb subsystem for alsa to work.
so, we might as well check against this path in scan as we are already doing check already in device_monitor.c in other places

I accept , that the Norns hardware is only extendable via USB, but for those using the software on other platforms this is an unnecessary restriction and stops midi devices being detected on (e.g.) gpio.
and I dont see any real advantage for the Norns hardware to do this additional check.

thoughts?

[catfact]

(sorry wrong button.)

i'm not sure what you're suggesting. we have the usb subsystem monitor to support hotplugging of USB MIDI devices. i suppose you wouldn't need it if you had GPIO midi but of course the norns hardware doesn't.

[TheTechnobear]

yes, but the scan in dev_monitor is used to initialise all devices... and excludes anything not from the usb subsystem - which as I said is unnecessary...
for sure, the hotplug support is only needed for usb !

[TheTechnobear]

dev_monitor_scan()
device_monitor.c : 154

[catfact]

ok thanks, so IIUC you just want to remove these tests:
https://github.com/monome/norns/blob/master/matron/src/device/device_monitor.c#L153
https://github.com/monome/norns/blob/master/matron/src/device/device_monitor.c#L250

these of course affect all devices, not just midi. they are not unnecessary if you have PCI or other subsystems that collide with the naming rules. (which was the case on the ubuntu laptop where this module was originally authored.)

i agree that it might be fine to remove these entirely on norns or other stripped down hardware. (these is even alluded to in the comments.)

[TheTechnobear]

I’d say that’s more a distribution thing, rather than architecture - but sure paths should not be hardcoded.
Really the midi detection stuff should all be done thru the alsa api , not filenames.
And then the udev should just be used as a ‘watch’ for new devices.
(And this is only applicable to usb since that’s the only thing we are supporting hotswap for )

[artfwo]

We used ALSA originally for midi detection, but decided to stick with udev for having a unified device detection mechanism for grid, midi, and hid. The i/o was also switched to alsa rawmidi interface so we could handle midi in lua, and since rawmidi requires /dev/midi paths there's a path finder in device_monitor too.

[TheTechnobear]

@artfwo ok, fair enough... I should have raised the alsa api as a separate discussion point.

my focus here really is on using udev /dev_monitor usb as its the only hot-swapping we have.
but we should still allow for valid midi devices to be from other sources. (e.g. gpio) (possibly same is true for hid?) - this just means removing checks from dev_monitor_scan, which is used at startup (i.e non-hotswap)

yes @catfact exactly.

Ive cleaned up my fork (a whole load of rebasing fun that was ;) )

so my plan if your interested in this change is:
a) create feature branch , make change , test
b) offer PR

it'll just be these changes, to keep the scope limited.
(if other changes become necessary , I'll be back to comment here :))

Ive also noticed an issue with my synth not working with USB midi , which I think is possibly a midi parser bug (other controllers midi controller i have work fine) - ive not debugged yet, so no details yet :) - but again, Id put on a different branch , and offer a PR.

I assume this is how you are working it? (feature branches with PRs)

[TheTechnobear]

ok, done and tested, and PR raised.
one small additional change required

non-usb devices do not have product name (used for display) , so i default to the id when product name is not found.

also Ive fixed a small bug, where get_device_name tries to strdup, even if current name = null, though this should not really happen with usb devices.

[TheTechnobear]

use case irrelevant to norns - issue closed