feat(HardwareSerial): Add function to invert Hardware UART's Tx line #11421
Conversation
…alongside existing Rx functionality Simply clone existing Rx functionality for Tx. The goal here is to allow granular control over both lines but avoid overloading HardwareSerial::begin() to change the boolean 'invert' parameter to a bitmask type.
|
|
👋 Hello asund, 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 ...
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality for inverting the HardwareSerial TX line, mirroring the existing RX inversion capability without altering the API.
- Added test cases in uart.ino to verify TX inversion during both running and stopped UART states.
- Declared and implemented uartSetTxInvert in the ESP32 HAL files with platform-specific behavior.
- Extended the HardwareSerial API to expose the new setTxInvert method.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/validation/uart/uart.ino | Added test calls for TX inversion verification under two scenarios. |
| cores/esp32/esp32-hal-uart.h | Declared the new uartSetTxInvert function. |
| cores/esp32/esp32-hal-uart.c | Implemented uartSetTxInvert with platform-dependent handling. |
| cores/esp32/HardwareSerial.h | Updated the HardwareSerial class interface with setTxInvert. |
| cores/esp32/HardwareSerial.cpp | Implemented setTxInvert by delegating to uartSetTxInvert. |
| // ESP_ERROR_CHECK(uart_set_line_inverse(uart->num, UART_SIGNAL_TXD_INV)); | ||
| // else | ||
| // ESP_ERROR_CHECK(uart_set_line_inverse(uart->num, UART_SIGNAL_INV_DISABLE)); | ||
| log_e("uartSetTxInvert is not supported in ESP32C6, ESP32H2 and ESP32P4"); |
There was a problem hiding this comment.
Consider updating the function's documentation and header comments to clearly state that TX inversion is not supported on ESP32C6, ESP32H2, and ESP32P4, so that users are aware of this platform limitation.
Copilot uses AI. Check for mistakes.
| return; | ||
| } | ||
| #if CONFIG_IDF_TARGET_ESP32C6 || CONFIG_IDF_TARGET_ESP32H2 || CONFIG_IDF_TARGET_ESP32P4 | ||
| // POTENTIAL ISSUE :: original code only set/reset txd_inv bit |
There was a problem hiding this comment.
[nitpick] Consider removing or clarifying the commented-out code block if it is no longer needed, to maintain clarity and reduce potential confusion for future maintainers.
Copilot uses AI. Check for mistakes.
Test Results 76 files 76 suites 12m 45s ⏱️ Results for commit f3ac90d. |
Description of Change
Simply clone existing HardwareSerial Rx pin inverting functionality for the Tx pin. The goal here is to allow granular control over both lines but avoid overloading HardwareSerial::begin() to change the boolean 'invert' parameter to a bitmask type (though this would be more featureful, I don't want to break the API).
Tests scenarios
I have tested my Pull Request on Arduino-esp32 core v3.2.0 and on ESP32-S2 hardware. I also updated the HardwareSerial test sketch although that functionality just verifies there's no crash, copying existing test for Rx line.
Related links
N/A