From cf514906b31946c17ee3d7823618373c24b123fb Mon Sep 17 00:00:00 2001 From: Tomas Rezucha Date: Wed, 28 Jun 2023 13:22:22 +0200 Subject: [PATCH] refactor(esp_tinyusb/cdc): Remove receive ring buffer - CDC-ACM: Remove intermediate RX ringbuffer - CDC-ACM: Increase default FIFO size to 512 bytes - CDC-ACM: Fix Virtual File System binding - CMake: Remove unnecessary dependencies --- .../workflows/build_and_run_test_app_usb.yml | 4 +- pytest.ini | 1 + usb/esp_tinyusb/CHANGELOG.md | 38 ++++ usb/esp_tinyusb/CMakeLists.txt | 47 +++-- usb/esp_tinyusb/Kconfig | 4 +- usb/esp_tinyusb/idf_component.yml | 2 +- usb/esp_tinyusb/include/tusb_cdc_acm.h | 76 ++++---- usb/esp_tinyusb/include/vfs_tinyusb.h | 40 +++- usb/esp_tinyusb/sbom.yml | 2 + usb/esp_tinyusb/test/CMakeLists.txt | 4 + usb/esp_tinyusb/test/test_esp_tinyusb.c | 130 +++++++++++++ usb/esp_tinyusb/tusb_cdc_acm.c | 177 +++++------------- usb/esp_tinyusb/vfs_tinyusb.c | 124 ++++++------ usb/test_app/CMakeLists.txt | 6 + usb/test_app/pytest_usb_device.py | 77 ++++++++ usb/usb_host_cdc_acm/test/usb_device.c | 5 +- 16 files changed, 480 insertions(+), 257 deletions(-) create mode 100644 usb/esp_tinyusb/CHANGELOG.md create mode 100644 usb/esp_tinyusb/sbom.yml create mode 100644 usb/esp_tinyusb/test/CMakeLists.txt create mode 100644 usb/esp_tinyusb/test/test_esp_tinyusb.c create mode 100644 usb/test_app/pytest_usb_device.py diff --git a/.github/workflows/build_and_run_test_app_usb.yml b/.github/workflows/build_and_run_test_app_usb.yml index 081711d788..3332961b61 100644 --- a/.github/workflows/build_and_run_test_app_usb.yml +++ b/.github/workflows/build_and_run_test_app_usb.yml @@ -64,7 +64,7 @@ jobs: - name: Install Python packages env: PIP_EXTRA_INDEX_URL: "https://dl.espressif.com/pypi/" - run: pip install --only-binary cryptography pytest-embedded pytest-embedded-serial-esp pytest-embedded-idf + run: pip install --only-binary cryptography pytest-embedded pytest-embedded-serial-esp pytest-embedded-idf pyserial - name: Run USB Test App on target working-directory: usb/test_app - run: pytest --target=${{ matrix.idf_target }} + run: pytest --target=${{ matrix.idf_target }} -m usb_host diff --git a/pytest.ini b/pytest.ini index 7e0bccc6d2..2cdf30fb45 100644 --- a/pytest.ini +++ b/pytest.ini @@ -20,6 +20,7 @@ markers = # env markers generic: generic runner usb_host: usb host runners + usb_device: usb device runners ethernet: ethernet runners # log related diff --git a/usb/esp_tinyusb/CHANGELOG.md b/usb/esp_tinyusb/CHANGELOG.md new file mode 100644 index 0000000000..2202678963 --- /dev/null +++ b/usb/esp_tinyusb/CHANGELOG.md @@ -0,0 +1,38 @@ +## 1.4.0 + +- MSC: Fix integer overflows +- CDC-ACM: Remove intermediate RX ringbuffer +- CDC-ACM: Increase default FIFO size to 512 bytes +- CDC-ACM: Fix Virtual File System binding + +## 1.3.0 + +- Add NCM extension + +## 1.2.1 - 1.2.2 + +- Minor bugfixes + +## 1.2.0 + +- Add MSC extension for accessing SPI Flash on memory card https://github.com/espressif/idf-extra-components/commit/a8c00d7707ba4ceeb0970c023d702c7768dba3dc + +## 1.1.0 + +- Add support for NCM, ECM/RNDIS, DFU and Bluetooth TinyUSB drivers https://github.com/espressif/idf-extra-components/commit/79f35c9b047b583080f93a63310e2ee7d82ef17b + +## 1.0.4 + +- Clean up string descriptors handling https://github.com/espressif/idf-extra-components/commit/046cc4b02f524d5c7e3e56480a473cfe844dc3d6 + +## 1.0.2 - 1.0.3 + +- Minor bugfixes + +## 1.0.1 + +- CDC-ACM: Return ESP_OK if there is nothing to flush https://github.com/espressif/idf-extra-components/commit/388ff32eb09aa572d98c54cb355f1912ce42707c + +## 1.0.0 + +- Initial version based on [esp-idf v4.4.3](https://github.com/espressif/esp-idf/tree/v4.4.3/components/tinyusb) diff --git a/usb/esp_tinyusb/CMakeLists.txt b/usb/esp_tinyusb/CMakeLists.txt index 8ecf106f84..dba855c543 100644 --- a/usb/esp_tinyusb/CMakeLists.txt +++ b/usb/esp_tinyusb/CMakeLists.txt @@ -1,24 +1,9 @@ -set(includes_private - "include_private" - ) - -set(includes_public - "include" - ) - set(srcs "descriptors_control.c" "tinyusb.c" "usb_descriptors.c" ) -set(priv_requires - usb - vfs - esp_ringbuf - fatfs - ) - if(NOT CONFIG_TINYUSB_NO_DEFAULT_TASK) list(APPEND srcs "tusb_tasks.c") endif() # CONFIG_TINYUSB_NO_DEFAULT_TASK @@ -28,12 +13,12 @@ if(CONFIG_TINYUSB_CDC_ENABLED) "cdc.c" "tusb_cdc_acm.c" ) -if(CONFIG_VFS_SUPPORT_IO) - list(APPEND srcs - "tusb_console.c" - "vfs_tinyusb.c" - ) -endif() # CONFIG_VFS_SUPPORT_IO + if(CONFIG_VFS_SUPPORT_IO) + list(APPEND srcs + "tusb_console.c" + "vfs_tinyusb.c" + ) + endif() # CONFIG_VFS_SUPPORT_IO endif() # CONFIG_TINYUSB_CDC_ENABLED if(CONFIG_TINYUSB_MSC_ENABLED) @@ -41,6 +26,7 @@ if(CONFIG_TINYUSB_MSC_ENABLED) tusb_msc_storage.c ) endif() # CONFIG_TINYUSB_MSC_ENABLED + if(CONFIG_TINYUSB_NET_MODE_NCM) list(APPEND srcs tinyusb_net.c @@ -48,9 +34,9 @@ if(CONFIG_TINYUSB_NET_MODE_NCM) endif() # CONFIG_TINYUSB_NET_MODE_NCM idf_component_register(SRCS ${srcs} - INCLUDE_DIRS ${includes_public} - PRIV_INCLUDE_DIRS ${includes_private} - PRIV_REQUIRES ${priv_requires} + INCLUDE_DIRS "include" + PRIV_INCLUDE_DIRS "include_private" + PRIV_REQUIRES usb REQUIRED_IDF_TARGETS esp32s2 esp32s3 ) @@ -64,5 +50,16 @@ endif() # Pass tusb_config.h from this component to TinyUSB idf_component_get_property(tusb_lib ${tinyusb_name} COMPONENT_LIB) - target_include_directories(${tusb_lib} PRIVATE "include") + +# Link required libraries +# This must be done after idf_component_register, because Kconfig values are not resolved before it +if(CONFIG_TINYUSB_MSC_ENABLED) + idf_component_get_property(fatfs fatfs COMPONENT_LIB) + target_link_libraries(${COMPONENT_LIB} PRIVATE ${fatfs}) +endif() + +if((CONFIG_TINYUSB_CDC_ENABLED) AND (CONFIG_VFS_SUPPORT_IO)) + idf_component_get_property(vfs vfs COMPONENT_LIB) + target_link_libraries(${COMPONENT_LIB} PUBLIC ${vfs}) +endif() diff --git a/usb/esp_tinyusb/Kconfig b/usb/esp_tinyusb/Kconfig index 1d852b82da..7cdaa3e65a 100644 --- a/usb/esp_tinyusb/Kconfig +++ b/usb/esp_tinyusb/Kconfig @@ -171,7 +171,7 @@ menu "TinyUSB Stack" config TINYUSB_CDC_RX_BUFSIZE depends on TINYUSB_CDC_ENABLED int "CDC FIFO size of RX channel" - default 64 + default 512 range 64 10000 help CDC FIFO size of RX channel. @@ -179,7 +179,7 @@ menu "TinyUSB Stack" config TINYUSB_CDC_TX_BUFSIZE depends on TINYUSB_CDC_ENABLED int "CDC FIFO size of TX channel" - default 64 + default 512 help CDC FIFO size of TX channel. endmenu # "Communication Device Class" diff --git a/usb/esp_tinyusb/idf_component.yml b/usb/esp_tinyusb/idf_component.yml index 3c00005113..89a51f7fb6 100644 --- a/usb/esp_tinyusb/idf_component.yml +++ b/usb/esp_tinyusb/idf_component.yml @@ -1,6 +1,6 @@ description: Espressif's additions to TinyUSB documentation: "https://docs.espressif.com/projects/esp-idf/en/latest/esp32s2/api-reference/peripherals/usb_device.html" -version: 1.3.1 +version: 1.4.0 url: https://github.com/espressif/idf-extra-components/tree/master/usb/esp_tinyusb dependencies: idf: '>=5.0' # IDF 4.x contains TinyUSB as submodule diff --git a/usb/esp_tinyusb/include/tusb_cdc_acm.h b/usb/esp_tinyusb/include/tusb_cdc_acm.h index ccdb577adc..89d4937c6b 100644 --- a/usb/esp_tinyusb/include/tusb_cdc_acm.h +++ b/usb/esp_tinyusb/include/tusb_cdc_acm.h @@ -11,9 +11,11 @@ extern "C" { #endif #include -#include "tinyusb_types.h" -#include "esp_err.h" #include "sdkconfig.h" +#include "esp_err.h" + +#include "tinyusb_types.h" +#include "class/cdc/cdc.h" #if (CONFIG_TINYUSB_CDC_ENABLED != 1) #error "TinyUSB CDC driver must be enabled in menuconfig" @@ -91,7 +93,7 @@ typedef void(*tusb_cdcacm_callback_t)(int itf, cdcacm_event_t *event); typedef struct { tinyusb_usbdev_t usb_dev; /*!< Usb device to set up */ tinyusb_cdcacm_itf_t cdc_port; /*!< CDC port */ - size_t rx_unread_buf_sz; /*!< Amount of data that can be passed to the ACM at once */ + size_t rx_unread_buf_sz __attribute__((deprecated("This parameter is not used any more. Configure RX buffer in menuconfig."))); tusb_cdcacm_callback_t callback_rx; /*!< Pointer to the function with the `tusb_cdcacm_callback_t` type that will be handled as a callback */ tusb_cdcacm_callback_t callback_rx_wanted_char; /*!< Pointer to the function with the `tusb_cdcacm_callback_t` type that will be handled as a callback */ tusb_cdcacm_callback_t callback_line_state_changed; /*!< Pointer to the function with the `tusb_cdcacm_callback_t` type that will be handled as a callback */ @@ -105,89 +107,87 @@ typedef struct { * @brief Initialize CDC ACM. Initialization will be finished with * the `tud_cdc_line_state_cb` callback * - * @param cfg - init configuration structure + * @param[in] cfg Configuration structure * @return esp_err_t */ esp_err_t tusb_cdc_acm_init(const tinyusb_config_cdcacm_t *cfg); - /** * @brief Register a callback invoking on CDC event. If the callback had been * already registered, it will be overwritten * - * @param itf - number of a CDC object - * @param event_type - type of registered event for a callback - * @param callback - callback function + * @param[in] itf Index of CDC interface + * @param[in] event_type Type of registered event for a callback + * @param[in] callback Callback function * @return esp_err_t - ESP_OK or ESP_ERR_INVALID_ARG */ esp_err_t tinyusb_cdcacm_register_callback(tinyusb_cdcacm_itf_t itf, cdcacm_event_type_t event_type, tusb_cdcacm_callback_t callback); - /** - * @brief Unregister a callback invoking on CDC event. + * @brief Unregister a callback invoking on CDC event * - * @param itf - number of a CDC object - * @param event_type - type of registered event for a callback + * @param[in] itf Index of CDC interface + * @param[in] event_type Type of registered event for a callback * @return esp_err_t - ESP_OK or ESP_ERR_INVALID_ARG */ esp_err_t tinyusb_cdcacm_unregister_callback(tinyusb_cdcacm_itf_t itf, cdcacm_event_type_t event_type); - /** * @brief Sent one character to a write buffer * - * @param itf - number of a CDC object - * @param ch - character to send + * @param[in] itf Index of CDC interface + * @param[in] ch Character to send * @return size_t - amount of queued bytes */ size_t tinyusb_cdcacm_write_queue_char(tinyusb_cdcacm_itf_t itf, char ch); - /** - * @brief Write data to write buffer from a byte array + * @brief Write data to write buffer * - * @param itf - number of a CDC object - * @param in_buf - a source array - * @param in_size - size to write from arr_src + * @param[in] itf Index of CDC interface + * @param[in] in_buf Data + * @param[in] in_size Data size in bytes * @return size_t - amount of queued bytes */ size_t tinyusb_cdcacm_write_queue(tinyusb_cdcacm_itf_t itf, const uint8_t *in_buf, size_t in_size); /** - * @brief Send all data from a write buffer. Use `tinyusb_cdcacm_write_queue` to add data to the buffer. + * @brief Flush data in write buffer of CDC interface + * + * Use `tinyusb_cdcacm_write_queue` to add data to the buffer * * WARNING! TinyUSB can block output Endpoint for several RX callbacks, after will do additional flush - * after the each trasfer. That can leads to the situation when you requested a flush, but it will fail until - * ont of the next callbacks ends. - * SO USING OF THE FLUSH WITH TIMEOUTS IN CALLBACKS IS NOT RECOMENDED - YOU CAN GET A LOCK FOR THE TIMEOUT + * after the each transfer. That can leads to the situation when you requested a flush, but it will fail until + * one of the next callbacks ends. + * SO USING OF THE FLUSH WITH TIMEOUTS IN CALLBACKS IS NOT RECOMMENDED - YOU CAN GET A LOCK FOR THE TIMEOUT * - * @param itf - number of a CDC object - * @param timeout_ticks - waiting until flush will be considered as failed - * @return esp_err_t - ESP_OK if (timeout_ticks > 0) and and flush was successful, - * ESP_ERR_TIMEOUT if timeout occurred3 or flush was successful with (timeout_ticks == 0) - * ESP_FAIL if flush was unsuccessful + * @param[in] itf Index of CDC interface + * @param[in] timeout_ticks Transfer timeout. Set to zero for non-blocking mode + * @return - ESP_OK All data flushed + * - ESP_ERR_TIMEOUT Time out occurred in blocking mode + * - ESP_NOT_FINISHED The transfer is still in progress in non-blocking mode */ esp_err_t tinyusb_cdcacm_write_flush(tinyusb_cdcacm_itf_t itf, uint32_t timeout_ticks); /** - * @brief Read a content to the array, and defines it's size to the sz_store + * @brief Receive data from CDC interface * - * @param itf - number of a CDC object - * @param out_buf - to this array will be stored the object from a CDC buffer - * @param out_buf_sz - size of buffer for results - * @param rx_data_size - to this address will be stored the object's size + * @param[in] itf Index of CDC interface + * @param[out] out_buf Data buffer + * @param[in] out_buf_sz Data buffer size in bytes + * @param[out] rx_data_size Number of bytes written to out_buf * @return esp_err_t ESP_OK, ESP_FAIL or ESP_ERR_INVALID_STATE */ esp_err_t tinyusb_cdcacm_read(tinyusb_cdcacm_itf_t itf, uint8_t *out_buf, size_t out_buf_sz, size_t *rx_data_size); - /** - * @brief Check if the ACM initialized + * @brief Check if the CDC interface is initialized * - * @param itf - number of a CDC object - * @return true or false + * @param[in] itf Index of CDC interface + * @return - true Initialized + * - false Not Initialized */ bool tusb_cdc_acm_initialized(tinyusb_cdcacm_itf_t itf); diff --git a/usb/esp_tinyusb/include/vfs_tinyusb.h b/usb/esp_tinyusb/include/vfs_tinyusb.h index 294e46f3db..9c9ba910b1 100644 --- a/usb/esp_tinyusb/include/vfs_tinyusb.h +++ b/usb/esp_tinyusb/include/vfs_tinyusb.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -7,6 +7,7 @@ #pragma once #include "esp_err.h" +#include "esp_vfs_common.h" // For esp_line_endings_t definitions #ifdef __cplusplus extern "C" { @@ -17,21 +18,52 @@ extern "C" { /** * @brief Register TinyUSB CDC at VFS with path - * @param cdc_intf - interface number of TinyUSB's CDC - * @param path - path where the CDC will be registered, `/dev/tusb_cdc` will be used if left NULL. * + * Know limitation: + * In case there are multiple CDC interfaces in the system, only one of them can be registered to VFS. + * + * @param[in] cdc_intf Interface number of TinyUSB's CDC + * @param[in] path Path where the CDC will be registered, `/dev/tusb_cdc` will be used if left NULL. * @return esp_err_t ESP_OK or ESP_FAIL */ esp_err_t esp_vfs_tusb_cdc_register(int cdc_intf, char const *path); /** * @brief Unregister TinyUSB CDC from VFS - * @param path - path where the CDC will be unregistered if NULL will be used `/dev/tusb_cdc` * + * @param[in] path Path where the CDC will be unregistered if NULL will be used `/dev/tusb_cdc` * @return esp_err_t ESP_OK or ESP_FAIL */ esp_err_t esp_vfs_tusb_cdc_unregister(char const *path); +/** + * @brief Set the line endings to sent + * + * This specifies the conversion between newlines ('\n', LF) on stdout and line + * endings sent: + * + * - ESP_LINE_ENDINGS_CRLF: convert LF to CRLF + * - ESP_LINE_ENDINGS_CR: convert LF to CR + * - ESP_LINE_ENDINGS_LF: no modification + * + * @param[in] mode line endings to send + */ +void esp_vfs_tusb_cdc_set_tx_line_endings(esp_line_endings_t mode); + +/** + * @brief Set the line endings expected to be received + * + * This specifies the conversion between line endings received and + * newlines ('\n', LF) passed into stdin: + * + * - ESP_LINE_ENDINGS_CRLF: convert CRLF to LF + * - ESP_LINE_ENDINGS_CR: convert CR to LF + * - ESP_LINE_ENDINGS_LF: no modification + * + * @param[in] mode line endings expected + */ +void esp_vfs_tusb_cdc_set_rx_line_endings(esp_line_endings_t mode); + #ifdef __cplusplus } #endif diff --git a/usb/esp_tinyusb/sbom.yml b/usb/esp_tinyusb/sbom.yml new file mode 100644 index 0000000000..f1e805b9c6 --- /dev/null +++ b/usb/esp_tinyusb/sbom.yml @@ -0,0 +1,2 @@ +supplier: 'Organization: Espressif Systems (Shanghai) CO LTD' +originator: 'Organization: Espressif Systems (Shanghai) CO LTD' diff --git a/usb/esp_tinyusb/test/CMakeLists.txt b/usb/esp_tinyusb/test/CMakeLists.txt new file mode 100644 index 0000000000..fc89d7e002 --- /dev/null +++ b/usb/esp_tinyusb/test/CMakeLists.txt @@ -0,0 +1,4 @@ +idf_component_register(SRCS "test_esp_tinyusb.c" + INCLUDE_DIRS "." + REQUIRES unity esp_tinyusb + ) diff --git a/usb/esp_tinyusb/test/test_esp_tinyusb.c b/usb/esp_tinyusb/test/test_esp_tinyusb.c new file mode 100644 index 0000000000..3d5b3edb6b --- /dev/null +++ b/usb/esp_tinyusb/test/test_esp_tinyusb.c @@ -0,0 +1,130 @@ +/* + * SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD + * + * SPDX-License-Identifier: CC0-1.0 + */ + +#include "soc/soc_caps.h" +#if SOC_USB_OTG_SUPPORTED + +#include +#include +#include "esp_system.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "esp_log.h" +#include "esp_err.h" + +#include "unity.h" +#include "tinyusb.h" +#include "tusb_cdc_acm.h" +#include "vfs_tinyusb.h" + +#define VFS_PATH "/dev/usb-cdc1" + +static const tusb_desc_device_t cdc_device_descriptor = { + .bLength = sizeof(cdc_device_descriptor), + .bDescriptorType = TUSB_DESC_DEVICE, + .bcdUSB = 0x0200, + .bDeviceClass = TUSB_CLASS_MISC, + .bDeviceSubClass = MISC_SUBCLASS_COMMON, + .bDeviceProtocol = MISC_PROTOCOL_IAD, + .bMaxPacketSize0 = CFG_TUD_ENDPOINT0_SIZE, + .idVendor = USB_ESPRESSIF_VID, + .idProduct = 0x4002, + .bcdDevice = 0x0100, + .iManufacturer = 0x01, + .iProduct = 0x02, + .iSerialNumber = 0x03, + .bNumConfigurations = 0x01 +}; + +static const uint16_t cdc_desc_config_len = TUD_CONFIG_DESC_LEN + CFG_TUD_CDC * TUD_CDC_DESC_LEN; +static const uint8_t cdc_desc_configuration[] = { + TUD_CONFIG_DESCRIPTOR(1, 4, 0, cdc_desc_config_len, TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP, 100), + TUD_CDC_DESCRIPTOR(0, 4, 0x81, 8, 0x02, 0x82, 64), + TUD_CDC_DESCRIPTOR(2, 4, 0x83, 8, 0x04, 0x84, 64), +}; + +static void tinyusb_cdc_rx_callback(int itf, cdcacm_event_t *event) +{ +} + +/** + * @brief TinyUSB CDC testcase + * + * This is not a 'standard' testcase, as it never exits. The testcase runs in a loop where it echoes back all the data received. + * The accompanying pytest script can be found in usb/test_app/pytest_usb_device.py + * + * - Init TinyUSB with standard CDC device and configuration descriptors + * - Init 2 CDC-ACM interfaces + * - Map CDC1 to Virtual File System + * - In a loop: Read data from CDC0 and CDC1. Echo received data back + * + * Note: CDC0 appends 'novfs' to echoed data, so the host (test runner) can easily determine which port is which. + */ +TEST_CASE("tinyusb_cdc", "[esp_tinyusb]") +{ + // Install TinyUSB driver + const tinyusb_config_t tusb_cfg = { + .external_phy = false, + .device_descriptor = &cdc_device_descriptor, + .configuration_descriptor = cdc_desc_configuration + }; + TEST_ASSERT_EQUAL(ESP_OK, tinyusb_driver_install(&tusb_cfg)); + + tinyusb_config_cdcacm_t acm_cfg = { + .usb_dev = TINYUSB_USBDEV_0, + .cdc_port = TINYUSB_CDC_ACM_0, + .rx_unread_buf_sz = 64, + .callback_rx = &tinyusb_cdc_rx_callback, + .callback_rx_wanted_char = NULL, + .callback_line_state_changed = NULL, + .callback_line_coding_changed = NULL + }; + + // Init CDC 0 + TEST_ASSERT_FALSE(tusb_cdc_acm_initialized(TINYUSB_CDC_ACM_0)); + TEST_ASSERT_EQUAL(ESP_OK, tusb_cdc_acm_init(&acm_cfg)); + TEST_ASSERT_TRUE(tusb_cdc_acm_initialized(TINYUSB_CDC_ACM_0)); + + // Init CDC 1 + acm_cfg.cdc_port = TINYUSB_CDC_ACM_1; + acm_cfg.callback_rx = NULL; + TEST_ASSERT_FALSE(tusb_cdc_acm_initialized(TINYUSB_CDC_ACM_1)); + TEST_ASSERT_EQUAL(ESP_OK, tusb_cdc_acm_init(&acm_cfg)); + TEST_ASSERT_TRUE(tusb_cdc_acm_initialized(TINYUSB_CDC_ACM_1)); + + // Install VFS to CDC 1 + TEST_ASSERT_EQUAL(ESP_OK, esp_vfs_tusb_cdc_register(TINYUSB_CDC_ACM_1, VFS_PATH)); + esp_vfs_tusb_cdc_set_rx_line_endings(ESP_LINE_ENDINGS_CRLF); + esp_vfs_tusb_cdc_set_tx_line_endings(ESP_LINE_ENDINGS_LF); + FILE *cdc = fopen(VFS_PATH, "r+"); + TEST_ASSERT_NOT_NULL(cdc); + + uint8_t buf[CONFIG_TINYUSB_CDC_RX_BUFSIZE + 1]; + while (true) { + size_t b = fread(buf, 1, sizeof(buf), cdc); + if (b > 0) { + printf("Intf VFS, RX %d bytes\n", b); + //ESP_LOG_BUFFER_HEXDUMP("test", buf, b, ESP_LOG_INFO); + fwrite(buf, 1, b, cdc); + } + vTaskDelay(1); + + size_t rx_size = 0; + int itf = 0; + ESP_ERROR_CHECK(tinyusb_cdcacm_read(itf, buf, CONFIG_TINYUSB_CDC_RX_BUFSIZE, &rx_size)); + if (rx_size > 0) { + printf("Intf %d, RX %d bytes\n", itf, rx_size); + + // Add 'novfs' to reply so the host can identify the port + strcpy((char *)&buf[rx_size - 2], "novfs\r\n"); + tinyusb_cdcacm_write_queue(itf, buf, rx_size + sizeof("novfs") - 1); + + tinyusb_cdcacm_write_flush(itf, 0); + } + } +} + +#endif diff --git a/usb/esp_tinyusb/tusb_cdc_acm.c b/usb/esp_tinyusb/tusb_cdc_acm.c index 48373cd8db..5ef6637b6c 100644 --- a/usb/esp_tinyusb/tusb_cdc_acm.c +++ b/usb/esp_tinyusb/tusb_cdc_acm.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -10,21 +10,19 @@ #include "esp_log.h" #include "freertos/FreeRTOS.h" #include "freertos/task.h" -#include "freertos/ringbuf.h" #include "tusb.h" #include "tusb_cdc_acm.h" #include "cdc.h" #include "sdkconfig.h" -#define RX_UNREADBUF_SZ_DEFAULT 64 // buffer storing all unread RX data #define MIN(x, y) (((x) < (y)) ? (x) : (y)) +// CDC-ACM spinlock +static portMUX_TYPE cdc_acm_lock = portMUX_INITIALIZER_UNLOCKED; +#define CDC_ACM_ENTER_CRITICAL() portENTER_CRITICAL(&cdc_acm_lock) +#define CDC_ACM_EXIT_CRITICAL() portEXIT_CRITICAL(&cdc_acm_lock) typedef struct { - size_t rx_unread_buf_sz; - RingbufHandle_t rx_unread_buf; - SemaphoreHandle_t ringbuf_read_mux; - uint8_t *rx_tfbuf; tusb_cdcacm_callback_t callback_rx; tusb_cdcacm_callback_t callback_rx_wanted_char; tusb_cdcacm_callback_t callback_line_state_changed; @@ -65,7 +63,9 @@ void tud_cdc_line_state_cb(uint8_t itf, bool dtr, bool rts) } } if (acm) { + CDC_ACM_ENTER_CRITICAL(); tusb_cdcacm_callback_t cb = acm->callback_line_state_changed; + CDC_ACM_EXIT_CRITICAL(); if (cb) { cdcacm_event_t event = { .type = CDC_EVENT_LINE_STATE_CHANGED, @@ -84,29 +84,9 @@ void tud_cdc_rx_cb(uint8_t itf) { esp_tusb_cdcacm_t *acm = get_acm(itf); if (acm) { - if (!acm->rx_unread_buf) { - ESP_LOGE(TAG, "There is no RX buffer created"); - abort(); - } - } else { - tud_cdc_n_read_flush(itf); // we have no place to store data, so just drop it - return; - } - while (tud_cdc_n_available(itf)) { - int read_res = tud_cdc_n_read( itf, - acm->rx_tfbuf, - CONFIG_TINYUSB_CDC_RX_BUFSIZE ); - int res = xRingbufferSend(acm->rx_unread_buf, - acm->rx_tfbuf, - read_res, 0); - if (res != pdTRUE) { - ESP_LOGW(TAG, "The unread buffer is too small, the data has been lost"); - } else { - ESP_LOGV(TAG, "Sent %d bytes to the buffer", read_res); - } - } - if (acm) { + CDC_ACM_ENTER_CRITICAL(); tusb_cdcacm_callback_t cb = acm->callback_rx; + CDC_ACM_EXIT_CRITICAL(); if (cb) { cdcacm_event_t event = { .type = CDC_EVENT_RX @@ -121,7 +101,9 @@ void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const *p_line_coding) { esp_tusb_cdcacm_t *acm = get_acm(itf); if (acm) { + CDC_ACM_ENTER_CRITICAL(); tusb_cdcacm_callback_t cb = acm->callback_line_coding_changed; + CDC_ACM_EXIT_CRITICAL(); if (cb) { cdcacm_event_t event = { .type = CDC_EVENT_LINE_CODING_CHANGED, @@ -131,8 +113,6 @@ void tud_cdc_line_coding_cb(uint8_t itf, cdc_line_coding_t const *p_line_coding) }; cb(itf, &event); } - } else { - return; } } @@ -141,7 +121,9 @@ void tud_cdc_rx_wanted_cb(uint8_t itf, char wanted_char) { esp_tusb_cdcacm_t *acm = get_acm(itf); if (acm) { + CDC_ACM_ENTER_CRITICAL(); tusb_cdcacm_callback_t cb = acm->callback_rx_wanted_char; + CDC_ACM_EXIT_CRITICAL(); if (cb) { cdcacm_event_t event = { .type = CDC_EVENT_RX_WANTED_CHAR, @@ -151,8 +133,6 @@ void tud_cdc_rx_wanted_cb(uint8_t itf, char wanted_char) }; cb(itf, &event); } - } else { - return; } } @@ -164,16 +144,24 @@ esp_err_t tinyusb_cdcacm_register_callback(tinyusb_cdcacm_itf_t itf, if (acm) { switch (event_type) { case CDC_EVENT_RX: + CDC_ACM_ENTER_CRITICAL(); acm->callback_rx = callback; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; case CDC_EVENT_RX_WANTED_CHAR: + CDC_ACM_ENTER_CRITICAL(); acm->callback_rx_wanted_char = callback; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; case CDC_EVENT_LINE_STATE_CHANGED: + CDC_ACM_ENTER_CRITICAL(); acm->callback_line_state_changed = callback; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; case CDC_EVENT_LINE_CODING_CHANGED: + CDC_ACM_ENTER_CRITICAL(); acm->callback_line_coding_changed = callback; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; default: ESP_LOGE(TAG, "Wrong event type"); @@ -195,16 +183,24 @@ esp_err_t tinyusb_cdcacm_unregister_callback(tinyusb_cdcacm_itf_t itf, } switch (event_type) { case CDC_EVENT_RX: + CDC_ACM_ENTER_CRITICAL(); acm->callback_rx = NULL; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; case CDC_EVENT_RX_WANTED_CHAR: + CDC_ACM_ENTER_CRITICAL(); acm->callback_rx_wanted_char = NULL; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; case CDC_EVENT_LINE_STATE_CHANGED: + CDC_ACM_ENTER_CRITICAL(); acm->callback_line_state_changed = NULL; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; case CDC_EVENT_LINE_CODING_CHANGED: + CDC_ACM_ENTER_CRITICAL(); acm->callback_line_coding_changed = NULL; + CDC_ACM_EXIT_CRITICAL(); return ESP_OK; default: ESP_LOGE(TAG, "Wrong event type"); @@ -216,56 +212,16 @@ esp_err_t tinyusb_cdcacm_unregister_callback(tinyusb_cdcacm_itf_t itf, /* CDC-ACM ********************************************************************* */ -static esp_err_t read_from_rx_unread_to_buffer(esp_tusb_cdcacm_t *acm, uint8_t *out_buf, size_t req_bytes, size_t *read_bytes) -{ - uint8_t *buf = xRingbufferReceiveUpTo(acm->rx_unread_buf, read_bytes, 0, req_bytes); - if (buf) { - memcpy(out_buf, buf, *read_bytes); - vRingbufferReturnItem(acm->rx_unread_buf, (void *)(buf)); - return ESP_OK; - } else { - return ESP_ERR_NO_MEM; - } -} - -static esp_err_t ringbuf_mux_take(esp_tusb_cdcacm_t *acm) -{ - if (xSemaphoreTake(acm->ringbuf_read_mux, 0) != pdTRUE) { - ESP_LOGW(TAG, "Read error: ACM is busy"); - return ESP_ERR_INVALID_STATE; - } - return ESP_OK; -} - -static esp_err_t ringbuf_mux_give(esp_tusb_cdcacm_t *acm) -{ - BaseType_t ret = xSemaphoreGive(acm->ringbuf_read_mux); - assert(ret == pdTRUE); - return ESP_OK; -} - esp_err_t tinyusb_cdcacm_read(tinyusb_cdcacm_itf_t itf, uint8_t *out_buf, size_t out_buf_sz, size_t *rx_data_size) { esp_tusb_cdcacm_t *acm = get_acm(itf); ESP_RETURN_ON_FALSE(acm, ESP_ERR_INVALID_STATE, TAG, "Interface is not initialized. Use `tinyusb_cdc_init` for initialization"); - size_t read_sz; - - /* Take a mutex to proceed two uninterrupted read operations */ - ESP_RETURN_ON_ERROR(ringbuf_mux_take(acm), TAG, "ringbuf_mux_take failed"); - esp_err_t res = read_from_rx_unread_to_buffer(acm, out_buf, out_buf_sz, &read_sz); - if (res != ESP_OK) { - ESP_RETURN_ON_ERROR(ringbuf_mux_give(acm), TAG, "ringbuf_mux_give failed"); - return res; - } - - *rx_data_size = read_sz; - /* Buffer's data can be wrapped, at that situations we should make another retrievement */ - if (read_from_rx_unread_to_buffer(acm, out_buf + read_sz, out_buf_sz - read_sz, &read_sz) == ESP_OK) { - *rx_data_size += read_sz; + if (tud_cdc_n_available(itf) == 0) { + *rx_data_size = 0; + } else { + *rx_data_size = tud_cdc_n_read(itf, out_buf, out_buf_sz); } - - ESP_RETURN_ON_ERROR(ringbuf_mux_give(acm), TAG, "ringbuf_mux_give failed"); return ESP_OK; } @@ -298,28 +254,21 @@ esp_err_t tinyusb_cdcacm_write_flush(tinyusb_cdcacm_itf_t itf, uint32_t timeout_ } if (!timeout_ticks) { // if no timeout - nonblocking mode - int res = tud_cdc_n_write_flush(itf); - if (!res) { - ESP_LOGW(TAG, "flush failed (res: %d)", res); - return ESP_FAIL; - } else { - if (tud_cdc_n_write_occupied(itf)) { - ESP_LOGW(TAG, "remained data to flush!"); - return ESP_FAIL; - } else { - return ESP_OK; - } + // It might take some time until TinyUSB flushes the endpoint + // Since this call is non-blocking, we don't wait for flush finished, + // We only inform the user by returning ESP_ERR_NOT_FINISHED + tud_cdc_n_write_flush(itf); + if (tud_cdc_n_write_occupied(itf)) { + return ESP_ERR_NOT_FINISHED; } } else { // trying during the timeout uint32_t ticks_start = xTaskGetTickCount(); uint32_t ticks_now = ticks_start; while (1) { // loop until success or until the time runs out ticks_now = xTaskGetTickCount(); - if (!tud_cdc_n_write_occupied(itf)) { // if nothing to write - nothing to flush - break; - } - if (tud_cdc_n_write_flush(itf)) { // Success - break; + tud_cdc_n_write_flush(itf); + if (tud_cdc_n_write_occupied(itf) == 0) { + break; // All data flushed } if ( (ticks_now - ticks_start) > timeout_ticks ) { // Time is up ESP_LOGW(TAG, "Flush failed"); @@ -327,26 +276,21 @@ esp_err_t tinyusb_cdcacm_write_flush(tinyusb_cdcacm_itf_t itf, uint32_t timeout_ } vTaskDelay(1); } - return ESP_OK; } + return ESP_OK; } static esp_err_t alloc_obj(tinyusb_cdcacm_itf_t itf) { esp_tusb_cdc_t *cdc_inst = tinyusb_cdc_get_intf(itf); + if (cdc_inst == NULL) { + return ESP_FAIL; + } cdc_inst->subclass_obj = calloc(1, sizeof(esp_tusb_cdcacm_t)); - if (!cdc_inst->subclass_obj) { + if (cdc_inst->subclass_obj == NULL) { return ESP_FAIL; - } else { - return ESP_OK; } -} - -static void free_obj(tinyusb_cdcacm_itf_t itf) -{ - esp_tusb_cdc_t *cdc_inst = tinyusb_cdc_get_intf(itf); - free(cdc_inst->subclass_obj); - cdc_inst->subclass_obj = NULL; + return ESP_OK; } esp_err_t tusb_cdc_acm_init(const tinyusb_config_cdcacm_t *cfg) @@ -361,7 +305,7 @@ esp_err_t tusb_cdc_acm_init(const tinyusb_config_cdcacm_t *cfg) }; ESP_RETURN_ON_ERROR(tinyusb_cdc_init(itf, &cdc_cfg), TAG, "tinyusb_cdc_init failed"); - ESP_GOTO_ON_ERROR(alloc_obj(itf), out4, TAG, "alloc_obj failed"); + ESP_GOTO_ON_ERROR(alloc_obj(itf), fail, TAG, "alloc_obj failed"); /* Callbacks setting up*/ if (cfg->callback_rx) { @@ -377,29 +321,8 @@ esp_err_t tusb_cdc_acm_init(const tinyusb_config_cdcacm_t *cfg) tinyusb_cdcacm_register_callback( itf, CDC_EVENT_LINE_CODING_CHANGED, cfg->callback_line_coding_changed); } - /* Buffers */ - esp_tusb_cdcacm_t *acm = get_acm(itf); - - acm->ringbuf_read_mux = xSemaphoreCreateMutex(); - ESP_GOTO_ON_FALSE(acm->ringbuf_read_mux, ESP_ERR_NO_MEM, out3, TAG, "Creation of a ringbuf mutex failed"); - - acm->rx_tfbuf = malloc(CONFIG_TINYUSB_CDC_RX_BUFSIZE); - ESP_GOTO_ON_FALSE(acm->rx_tfbuf, ESP_ERR_NO_MEM, out2, TAG, "Creation buffer error"); - - acm->rx_unread_buf_sz = cfg->rx_unread_buf_sz == 0 ? RX_UNREADBUF_SZ_DEFAULT : cfg->rx_unread_buf_sz; - acm->rx_unread_buf = xRingbufferCreate(acm->rx_unread_buf_sz, RINGBUF_TYPE_BYTEBUF); - ESP_GOTO_ON_FALSE(acm->rx_unread_buf, ESP_ERR_NO_MEM, out1, TAG, "Creation buffer error"); - - ESP_LOGD(TAG, "Comm Initialized buff:%d bytes", cfg->rx_unread_buf_sz); return ESP_OK; - -out1: - free(acm->rx_tfbuf); -out2: - vSemaphoreDelete(acm->ringbuf_read_mux); -out3: - free_obj(itf); -out4: +fail: tinyusb_cdc_deinit(itf); return ret; } diff --git a/usb/esp_tinyusb/vfs_tinyusb.c b/usb/esp_tinyusb/vfs_tinyusb.c index 2a60a3e68a..8b8cea9ee3 100644 --- a/usb/esp_tinyusb/vfs_tinyusb.c +++ b/usb/esp_tinyusb/vfs_tinyusb.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -67,19 +67,17 @@ static vfs_tinyusb_t s_vfstusb; static esp_err_t apply_path(char const *path) { - if (path != NULL) { - size_t path_len = strlen(path) + 1; - if (path_len > VFS_TUSB_MAX_PATH) { - ESP_LOGE(TAG, "The path is too long; maximum is %d characters", VFS_TUSB_MAX_PATH); - return ESP_ERR_INVALID_ARG; - } - strncpy(s_vfstusb.vfs_path, path, (VFS_TUSB_MAX_PATH - 1)); - } else { - strncpy(s_vfstusb.vfs_path, - VFS_TUSB_PATH_DEFAULT, - (VFS_TUSB_MAX_PATH - 1)); + if (path == NULL) { + path = VFS_TUSB_PATH_DEFAULT; } - ESP_LOGV(TAG, "Path is set to `%s`", s_vfstusb.vfs_path); + + size_t path_len = strlen(path) + 1; + if (path_len > VFS_TUSB_MAX_PATH) { + ESP_LOGE(TAG, "The path is too long; maximum is %d characters", VFS_TUSB_MAX_PATH); + return ESP_ERR_INVALID_ARG; + } + strncpy(s_vfstusb.vfs_path, path, (VFS_TUSB_MAX_PATH - 1)); + ESP_LOGV(TAG, "Path is set to `%s`", path); return ESP_OK; } @@ -107,7 +105,6 @@ static void vfstusb_deinit(void) memset(&s_vfstusb, 0, sizeof(s_vfstusb)); } - static int tusb_open(const char *path, int flags, int mode) { (void) mode; @@ -124,24 +121,25 @@ static ssize_t tusb_write(int fd, const void *data, size_t size) _lock_acquire(&(s_vfstusb.write_lock)); for (size_t i = 0; i < size; i++) { int c = data_c[i]; - /* handling the EOL */ - if (c == '\n' && s_vfstusb.tx_mode != ESP_LINE_ENDINGS_LF) { - if (tinyusb_cdcacm_write_queue_char(s_vfstusb.cdc_intf, '\r')) { - written_sz++; - } else { + if (c != '\n') { + if (!tinyusb_cdcacm_write_queue_char(s_vfstusb.cdc_intf, c)) { break; // can't write anymore } - if (s_vfstusb.tx_mode == ESP_LINE_ENDINGS_CR) { - continue; - } - } - /* write a char */ - if (tinyusb_cdcacm_write_queue_char(s_vfstusb.cdc_intf, c)) { - written_sz++; } else { - break; // can't write anymore + if (s_vfstusb.tx_mode == ESP_LINE_ENDINGS_CRLF || s_vfstusb.tx_mode == ESP_LINE_ENDINGS_CR) { + char cr = '\r'; + if (!tinyusb_cdcacm_write_queue_char(s_vfstusb.cdc_intf, cr)) { + break; // can't write anymore + } + } + if (s_vfstusb.tx_mode == ESP_LINE_ENDINGS_CRLF || s_vfstusb.tx_mode == ESP_LINE_ENDINGS_LF) { + char lf = '\n'; + if (!tinyusb_cdcacm_write_queue_char(s_vfstusb.cdc_intf, lf)) { + break; // can't write anymore + } + } } - + written_sz++; } tud_cdc_n_write_flush(s_vfstusb.cdc_intf); _lock_release(&(s_vfstusb.write_lock)); @@ -160,31 +158,40 @@ static ssize_t tusb_read(int fd, void *data, size_t size) char *data_c = (char *) data; size_t received = 0; _lock_acquire(&(s_vfstusb.read_lock)); - int cm1 = NONE; - int c = NONE; + if (tud_cdc_n_available(s_vfstusb.cdc_intf) == 0) { + goto finish; + } while (received < size) { - cm1 = c; // store the old char - int c = tud_cdc_n_read_char(0); // get a new one + int c = tud_cdc_n_read_char(s_vfstusb.cdc_intf); + if ( c == NONE) { // if data ends + break; + } + + // Handle line endings. From configured mode -> LF mode if (s_vfstusb.rx_mode == ESP_LINE_ENDINGS_CR) { + // Change CRs to newlines if (c == '\r') { c = '\n'; } - } else if (s_vfstusb.rx_mode == ESP_LINE_ENDINGS_CR) { - if ((c == '\n') & (cm1 == '\r')) { - --received; // step back - c = '\n'; + } else if (s_vfstusb.rx_mode == ESP_LINE_ENDINGS_CRLF) { + if (c == '\r') { + uint8_t next_char = NONE; + // Check if next char is newline. If yes, we got CRLF sequence + tud_cdc_n_peek(s_vfstusb.cdc_intf, &next_char); + if (next_char == '\n') { + c = tud_cdc_n_read_char(s_vfstusb.cdc_intf); // Remove '\n' from the fifo + } } } - if ( c == NONE) { // if data ends - break; - } + data_c[received] = (char) c; ++received; if (c == '\n') { break; } } +finish: _lock_release(&(s_vfstusb.read_lock)); if (received > 0) { return received; @@ -193,7 +200,6 @@ static ssize_t tusb_read(int fd, void *data, size_t size) return -1; } - static int tusb_fstat(int fd, struct stat *st) { FD_CHECK(fd, -1); @@ -223,39 +229,33 @@ static int tusb_fcntl(int fd, int cmd, int arg) esp_err_t esp_vfs_tusb_cdc_unregister(char const *path) { - ESP_LOGD(TAG, "Unregistering TinyUSB driver"); + ESP_LOGD(TAG, "Unregistering CDC-VFS driver"); int res; if (path == NULL) { // NULL means using the default path for unregistering: VFS_TUSB_PATH_DEFAULT - res = strcmp(s_vfstusb.vfs_path, VFS_TUSB_PATH_DEFAULT); - } else { - res = strcmp(s_vfstusb.vfs_path, path); + path = VFS_TUSB_PATH_DEFAULT; } + res = strcmp(s_vfstusb.vfs_path, path); if (res) { res = ESP_ERR_INVALID_ARG; - ESP_LOGE(TAG, "There is no TinyUSB driver registerred to the path '%s' (err: 0x%x)", s_vfstusb.vfs_path, res); + ESP_LOGE(TAG, "There is no CDC-VFS driver registered to path '%s' (err: 0x%x)", path, res); return res; } - - res = esp_vfs_unregister(s_vfstusb.vfs_path); if (res != ESP_OK) { - ESP_LOGE(TAG, "Can't unregister TinyUSB driver from '%s' (err: 0x%x)", s_vfstusb.vfs_path, res); + ESP_LOGE(TAG, "Can't unregister CDC-VFS driver from '%s' (err: 0x%x)", s_vfstusb.vfs_path, res); } else { - ESP_LOGD(TAG, "Unregistered TinyUSB driver"); + ESP_LOGD(TAG, "Unregistered CDC-VFS driver"); vfstusb_deinit(); } return res; } - - - esp_err_t esp_vfs_tusb_cdc_register(int cdc_intf, char const *path) { - ESP_LOGD(TAG, "Registering TinyUSB CDC driver"); + ESP_LOGD(TAG, "Registering CDC-VFS driver"); int res; if (!tusb_cdc_acm_initialized(cdc_intf)) { ESP_LOGE(TAG, "TinyUSB CDC#%d is not initialized", cdc_intf); @@ -279,9 +279,23 @@ esp_err_t esp_vfs_tusb_cdc_register(int cdc_intf, char const *path) res = esp_vfs_register(s_vfstusb.vfs_path, &vfs, NULL); if (res != ESP_OK) { - ESP_LOGE(TAG, "Can't register TinyUSB driver (err: %x)", res); + ESP_LOGE(TAG, "Can't register CDC-VFS driver (err: %x)", res); } else { - ESP_LOGD(TAG, "TinyUSB CDC registered (%s)", s_vfstusb.vfs_path); + ESP_LOGD(TAG, "CDC-VFS registered (%s)", s_vfstusb.vfs_path); } return res; } + +void esp_vfs_tusb_cdc_set_rx_line_endings(esp_line_endings_t mode) +{ + _lock_acquire(&(s_vfstusb.read_lock)); + s_vfstusb.rx_mode = mode; + _lock_release(&(s_vfstusb.read_lock)); +} + +void esp_vfs_tusb_cdc_set_tx_line_endings(esp_line_endings_t mode) +{ + _lock_acquire(&(s_vfstusb.write_lock)); + s_vfstusb.tx_mode = mode; + _lock_release(&(s_vfstusb.write_lock)); +} diff --git a/usb/test_app/CMakeLists.txt b/usb/test_app/CMakeLists.txt index 86a2ef4d96..90520ce070 100755 --- a/usb/test_app/CMakeLists.txt +++ b/usb/test_app/CMakeLists.txt @@ -1,6 +1,7 @@ # The following lines of boilerplate have to be in your project's # CMakeLists in this exact order for cmake to work correctly cmake_minimum_required(VERSION 3.16) +include($ENV{IDF_PATH}/tools/cmake/version.cmake) set(EXTRA_COMPONENT_DIRS ../usb_host_cdc_acm ../usb_host_msc @@ -15,5 +16,10 @@ set(EXTRA_COMPONENT_DIRS ../usb_host_cdc_acm # Set the components to include the tests for. set(TEST_COMPONENTS "usb_host_cdc_acm" "usb_host_msc" "usb_host_uvc" "usb_host_hid" CACHE STRING "List of components to test") +if("${IDF_VERSION_MAJOR}.${IDF_VERSION_MINOR}" VERSION_GREATER_EQUAL "5.0") + list(APPEND EXTRA_COMPONENT_DIRS ../esp_tinyusb) + list(APPEND TEST_COMPONENTS "esp_tinyusb") +endif() + include($ENV{IDF_PATH}/tools/cmake/project.cmake) project(usb_test_app) diff --git a/usb/test_app/pytest_usb_device.py b/usb/test_app/pytest_usb_device.py new file mode 100644 index 0000000000..2c5e60a578 --- /dev/null +++ b/usb/test_app/pytest_usb_device.py @@ -0,0 +1,77 @@ +# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD +# SPDX-License-Identifier: CC0-1.0 + +from typing import Tuple + +import pytest +from pytest_embedded_idf.dut import IdfDut +from time import sleep +from serial import Serial +from serial.tools.list_ports import comports + + +@pytest.mark.esp32s2 +@pytest.mark.esp32s3 +@pytest.mark.usb_device +def test_usb_device(dut) -> None: + ''' + This is a pytest script for esp_tinyusb testcase in usb/esp_tinyusb/test/test_esp_tinyusb.c + + Running the test locally: + 1. Build the testa app for your DUT (ESP32-S2 or S3) + 2. Connect you DUT to your test runner (local machine) with USB port and flashing port + 3. Run `pytest --target esp32s3` + + Test procedure: + 1. Run the test on the DUT + 2. Expect 2 Virtual COM Ports in the system + 3. Open both comports and send some data. Expect echoed data + ''' + dut.expect_exact('Press ENTER to see the list of tests.') + dut.write('[esp_tinyusb]') + dut.expect_exact('TinyUSB: TinyUSB Driver installed') + sleep(2) # Some time for the OS to enumerate our USB device + + # Find devices with Espressif TinyUSB VID/PID + s = [] + ports = comports() + for port, _, hwid in ports: + if '303A:4002' in hwid: + s.append(port) + + if len(s) != 2: + raise Exception('TinyUSB COM port not found') + + with Serial(s[0]) as cdc0: + with Serial(s[1]) as cdc1: + # Write dummy string and check for echo + cdc0.write('text\r\n'.encode()) + res = cdc0.readline() + assert b'text' in res + if b'novfs' in res: + novfs_cdc = cdc0 + vfs_cdc = cdc1 + + cdc1.write('text\r\n'.encode()) + res = cdc1.readline() + assert b'text' in res + if b'novfs' in res: + novfs_cdc = cdc1 + vfs_cdc = cdc0 + + # Write more than MPS, check that the transfer is not divided + novfs_cdc.write(bytes(100)) + dut.expect_exact("Intf 0, RX 100 bytes") + + # Write more than RX buffer, check correct reception + novfs_cdc.write(bytes(600)) + transfer_len1 = int(dut.expect(r'Intf 0, RX (\d+) bytes')[1].decode()) + transfer_len2 = int(dut.expect(r'Intf 0, RX (\d+) bytes')[1].decode()) + assert transfer_len1 + transfer_len2 == 600 + + # The VFS is setup for CRLF RX and LF TX + vfs_cdc.write('text\r\n'.encode()) + res = vfs_cdc.readline() + assert b'text\n' in res + + return diff --git a/usb/usb_host_cdc_acm/test/usb_device.c b/usb/usb_host_cdc_acm/test/usb_device.c index 9b180a30f6..74793ed45a 100644 --- a/usb/usb_host_cdc_acm/test/usb_device.c +++ b/usb/usb_host_cdc_acm/test/usb_device.c @@ -11,7 +11,7 @@ #include "esp_idf_version.h" static uint8_t buf[CONFIG_TINYUSB_CDC_RX_BUFSIZE + 1]; -void tinyusb_cdc_rx_callback(int itf, cdcacm_event_t *event) +static void tinyusb_cdc_rx_callback(int itf, cdcacm_event_t *event) { size_t rx_size = 0; /* read and write back */ @@ -38,7 +38,7 @@ static const tusb_desc_device_t cdc_device_descriptor = { .bNumConfigurations = 0x01 }; -const uint16_t cdc_desc_config_len = TUD_CONFIG_DESC_LEN + 2 * TUD_CDC_DESC_LEN; +static const uint16_t cdc_desc_config_len = TUD_CONFIG_DESC_LEN + CFG_TUD_CDC * TUD_CDC_DESC_LEN; static const uint8_t cdc_desc_configuration[] = { TUD_CONFIG_DESCRIPTOR(1, 4, 0, cdc_desc_config_len, TUSB_DESC_CONFIG_ATT_REMOTE_WAKEUP, 100), TUD_CDC_DESCRIPTOR(0, 4, 0x81, 8, 0x02, 0x82, 64), @@ -60,7 +60,6 @@ void run_usb_dual_cdc_device(void) tinyusb_config_cdcacm_t amc_cfg = { .usb_dev = TINYUSB_USBDEV_0, .cdc_port = TINYUSB_CDC_ACM_0, - .rx_unread_buf_sz = 64, .callback_rx = &tinyusb_cdc_rx_callback, .callback_rx_wanted_char = NULL, .callback_line_state_changed = NULL,