Feat/add winusb#27
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds two TinyUSB-based USB device examples (WebUSB and WinUSB) for ESP32, including TinyUSB integration, board configs, USB descriptors, application logic, HTML test page, component manifests, CMake files, and updates build_config/README IDF references to release-v5.5. Changes
Sequence Diagram(s)sequenceDiagram
participant Host as USB Host (PC / Browser)
participant PHY as USB PHY (usb_phy_init)
participant TinyUSB as TinyUSB Stack (tud_*/tud_task)
participant App as Example App (web_usb.c / win_usb.c)
Host->>PHY: Connect & reset
activate PHY
PHY->>TinyUSB: initialize (tud_init)
TinyUSB->>App: app_main starts tusb task
deactivate PHY
Host->>TinyUSB: Get Device Descriptor
TinyUSB->>App: tud_descriptor_device_cb()
App-->>TinyUSB: device descriptor
Host->>TinyUSB: Get Configuration / BOS / MS OS
TinyUSB->>App: tud_descriptor_configuration_cb() / tud_descriptor_bos_cb()
App-->>TinyUSB: configuration / BOS / MS OS descriptor
Host->>TinyUSB: Vendor Control (GET_MS_OS_20)
TinyUSB->>App: tud_vendor_control_xfer_cb()
App-->>TinyUSB: MS OS 2.0 descriptor
TinyUSB-->>Host: MS OS 2.0 descriptor
Host->>TinyUSB: Vendor Bulk OUT (send data)
TinyUSB->>App: tud_vendor_rx_cb()
App->>TinyUSB: tud_vendor_write() (echo)
TinyUSB-->>Host: Vendor Bulk IN (echo)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
examples/usb/device/win_usb/main/idf_component.yml (1)
4-5: Consider aligning IDF version constraint with actual usage.The manifest allows IDF
>=4.1.0, but the project targets IDF 5.5 perbuild_config.yml. While this broad constraint provides flexibility, TinyUSB features used in this example may require a more recent IDF version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/usb/device/win_usb/main/idf_component.yml` around lines 4 - 5, The idf_component.yml currently sets the IDF constraint to "version: '>=4.1.0'" which is inconsistent with build_config.yml targeting IDF 5.5; update the idf.version constraint in idf_component.yml (the idf -> version entry) to match the project's target (for example ">=5.5.0" or a range that includes 5.5) so TinyUSB-dependent features align with the actual IDF used.examples/usb/device/win_usb/main/tusb/tusb_config.h (1)
34-117: Reduce duplication with a shared TinyUSB config base header.This file duplicates most of
examples/usb/device/web_usb/main/tusb/tusb_config.h. Consider moving common macros to a shared header and only overriding PID/product/example-specific values per target. This will reduce config drift and maintenance cost.♻️ Refactor direction
-// full duplicated config in each example +// shared: examples/usb/device/common/tusb/tusb_config_base.h +// web_usb: defines USB_PID/USB_PRODUCT + includes tusb_config_base.h +// win_usb: defines USB_PID/USB_PRODUCT + includes tusb_config_base.h🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/usb/device/win_usb/main/tusb/tusb_config.h` around lines 34 - 117, This file duplicates most TinyUSB config macros; extract common definitions (e.g., CFG_TUSB_RHPORT0_MODE/CFG_TUSB_RHPORT1_MODE, CFG_TUSB_MCU, CFG_TUSB_OS, CFG_TUSB_OS_INC_PATH, CFG_TUSB_DEBUG, CFG_TUSB_MEM_SECTION/CFG_TUSB_MEM_ALIGN, CFG_TUD_ENABLED, CFG_TUD_ENDPOINT0_SIZE, CFG_TUD_VENDOR/CFG_TUD_CDC, VENDOR_BUF_SIZE, CFG_TUD_VENDOR_RX_BUFSIZE/CFG_TUD_VENDOR_TX_BUFSIZE, CFG_TUD_VENDOR_EPSIZE) into a new shared header (e.g., tusb_config_common.h), include that shared header from this file, and leave only example-specific overrides here (USB_PID, USB_PRODUCT, USB_MANUFACTURER and any rhport HS/FS selection if different). Ensure this file no longer redefines symbols already provided by the shared header and that per-board options like CONFIG_TINYUSB_RHPORT_HS and CONFIG_USB_HS remain supported via conditional overrides.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build_config.yml`:
- Around line 162-164: Add a new YAML project entry for the missing web_usb
example so CI builds it; mirror the existing win_usb entry by adding an entry
with id "web_usb", path pointing to examples/usb/device/web_usb, and the same
idf_version: release-v5.5 and target: esp32s3 values as the win_usb entry,
ensuring proper YAML indentation and ordering within build_config.yml.
In `@examples/usb/device/web_usb/main/tusb/usb_descriptors.c`:
- Around line 105-110: The serial string at string_desc_arr index 3 is a fixed
literal ("012-2021"); replace it by generating a unique USB serial at runtime
from chip-unique data (e.g., eFuse MAC or chip ID) and populate
string_desc_arr[3] before USB initialization. Implement a helper (e.g.,
read_chip_unique_id() or get_efuse_mac()) to read the hardware ID, format it
into an ASCII serial string, and assign that buffer to string_desc_arr[3]
(ensure the buffer lifetime spans USB use and follows descriptor encoding
rules). Ensure this initialization happens prior to calling tusb or USB stack
start so the device advertises the unique serial.
In `@examples/usb/device/web_usb/main/web_usb.c`:
- Line 30: Update the misleading comment to reference the actual symbolic
constant used: replace the hard-coded bRequest=0x20 text with
bRequest=VENDOR_REQUEST_MICROSOFT (or state "bRequest=VENDOR_REQUEST_MICROSOFT
(implements MS OS 2.0 vendor request)") so the comment matches the
implementation that uses VENDOR_REQUEST_MICROSOFT.
- Around line 12-21: Modify usb_phy_init to return an esp_err_t (or int) and
check the return value from usb_new_phy inside it; propagate and return any
failure to the caller (refer to function usb_phy_init and call site app_main).
In app_main, call usb_phy_init and verify its return status, logging an error
and aborting or returning if initialization fails. Also check the return value
of xTaskCreate where the USB task is created (refer to the xTaskCreate call in
app_main), and log and handle task-creation failures (e.g., delete partial
state/return error). Ensure all logs use existing logging utilities and that
error paths clean up or stop further USB setup.
In `@examples/usb/device/win_usb/main/win_usb.c`:
- Around line 1-8: The source is missing the stdio.h header required for printf
usage; add `#include` <stdio.h> to the top-of-file include block alongside the
existing headers (near "string.h", "freertos/FreeRTOS.h", etc.) so printf calls
in this file compile cleanly—look for the include list in win_usb.c to insert
the new include.
- Around line 12-21: usb_new_phy() and xTaskCreate() return status values that
are currently ignored; update usb_phy_init() to check the esp_err_t returned by
usb_new_phy (use ESP_ERROR_CHECK or handle non-ESP_OK by logging and
returning/aborting) and update the task-creation call (the xTaskCreate
invocation) to check its BaseType_t result against pdPASS, logging and handling
failures (e.g., delete resources, return error, or abort) so initialization
errors don't fail silently; reference usb_phy_init, usb_new_phy, and the
xTaskCreate call when making these changes.
---
Nitpick comments:
In `@examples/usb/device/win_usb/main/idf_component.yml`:
- Around line 4-5: The idf_component.yml currently sets the IDF constraint to
"version: '>=4.1.0'" which is inconsistent with build_config.yml targeting IDF
5.5; update the idf.version constraint in idf_component.yml (the idf -> version
entry) to match the project's target (for example ">=5.5.0" or a range that
includes 5.5) so TinyUSB-dependent features align with the actual IDF used.
In `@examples/usb/device/win_usb/main/tusb/tusb_config.h`:
- Around line 34-117: This file duplicates most TinyUSB config macros; extract
common definitions (e.g., CFG_TUSB_RHPORT0_MODE/CFG_TUSB_RHPORT1_MODE,
CFG_TUSB_MCU, CFG_TUSB_OS, CFG_TUSB_OS_INC_PATH, CFG_TUSB_DEBUG,
CFG_TUSB_MEM_SECTION/CFG_TUSB_MEM_ALIGN, CFG_TUD_ENABLED,
CFG_TUD_ENDPOINT0_SIZE, CFG_TUD_VENDOR/CFG_TUD_CDC, VENDOR_BUF_SIZE,
CFG_TUD_VENDOR_RX_BUFSIZE/CFG_TUD_VENDOR_TX_BUFSIZE, CFG_TUD_VENDOR_EPSIZE) into
a new shared header (e.g., tusb_config_common.h), include that shared header
from this file, and leave only example-specific overrides here (USB_PID,
USB_PRODUCT, USB_MANUFACTURER and any rhport HS/FS selection if different).
Ensure this file no longer redefines symbols already provided by the shared
header and that per-board options like CONFIG_TINYUSB_RHPORT_HS and
CONFIG_USB_HS remain supported via conditional overrides.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
README.mdbuild_config.ymlexamples/usb/device/web_usb/CMakeLists.txtexamples/usb/device/web_usb/main/CMakeLists.txtexamples/usb/device/web_usb/main/idf_component.ymlexamples/usb/device/web_usb/main/tusb/tusb_config.hexamples/usb/device/web_usb/main/tusb/usb_descriptors.cexamples/usb/device/web_usb/main/tusb/usb_descriptors.hexamples/usb/device/web_usb/main/web_usb.cexamples/usb/device/web_usb/web_usb_test.htmlexamples/usb/device/win_usb/CMakeLists.txtexamples/usb/device/win_usb/README.mdexamples/usb/device/win_usb/main/CMakeLists.txtexamples/usb/device/win_usb/main/idf_component.ymlexamples/usb/device/win_usb/main/tusb/tusb_config.hexamples/usb/device/win_usb/main/tusb/usb_descriptors.cexamples/usb/device/win_usb/main/tusb/usb_descriptors.hexamples/usb/device/win_usb/main/win_usb.cexamples/usb/device/win_usb/sdkconfig.defaults
Comment on lines
+162
to
164
| - path: examples/usb/device/win_usb | ||
| idf_version: release-v5.5 | ||
| target: esp32s3 |
There was a problem hiding this comment.
Missing web_usb project entry.
The win_usb project is correctly added to the build configuration, but the web_usb example (also introduced in this PR at examples/usb/device/web_usb/) is not included. This means web_usb won't be built by CI.
Proposed fix to add web_usb entry
- path: examples/usb/device/win_usb
idf_version: release-v5.5
target: esp32s3
+
+ - path: examples/usb/device/web_usb
+ idf_version: release-v5.5
+ target: esp32s3📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - path: examples/usb/device/win_usb | |
| idf_version: release-v5.5 | |
| target: esp32s3 | |
| - path: examples/usb/device/win_usb | |
| idf_version: release-v5.5 | |
| target: esp32s3 | |
| - path: examples/usb/device/web_usb | |
| idf_version: release-v5.5 | |
| target: esp32s3 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build_config.yml` around lines 162 - 164, Add a new YAML project entry for
the missing web_usb example so CI builds it; mirror the existing win_usb entry
by adding an entry with id "web_usb", path pointing to
examples/usb/device/web_usb, and the same idf_version: release-v5.5 and target:
esp32s3 values as the win_usb entry, ensuring proper YAML indentation and
ordering within build_config.yml.
Comment on lines
+105
to
+110
| char const *string_desc_arr [] = { | ||
| (const char[]) { 0x09, 0x04 }, // 0: language (English) | ||
| USB_MANUFACTURER, // 1: Manufacturer | ||
| USB_PRODUCT, // 2: Product | ||
| "012-2021", // 3: Serial | ||
| }; |
There was a problem hiding this comment.
Use a unique USB serial instead of a fixed literal.
Line 109 hardcodes the serial ("012-2021"). This can cause host-side ambiguity when multiple boards are connected or re-enumerated. Please derive serial from chip-unique data (for example, eFuse MAC) before exposing string index 3.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/usb/device/web_usb/main/tusb/usb_descriptors.c` around lines 105 -
110, The serial string at string_desc_arr index 3 is a fixed literal
("012-2021"); replace it by generating a unique USB serial at runtime from
chip-unique data (e.g., eFuse MAC or chip ID) and populate string_desc_arr[3]
before USB initialization. Implement a helper (e.g., read_chip_unique_id() or
get_efuse_mac()) to read the hardware ID, format it into an ASCII serial string,
and assign that buffer to string_desc_arr[3] (ensure the buffer lifetime spans
USB use and follows descriptor encoding rules). Ensure this initialization
happens prior to calling tusb or USB stack start so the device advertises the
unique serial.
Comment on lines
+12
to
+21
| static void usb_phy_init(void) | ||
| { | ||
| usb_phy_handle_t phy_hdl; | ||
| usb_phy_config_t phy_conf = { | ||
| .controller = USB_PHY_CTRL_OTG, | ||
| .otg_mode = USB_OTG_MODE_DEVICE, | ||
| .target = USB_PHY_TARGET_INT | ||
| }; | ||
| usb_new_phy(&phy_conf, &phy_hdl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the files exist
echo "=== Checking file existence ==="
ls -la examples/usb/device/web_usb/main/web_usb.c 2>&1 || echo "File not found"
ls -la examples/usb/device/win_usb/main/win_usb.c 2>&1 || echo "File not found"
echo -e "\n=== Content of web_usb.c (lines 1-50) ==="
head -50 examples/usb/device/web_usb/main/web_usb.c 2>&1 || echo "Cannot read file"
echo -e "\n=== Content of web_usb.c (lines 80-100) ==="
sed -n '80,100p' examples/usb/device/web_usb/main/web_usb.c 2>&1 || echo "Cannot read file"
echo -e "\n=== Line count ==="
wc -l examples/usb/device/web_usb/main/web_usb.c 2>&1 || echo "Cannot count lines"Repository: YanKE01/esp_demos
Length of output: 2090
🏁 Script executed:
#!/bin/bash
echo "=== Content of win_usb.c (entire file) ==="
cat -n examples/usb/device/win_usb/main/win_usb.c
echo -e "\n=== Search for usb_new_phy and xTaskCreate calls ==="
rg -n 'usb_new_phy|xTaskCreate' examples/usb/device/web_usb/main/web_usb.c examples/usb/device/win_usb/main/win_usb.cRepository: YanKE01/esp_demos
Length of output: 3737
Check return values for USB PHY initialization and task creation.
Both usb_new_phy() (line 20) and xTaskCreate() (line 92) return error codes that are currently unchecked. Failures at these points will leave USB non-functional without logs. The same issue exists in examples/usb/device/win_usb/main/win_usb.c (lines 20 and 96).
Modify usb_phy_init() to return a status code and check it in app_main(), and also check the return value of xTaskCreate():
Proposed fix
-static void usb_phy_init(void)
+static bool usb_phy_init(void)
{
usb_phy_handle_t phy_hdl;
usb_phy_config_t phy_conf = {
.controller = USB_PHY_CTRL_OTG,
.otg_mode = USB_OTG_MODE_DEVICE,
.target = USB_PHY_TARGET_INT
};
- usb_new_phy(&phy_conf, &phy_hdl);
+ if (usb_new_phy(&phy_conf, &phy_hdl) != ESP_OK) {
+ ESP_LOGE(TAG, "usb_new_phy failed");
+ return false;
+ }
+ return true;
}
void app_main(void)
{
- usb_phy_init();
+ if (!usb_phy_init()) {
+ return;
+ }
if (!tusb_init()) {
ESP_LOGE(TAG, "TinyUSB init failed");
return;
}
- xTaskCreate(tusb_device_task, "tusb_device_task", 4096, NULL, 5, NULL);
+ if (xTaskCreate(tusb_device_task, "tusb_device_task", 4096, NULL, 5, NULL) != pdPASS) {
+ ESP_LOGE(TAG, "Failed to create tusb_device_task");
+ return;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/usb/device/web_usb/main/web_usb.c` around lines 12 - 21, Modify
usb_phy_init to return an esp_err_t (or int) and check the return value from
usb_new_phy inside it; propagate and return any failure to the caller (refer to
function usb_phy_init and call site app_main). In app_main, call usb_phy_init
and verify its return status, logging an error and aborting or returning if
initialization fails. Also check the return value of xTaskCreate where the USB
task is created (refer to the xTaskCreate call in app_main), and log and handle
task-creation failures (e.g., delete partial state/return error). Ensure all
logs use existing logging utilities and that error paths clean up or stop
further USB setup.
| } | ||
| } | ||
|
|
||
| /* Vendor control: MS OS 2.0 (bRequest=0x20, wIndex=7). WebUSB BOS uses vendor code 0x01 but iLandingPage=0 so no URL. */ |
There was a problem hiding this comment.
Comment is misleading about vendor request value.
Line 30 says bRequest=0x20, but the implementation uses VENDOR_REQUEST_MICROSOFT. Please align the comment with the actual symbolic constant used by this example.
✏️ Suggested comment fix
-/* Vendor control: MS OS 2.0 (bRequest=0x20, wIndex=7). WebUSB BOS uses vendor code 0x01 but iLandingPage=0 so no URL. */
+/* Vendor control: MS OS 2.0 (bRequest=VENDOR_REQUEST_MICROSOFT, wIndex=7). WebUSB BOS uses vendor code 0x01 but iLandingPage=0 so no URL. */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /* Vendor control: MS OS 2.0 (bRequest=0x20, wIndex=7). WebUSB BOS uses vendor code 0x01 but iLandingPage=0 so no URL. */ | |
| /* Vendor control: MS OS 2.0 (bRequest=VENDOR_REQUEST_MICROSOFT, wIndex=7). WebUSB BOS uses vendor code 0x01 but iLandingPage=0 so no URL. */ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/usb/device/web_usb/main/web_usb.c` at line 30, Update the misleading
comment to reference the actual symbolic constant used: replace the hard-coded
bRequest=0x20 text with bRequest=VENDOR_REQUEST_MICROSOFT (or state
"bRequest=VENDOR_REQUEST_MICROSOFT (implements MS OS 2.0 vendor request)") so
the comment matches the implementation that uses VENDOR_REQUEST_MICROSOFT.
Comment on lines
+1
to
+8
| #include <string.h> | ||
| #include "freertos/FreeRTOS.h" | ||
| #include "freertos/task.h" | ||
| #include "esp_log.h" | ||
| #include "tusb.h" | ||
| #include "device/usbd.h" | ||
| #include "esp_private/usb_phy.h" | ||
| #include "usb_descriptors.h" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n examples/usb/device/win_usb/main/win_usb.c | head -70Repository: YanKE01/esp_demos
Length of output: 2561
🏁 Script executed:
rg 'printf' examples/usb/device/win_usb/main/win_usb.c -nRepository: YanKE01/esp_demos
Length of output: 125
🏁 Script executed:
rg '#include.*stdio\.h' examples/usb/device/win_usb/main/win_usb.cRepository: YanKE01/esp_demos
Length of output: 43
Add <stdio.h> include for printf usage.
Lines 61 and 63 use printf without including <stdio.h>, which will break builds with strict compiler settings.
Fix
+#include <stdio.h>
`#include` <string.h>
`#include` "freertos/FreeRTOS.h"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #include <string.h> | |
| #include "freertos/FreeRTOS.h" | |
| #include "freertos/task.h" | |
| #include "esp_log.h" | |
| #include "tusb.h" | |
| #include "device/usbd.h" | |
| #include "esp_private/usb_phy.h" | |
| #include "usb_descriptors.h" | |
| `#include` <stdio.h> | |
| `#include` <string.h> | |
| `#include` "freertos/FreeRTOS.h" | |
| `#include` "freertos/task.h" | |
| `#include` "esp_log.h" | |
| `#include` "tusb.h" | |
| `#include` "device/usbd.h" | |
| `#include` "esp_private/usb_phy.h" | |
| `#include` "usb_descriptors.h" |
🧰 Tools
🪛 Clang (14.0.6)
[error] 1-1: 'string.h' file not found
(clang-diagnostic-error)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/usb/device/win_usb/main/win_usb.c` around lines 1 - 8, The source is
missing the stdio.h header required for printf usage; add `#include` <stdio.h> to
the top-of-file include block alongside the existing headers (near "string.h",
"freertos/FreeRTOS.h", etc.) so printf calls in this file compile cleanly—look
for the include list in win_usb.c to insert the new include.
Comment on lines
+12
to
+21
| static void usb_phy_init(void) | ||
| { | ||
| usb_phy_handle_t phy_hdl; | ||
| usb_phy_config_t phy_conf = { | ||
| .controller = USB_PHY_CTRL_OTG, | ||
| .otg_mode = USB_OTG_MODE_DEVICE, | ||
| .target = USB_PHY_TARGET_INT | ||
| }; | ||
| usb_new_phy(&phy_conf, &phy_hdl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n examples/usb/device/win_usb/main/win_usb.cRepository: YanKE01/esp_demos
Length of output: 3228
🏁 Script executed:
# Search for usb_new_phy function signature
rg "usb_new_phy" --type c --type h -A 3Repository: YanKE01/esp_demos
Length of output: 1754
🏁 Script executed:
# Search for xTaskCreate usage patterns
rg "xTaskCreate" --type c --type h -B 1 -A 1 | head -40Repository: YanKE01/esp_demos
Length of output: 2613
Handle failures from usb_new_phy and xTaskCreate.
At line 20, usb_new_phy() returns esp_err_t but the return value is ignored (other examples in this repo use ESP_ERROR_CHECK() for this function). At line 96, xTaskCreate() returns BaseType_t indicating success or failure (pdPASS/pdFAIL), but the return value is also ignored. Both initialization failures can occur silently.
🛠️ Proposed reliability fix
-static void usb_phy_init(void)
+static bool usb_phy_init(void)
{
usb_phy_handle_t phy_hdl;
usb_phy_config_t phy_conf = {
.controller = USB_PHY_CTRL_OTG,
.otg_mode = USB_OTG_MODE_DEVICE,
.target = USB_PHY_TARGET_INT
};
- usb_new_phy(&phy_conf, &phy_hdl);
+ if (usb_new_phy(&phy_conf, &phy_hdl) != ESP_OK) {
+ ESP_LOGE(TAG, "usb_new_phy failed");
+ return false;
+ }
+ return true;
}
@@
void app_main(void)
{
- usb_phy_init();
+ if (!usb_phy_init()) {
+ return;
+ }
if (!tusb_init()) {
ESP_LOGE(TAG, "TinyUSB init failed");
return;
}
- xTaskCreate(tusb_device_task, "tusb_device_task", 4096, NULL, 5, NULL);
+ if (xTaskCreate(tusb_device_task, "tusb_device_task", 4096, NULL, 5, NULL) != pdPASS) {
+ ESP_LOGE(TAG, "Failed to create tusb_device_task");
+ return;
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/usb/device/win_usb/main/win_usb.c` around lines 12 - 21,
usb_new_phy() and xTaskCreate() return status values that are currently ignored;
update usb_phy_init() to check the esp_err_t returned by usb_new_phy (use
ESP_ERROR_CHECK or handle non-ESP_OK by logging and returning/aborting) and
update the task-creation call (the xTaskCreate invocation) to check its
BaseType_t result against pdPASS, logging and handling failures (e.g., delete
resources, return error, or abort) so initialization errors don't fail silently;
reference usb_phy_init, usb_new_phy, and the xTaskCreate call when making these
changes.
YanKE01
force-pushed
the
feat/add_winusb
branch
from
7b09c8e to
992f887
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
examples/usb/device/web_usb/main/web_usb.c (1)
49-62: Consider flushing the TX buffer after write.The echo implementation reads data and writes it back, but
tud_vendor_n_write()may buffer the data. Consider callingtud_vendor_n_write_flush(itf)after the write to ensure data is sent promptly, especially for interactive use cases.Proposed improvement
ESP_LOGI(TAG, "WebUSB received %d bytes", n); tud_vendor_n_write(itf, buf, (size_t)n); + tud_vendor_n_write_flush(itf); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/usb/device/web_usb/main/web_usb.c` around lines 49 - 62, The echo handler tud_vendor_rx_cb currently writes echoed data with tud_vendor_n_write but doesn't flush the vendor TX buffer, which can delay transmission; after calling tud_vendor_n_write(itf, buf, (size_t)n) add a call to tud_vendor_n_write_flush(itf) to ensure the data is sent immediately (keep existing behavior around tud_vendor_n_available, tud_vendor_n_read and existing logging via ESP_LOGI(TAG, ...)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@examples/usb/device/web_usb/main/web_usb.c`:
- Around line 49-62: The echo handler tud_vendor_rx_cb currently writes echoed
data with tud_vendor_n_write but doesn't flush the vendor TX buffer, which can
delay transmission; after calling tud_vendor_n_write(itf, buf, (size_t)n) add a
call to tud_vendor_n_write_flush(itf) to ensure the data is sent immediately
(keep existing behavior around tud_vendor_n_available, tud_vendor_n_read and
existing logging via ESP_LOGI(TAG, ...)).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.mdexamples/usb/device/web_usb/CMakeLists.txtexamples/usb/device/web_usb/main/CMakeLists.txtexamples/usb/device/web_usb/main/idf_component.ymlexamples/usb/device/web_usb/main/tusb/tusb_config.hexamples/usb/device/web_usb/main/tusb/usb_descriptors.cexamples/usb/device/web_usb/main/tusb/usb_descriptors.hexamples/usb/device/web_usb/main/web_usb.cexamples/usb/device/web_usb/web_usb_test.html
🚧 Files skipped from review as they are similar to previous changes (3)
- examples/usb/device/web_usb/CMakeLists.txt
- examples/usb/device/web_usb/main/idf_component.yml
- examples/usb/device/web_usb/web_usb_test.html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores