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

include pin_arduino.h for variant USB defines #5719

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

hathach
Copy link
Contributor

@hathach hathach commented Sep 29, 2021

Summary

add pin_arduino.h include in core's USB.cpp. This allows macro defined by variant e.g USB_VID, USB_PID to be used correct per board.

Impact

Correct the USB defines such as VID/PID, manufacturer in usb descriptor. Following is lsusb output with Adafruit Metro ESP32-S2 board before and after the PR

Before

Bus 003 Device 094: ID 303a:0002 Espressif Systems METRO_ESP32S2

After

Bus 003 Device 096: ID 239a:80df Adafruit Metro ESP32-S2

@ladyada

@me-no-dev me-no-dev merged commit c5bb833 into espressif:master Oct 1, 2021
@me-no-dev
Copy link
Member

@hathach let me know if you have any issues with Arduino's USB stack :)
BTW if Adafruit's lib uses tinyusb from here directly, you can set CDC control endpoint to 0x85 and it will be assigned an empty fifo, thus allowing one more device to be added (CDC+MSC+HID+Vendor)

@hathach
Copy link
Contributor Author

hathach commented Oct 1, 2021

thanks @me-no-dev , the adafruit library actually use ESP core's configuration descriptor building and USBCDC Serial. Therefore it is all good. For empty fifo for CDC's notification endpoint, yeah, I did notice you have some modification to have 0x85 as empty fifo. I actually plan to do it in a more generic way for tinyusb, basically adding an extra api for cdc driver to call if it is esp32s2 but haven't got time to get into the detail. will tag you if there is an PR for it.

@hathach hathach deleted the include-usb-variant-defines branch December 10, 2021 17:06
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

Successfully merging this pull request may close these issues.

2 participants