-
Notifications
You must be signed in to change notification settings - Fork 81
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
fix(usb) compile tinyusb_net.c when RNDIS is enabled (IEC-96) #313
base: master
Are you sure you want to change the base?
Conversation
…o_v_2_6_2 feat: Update expat to v2.6.2
@voedipus Thank you for the PR! we did not test RNDIS, only NCM. That is why why enabled NCM only |
@tore-espressif Actually, it is not working with this MR, but I was able to get RNDIS working with couple of other fixes. I will add changes soon |
@tore-espressif It works now, checked on Linux with esp idf 5.1.3 and Windows 10 (RNDIS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for this PR!
I'm still not sure whether we want to claim support form ECM/RNDIS when we already support NCM. I'll check internally and let you know
config TINYUSB_NET_MODE_RNDIS | ||
bool "RNDIS" | ||
|
||
config TINYUSB_NET_MODE_ECM | ||
bool "ECM" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep it tha way it was? ECM+RNDIS together? There are 2 reasons for this:
- The TinyUSB driver has ECM+RNDIS protocol together
- We usually don't want to enable ECM and disalble RNDIS (or vice versa). RNDIS works with windows, while ECM workds with linux and macOS. Only with both we can support all operating systems
Checklist
url
field definedChange description
RNDIS and ECM were not working when selected in menuconfig