Skip to content

Commit

Permalink
fix(cdc_acm): Fix closing of already closed device
Browse files Browse the repository at this point in the history
  • Loading branch information
tore-espressif committed Feb 8, 2024
1 parent f950437 commit ea6f78e
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 7 deletions.
1 change: 1 addition & 0 deletions host/class/cdc/usb_host_cdc_acm/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## [Unreleased]

- 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

## 2.0.2

Expand Down
23 changes: 20 additions & 3 deletions host/class/cdc/usb_host_cdc_acm/cdc_acm_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -845,13 +845,30 @@ esp_err_t cdc_acm_host_close(cdc_acm_dev_hdl_t cdc_hdl)

xSemaphoreTake(p_cdc_acm_obj->open_close_mutex, portMAX_DELAY);

cdc_dev_t *cdc_dev = (cdc_dev_t *)cdc_hdl;

// Cancel polling of BULK IN and INTERRUPT IN
// Make sure that the device is in the devices list (that it is not already closed)
cdc_dev_t *cdc_dev;
bool device_found = false;
CDC_ACM_ENTER_CRITICAL();
SLIST_FOREACH(cdc_dev, &p_cdc_acm_obj->cdc_devices_list, list_entry) {
if (cdc_dev == (cdc_dev_t *)cdc_hdl) {
device_found = true;
break;
}
}

// Device was not found in the cdc_devices_list; it was already closed, return OK
if (!device_found) {
CDC_ACM_EXIT_CRITICAL();
xSemaphoreGive(p_cdc_acm_obj->open_close_mutex);
return ESP_OK;
}

// No user callbacks from this point
cdc_dev->notif.cb = NULL;
cdc_dev->data.in_cb = NULL;
CDC_ACM_EXIT_CRITICAL();

// Cancel polling of BULK IN and INTERRUPT IN
if (cdc_dev->data.in_xfer) {
ESP_ERROR_CHECK(cdc_acm_reset_transfer_endpoint(cdc_dev->dev_hdl, cdc_dev->data.in_xfer));
}
Expand Down
6 changes: 4 additions & 2 deletions host/class/cdc/usb_host_cdc_acm/include/usb/cdc_acm_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,10 @@ esp_err_t cdc_acm_host_open_vendor_specific(uint16_t vid, uint16_t pid, uint8_t
* @brief Close CDC device and release its resources
*
* @note All in-flight transfers will be prematurely canceled.
* @param cdc_hdl CDC handle obtained from cdc_acm_host_open()
* @return esp_err_t
* @param[in] cdc_hdl CDC handle obtained from cdc_acm_host_open()
* @return
* - ESP_OK: Success - device closed
* - ESP_ERR_INVALID_STATE: cdc_hdl is NULL or the CDC driver is not installed
*/
esp_err_t cdc_acm_host_close(cdc_acm_dev_hdl_t cdc_hdl);

Expand Down
37 changes: 35 additions & 2 deletions host/class/cdc/usb_host_cdc_acm/test/test_cdc_acm_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ TEST_CASE("read_write", "[cdc_acm]")
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);
cdc_acm_host_desc_print(cdc_dev);
vTaskDelay(100);
vTaskDelay(10);

TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_data_tx_blocking(cdc_dev, tx_buf, sizeof(tx_buf), 1000));
TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_data_tx_blocking(cdc_dev, tx_buf, sizeof(tx_buf), 1000));
Expand Down Expand Up @@ -566,6 +566,39 @@ TEST_CASE("functional_descriptor", "[cdc_acm]")
vTaskDelay(20);
}

/**
* @brief Closing procedure test
*
* -# Close already closed device
*/
TEST_CASE("closing", "[cdc_acm]")
{
nb_of_responses = 0;
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);

TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_close(cdc_dev));
printf("Closing already closed device \n");
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 All @@ -576,4 +609,4 @@ TEST_CASE("mock_device_app", "[cdc_acm_device][ignore]")
}
}

#endif
#endif // SOC_USB_OTG_SUPPORTED

0 comments on commit ea6f78e

Please sign in to comment.