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

Cheap MIDI interface fails with MIDINOTE operator #287

Closed
lazzarello opened this issue Nov 3, 2017 · 24 comments
Closed

Cheap MIDI interface fails with MIDINOTE operator #287

lazzarello opened this issue Nov 3, 2017 · 24 comments

Comments

@lazzarello
Copy link

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
Copy link
Collaborator

catfact commented Nov 3, 2017

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:

  • add some print_dbg statements in (for example) the bees midi connect handler and/or lower-level midi-usb functions in the avr32 lib (see the many many commented-out prints in these sources)
  • build a debug version of BEES (make clean && make without R=1)
  • copy debug version to sdcard and flash it with the bootloader
  • connect aleph usb device port to host computer
  • open a 115200 baud serial connection to see debug messages

@lazzarello
Copy link
Author

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
Copy link
Member

ngwese commented Nov 4, 2017

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
Copy link
Collaborator

catfact commented Nov 4, 2017

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
Copy link
Member

ngwese commented Nov 4, 2017

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

@catfact
Copy link
Collaborator

catfact commented Nov 4, 2017

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
Copy link
Collaborator

catfact commented Nov 4, 2017

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

@catfact
Copy link
Collaborator

catfact commented Nov 4, 2017

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

@lazzarello
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

catfact commented Nov 4, 2017

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
Copy link
Member

tehn commented Nov 4, 2017 via email

@catfact
Copy link
Collaborator

catfact commented Nov 4, 2017

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
Copy link
Author

@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
Copy link
Author

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
Copy link
Member

ngwese commented Nov 5, 2017

@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
Copy link
Member

ngwese commented Nov 5, 2017

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
Copy link
Contributor

boqs commented Nov 6, 2017

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
Copy link
Contributor

boqs commented Nov 14, 2017

@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 🤘

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
Copy link
Author

lazzarello commented Nov 21, 2017

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

@lazzarello
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants