-
-
Notifications
You must be signed in to change notification settings - Fork 180
Add UART versions of main S3 builds #3173
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
Conversation
WalkthroughTwo new ESP32_S3 board variants, "ESP32_S3_BLE_UART" and "ESP32_S3_ALL_UART," were introduced. Updates include documentation in both English and Chinese READMEs, build matrix additions in the Azure Pipelines configuration, and new CMake configure/build presets for these targets. No other features or code changes were made. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CI Pipeline
participant CMake
participant Documentation
Developer->>Documentation: Add ESP32_S3_BLE_UART/ALL_UART entries
Developer->>CMake: Add new presets for BLE_UART/ALL_UART
Developer->>CI Pipeline: Update build matrix with new targets
CI Pipeline->>CMake: Build using new presets
CI Pipeline->>Documentation: Publish new version badges
Possibly related PRs
Suggested labels
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (2)
75-75
: Fix table cell formattingAdd a space before the pipe to ensure proper Markdown column separation.
- | ESP32_S3_BLE_UART | Display, BLE, Quad spiram support, connection via UART| [![Latest Version @ Cloudsmith] + | ESP32_S3_BLE_UART | Display, BLE, Quad spiram support, connection via UART | [![Latest Version @ Cloudsmith]
77-77
: Normalize table indentationUse a single space before “Display” for consistency with other rows.
- | ESP32_S3_ALL_UART | Display, BLE, Octal spiram support, connection via UART | [![Latest Version @ Cloudsmith] + | ESP32_S3_ALL_UART | Display, BLE, Octal spiram support, connection via UART | [![Latest Version @ Cloudsmith]targets/ESP32/CMakePresets.json (1)
377-402
: Configure presets correctly addedThe
ESP32_S3_BLE_UART
configure preset inherits the common S3 settings and omits the USB CDC flag as intended. For clarity and to prevent default behavior, consider explicitly disabling USB CDC."cacheVariables": { "TARGET_NAME": "${presetName}", "SDK_CONFIG_FILE": "sdkconfig.default_ble.esp32s3", "NF_BUILD_RTM": "OFF", "NF_FEATURE_DEBUGGER": "ON", "NF_FEATURE_RTC": "ON", "NF_FEATURE_HAS_SDCARD": "ON", + "ESP32_USB_CDC": "OFF", "API_nanoFramework.Device.OneWire": "ON", "API_nanoFramework.Device.Bluetooth": "ON", // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md
(1 hunks)README.zh-cn.md
(1 hunks)azure-pipelines-nightly.yml
(1 hunks)targets/ESP32/CMakePresets.json
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_ETHERNET_KIT_1.2)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_S3_ALL)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_H2_THREAD)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C6_THREAD)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C3)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_BLE_REV0)
- GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_PSRAM_REV0)
- GitHub Check: nf-interpreter (Check_Code_Style)
- GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
- GitHub Check: nf-interpreter (Check_Build_Options)
🔇 Additional comments (3)
README.zh-cn.md (1)
52-53
: Entries correctly added for UART variantsThe two new targets
ESP32_S3_BLE_UART
andESP32_S3_ALL_UART
have been appended to the Chinese README table, matching the English version and CI configuration.azure-pipelines-nightly.yml (1)
535-543
: New UART targets integrated into ESP32 build matrixThe
ESP32_S3_BLE_UART
andESP32_S3_ALL_UART
entries are correctly added to the nightly pipeline matrix, mirroring the existing BLE and ALL presets. Indentation, key names, and field ordering align with the surrounding ESP32 entries.Also applies to: 544-552
targets/ESP32/CMakePresets.json (1)
1005-1010
: Build presets addedThe
ESP32_S3_BLE_UART
andESP32_S3_ALL_UART
build presets correctly inherit frombase-user
and reference the new configure presets. The naming and ordering are consistent with the existing build presets.Also applies to: 1017-1022
Description
There are a number of new S3 development boards that only have a connection via a USB to serial chip. The current firmware is configured to work via the native USB connection which will not work for these boards.
This PR adds the following new targets:
ESP32_S3_BLE_UART
ESP32_S3_ALL_UART
Readme.md and Readme.zh_cn.md updated with new targets.
azure nightly updated to include new builds.
Motivation and Context
Discussed on discord
How Has This Been Tested?
Build tested locally but unable to test run as no S3 test board available with UART connection
Will be tested by users
Types of changes
Checklist
Summary by CodeRabbit