monome / aleph

open source sound computer
Other
79 stars 39 forks source link

Cheap MIDI interface fails with MIDINOTE operator #287

Closed lazzarello closed 6 years ago

lazzarello commented 6 years ago

I was going through the MIDI tutorial and plugged in a Midiplus AKM320. No note on messages were recognized. Then I swapped the controller out for a M-Audio Keystation 61es. Note on messages controlled the sound.

I checked the midiplus on my laptop in PD and indeed, noteon data is recieved via the [notein] object.

Not sure how to debug this problem in the Aleph MIDI implementation.

catfact commented 6 years ago

cool thanks for the data point. this is one of those things that is hard to test on enough devices.

one thing that would help would be to see the raw MIDI bytes coming in from the controller. for example, the raw output of pd's [midiin].

though if i had to guess, i'd bet on a problem at the point of USB connection/handshake, not with parsing the midi stream itself. (that is, OS-level stuff, not app stuff.)

if you do want to help debug on the aleph side:

lazzarello commented 6 years ago

here's the dump of from the pd [midiin]

Keystation (working with Aleph)
print: 144
print: 60
print: 100
print: 144
print: 60
print: 0

Midiplus (not working with Aleph)
print: 144
print: 60
print: 127
print: 128
print: 60
print: 64

Turns out the cheap Chinese keyboard has correctly implemented the MIDI note off spec. It's sending a first byte value of 0x80 for noteoff messages, rather than 0x90 with a velocity of 0.

I can probably fix this myself but I'm not too familiar with where in the codebase I'd find functions touching this data.

ngwese commented 6 years ago

IIRC a note-on with velocity 0 should be treated like a note off in addition to a note-off with any velocity (release velocity). I vaguely recall handling this in the monome module firmware but I don't remember the state of the aleph firmware (off to go look).

catfact commented 6 years ago

bees midi note operator does indeed handle 0x8 noteoff nybbles, if that's what you are implying. that code is here: https://github.com/monome/aleph/blob/dev/apps/bees/src/ops/op_midi_note.c

I am now doubling my bet on an issue with handshaking / connection. my suggestion of printing in the connection event handler is the next diagnostic step.

ngwese commented 6 years ago

Agreed regarding midi_note - it does indeed produce the same bees network activations for both those cases.

catfact commented 6 years ago

if bees doesn't get a connection event then we have to look at the UHI code. which is hairy. I can't guess off the top of my head why UHI stack wouldn't be recognizing the device class, but the triage would be straightforward (albeit requiring the hardware)

