-
Notifications
You must be signed in to change notification settings - Fork 22
feat(cli): Add support for USB CDC to cli component, for use on ESP32-S2 for example
#564
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| idf_component_register( | ||
| INCLUDE_DIRS "include" "detail/cli/include" | ||
| SRC_DIRS "src" | ||
| REQUIRES driver esp_driver_uart esp_driver_usb_serial_jtag vfs logger) | ||
| REQUIRES driver esp_driver_uart esp_driver_usb_serial_jtag vfs esp_vfs_console logger) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # on the ESP32S2, which has native USB, we need to set the console so that the | ||
| # CLI can be configured correctly: | ||
| CONFIG_ESP_CONSOLE_USB_CDC=y |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,16 +11,14 @@ | |||||||||||||||||||||||||||
| #include "driver/usb_serial_jtag_vfs.h" | ||||||||||||||||||||||||||||
| #include "esp_err.h" | ||||||||||||||||||||||||||||
| #include "esp_system.h" | ||||||||||||||||||||||||||||
| #include <fcntl.h> | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #include "esp_vfs_cdcacm.h" | ||||||||||||||||||||||||||||
| #include "esp_vfs_dev.h" | ||||||||||||||||||||||||||||
| #include "esp_vfs_usb_serial_jtag.h" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #include "line_input.hpp" | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #ifdef CONFIG_ESP_CONSOLE_USB_CDC | ||||||||||||||||||||||||||||
| #error The cli component is currently incompatible with CONFIG ESP_CONSOLE_USB_CDC console. | ||||||||||||||||||||||||||||
| #endif // CONFIG_ESP_CONSOLE_USB_CDC | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #ifndef STRINGIFY | ||||||||||||||||||||||||||||
| #define STRINGIFY(s) STRINGIFY2(s) | ||||||||||||||||||||||||||||
| #define STRINGIFY2(s) #s | ||||||||||||||||||||||||||||
|
|
@@ -38,9 +36,9 @@ namespace espp { | |||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * @note You should call configure_stdin_stdout() before creating a Cli object | ||||||||||||||||||||||||||||
| * to ensure that std::cin works as needed. If you do not want to use the | ||||||||||||||||||||||||||||
| * Cli over the ESP CONSOLE (e.g. the ESP's UART, USB Serial/JTAG) and | ||||||||||||||||||||||||||||
| * instead want to run it over a different UART port, VFS, or some other | ||||||||||||||||||||||||||||
| * configuration, then you should call one of | ||||||||||||||||||||||||||||
| * Cli over the ESP CONSOLE (e.g. the ESP's UART, USB Serial/JTAG, USB | ||||||||||||||||||||||||||||
| * CDC) and instead want to run it over a different UART port, VFS, or | ||||||||||||||||||||||||||||
| * some other configuration, then you should call one of | ||||||||||||||||||||||||||||
| * - configure_stdin_stdout_uart() | ||||||||||||||||||||||||||||
| * - configure_stdin_stdout_vfs() | ||||||||||||||||||||||||||||
| * - configure_stdin_stdout_custom() | ||||||||||||||||||||||||||||
|
|
@@ -58,6 +56,7 @@ class Cli : private cli::CliSession { | |||||||||||||||||||||||||||
| * compiled to use. This will only work if the ESP_CONSOLE was | ||||||||||||||||||||||||||||
| * configured to use one of the following: | ||||||||||||||||||||||||||||
| * - CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG | ||||||||||||||||||||||||||||
| * - CONFIG_ESP_CONSOLE_USB_CDC | ||||||||||||||||||||||||||||
| * - CONFIG_ESP_CONSOLE_UART | ||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||
| * If you want to use a different console, you should use one of the | ||||||||||||||||||||||||||||
|
|
@@ -77,6 +76,8 @@ class Cli : private cli::CliSession { | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| #if CONFIG_ESP_CONSOLE_USB_SERIAL_JTAG | ||||||||||||||||||||||||||||
| configure_stdin_stdout_usb_serial_jtag(); | ||||||||||||||||||||||||||||
| #elif CONFIG_ESP_CONSOLE_USB_CDC | ||||||||||||||||||||||||||||
| configure_stdin_stdout_usb_cdc(); | ||||||||||||||||||||||||||||
| #elif CONFIG_ESP_CONSOLE_UART | ||||||||||||||||||||||||||||
| configure_stdin_stdout_uart((uart_port_t)CONFIG_ESP_CONSOLE_UART_NUM, | ||||||||||||||||||||||||||||
| CONFIG_ESP_CONSOLE_UART_BAUDRATE); | ||||||||||||||||||||||||||||
|
|
@@ -182,6 +183,47 @@ class Cli : private cli::CliSession { | |||||||||||||||||||||||||||
| configured_ = true; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * @brief Configure the USB CDC driver to support blocking input read, so | ||||||||||||||||||||||||||||
| * that std::cin (which assumes a blocking read) will function. This | ||||||||||||||||||||||||||||
| * should be primarily used when you want to use the std::cin/std::getline | ||||||||||||||||||||||||||||
| * and other std input functions or you want to use the cli library. | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| static void configure_stdin_stdout_usb_cdc(void) { | ||||||||||||||||||||||||||||
| if (configured_) { | ||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // drain stdout before reconfiguring it | ||||||||||||||||||||||||||||
| fflush(stdout); | ||||||||||||||||||||||||||||
| fsync(fileno(stdout)); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const std::string_view dev_name = "/dev/cdcacm"; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // redirect stdin, stdout, stderr to the USB CDC interface | ||||||||||||||||||||||||||||
| console_.in = freopen(dev_name.data(), "r", stdin); | ||||||||||||||||||||||||||||
| console_.out = freopen(dev_name.data(), "w", stdout); | ||||||||||||||||||||||||||||
| console_.err = freopen(dev_name.data(), "w", stderr); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| esp_vfs_dev_cdcacm_register(); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
Comment on lines
+203
to
+209
|
||||||||||||||||||||||||||||
| // redirect stdin, stdout, stderr to the USB CDC interface | |
| console_.in = freopen(dev_name.data(), "r", stdin); | |
| console_.out = freopen(dev_name.data(), "w", stdout); | |
| console_.err = freopen(dev_name.data(), "w", stderr); | |
| esp_vfs_dev_cdcacm_register(); | |
| // Register the VFS device before redirecting streams | |
| esp_vfs_dev_cdcacm_register(); | |
| // redirect stdin, stdout, stderr to the USB CDC interface | |
| console_.in = freopen(dev_name.data(), "r", stdin); | |
| console_.out = freopen(dev_name.data(), "w", stdout); | |
| console_.err = freopen(dev_name.data(), "w", stderr); |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "Initialize VFS & UART" is misleading as this function configures USB CDC, not UART. It should say "Initialize VFS & USB CDC" to be accurate.
| // Initialize VFS & UART so we can use std::cout/cin | |
| // Initialize VFS & USB CDC so we can use std::cout/cin |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setvbuf call should be placed before the VFS registration and freopen calls, similar to the pattern in configure_stdin_stdout_vfs() (line 258) and configure_stdin_stdout_uart() (line 121). The current placement after freopen may not properly configure stdin's buffering mode. Move this call to after the fflush/fsync operations (around line 199-200).
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Missing a final fflush/fsync sequence before setting configured_ = true. The configure_stdin_stdout_uart() (lines 146-147) and configure_stdin_stdout_usb_serial_jtag() (lines 180-181) functions both call fflush(stdout) and fsync(fileno(stdout)) at the end to ensure all output is written before completing configuration. Consider adding this for consistency:
fflush(stdout);
fsync(fileno(stdout));
configured_ = true;| fflush(stdout); | |
| fsync(fileno(stdout)); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| #include "cli.hpp" | ||
|
|
||
| bool espp::Cli::configured_ = false; | ||
| espp::Cli::console_handle_t espp::Cli::console_ = {nullptr, nullptr, nullptr}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
freopencalls can return NULL on failure, but these return values are not checked. Whileconfigure_stdin_stdout_vfs()also doesn't check these (lines 264-266), it would be good practice to verify that the operations succeeded before proceeding. Consider adding checks like: