Allow using an arbitrary number of ports

[Windfisch]

Hi,

thanks for your great work!

I made adjustments to support an arbitrary, user-selectable number of input and output MIDI ports (I have tested this with up to 16; however, usb-device needs to be patched as in https://github.com/Windfisch/usb-device/tree/buffer-sizes to allow for a control buffer size large enough to hold the rather large descriptors.).

Also I corrected the ports in the descriptor: It seems that the right way[tm] is to have one external in/out jack per embedded out/in jack; Windows ignores embedded jacks that aren't connected to any external jack (or other element possibly), while Linux does just fine.

Only one in/out pair can be used with the default settings, though, but by using the control-buffer-256 feature of usb-device, up to 5 in/out port pairs can be used. Using more ports require aforementioned patch to usb-device.

_Note: tdd.exe, a USB descriptor dumper and validator, still reports two errors (as with the original code): The in and out bulk endpoint descriptors are too short, because the MIDI audio class requires two more bytes. I've fixed this in https://github.com/Windfisch/usbd-midi (master), but this requires a patched version of usb-device._

I have changed the signature of the new() method; one might consider adding a differently-named function instead in order to retain compatibility.

If you have any questions or requests, give me a shout :).

[ho-ho-ho]

I've just tried out this change and it works fine :) 4 in + 4 out directly worked out of the box ... but there is one problem: No matter what cable I try to send data to, it is always sent on the first cable. I've made some tests and it seems like the cable_number field of the UsbMidiEventPacket is ignored:

                    let packet = UsbMidiEventPacket::from_midi(
                        CableNumber::Cable5,
                        packet.message,
                    );
                    defmt::info!("packet {:?}", defmt::Debug2Format(&packet));
                    let bytes: [u8;4] = packet.into();
                    defmt::info!("bytes {=[u8]:X}", bytes);

the output is:

       0 INFO  packet "UsbMidiEventPacket { cable_number: Cable5, message: ControlChange(Channel1, ControlFunction(U7(2)), U7(35)) }"
└─ midi_loopback::__cortex_m_rt_main @ src/bin/midi_loopback.rs:73
       1 INFO  bytes [0xF, 0xB0, 0x2, 0x23]
└─ midi_loopback::__cortex_m_rt_main @ src/bin/midi_loopback.rs:75

when using this, you can see the first byte is 0x0F and not 0x5F 0x5B as expected.

I've tried to find the root cause and found this in the U4::combine implementation:

let upper = upper.0.overflowing_shl(8).0;

the upper nibble is shifted left by 8 bits and not 4. This surely is a problem, but changing it to 4 didn't fix the cable number problem. So I'm still not sure where the cable number problem is coming from, maybe you have an idea :)

Edit: forget the shift stuff in the U4::combine function ... I didn't know how overflowing_shl works.

Further edit: After more testing, it was the shift ... changing from 8 to 4 made it work. I just had some inconsistencies in my git checkouts. Starting from clean ones and only changing this part made it work like it should :)

Aftermath: So the use of overflowing_shl(8) in the U4::combine function is the main source of error here because the return value is not checked (in case of overflow the cable number is just returned unshifted resulting in the upper and lower nibble being or'ed together, that resulted in the 0x0F in the test above instead of 0x0B which would be correct: 0x0B | 0x05 = 0x0F ) and the shift is using 8 instead of 4 bits. Maybe I'll make a pull request with a corrected version tho I'm not sure if it's worth the work since this repository seems to be abandoned :(

[Windfisch]

Hey, thanks for the debugging work. I thought i have tested that, tho; what platform are you on?

I am actively using this driver so I'm willing to fork and maintain it. So feel free so file these PRs against my repo :)

[ho-ho-ho]

Yeah that was a strange error to find. As a device, I'm using a STM32F303CC, tested it on a Windows 10 host, will check on arch linux later. The problem was only there when sending on cables other than the first one, receiving worked fine with the all cables. But well, looking at that 8bit shift that's no surprise ... the question is more like: how did it even work for that long :laughing: (we've all been there).

Nice to see someone take care, I'll hand in a PR later :)

[Windfisch]

This is interesting. I'd have thought that it should behave more or less identical to the STM32F103. Yea, the old question in software development: how did that ever work?! :D

edit: ah I can tell you how: It likely didn't I did not use that functionality; instead, I parsed the raw packets by hand. :D

[btrepp]

Hi all. I'm happy to give contributor access here. Apologies I didn't get notifications.

One of the initial decisions about small in/outs was to make it easier to use in daw software. A bunch of never plugged up outs/ins are confusing. I always intended to provide a more dynamic intializer, allowing people to have there use case.