felis / UHS30

For information about the project see README below
GNU General Public License v2.0
133 stars 39 forks source link

Enumerate fails when interface has not endpoint #25

Closed YuuichiAkagawa closed 7 years ago

YuuichiAkagawa commented 7 years ago

Hello, I implementing MIDI class. Some USB MIDI device have multiple interface. But the first interface may not have an endpoint. In this case, enumeration is fail.

Configuration descriptor
0x0000 09 02 65 00 02 01 00 A0 32 09 04 00 00 00 01 01
0x0010 00 00 09 24 01 00 01 09 00 01 01 09 04 01 00 02
0x0020 01 03 00 00 07 24 01 00 01 41 00 06 24 02 01 01
0x0030 00 06 24 02 02 02 00 09 24 03 01 03 01 02 01 00
0x0040 09 24 03 02 04 01 01 01 00 09 05 01 02 40 00 00
0x0050 00 00 05 25 01 01 01 09 05 81 02 40 00 00 00 00
0x0060 05 25 01 01 03
Host initialized.
configs: 1
CONFIGURATION: 0, bNumInterfaces 2, wTotalLength 101
bLength: 9 bDescriptorType: 02
bLength: 9 bDescriptorType: 04
INTERFACE DESCRIPTOR FOUND
Getting 0 endpoints
ENDPOINT DESCRIPTORS FILLED
TestInterface VID:09e8 PID:007c Class:00 Subclass:00 Protocol 00
MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 1
bLength: 36 bDescriptorType: 01     <- wrong
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 05
bLength: 5 bDescriptorType: 25
bLength: 9 bDescriptorType: 05
bLength: 5 bDescriptorType: 25
USB_INTERFACE END OF STREAM

I tried commenting out the following line. https://github.com/felis/UHS30/blob/master/libraries/UHS_host/UHS_host_INLINE.h#L764-L766

                        if(!ei->interface.numep) {
//                                rcode = getone(pep, left, read, data, offset);
//                                if(rcode)
//                                        return rcode;
                        }

It's work fine.

Host initialized.
configs: 1
CONFIGURATION: 0, bNumInterfaces 2, wTotalLength 101
bLength: 9 bDescriptorType: 02
bLength: 9 bDescriptorType: 04
INTERFACE DESCRIPTOR FOUND
Getting 0 endpoints
ENDPOINT DESCRIPTORS FILLED
TestInterface VID:09e8 PID:007c Class:00 Subclass:00 Protocol 00
MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 1
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 04
INTERFACE DESCRIPTOR FOUND
Getting 2 endpoints
bLength: 7 bDescriptorType: 24
bLength: 6 bDescriptorType: 24
bLength: 6 bDescriptorType: 24
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 05
bLength: 5 bDescriptorType: 25
bLength: 9 bDescriptorType: 05
ENDPOINT DESCRIPTORS FILLED
TestInterface VID:09e8 PID:007c Class:00 Subclass:00 Protocol 00
MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 3
bLength: 5 bDescriptorType: 25
USB_INTERFACE END OF STREAM
bLength: 9 bDescriptorType: 02
bLength: 9 bDescriptorType: 04
INTERFACE DESCRIPTOR FOUND
Getting 0 endpoints
ENDPOINT DESCRIPTORS FILLED
MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 1
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 04
INTERFACE DESCRIPTOR FOUND
Getting 2 endpoints
bLength: 7 bDescriptorType: 24
bLength: 6 bDescriptorType: 24
bLength: 6 bDescriptorType: 24
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 24
bLength: 9 bDescriptorType: 05
bLength: 5 bDescriptorType: 25
bLength: 9 bDescriptorType: 05
ENDPOINT DESCRIPTORS FILLED
MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 3
MIDI: SetInterface
bLength: 5 bDescriptorType: 25
MIDI: device detected
Connected to MIDI

Am I wrong? or correct?

xxxajk commented 7 years ago

I'll look into it later tonight.

On Feb 27, 2017 7:51 AM, "Yuuichi Akagawa" notifications@github.com wrote:

Hello, I implementing MIDI class. Some USB MIDI device have multiple interface. But the first interface may not have an endpoint. In this case, enumeration is fail.

