fix(uart): fixes setPins() when changing RTS and CTS#12201
Merged
fix(uart): fixes setPins() when changing RTS and CTS#12201
Conversation
SuGlider
added
Type: Bug 🐛
Contributor
👋 Hello SuGlider, we appreciate your contribution to this project! 📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more. 🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project. Click to see more instructions ...
Review and merge process you can expect ...
|
Contributor
Test Results 90 files 90 suites 28m 25s ⏱️ Results for commit bef7ed0. ♻️ This comment has been updated with latest results. |
1 task
Contributor
Memory usage test (comparing PR against master branch)The table below shows the summary of memory usage change (decrease - increase) in bytes and percentage for each target.
Click to expand the detailed deltas report [usage change in BYTES]
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
P-R-O-C-H-Y
approved these changes
Jan 5, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue with HardwareSerial::setPins() when changing RTS and CTS pins before calling begin(). The fix leverages improvements in ESP-IDF 5.1+ that have been backported from 5.5.2 to 5.1.0, which now properly support changing individual UART pins. The PR also adds a comprehensive hardware flow control example and documentation for testing functions.
Key changes:
- Replaces internal
_uartInternalSetPin()implementation with IDF'suart_set_pin()to properly handle RTS/CTS pin configuration - Adds new
uart_internal_hw_flow_ctrl_loopback()helper function for testing hardware flow control without external wires - Provides a complete HardwareFlowControl_Demo example demonstrating both hardware-controlled and GPIO-controlled flow control modes
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cores/esp32/esp32-hal-uart.c | Removes custom _uartInternalSetPin() implementation and replaces calls with IDF's uart_set_pin(); adds uart_internal_hw_flow_ctrl_loopback() function for testing; fixes spelling errors in comments |
| cores/esp32/esp32-hal-uart.h | Adds declaration for uart_internal_hw_flow_ctrl_loopback() function; fixes spelling error in comment |
| libraries/ESP32/examples/Serial/HardwareFlowControl_Demo/HardwareFlowControl_Demo.ino | New comprehensive example demonstrating UART hardware flow control with RTS/CTS signals, supporting both internal GPIO matrix loopback and external wire connections |
| libraries/ESP32/examples/Serial/HardwareFlowControl_Demo/README.md | New detailed documentation explaining hardware flow control concepts, pin connections, configuration options, and expected behavior for both hardware-controlled and GPIO-controlled modes |
| docs/en/api/serial.rst | Adds documentation for uart_internal_loopback() and uart_internal_hw_flow_ctrl_loopback() testing functions with usage examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libraries/ESP32/examples/Serial/HardwareFlowControl_Demo/README.md
Outdated
Show resolved
Hide resolved
libraries/ESP32/examples/Serial/HardwareFlowControl_Demo/README.md
Outdated
Show resolved
Hide resolved
libraries/ESP32/examples/Serial/HardwareFlowControl_Demo/README.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Updated README.md to include additional pin connection details and modified function descriptions for hardware flow control.
Member
|
Can we add a test case in our UART testing for this ? |
Collaborator
Author
Sure! Do you want to do that or you expect me to add it? |
Member
|
@SuGlider If you can add it in this PR would be good so we can check that everything passes in the test. |
pedrominatel
approved these changes
Jan 6, 2026
…m/espressif/arduino-esp32 into fix/uart_set_pins_rts_cts_failure
Collaborator
Author
CI HW flow control test case added. |
Collaborator
Author
|
@lucasssvaz | @me-no-dev : PR is completed. Please review it. |
Member
|
@SuGlider Tests are failing. PTAL |
Collaborator
Author
Thre is nothing that I do for it. It works fine with real SoC. ESP32 Log:ESP32-S3 Log: |
Member
|
Wokwi is fixed and CI is passing again. |
lucasssvaz
approved these changes
Jan 7, 2026
lucasssvaz
added
Status: Pending Merge
github-actions
bot
added
the
Status: Pre-commit fixes required ⚠️
Contributor
|
Collaborator
Author
|
@P-R-O-C-H-Y - It is failing Baudrate Detection with the ESP32-S2. Wokwi CI Failure (baudrate test): This is the Real SoC output: |
github-actions
bot
removed
the
Status: Pre-commit fixes required ⚠️
Member
|
Please wait for #12215 so Wokwi CI can be re-triggered for all Socs. |
P-R-O-C-H-Y
added
the
Re-trigger Wokwi Tests
github-actions
bot
removed
the
Re-trigger Wokwi Tests
Member
|
PR is good to go! |
github-project-automation
bot
moved this from In Review
to Done
in Arduino ESP32 Core Project Roadmap
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
5 participants
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.
Description of Change
IDF 5.1+ has finally fixed the issue related to changing a single UART pin (rx, tx, cts, rts).
It has been backported from 5.5.2 to 5.1.0.
An issue was found that prevents RTS and CTS pins to be correctly configures when
HardwareSerial::setPins()is executed beforeHardwareSerial::begin(). This issue is fixed by the change from IDF in conjuction with the changes of this PR.As part of the testing and also for demonstation purpose, a new RX-TX + RTS-CTS loopback using Hardware Flow Control has been provided as part of this PR.
Test Scenarios
tested using an ESP32-S3 and the provided examples in different configurations.
Related links
Closes #12199