if bees gets a connection event but notein op isn't getting anything, then I dunno. most recent midi input packet is just in a static global (sorry) that any op can access. so if a controller is sending a lot of weird extra packets it could get fubard, but that doesn't seem to be the case here. (unless! unless pd is filtering out packets that it doesn't think are standard midi... but nah, i doubt it)

catfact commented 6 years ago

yeah i mean... its not actually the first time I've made a midi input :)

catfact commented 6 years ago

it is however the first time tried to make a usb midi host :D

lazzarello commented 6 years ago

apologies if I implied the midi implementation wasn't to spec. It was my confusion.

More noob questions, how do I "copy debug version to sdcard and flash it with the bootloader"?

lazzarello commented 6 years ago

I found the bootloader menu, got a serial console and found the logic where I can output debugging infos. Here's a picture for fun.

img_20171104_011353

lazzarello commented 6 years ago

Okay, so here's what I got. Snarky debugging comments in output are my own.

Controller that works outputs

 received device descriptor. 
 address: 00000001
 speed: 00000001

 dev desc -> bLength : 00000012
 dev desc -> bDescriptorType : 00000001
 dev desc -> bcdUSB : 00000001
 dev desc -> bDeviceClass : 00000000
 dev desc -> bDeviceSubClass : 00000000
 dev desc -> bDeviceProtocol : 00000000
 dev desc -> bMaxPacketSize0 : 00000040
 dev desc -> idVendor : 00004D0A
 dev desc -> idProduct : 00009100
 dev desc -> bcdDevice : 00001301
 dev desc -> iManufacturer : 00000001
 dev desc -> iProduct : 00000002
 dev desc -> iSerialNumber : 00000000
 dev desc -> bNumConfigurations : 00000001

 conf desc -> bLength : 00000009
 conf desc -> bDescriptorType : 00000002
 conf desc -> wTotalLength : 00006500
 conf desc -> bNumInterfaces : 00000002
 conf desc -> bConfigurationValue : 00000001
 conf desc -> iConfiguration : 00000003
 conf desc -> bmAttributes : 000000C0
 conf desc -> bMaxPower : 00000000
 run uhi_ftdi_install
 uhi_midi_install ignoring descriptor; type: 0x00000002
 bLength : 0x00000009
 bInterfaceNumber : 0x00000000
 bAlternateSetting : 0x00000000
 bNumEndpoints : 0x00000000
 bInterfaceClass : 0x00000001
 bInterfaceSubClass : 0x00000001
 bInterfaceProtocol : 0x00000000
 iInterface : 0x00000000

 uhi_midi_install ignoring interface; class: 0x00000001 ; subclass: 0x00000001
 sorry nerd, your interface is not supported.
 uhi_midi_install ignoring descriptor; type: 0x00000024
 bLength : 0x00000009
 bInterfaceNumber : 0x00000001
 bAlternateSetting : 0x00000000
 bNumEndpoints : 0x00000002
 bInterfaceClass : 0x00000001
 bInterfaceSubClass : 0x00000003
 bInterfaceProtocol : 0x00000000
 iInterface : 0x00000000

 class/subclass matches audio/MIDI. 
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 allocating bulk endpoint (  input )
 uhi_midi_install ignoring descriptor; type: 0x00000025
 allocating bulk endpoint (  output )
 uhi_midi_install ignoring descriptor; type: 0x00000025
 completed MIDI device install
 enabling UHI, idx: 0
 enabling UHI, idx: 1
 enabling UHI, idx: 2
 finished uhi_midi_enable

Controller that doesn't work outputs

received device descriptor. 
 address: 00000001
 speed: 00000001

 dev desc -> bLength : 00000012
 dev desc -> bDescriptorType : 00000001
 dev desc -> bcdUSB : 00000002
 dev desc -> bDeviceClass : 00000000
 dev desc -> bDeviceSubClass : 00000000
 dev desc -> bDeviceProtocol : 00000000
 dev desc -> bMaxPacketSize0 : 00000040
 dev desc -> idVendor : 0000CC1A
 dev desc -> idProduct : 0000013C
 dev desc -> bcdDevice : 00001300
 dev desc -> iManufacturer : 00000001
 dev desc -> iProduct : 00000002
 dev desc -> iSerialNumber : 00000003
 dev desc -> bNumConfigurations : 00000001

 conf desc -> bLength : 00000009
 conf desc -> bDescriptorType : 00000002
 conf desc -> wTotalLength : 00006500
 conf desc -> bNumInterfaces : 00000002
 conf desc -> bConfigurationValue : 00000001
 conf desc -> iConfiguration : 00000000
 conf desc -> bmAttributes : 000000A0
 conf desc -> bMaxPower : 00000014
 run uhi_ftdi_install
 uhi_midi_install ignoring descriptor; type: 0x00000002
 bLength : 0x00000009
 bInterfaceNumber : 0x00000000
 bAlternateSetting : 0x00000000
 bNumEndpoints : 0x00000000
 bInterfaceClass : 0x00000001
 bInterfaceSubClass : 0x00000001
 bInterfaceProtocol : 0x00000000
 iInterface : 0x00000000

 uhi_midi_install ignoring interface; class: 0x00000001 ; subclass: 0x00000001
 sorry nerd, your interface is not supported.
 uhi_midi_install ignoring descriptor; type: 0x00000024
 bLength : 0x00000009
 bInterfaceNumber : 0x00000001
 bAlternateSetting : 0x00000000
 bNumEndpoints : 0x00000002
 bInterfaceClass : 0x00000001
 bInterfaceSubClass : 0x00000003
 bInterfaceProtocol : 0x00000000
 iInterface : 0x00000000

 class/subclass matches audio/MIDI. 
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 uhi_midi_install ignoring descriptor; type: 0x00000024
 allocating bulk endpoint (  output )
 uhi_midi_install ignoring descriptor; type: 0x00000025
 allocating bulk endpoint (  input )
 uhi_midi_install ignoring descriptor; type: 0x00000025
 completed MIDI device install
 enabling UHI, idx: 0
 enabling UHI, idx: 1
 enabling UHI, idx: 2
 finished uhi_midi_enable
 set_param_value, index: 73 , value: 0x00002D00
 requesting note_scaler value for input: 0x00002D00 ; result: 0x006E0000 , scaled: 0x006E0000
 set_param_value, index: 72 , value: 0x00002D00
 requesting note_scaler value for input: 0x00002D00 ; result: 0x006E0000 , sca
 event queue full!
 event queue full!led: 0x006E0000
 set_param_value, index: 69 , value: 0x00000000 , scaled: 0x00000000
 set_param_value, index: 68 , value: 0x00000000 , scaled: 0x00000000
 event queue full!
 event queue full!
 set_param_value, index: 73 , value: 0x00007200
 requesting note_scaler value for input: 0x00007200 ; result: 0x171FE927 , scaled: 0x171FE927
 set_param_value, index: 72 , value: 0x000072
 event queue full!
 event queue full!00
 requesting note_scaler value for input: 0x00007200 ; result: 0x171FE927 , scaled: 0x171FE927
 set_param_value, index: 69 , value: 0x00000000 , scaled: 0x00000000
 set_param_value, index: 68 , value: 0x00000000 , scaled: 
 event queue full!0x00000000
 set_param_value, index: 73 , value: 0x00001500
 requesting note_scaler value for input: 0x00001500 ; result: 0x001B8000 , scaled: 
 event queue full!
 event queue full!0x001B8000
 set_param_value, index: 72 , value: 0x00001500
 requesting note_scaler value for input: 0x00001500 ; result: 0x001B8000 , scaled: 0x001B8000
 set_param_value, index: 69 
 event queue full!
 event queue full!, value: 0x00003500 , scaled: 0x03B604EF
 set_param_value, index: 68 , value: 0x00003500 , scaled: 0x03B604EF
 set_param_value, index: 73 , value: 0x00005A00
 requesting
 event queue full!
 event queue full! note_scaler value for input: 0x00005A00 ; result: 0x05C7FA49 , scaled: 0x05C7FA49
 set_param_value, index: 72 , value: 0x00005A00
 requesting note_scaler value for input
 event queue full!: 0x00005A00 ; result: 0x05C7FA49 , scaled: 0x05C7FA49
 set_param_value, index: 69 , value: 0x00005400 , scaled: 0x0BEFBEFA
 set_param_value, index: 68 , value: 0x00005400 , scaled: 0x0BEFBEFA

 event queue full!
 event queue full!set_param_value, index: 73 , value: 0x00001500
 requesting note_scaler value for input: 0x00001500 ; result: 0x001B8000 , scaled: 0x001B8000
 set_param_value, index: 72 , 
 event queue full!
 event queue full!value: 0x00001500
 requesting note_scaler value for input: 0x00001500 ; result: 0x001B8000 , scaled: 0x001B8000
 set_param_value, index: 69 , value: 0x00003500 , scaled: 0x03B604EF
 event queue full!
 event queue full!
 set_param_value, index: 68 , value: 0x00003500 , scaled: 0x03B604EF
 set_param_value, index: 73 , value: 0x00005A00
 requesting note_scaler value for input: 0x00005A00 ; result: 0x05C7FA49 , scaled: 0x05C7FA49
 set_param_value, index: 72 , value: 0x00005A00
 requesting note_scaler value for input: 0x00005A00 ; result: 0x05C7FA49 , scaled: 0x05C7FA49
 set_param_value, inde
 event queue full!
 event queue full!x: 69 , value: 0x00005400 , scaled: 0x0BEFBEFA
 set_param_value, index: 68 , value: 0x00005400 , scaled: 0x0BEFBEFA
 set_param_value, index: 73 , value: 0x00005A00
 requesting note_scaler value for input: 0x00005A00 ; result: 0x05C7FA49 , scaled: 0x05C7FA49
 set_param_value, index: 72 , value: 0x00005A00
 requesting note_scaler value for input: 0x00005A00 ; result: 0x05C7FA49 , scaled: 0x05C7FA49
 set_param_value, index: 69 , value: 0x00005400 , scaled: 0x0BEFBEFA
 set_param_value, index: 68 , value: 0x00005400 , scaled: 0x0BEFBEFA
 set_param_value, index: 73 , value: 0x00004D00
 requesting note_scaler value for input: 0x00004D00 ; result: 0x02BA74DA , scaled: 0x02BA74DA
 set_param_value, index: 72 , value: 0x00004D00
 requesting note_scaler value for input: 0x00004D00 ; result: 0x02BA74DA , scaled: 0x02BA74DA
 set_param_value, index: 69 , value: 0x00000100 , scaled: 0x0011ECC5
 set_param_value, index: 68 , value: 0x00000100 , scaled: 0x0011ECC5
 set_param_value, index: 73 , value: 0x00003600
 requesting note_scaler value for input: 0x00003600 ; result: 0x00B8FF49 , scaled: 0x00B8FF49
 set_param_value, index: 72 , value: 0x00003600
 requesting note_scaler value for input: 0x00003600 ; result: 0x00B8FF49 , scaled: 0x00B8FF49
 set_param_value, index: 69 , value: 0x00000000 , scaled: 0x00000000
 set_param_value, index: 68 , value: 0x00000000 , scaled: 0x00000000
 set_param_value, index: 73 , value: 0x00001500
 requesting note_scaler value for input: 0x00001500 ; result: 0x001B8000 , scaled: 0x001B8000
 set_param_value, index: 72 , value: 0x00001500
 requesting note_scaler value for input: 0x00001500 ; result: 0x001B8000 , scaled: 0x001B8000
 set_param_value, index: 69 , value: 0x00003500 , scaled: 0x03B604EF
 set_param_value, index: 68 , value: 0x00003500 , scaled: 0x03B604EF
 set_param_value, index: 73 , value: 0x00007400
 requesting note_scaler value for input: 0x00007400 ; result: 0x19F4E00A , scaled: 0x19F4E00A
 set_param_value, index: 72 , value: 0x00007400
 requesting note_scaler value for input: 0x00007400 ; result: 0x19F4E00A , scaled: 0x19F4E00A
 set_param_value, index: 69 , value: 0x00001F00 , scaled: 0x022BABF1
 set_param_value, index: 68 , value: 0x00001F00 , scaled: 0x022BABF1

Both seem to enter the block in uhi_midi.c which sets the iface_supported boolean, and both set that to false. But the second (broken) interface seems to spam the USB port with values without any physical interaction and generates the frequent "event queue full!" messages.

lazzarello commented 6 years ago

It's probably also worth noting that the OLED screen becomes 100% white for about one cycle when I plug in the broken controller.

lazzarello commented 6 years ago

more interesting debugging output. Moving the volume fader on the keyboard spam outputs this debugging message to the console, produces an all white OLED screen.

 midi rx endpoint error
 uhd error: job is already underway
catfact commented 6 years ago

ok, so i haven't looked at this stuff in a while, but there's nothing in there that actually looks wrong. (which just means the debug print statements that happen to be in there are not pointing at the exact problem - to be expected.)

usb is complicated. its normal for a device to supply multiple interfaces - in fact for audio/midi devices it's required. the [0x1 , 0x1] interface that we're skipping is the "audio control" interface (audio devices have to supply it, and midi devices are subclasses of audio devices, but they generally don't have a meaningful use for the AC interface.) midi devices also have to supply a "midi streaming" interface, which must include "bulk data transfer" or "interrupt" endpoints, and most in addition have "isochronous" endpoints for hosts that can support the necessary bandwidth. we're polling the device, so we want to the use bulk/interrupt async methods.

anyways: theory 1): for some reason we're mistakenly using the isochronous endpoints for this device, when we should be using the bulk/interrupt endpoints. thus, an endless number of packets are available every single time we poll the device.

theory 2): we are making some kind of bad assumption in midi.c:midi_parse() - which is not about parsing note messages &c, but about watching incoming packets for status/data and signalling events when completed packet comes in. (obviously it's a pretty crude system; i had a lot of functionality to implement, midi wasn't actually top priority, and just did what i could to get it working with limited test coverage for different devices.) i'm sure there is a way to break this if the controller sends extra keep-alive messages or something.

