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

fix(host/cdc): Fix TX timeout reaction #41

Merged
merged 1 commit into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion host/class/cdc/usb_host_cdc_acm/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## [Unreleased]
## 2.0.3

- Added `cdc_acm_host_cdc_desc_get()` function that enables users to get CDC functional descriptors of the device
- Fixed closing of a CDC device from multiple threads
- Fixed reaction on TX transfer timeout (https://github.com/espressif/esp-protocols/issues/514)

## 2.0.2

Expand Down
10 changes: 6 additions & 4 deletions host/class/cdc/usb_host_cdc_acm/cdc_acm_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,6 @@ static esp_err_t cdc_acm_transfers_allocate(cdc_dev_t *cdc_dev, const usb_ep_des
cdc_dev->ctrl_transfer->timeout_ms = 1000;
cdc_dev->ctrl_transfer->bEndpointAddress = 0;
cdc_dev->ctrl_transfer->device_handle = cdc_dev->dev_hdl;
cdc_dev->ctrl_transfer->context = cdc_dev;
cdc_dev->ctrl_transfer->callback = out_xfer_cb;
cdc_dev->ctrl_transfer->context = xSemaphoreCreateBinary();
ESP_GOTO_ON_FALSE(cdc_dev->ctrl_transfer->context, ESP_ERR_NO_MEM, err, TAG,);
Expand Down Expand Up @@ -1152,16 +1151,19 @@ esp_err_t cdc_acm_host_data_tx_blocking(cdc_acm_dev_hdl_t cdc_hdl, const uint8_t
}

ESP_LOGD("CDC_ACM", "Submitting BULK OUT transfer");
SemaphoreHandle_t transfer_finished_semaphore = (SemaphoreHandle_t)cdc_dev->data.out_xfer->context;
xSemaphoreTake(transfer_finished_semaphore, 0); // Make sure the semaphore is taken before we submit new transfer

memcpy(cdc_dev->data.out_xfer->data_buffer, data, data_len);
cdc_dev->data.out_xfer->num_bytes = data_len;
cdc_dev->data.out_xfer->timeout_ms = timeout_ms;
ESP_GOTO_ON_ERROR(usb_host_transfer_submit(cdc_dev->data.out_xfer), unblock, TAG,);

// Wait for OUT transfer completion
taken = xSemaphoreTake((SemaphoreHandle_t)cdc_dev->data.out_xfer->context, pdMS_TO_TICKS(timeout_ms));
taken = xSemaphoreTake(transfer_finished_semaphore, pdMS_TO_TICKS(timeout_ms));
if (!taken) {
// Reset the endpoint
cdc_acm_reset_transfer_endpoint(cdc_dev->dev_hdl, cdc_dev->data.out_xfer);
cdc_acm_reset_transfer_endpoint(cdc_dev->dev_hdl, cdc_dev->data.out_xfer); // Resetting the endpoint will cause all in-progress transfers to complete
ESP_LOGW(TAG, "TX transfer timeout");
ret = ESP_ERR_TIMEOUT;
goto unblock;
}
Expand Down
6 changes: 5 additions & 1 deletion host/class/cdc/usb_host_cdc_acm/idf_component.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
## IDF Component Manager Manifest File
version: "2.0.2~1"
version: "2.0.3"
description: USB Host CDC-ACM driver
tags:
- usb
- usb_host
- cdc
url: https://github.com/espressif/esp-usb/tree/master/host/class/cdc/usb_host_cdc_acm
dependencies:
idf: ">=4.4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,6 @@ TEST_CASE("functional_descriptor", "[cdc_acm]")
*/
TEST_CASE("closing", "[cdc_acm]")
{
nb_of_responses = 0;
cdc_acm_dev_hdl_t cdc_dev = NULL;

test_install_cdc_driver();
Expand All @@ -599,6 +598,40 @@ TEST_CASE("closing", "[cdc_acm]")
vTaskDelay(20); //Short delay to allow task to be cleaned up
}

/* Basic test to check CDC driver reaction to TX timeout */
TEST_CASE("tx_timeout", "[cdc_acm]")
{
cdc_acm_dev_hdl_t cdc_dev = NULL;

test_install_cdc_driver();

const cdc_acm_host_device_config_t dev_config = {
.connection_timeout_ms = 500,
.out_buffer_size = 64,
.event_cb = notif_cb,
.data_cb = handle_rx,
.user_arg = tx_buf,
};

printf("Opening CDC-ACM device\n");
TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_open(0x303A, 0x4002, 0, &dev_config, &cdc_dev)); // 0x303A:0x4002 (TinyUSB Dual CDC device)
TEST_ASSERT_NOT_NULL(cdc_dev);
vTaskDelay(10);

// TX some data with timeout_ms=0. This will cause a timeout
TEST_ASSERT_EQUAL(ESP_ERR_TIMEOUT, cdc_acm_host_data_tx_blocking(cdc_dev, tx_buf, sizeof(tx_buf), 0));
vTaskDelay(100); // Wait before trying new TX

// TX some data again with greater timeout. This will check normal operation
TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_data_tx_blocking(cdc_dev, tx_buf, sizeof(tx_buf), 1000));
tore-espressif marked this conversation as resolved.
Show resolved Hide resolved
vTaskDelay(100);

TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_close(cdc_dev));
TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_uninstall());

vTaskDelay(20); //Short delay to allow task to be cleaned up
}

/* Following test case implements dual CDC-ACM USB device that can be used as mock device for CDC-ACM Host tests */
void run_usb_dual_cdc_device(void);
TEST_CASE("mock_device_app", "[cdc_acm_device][ignore]")
Expand Down
Loading