Configuration descriptor 0x0000 09 02 65 00 02 01 00 A0 32 09 04 00 00 00 01 01 0x0010 00 00 09 24 01 00 01 09 00 01 01 09 04 01 00 02 0x0020 01 03 00 00 07 24 01 00 01 41 00 06 24 02 01 01 0x0030 00 06 24 02 02 02 00 09 24 03 01 03 01 02 01 00 0x0040 09 24 03 02 04 01 01 01 00 09 05 01 02 40 00 00 0x0050 00 00 05 25 01 01 01 09 05 81 02 40 00 00 00 00 0x0060 05 25 01 01 03

Host initialized. configs: 1 CONFIGURATION: 0, bNumInterfaces 2, wTotalLength 101 bLength: 9 bDescriptorType: 02 bLength: 9 bDescriptorType: 04 INTERFACE DESCRIPTOR FOUND Getting 0 endpoints ENDPOINT DESCRIPTORS FILLED TestInterface VID:09e8 PID:007c Class:00 Subclass:00 Protocol 00 MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 1 bLength: 36 bDescriptorType: 01 <- wrong bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 05 bLength: 5 bDescriptorType: 25 bLength: 9 bDescriptorType: 05 bLength: 5 bDescriptorType: 25 USB_INTERFACE END OF STREAM