@ngwese - didn't you do a lot of midi cleanup for libavr32 at some point? can we ditch all this horrible old code?

if i have time this weekend i'll go through the code in more detail and try to pinpoint some other things to look at.

if you really want to get into this stuff you could also try using usbmon or wireshark to get raw usb packets from the device on linux, see how the OS deals with handshaking for this keyboard.

here's a good page of reference material, in addition to the official usb class specs: http://www.beyondlogic.org/usbnutshell/usb5.shtml#InterfaceDescriptors this appnote from silabs may also help: https://www.silabs.com/documents/public/application-notes/AN295.pdf and of course there is the usb.org spec: http://www.usb.org/developers/docs/devclass_docs/midi10.pdf

btw: "event queue full" and screen glitches are common sights in aleph debugging sessions if there is tons of serial output being printed, because this in itself can make it hard for the processor to keep up with incoming realtime events.

tehn commented 6 years ago

apologies for the candor, but a quicker path to victory (and to save dev energy for more fun things) might be to get a different cheap midi interface that is known to work.

it's a rabbit hole to fix midi inplementations for every interface on the market.

On Sat, Nov 4, 2017 at 4:27 PM ezra buchla notifications@github.com wrote:

ok, so i haven't looked at this stuff in a while, but there's nothing in there that actually looks wrong. (which just means the debug print statements that happen to in there are not pointing at the exact problem - to be expected.)

