-
Notifications
You must be signed in to change notification settings - Fork 93
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
Please do include RNDIS support. The upstream is rewriting the NCM driver and some unstablility will be expected. Also, the current NCM driver is buggy and slower than the RNDIS. If we would like to enable the USB network feature at this moment I think RNDIS is still the way to go |
The current RNDIS driver in TinyUSB does not support FIFO buffering and lacks an effective API to notify the application layer when the previous transmission is complete. Using the more efficient Furthermore, supporting RNDIS is necessary because, in network card applications, Windows 10 does not support the NCM driver. |
Here's a step-by-step manual how to install NCM driver on Windows 10 , if anyone is interested UPDATE: We will update TinyUSB in esp-idf to v0.17 soon. This release brings refactored NCM class and DMA support. If the NCM performs well, we plan to support NCM only, due to our limited resources to maintain 2 USB networking solutions |
Checklist
url
field definedChange description
RNDIS and ECM were not working when selected in menuconfig