I tried commenting out the following line. https://github.com/felis/UHS30/blob/master/libraries/ UHS_host/UHS_host_INLINE.h#L764-L766

                    if(!ei->interface.numep) {

// rcode = getone(pep, left, read, data, offset); // if(rcode) // return rcode; }

It's work fine.

Host initialized. configs: 1 CONFIGURATION: 0, bNumInterfaces 2, wTotalLength 101 bLength: 9 bDescriptorType: 02 bLength: 9 bDescriptorType: 04 INTERFACE DESCRIPTOR FOUND Getting 0 endpoints ENDPOINT DESCRIPTORS FILLED TestInterface VID:09e8 PID:007c Class:00 Subclass:00 Protocol 00 MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 1 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 04 INTERFACE DESCRIPTOR FOUND Getting 2 endpoints bLength: 7 bDescriptorType: 24 bLength: 6 bDescriptorType: 24 bLength: 6 bDescriptorType: 24 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 05 bLength: 5 bDescriptorType: 25 bLength: 9 bDescriptorType: 05 ENDPOINT DESCRIPTORS FILLED TestInterface VID:09e8 PID:007c Class:00 Subclass:00 Protocol 00 MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 3 bLength: 5 bDescriptorType: 25 USB_INTERFACE END OF STREAM bLength: 9 bDescriptorType: 02 bLength: 9 bDescriptorType: 04 INTERFACE DESCRIPTOR FOUND Getting 0 endpoints ENDPOINT DESCRIPTORS FILLED MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 1 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 04 INTERFACE DESCRIPTOR FOUND Getting 2 endpoints bLength: 7 bDescriptorType: 24 bLength: 6 bDescriptorType: 24 bLength: 6 bDescriptorType: 24 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 24 bLength: 9 bDescriptorType: 05 bLength: 5 bDescriptorType: 25 bLength: 9 bDescriptorType: 05 ENDPOINT DESCRIPTORS FILLED MIDI: checking vid 09e8, pid 007c, iface class 1, subclass 3 MIDI: SetInterface bLength: 5 bDescriptorType: 25 MIDI: device detected Connected to MIDI

Am I wrong? or correct?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/felis/UHS30/issues/25, or mute the thread https://github.com/notifications/unsubscribe-auth/ADskzDoUnq_RBc8HSyk2a5YVWIbIo1SYks5rgsbLgaJpZM4MNESo .

xxxajk commented 7 years ago

Can you issue a pull request for the implementation thus-far without this change so that I may compare it on what MIDI I have here?

YuuichiAkagawa commented 7 years ago

I send pull request. I test with Arduino UNO + USB Host Shield only.

If you can use LUFA, you can test it with this code. https://github.com/YuuichiAkagawa/LUFA_MIDI_Collection

This issue's device is "AKAI MPK mini".

xxxajk commented 7 years ago

I have this device too.

YuuichiAkagawa commented 7 years ago

Additional debug information. Running with USB_MIDI_Dump.

configuration descriptor
0x0000 09 02 65 00 02 01 00 A0
0x0008 32 09 04 00 00 00 01 01
0x0010 00 00 09 24 01 00 01 09
0x0018 00 01 01 09 04 01 00 02
0x0020 01 03 00 00 07 24 01 00
0x0028 01 41 00 06 24 02 01 01
0x0030 00 06 24 02 02 02 00 09
0x0038 24 03 01 03 01 02 01 00
0x0040 09 24 03 02 04 01 01 01
0x0048 00 09 05 01 02 40 00 00
0x0050 00 00 05 25 01 01 01 09
0x0058 05 81 02 40 00 00 00 00
0x0060 05 25 01 01 03

l:left, r:read, o:offset, d:data uhs30_midi_debug

xxxajk commented 7 years ago

Interesting find. Can you tell me if this change affects other devices negatively?

YuuichiAkagawa commented 7 years ago

Probably no problem. But I don't have any other devices. I can't check it.

I can't understand why this code is necessary. I think that it is unnecessary. https://github.com/felis/UHS30/blob/master/libraries/UHS_host/UHS_host_INLINE.h#L763-L767

                        if(!ei->interface.numep) {
                                rcode = getone(pep, left, read, data, offset);
                                if(rcode)
                                        return rcode;
                        }
xxxajk commented 7 years ago

As far as another device to try, any USB mass storage device will do, for example, a thumb drive or an SDcard to USB reader. As for the code -- This block of code handles a device without any endpoints, and also serves as a simple feedback for cases where there is a bug in the code. The device you are testing managed to trigger it, so in this case it is a potential bug. I'm not aware of any actual devices that could trigger this, except perhaps for some battery chargers, which is possible to connect on boards other than the USB Host Shield.

On Sun, Mar 26, 2017 at 8:43 PM, Yuuichi Akagawa notifications@github.com wrote:

Probably no problem. But I don't have any other devices. I can't check it.

I can't understand why this code is necessary. I think that it is unnecessary. https://github.com/felis/UHS30/blob/master/libraries/ UHS_host/UHS_host_INLINE.h#L763-L767

                    if(!ei->interface.numep) {
                            rcode = getone(pep, left, read, data, offset);
                            if(rcode)
                                    return rcode;
                    }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/felis/UHS30/issues/25#issuecomment-289329956, or mute the thread https://github.com/notifications/unsubscribe-auth/ADskzArSoPk7VDNa6_ZfLwh92XthwHkDks5rpwY7gaJpZM4MNESo .

-- Visit my github for awesome Arduino code @ https://github.com/xxxajk

YuuichiAkagawa commented 7 years ago

I see. When all interface has no endpoint, I understood this code mean. But it is wrong when first interface has no endpoint.

I propose PR: https://github.com/felis/UHS30/pull/27

My USB MIDI Keyboard is commercial device. http://www.akaipro.com/product/mpkmini akaimpkmini

xxxajk commented 7 years ago

Correct. UHS 3.0 is not device Centric instead it is interface Centric. This takes care of the classic problem of multiple interfaces on a single device.

On Mar 27, 2017 10:05 AM, "Yuuichi Akagawa" notifications@github.com wrote:

I see. When all interface has no endpoint, I understood this code mean. But it is wrong when first interface has no endpoint.

I propose PR: #27 https://github.com/felis/UHS30/pull/27

My USB MIDI Keyboard is commercial device. http://www.akaipro.com/product/mpkmini [image: akaimpkmini] https://cloud.githubusercontent.com/assets/926923/24358427/076f78f6-133c-11e7-8727-48e8a9d4459e.png

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/felis/UHS30/issues/25#issuecomment-289463472, or mute the thread https://github.com/notifications/unsubscribe-auth/ADskzHc0SKBzaywQ7dHBO1Gp7ZFnsIoCks5rp8IMgaJpZM4MNESo .

xxxajk commented 7 years ago

Will get a chance to see what the best fix is this coming weekend.

YuuichiAkagawa commented 7 years ago

Thank you for merge.