usb is complicated. its normal for a device to supply multiple interfaces

  • in fact for audio/midi devices it's required. the [0x1 , 0x1] interface that we're skipping is the "audio control" interface. midi devices also have to supply a "bulk data transfer" or "interrupt" interface, and most in addition have a "streaming" or "isochronous" interface for hosts that can support the necessary badnwidth.

here's a good page of reference material, in addition to the official usb class specs: http://www.beyondlogic.org/usbnutshell/usb5.shtml#InterfaceDescriptors

anyways: theory 1): for some reason we're mistakenly using the isochronous interface for this device, when we should be using the bulk/interrupt interface. an endless number of packets are available every single time we poll the device.

theory 2): we are making some kind of bad assumption in midi.c:midi_parse() - which is not about parsing note messages &c, but about watching incoming packets for status/data and signalling events when completed packet comes in. obviously it's a pretty crude system; i had a lot of functionality to implement, midi wasn't actually top priority, and just did what i could to get it working with limited test coverage for different devices.

@ngwese https://github.com/ngwese - didn't you do a lot of midi cleanup for libavr32 at some point?

if i have time this weekend i'll go through the code in more detail and try to pinpoint some other things to look at.

if you really want to get into this stuff you could also try using usbmon or wireshark to get raw usb packets from the device on linux, see how the OS deals with handshaking for this keyboard.

