Skip to content

fix(board): skip USB_VID/USB_PID injection on ESP32-S2/S3 (drops 149-156 redef warnings/build) - #413

Merged
zackees merged 3 commits into
mainfrom
fix/esp32-flag-hygiene-403-405-redo
Jun 4, 2026
Merged

fix(board): skip USB_VID/USB_PID injection on ESP32-S2/S3 (drops 149-156 redef warnings/build)#413
zackees merged 3 commits into
mainfrom
fix/esp32-flag-hygiene-403-405-redo

Conversation

@zackees

Copy link
Copy Markdown
Member

Closes#405.

Summary

On ESP32-S2/S3 the Arduino framework's variants/<board>/pins_arduino.h already #defines both USB_VID and USB_PID. Injecting the same value via -D from the board JSON's vid/pid fields produces a "USB_VID" redefined warning at every TU that includes pins_arduino.h149-156 warnings per build observed in CI (esp32s2, esp32s3, rgbw).

BoardConfig::get_defines now skips the unconditional injection when mcu == "esp32s2" || mcu == "esp32s3". User overrides via build.extra_flags = "-DUSB_VID=..." still take effect through the existing extra_flags processing loop.

Other USB-native boards (Leonardo, Micro, Adafruit SAM, etc.) continue to receive the injected defines.

Tests

Two new regression tests in crates/fbuild-config/src/board/tests.rs:

  • test_esp32s3_board_skips_usb_vid_pid_injection — confirms board-JSON vid/pid are NOT emitted as defines for ESP32-S3
  • test_esp32s3_board_extra_flag_usb_vid_override_wins — confirms user -DUSB_VID=... override still wins

71/71 board::tests pass locally.

On #403

This PR does NOT address #403 (scoping -Wno-error=array-bounds, -Wno-error=maybe-uninitialized, -Wno-error=uninitialized, -Wno-error=deprecated-declarations). Investigation shows only -Wno-error=deprecated-declarations actually lives in fbuild's ESP32 config (crates/fbuild-build/src/esp32/configs/*.json); the other three (array-bounds, maybe-uninitialized, uninitialized) do not appear anywhere in fbuild's source — they come from the upstream pioarduino framework platform.json. I will close#403 separately with that explanation.

Tracker

Filed from fbuild meta-audit: #408

On ESP32-S2/S3 the Arduino framework's
variants/<board>/pins_arduino.h already #defines both USB_VID and
USB_PID. Injecting the same value again via -D from the board JSON
vid/pid fields produces a "USB_VID redefined" warning at every TU
that includes pins_arduino.h — 149-156 warnings per build observed
in CI (esp32s2 + esp32s3 + rgbw).
BoardConfig::get_defines now skips the unconditional USB_VID/USB_PID
injection when the MCU is esp32s2 or esp32s3. User overrides via
build.extra_flags (-DUSB_VID=...) still win through the extra_flags
processing loop.
Adds two regression tests:
- test_esp32s3_board_skips_usb_vid_pid_injection asserts the JSON
vid/pid fields are NOT emitted as defines for an ESP32-S3 board
- test_esp32s3_board_extra_flag_usb_vid_override_wins asserts that
user overrides via extra_flags still take effect
Other USB-native boards (Leonardo, Micro, etc.) and Adafruit SAM
boards continue to receive the injected defines.
Closes#405.
@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@zackees, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 7 minutes and 57 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6961993e-2e16-48ec-a8a1-2810aea9ee08

📥 Commits

Reviewing files that changed from the base of the PR and between 5a3a7b9 and 00d9cf4.

📒 Files selected for processing (4)
  • crates/fbuild-config/src/board/methods.rs
  • crates/fbuild-config/src/board/mod.rs
  • crates/fbuild-config/src/board/tests.rs
  • crates/fbuild-config/src/board/tests_usb_vid.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/esp32-flag-hygiene-403-405-redo

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

zackees added 2 commits June 3, 2026 19:40
PR #413's CI hit the 1000-LOC gate on board/tests.rs (1005 lines).
Move the entire "USB VID/PID defines" test block (5 tests) into a
sibling sub-module file so the main tests.rs drops to 930 LOC.
No behavior change — same 5 tests, same assertions, just split into a
dedicated file. 72/72 board tests pass.
@zackees
zackees merged commit 013df1e into mainJun 4, 2026
84 checks passed
@zackees
zackees deleted the fix/esp32-flag-hygiene-403-405-redo branch June 4, 2026 03:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Triage

1 participant

@zackees