fix(host/cdc): Fix TX timeout reaction

After TX transfer timeout we reset the endpoint which causes all
in-flight transfers to complete. The transfer finished callback gives
a transfer finished semaphore which we must take, so we would not
affect next TX transfers.

Closes espressif/esp-protocols#514
Closes #40

host/class/cdc/usb_host_cdc_acm/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 - 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
 

host/class/cdc/usb_host_cdc_acm/cdc_acm_host.c

@@ -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,);
@@ -1158,10 +1157,12 @@ esp_err_t cdc_acm_host_data_tx_blocking(cdc_acm_dev_hdl_t cdc_hdl, const uint8_t
     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));
+    SemaphoreHandle_t transfer_finished_semaphore = (SemaphoreHandle_t)cdc_dev->data.out_xfer->context;
+    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
+        xSemaphoreTake(transfer_finished_semaphore, pdMS_TO_TICKS(1000)); // The transfer callback gives the semaphore, take it back
+        ESP_LOGW(TAG, "TX transfer timeout");
         ret = ESP_ERR_TIMEOUT;
         goto unblock;
     }

host/class/cdc/usb_host_cdc_acm/test_app/main/test_cdc_acm_host.c

@@ -599,6 +599,43 @@ 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]")
+{
+    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);
+
+    // TX some date 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
+
+    TEST_ASSERT_EQUAL(ESP_OK, cdc_acm_host_data_tx_blocking(cdc_dev, tx_buf, sizeof(tx_buf), 1000));
+    vTaskDelay(100);
+
+    // We sent two messages, 1 should timeout the second should be received OK
+    TEST_ASSERT_EQUAL(1, nb_of_responses);
+
+    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]")