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
espressif · Sep 4, 2023 · cf51490 · cf51490
1 parent 3c52bba
commit cf51490
Show file tree

Hide file tree

Showing 16 changed files with 480 additions and 257 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,29 +13,30 @@ 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)
     list(APPEND srcs
         tusb_msc_storage.c
         )
 endif() # CONFIG_TINYUSB_MSC_ENABLED
+
 if(CONFIG_TINYUSB_NET_MODE_NCM)
     list(APPEND srcs
          tinyusb_net.c
          )
 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
@@ -171,15 +171,15 @@ 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.
 
         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
@@ -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
@@ -11,9 +11,11 @@ extern "C" {
 #endif
 
 #include <stdint.h>
-#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
@@ -1,12 +1,13 @@
 /*
- * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #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
@@ -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
@@ -0,0 +1,4 @@
+idf_component_register(SRCS "test_esp_tinyusb.c"
+                       INCLUDE_DIRS "."
+                       REQUIRES unity esp_tinyusb
+                       )