-
Notifications
You must be signed in to change notification settings - Fork 134
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
[eppp]: Add support for USB host -- device transport #590
base: master
Are you sure you want to change the base?
[eppp]: Add support for USB host -- device transport #590
Conversation
@@ -14,7 +14,7 @@ brings in the WiFi connectivity from the "SLAVE" microcontroller. | |||
SLAVE micro HOST micro | |||
\|/ +----------------+ +----------------+ | |||
| | | serial line | | | |||
+---+ WiFi NAT PPPoS |======== UART / SPI =======| PPPoS client | | |||
+---+ WiFi NAT PPPoS |=== UART / SPI / SDIO =====| PPPoS client | |
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.
We could remove the “channels” from this image and phrase it as link channel or something like that.
@@ -39,14 +39,19 @@ brings in the WiFi connectivity from the "SLAVE" microcontroller. | |||
|
|||
## Throughput | |||
|
|||
Tested with WiFi-NAPT example, no IRAM optimizations | |||
Tested with WiFi-NAPT example | |||
|
|||
### UART @ 3Mbauds |
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.
Link | TCP | UDP |
---|---|---|
UART @ 3Mbauds | 2Mbits/s | 2Mbits/s |
SPI @ 16MHz | 5Mbits/s | 8Mbits/s |
SDIO | 9Mbits/s | 11Mbits/s |
@@ -128,16 +128,23 @@ void app_main(void) | |||
ESP_ERROR_CHECK(ret); | |||
|
|||
init_network_interface(); // WiFi station if withing SoC capabilities (otherwise a placeholder) | |||
// ESP_ERROR_CHECK(esp_netif_init()); |
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.
Clean?
ESP_GOTO_ON_FALSE((s_essl_mutex = xSemaphoreCreateMutex()), ESP_ERR_NO_MEM, err, TAG, "failed to create semaphore"); | ||
return ret; | ||
|
||
err: |
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.
Are we leaking s_card
on failure?
Please also review other initialized data for cleanup, e.g. s_essl_mutex
* @param[in] event Device event type and data | ||
* @param[in] user_ctx Argument we passed to the device open function | ||
*/ | ||
static void handle_event(const cdc_acm_host_dev_event_data_t *event, void *user_ctx) |
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.
Should we allow users to add a custom event handler? Maybe add a parameter to init function and use this default function if not provided.
depends on #511
both eppp server and client can choose to act as USB CDC ACM host or a device.
(may need a minor refactor, as we support more and more transports)