migrate cdc device to edpt stream API by hathach · Pull Request #3339 · hathach/tinyusb

hathach · 2025-11-13T05:05:17Z

migrate device cdc to use edpt stream API
tu_edpt_stream_open() won't clear fifo when open to allow persistent data for cdc
update host midi and cdc to new stream open behavior

tu_edpt_stream_open() does not clear fifo, allow for persistent stream when disconnect/reconnect

chatgpt-codex-connector · 2025-11-13T05:09:57Z

💡 Codex Review

Lines 372 to 379 in 0871c7d

    
           } else { 
        
             tu_edpt_stream_t *stream_rx = &p_cdc->stream.rx; 
        
             tu_edpt_stream_open(stream_rx, desc_ep); 
        
             if (!_cdcd_cfg.rx_persistent) { 
        
               tu_edpt_stream_clear(stream_rx); 
        
             } 
        
             TU_ASSERT(tu_edpt_stream_read_xfer(rhport, stream_rx) > 0, 0); // prepare for incoming data

Avoid asserting when persistent CDC RX FIFO is full

The new cdcd_open always asserts that tu_edpt_stream_read_xfer() succeeds after opening the OUT endpoint. When RX persistence is enabled (_cdcd_cfg.rx_persistent == true) the FIFO contents are intentionally preserved across bus resets, so the buffer can legitimately be full when cdcd_open() runs. In that case tu_edpt_stream_read_xfer() returns 0 because no free space is available, and the TU_ASSERT aborts the enumeration path. The previous implementation simply skipped queuing another transfer until the application drained some data, allowing the device to re‑enumerate successfully. This assertion should be removed or converted to a non-fatal check so that a full persistent FIFO does not block reconfiguration.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copilot

Pull Request Overview

This PR migrates the CDC device class to use the unified endpoint stream API, consolidating FIFO management and endpoint transfer logic into reusable stream abstractions. The changes simplify CDC device/host implementations while maintaining backward compatibility.

Key Changes:

Refactored CDC device to use tu_edpt_stream_t instead of direct FIFO management, removing ~150 lines of code
Updated MIDI and vendor device classes to use stream clear operations for consistency
Modified .clang-format to align function arguments on the same line (AlignAfterOpenBracket: Align) and added penalties to discourage breaking parameters

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/tusb.c	Changed ZLP check from `!tu_fifo_count()` to `tu_fifo_empty()` for clarity; fixed spacing in deinit function
src/common/tusb_private.h	Added `tu_edpt_stream_empty()` helper function; removed fifo clear from stream open
src/class/cdc/cdc_device.c	Major refactoring to use stream API instead of direct FIFO operations; restructured interface type with nested stream struct; simplified read/write/flush implementations
src/class/cdc/cdc_host.c	Added TU_ASSERT for stream init; added void casts for deinit return values; added stream clear calls; improved code organization
src/class/midi/midi_host.c	Removed ep_in/ep_out fields; updated endpoint count logic to use stream state; added stream clear calls
src/class/midi/midi_device.c	Added stream clear calls after opening endpoints
src/class/vendor/vendor_device.c	Improved code clarity with local stream variables; updated comments
examples/device/net_lwip_webserver/src/usb_descriptors.c	Removed PVS-Studio comment suppressions
.clang-format	Updated formatting rules for better parameter alignment and reduced line breaks

src/class/cdc/cdc_device.c

Copilot · 2025-11-13T05:11:18Z

src/class/cdc/cdc_device.c

+    tu_edpt_stream_init(&p_cdc->stream.rx, false, false, false, p_cdc->stream.rx_ff_buf, CFG_TUD_CDC_RX_BUFSIZE,
+                        p_epbuf->epout, CFG_TUD_CDC_EP_BUFSIZE);

    // TX fifo can be configured to change to overwritable if not connected (DTR bit not set). Without DTR we do not
    // know if data is actually polled by terminal. This way the most current data is prioritized.
    // Default: is overwritable
-    tu_fifo_config(&p_cdc->tx_ff, p_cdc->tx_ff_buf, TU_ARRAY_SIZE(p_cdc->tx_ff_buf), 1, _cdcd_cfg.tx_overwritabe_if_not_connected);
-
-    #if OSAL_MUTEX_REQUIRED
-    osal_mutex_t mutex_rd = osal_mutex_create(&p_cdc->rx_ff_mutex);
-    osal_mutex_t mutex_wr = osal_mutex_create(&p_cdc->tx_ff_mutex);
-    TU_ASSERT(mutex_rd != NULL && mutex_wr != NULL, );
-
-    tu_fifo_config_mutex(&p_cdc->rx_ff, NULL, mutex_rd);
-    tu_fifo_config_mutex(&p_cdc->tx_ff, mutex_wr, NULL);
-    #endif
+    tu_edpt_stream_init(&p_cdc->stream.tx, false, true, _cdcd_cfg.tx_overwritabe_if_not_connected,
+                        p_cdc->stream.tx_ff_buf, CFG_TUD_CDC_TX_BUFSIZE, p_epbuf->epin, CFG_TUD_CDC_EP_BUFSIZE);


Missing error handling for tu_edpt_stream_init(). This function can fail (returns bool), but the return value is not checked. Consider adding error checking similar to how it's done in cdc_host.c with TU_ASSERT().

src/class/cdc/cdc_device.c

Copilot · 2025-11-13T05:11:18Z

src/class/cdc/cdc_device.c

+    tu_edpt_stream_deinit(&p_cdc->stream.rx);
+    tu_edpt_stream_deinit(&p_cdc->stream.tx);


Missing error handling for tu_edpt_stream_deinit(). The function returns bool but the return values are not checked. If deinitialization fails, the function should propagate the error instead of always returning true.

Copilot · 2025-11-13T05:11:19Z

src/common/tusb_private.h

+  return tu_fifo_empty(&s->ff);
+}
+
+


