henla464 / USB_Host_Shield_Library_2.0

Forked "USB Host Shield Library 2.0" with CP210x driver
15 stars 2 forks source link

Incorrect SubClassID? #2

Closed syonbori closed 3 years ago

syonbori commented 4 years ago

Hi, I wanna use a CP210x device with ESP32, and found your great codes. I'm just a newbie about USB and CP210x so this repo is very helpful. really thanks!

Since your version not supports ESP32 chips, I tried to follow original version with your great cp210x related codes, and found some bugs.

https://github.com/henla464/USB_Host_Shield_Library_2.0/blob/9c8dc1c6293da0d6a8fdaf439aaa727e269d45fe/CP210x.cpp#L131

this specifies ClassID=FF, SubClassID=FF, ProtocolID=FF. but on my CP2102 device, it should be CID=FF, SCID=00, PID=00.

but this still works because:

https://github.com/henla464/USB_Host_Shield_Library_2.0/blob/9c8dc1c6293da0d6a8fdaf439aaa727e269d45fe/confdescparser.h#L164-L167

this delay(10); breaks the if block and runs next if(theXtractor) { ... block. In CID,SCID,PID=FF,FF,FF case, the isGoodInterface will be false. but the above code not mind it. Original library code respects the isGoodInterface ( https://github.com/felis/USB_Host_Shield_2.0/blob/cd87628af4a693eeafe1bf04486cf86ba01d29b8/confdescparser.h#L167-L170 )

In my case, use original confdescparser.h and fix your CP210x.cpp like:

ConfigDescParser < 0xFF, 0x00, 0x00, CP_MASK_COMPARE_ALL> confDescrParser(this);
// or:
// ConfigDescParser < 0xFF, 0xFF, 0xFF, CP_MASK_COMPARE_CLASS> confDescrParser(this);

solved most of porting problems.

is this reasonable?

henla464 commented 4 years ago

Hello, I am glad someone has use for this code :)  It was some time ago I wrote that, and I did copy and modify from some other code... I remember there was a delay that somehow made it work, haha. I think I never realized that it broke that if statement (I normally use {} but since this was code copied...).  So I think you have solved an old mystery!

henla464 commented 4 years ago

I checked in the changes you proposed. I don't have any way to test this...I don't have the hardware any longer... Let me know if you notice something that is wrong!

syonbori commented 4 years ago

Hello, thank you very much! I found FF,00,00 in CP210x datasheet so this change looks OK.

I gonna use the fixed version for my developing product. When it works as expected, i wanna update related codes to follow the upstream version.