remove tud cdc configure#3549
Conversation
…mpile-time macros for simplicity and reduced complexity
…rowse error handling update AGENTS.md with metrics compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd60008e89
ℹ️ 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".
| tusb_init(BOARD_TUD_RHPORT, &dev_init); | ||
| tud_cdc_configure_t cdc_cfg = CFG_TUD_CDC_CONFIGURE_DEFAULT(); | ||
| cdc_cfg.tx_persistent = true; | ||
| cdc_cfg.tx_overwritabe_if_not_connected = false; | ||
| tud_cdc_configure(&cdc_cfg); | ||
| board_init_after_tusb(); |
There was a problem hiding this comment.
Restore CDC TX persistence settings in host_info example
Removing the tud_cdc_configure() call here changes this example’s runtime behavior from persistent/non-overwritable TX to the new defaults (CFG_TUD_CDC_TX_PERSISTENT=0, CFG_TUD_CDC_TX_OVERWRITABLE_IF_NOT_CONNECTED=1), so CDC output produced before the terminal is fully connected (e.g. tuh_mount_cb()/tuh_umount_cb() writes) can be dropped or cleared on reconnect/reset. This regresses the example’s ability to reliably show early host events unless equivalent compile-time macros are set in its tusb_config.h.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR removes the runtime CDC device driver configuration API (tud_cdc_configure()) and replaces it with compile-time configuration macros to reduce firmware footprint. It also updates contributor guidance for collecting code size metrics and bumps several GitHub Actions workflow dependencies.
Changes:
- Remove
tud_cdc_configure()/tud_cdc_configure_tand replace the behavior with newCFG_TUD_CDC_*compile-time macros. - Update CDC device implementation to use the new compile-time settings for persistence and TX FIFO overwritability behavior.
- Update developer documentation (AGENTS.md) and refresh GitHub Actions workflow versions/behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/class/cdc/cdc_device.h | Removes runtime configuration API and introduces new compile-time CDC configuration macros. |
| src/class/cdc/cdc_device.c | Switches CDC driver behavior logic from runtime config struct to compile-time macros. |
| examples/dual/host_info_to_device_cdc/src/main.c | Removes example’s runtime call to tud_cdc_configure() (now deleted). |
| AGENTS.md | Adds instructions for generating and comparing code size metrics. |
| .github/workflows/metrics_comment.yml | Updates artifact download action version. |
| .github/workflows/membrowse-onboard.yml | Updates checkout and ccache action versions. |
| .github/workflows/claude.yml | Updates checkout action version. |
| .github/workflows/claude-code-review.yml | Updates checkout action version. |
| .github/workflows/cifuzz.yml | Updates artifact upload action version. |
| .github/workflows/build_util.yml | Makes “Membrowse Upload” non-fatal (continue-on-error: true). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Size Difference ReportBecause TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds. Note: If there is no change, only one value is shown. Changes >1% in size
Changes <1% in sizeNo entries. No changes
|
|
| target | .text | .rodata | .data | .bss | total | % diff |
|---|---|---|---|---|---|---|
| lpcxpresso1769/cdc_dual_ports | 10,028 → 9,944 (-84) | — | — | — | 10,044 → 9,960 (-84) | -0.8% |
| lpcxpresso1347/cdc_dual_ports | 9,724 → 9,644 (-80) | — | — | — | 9,740 → 9,660 (-80) | -0.8% |
| ea4088_quickstart/cdc_dual_ports | 10,000 → 9,924 (-76) | — | — | — | 10,016 → 9,940 (-76) | -0.8% |
| metro_m4_express/cdc_dual_ports | 10,072 → 9,996 (-76) | — | — | — | 10,080 → 10,004 (-76) | -0.8% |
| samg55_xplained/cdc_dual_ports | 10,156 → 10,080 (-76) | — | — | — | 10,164 → 10,088 (-76) | -0.7% |
| mm32f327x_mb39/cdc_dual_ports | 10,264 → 10,180 (-84) | — | — | — | 11,240 → 11,156 (-84) | -0.7% |
| lpcxpresso1549/cdc_dual_ports | 11,064 → 10,984 (-80) | — | — | — | 11,080 → 11,000 (-80) | -0.7% |
| msp_exp432e401y/cdc_dual_ports | 12,116 → 12,032 (-84) | — | — | — | 12,136 → 12,052 (-84) | -0.7% |
| cynthion_d11/cdc_dual_ports | 9,464 → 9,400 (-64) | — | — | — | 9,464 → 9,400 (-64) | -0.7% |
| metro_m0_express/cdc_dual_ports | 9,516 → 9,452 (-64) | — | — | — | 9,516 → 9,452 (-64) | -0.7% |
…ard compatibility with no-op macros
remove tud_cdc_configure(), move tem to compile-time macro to save footprint