"event queue full" and screen glitches are common sights in debugging sessions if there is tons of serial output being printed, because this in itself can make it hard for the processor to keep up with incoming realtime events.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/monome/aleph/issues/287#issuecomment-341927484, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPEcBN1Tbnqx8Qo3R_WyD2PIBHgwTvMks5szMilgaJpZM4QRynN .

catfact commented 6 years ago

to be clear, i absolutely agree. i doubt i'll actually be able to put any more time into this.

on linux, for example, broad support for usb midi- and hid-class devices, has been an incremental project spanning many many years and involving many many people.

if OP wants to use this as an opportunity to get hands dirty with the guts of USB interfaces and endpoints, while making midi support in aleph/libavr32 more robust, i'm all for it. but this is not really something that blindly debugging by email is going to help with.

lazzarello commented 6 years ago

@tehn I already have a MIDI interface that works. It's cool, I'm interested in this kind of hardware debugging. Thanks for the tips!

lazzarello commented 6 years ago

I have USB protocol dumps from wireshark and the midi_parse() function is fully commented with debug output. I also have a keyboard that works, as a reference. I'll see how far I can get with this.

@catfact wanna come to Noisebridge for a debugging session? :)

ngwese commented 6 years ago

@catfact regarding cleanup of the midi bits in libavr32 - yes I did end up modifying a bunch of stuff. After reading the USB MIDI spec I came to the conclusion that the aleph code was trying to handle cases which only occurred when interacting w/ a real MIDI DIN jack. USB MIDI is more constrained w.r.t. how messages are formatted in USB frames so I removed a bunch of code. IIRC the aleph MIDI code could fail in certain cases if USB frames got dropped or sysex came through.