[nitpick] Extra blank line. Consider removing one of the empty lines to maintain consistent spacing in the file.

Suggested change

src/class/cdc/cdc_device.c

Copilot

Pull Request Overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.

Copilot · 2025-11-13T05:44:38Z

src/class/midi/midi_host.c


-    tu_edpt_stream_read_xfer(dev_addr, &p_midi->ep_stream.rx); // prepare for next transfer
-  } else if (ep_addr == p_midi->ep_stream.tx.ep_addr) {
+    tu_edpt_stream_read_xfer(dev_addr, ep_str_rx); // prepare for next transfer


The return value of tu_edpt_stream_read_xfer() is not explicitly cast to void. For consistency with cdc_host.c (line 521) and to clearly indicate the intentional ignoring of the return value, consider adding (void) cast.

Suggested change

tu_edpt_stream_read_xfer(dev_addr, ep_str_rx); // prepare for next transfer

(void)tu_edpt_stream_read_xfer(dev_addr, ep_str_rx); // prepare for next transfer

Copilot · 2025-11-13T05:44:38Z

src/class/midi/midi_host.c

      // If there is no data left, a ZLP should be sent if
      // xferred_bytes is multiple of EP size and not zero
-      tu_edpt_stream_write_zlp_if_needed(dev_addr, &p_midi->ep_stream.tx, xferred_bytes);
+      tu_edpt_stream_write_zlp_if_needed(dev_addr, ep_str_tx, xferred_bytes);


The return value of tu_edpt_stream_write_zlp_if_needed() is not explicitly cast to void. For consistency with cdc_host.c (line 698) and to clearly indicate the intentional ignoring of the return value, consider adding (void) cast.

Suggested change

tu_edpt_stream_write_zlp_if_needed(dev_addr, ep_str_tx, xferred_bytes);

(void)tu_edpt_stream_write_zlp_if_needed(dev_addr, ep_str_tx, xferred_bytes);

Copilot · 2025-11-13T05:44:38Z

src/class/cdc/cdc_device.c

+
+          tu_edpt_stream_open(stream_tx, desc_ep);
+          if (_cdcd_cfg.tx_persistent) {
+            tu_edpt_stream_write_xfer(rhport, stream_tx); // flush pending data


The return value of tu_edpt_stream_write_xfer() is not explicitly cast to void. For consistency with similar calls in vendor_device.c (line 242) and to clearly indicate the intentional ignoring of the return value, consider adding (void) cast.

Suggested change

tu_edpt_stream_write_xfer(rhport, stream_tx); // flush pending data

(void)tu_edpt_stream_write_xfer(rhport, stream_tx); // flush pending data

Copilot · 2025-11-13T05:44:39Z

src/class/cdc/cdc_device.c

+    tu_edpt_stream_init(&p_cdc->stream.rx, false, false, false, p_cdc->stream.rx_ff_buf, CFG_TUD_CDC_RX_BUFSIZE,
+                        p_epbuf->epout, CFG_TUD_CDC_EP_BUFSIZE);


The return value of tu_edpt_stream_init() is not checked. If initialization fails, the interface will be in an inconsistent state. Consider adding TU_ASSERT() to validate the initialization succeeded, similar to how cdch_init() in cdc_host.c checks these return values.

Copilot · 2025-11-13T05:44:39Z

src/class/cdc/cdc_device.c


-    // prepare for OUT transaction
-    _prep_out_transaction(itf);
+    tu_edpt_stream_read_xfer(rhport, stream_rx); // prepare for more data


The return value of tu_edpt_stream_read_xfer() is not explicitly cast to void. For consistency with cdch_xfer_cb() in cdc_host.c (line 718) and to clearly indicate the intentional ignoring of the return value, consider adding (void) cast.

Suggested change

tu_edpt_stream_read_xfer(rhport, stream_rx); // prepare for more data

(void)tu_edpt_stream_read_xfer(rhport, stream_rx); // prepare for more data

Copilot · 2025-11-13T05:44:39Z

src/class/cdc/cdc_host.c

+    (void)tu_edpt_stream_deinit(&p_cdc->stream.tx);
+    (void)tu_edpt_stream_deinit(&p_cdc->stream.rx);


While the return values of tu_edpt_stream_deinit() are explicitly cast to void, consider checking the return values and returning false if any deinit operation fails, to properly propagate errors to callers.

Suggested change

(void)tu_edpt_stream_deinit(&p_cdc->stream.tx);

(void)tu_edpt_stream_deinit(&p_cdc->stream.rx);

if (!tu_edpt_stream_deinit(&p_cdc->stream.tx)) {

return false;

}

if (!tu_edpt_stream_deinit(&p_cdc->stream.rx)) {

return false;

}

Copilot · 2025-11-13T05:44:39Z

src/class/cdc/cdc_device.c

-      }
+    if (0 == tu_edpt_stream_write_xfer(rhport, stream_tx)) {
+      // If there is no data left, a ZLP should be sent if needed
+      tu_edpt_stream_write_zlp_if_needed(rhport, stream_tx, xferred_bytes);


The return value of tu_edpt_stream_write_zlp_if_needed() is not explicitly cast to void. For consistency with cdch_xfer_cb() in cdc_host.c (line 698) and to clearly indicate the intentional ignoring of the return value, consider adding (void) cast.

Suggested change

tu_edpt_stream_write_zlp_if_needed(rhport, stream_tx, xferred_bytes);

(void)tu_edpt_stream_write_zlp_if_needed(rhport, stream_tx, xferred_bytes);

Copilot · 2025-11-13T05:44:40Z

src/class/cdc/cdc_device.c

+  TU_VERIFY(itf < CFG_TUD_CDC, );
+  cdcd_interface_t *p_cdc = &_cdcd_itf[itf];
+  tu_edpt_stream_clear(&p_cdc->stream.rx);
+  tu_edpt_stream_read_xfer(p_cdc->rhport, &p_cdc->stream.rx);


The return value of tu_edpt_stream_read_xfer() is not explicitly cast to void. For consistency with tuh_cdc_read_clear() in cdc_host.c (line 521) and to clearly indicate the intentional ignoring of the return value, consider adding (void) cast.

Suggested change

tu_edpt_stream_read_xfer(p_cdc->rhport, &p_cdc->stream.rx);

(void)tu_edpt_stream_read_xfer(p_cdc->rhport, &p_cdc->stream.rx);

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…tream

sonarqubecloud · 2025-11-14T04:58:35Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

migrate cdc device to use edpt stream API

f11adb0

tu_edpt_stream_open() does not clear fifo, allow for persistent stream when disconnect/reconnect

Copilot AI review requested due to automatic review settings November 13, 2025 05:05

github-actions bot added the Prio Urgent label Nov 13, 2025

Copilot started reviewing on behalf of hathach November 13, 2025 05:06 View session

hathach force-pushed the cdc-edpt-stream branch from 0871c7d to 3be91bd Compare November 13, 2025 05:08

Copilot finished reviewing on behalf of hathach November 13, 2025 05:09

Copilot AI reviewed Nov 13, 2025

View reviewed changes

hathach force-pushed the cdc-edpt-stream branch from 3be91bd to 81f70bf Compare November 13, 2025 05:15

clear endpoint stream when open for cdc_host and midi_host

397a3af

hathach force-pushed the cdc-edpt-stream branch from 81f70bf to 397a3af Compare November 13, 2025 05:31

hathach requested a review from Copilot November 13, 2025 05:38

Copilot started reviewing on behalf of hathach November 13, 2025 05:38 View session

Copilot finished reviewing on behalf of hathach November 13, 2025 05:42

Copilot AI reviewed Nov 13, 2025

View reviewed changes

hathach and others added 3 commits November 13, 2025 12:47

Apply suggestions from code review

2b07fa6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

cdc device tx_persistent for dual host info to help with hil test

aac7ad1

Merge remote-tracking branch 'origin/cdc-edpt-stream' into cdc-edpt-s…

1f8968f

…tream

hathach force-pushed the cdc-edpt-stream branch 2 times, most recently from 53485b4 to 4e47871 Compare November 14, 2025 03:43

tweak to dual host info device cdc to make it easier to pass hil test

c9a9e94

hathach force-pushed the cdc-edpt-stream branch from 4e47871 to c9a9e94 Compare November 14, 2025 04:56

hathach merged commit a3a5a41 into master Nov 14, 2025
192 of 193 checks passed

hathach deleted the cdc-edpt-stream branch November 14, 2025 07:23

		tu_edpt_stream_deinit(&p_cdc->stream.rx);
		tu_edpt_stream_deinit(&p_cdc->stream.tx);

	tu_edpt_stream_read_xfer(dev_addr, ep_str_rx); // prepare for next transfer
	(void)tu_edpt_stream_read_xfer(dev_addr, ep_str_rx); // prepare for next transfer

	tu_edpt_stream_write_zlp_if_needed(dev_addr, ep_str_tx, xferred_bytes);
	(void)tu_edpt_stream_write_zlp_if_needed(dev_addr, ep_str_tx, xferred_bytes);

	tu_edpt_stream_write_xfer(rhport, stream_tx); // flush pending data
	(void)tu_edpt_stream_write_xfer(rhport, stream_tx); // flush pending data

		tu_edpt_stream_init(&p_cdc->stream.rx, false, false, false, p_cdc->stream.rx_ff_buf, CFG_TUD_CDC_RX_BUFSIZE,
		p_epbuf->epout, CFG_TUD_CDC_EP_BUFSIZE);

	tu_edpt_stream_read_xfer(rhport, stream_rx); // prepare for more data
	(void)tu_edpt_stream_read_xfer(rhport, stream_rx); // prepare for more data

		(void)tu_edpt_stream_deinit(&p_cdc->stream.tx);
		(void)tu_edpt_stream_deinit(&p_cdc->stream.rx);

-    (void)tu_edpt_stream_deinit(&p_cdc->stream.tx);
-    (void)tu_edpt_stream_deinit(&p_cdc->stream.rx);
+    if (!tu_edpt_stream_deinit(&p_cdc->stream.tx)) {
+      return false;
+    }
+    if (!tu_edpt_stream_deinit(&p_cdc->stream.rx)) {
+      return false;
+    }

	tu_edpt_stream_write_zlp_if_needed(rhport, stream_tx, xferred_bytes);
	(void)tu_edpt_stream_write_zlp_if_needed(rhport, stream_tx, xferred_bytes);

	tu_edpt_stream_read_xfer(p_cdc->rhport, &p_cdc->stream.rx);
	(void)tu_edpt_stream_read_xfer(p_cdc->rhport, &p_cdc->stream.rx);

Conversation

hathach commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector bot commented Nov 13, 2025

💡 Codex Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 14, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hathach commented Nov 13, 2025 •

edited

Loading