The simplified code in libavr32 basically shunts the incoming packets into the event queue: https://github.com/monome/libavr32/blob/master/src/usb/midi/midi.c#L63-L93

...compared to the aleph avr32 code which does much more: https://github.com/monome/aleph/blob/dev/avr32_lib/src/usb/midi/midi.c#L67-L149

I haven't back ported any of the libavr32 MIDI changes to the aleph code base but according to this post the changes apparently work fine on the aleph as well: https://llllllll.co/t/aleph-refactoring-and-integration/5130/55?u=ngwese

ngwese commented 6 years ago

I should add that various MIDI devices such as a ROLI block and Linnstrument were generating MIDI streams which would overwhelm and/or lockup the earthsea/ansible (aka libavr32) code.

Recent firmware for ROLI block does seem to fix the problems I was running into last year. I mention this in support of @tehn's comment earlier. I've come to realize how variable USB MIDI devices are and how complex USB is. Without a ton of devices on hand to test it is really difficult to make things bullet proof.

boqs commented 6 years ago

I haven't back ported any of the libavr32 MIDI changes to the aleph code base but according to this post the changes apparently work fine on the aleph as well: https://llllllll.co/t/aleph-refactoring-and-integration/5130/55?u=ngwese

Yup, my public dev branch is now using libavr32 & this is the firmware I'm running whilst working on fmsynth module. Haven't observed any unexpected regressions so far...

boqs commented 6 years ago

@lazzarello I've overhauled aleph's midi support in an effort to get things working properly with all my midi gear - would you be able to help me test that branch with your midi hardware?

https://github.com/boqs/aleph/tree/midi_common https://github.com/boqs/libavr32/tree/midi_common

So far things are working right with EMU midi1x1Tab (input/output) & a YouRock midi guitar :metal:

According to my observations the midi output support on monome/dev is simply buggy, and furthermore I claim that the changes on this branch fix the bugs! But should warn you before enlisting help there's a decent probability I'm barking up the wrong tree...

lazzarello commented 6 years ago

Branch built, firmware loaded and default waves scene loads as expected. Heading into the studio to try out the MIDI controller.

lazzarello commented 6 years ago

Confirmed MIDI works with the midi_common branch on my Aleph. Closing.