fix(uart): terminates uart driver whenever both rx and tx pins are detached (#12080)

SuGlider · Copilot · pre-commit-ci-lite[bot] · web-flow · commit 2dd42c2350d1 · 2025-12-04T11:23:10.000+02:00
* fix(uart): terminates uart driver whenever rx and tx are detached

* fix(uart): detaching pin check points

* fix(uart): extended lod messages

* fix(uart): fixes setPins to keep driver working

* fix(uart): peripheral manager CI test adjusting

Updated UART test configurations to handle I2C interactions correctly by detaching TX pin and restarting the UART driver when both RX and TX are detached.

* fix(uart): Refactor UART test to detach only one pin

Updated UART test configuration to detach only one pin instead of both, ensuring the UART driver continues to function. Removed redundant code related to UART driver restart when both pins are detached.

* fix(uart_test): do not detach both UART pins

Updated UART test configuration to detach TX pin while keeping RX pin active. Adjusted assertions to reflect the new pin settings.

* fix(uart_test): formatting

* fix(uart_doc): Document RX and TX pin detachment behavior

Added note about driver behavior when RX and TX pins are detached.

* fix(uart): code formatting

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;

* fix(uart): code formatting

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;

* fix(uart): code formatting

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;

* fix(uart): code formatting

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;

* feat(uart): C wrapper to end related Serial object

* fix(uart): fixes bad case value

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;

* fix(uart): C wrapper to end related Serial object

* fix(docs): adding comma

* ci(pre-commit): Apply automatic fixes

* fix(uart_ci): typo in UART driver comment

---------

Co-authored-by: Copilot &lt;175728472+Copilot@users.noreply.github.com&gt;
Co-authored-by: pre-commit-ci-lite[bot] &lt;117423508+pre-commit-ci-lite[bot]@users.noreply.github.com&gt;
diff --git a/cores/esp32/HardwareSerial.cpp b/cores/esp32/HardwareSerial.cpp
@@ -61,6 +61,31 @@ HardwareSerial Serial5(5);
 extern void HWCDCSerialEvent(void) __attribute__((weak));
 #endif
 
+// C-callable helper used by HAL when pins are detached and the high-level
+// HardwareSerial instance must be finalized.
+extern "C" void hal_uart_notify_pins_detached(int uart_num) {
+  log_d("hal_uart_notify_pins_detached: Notifying HardwareSerial for UART%d", uart_num);
+  switch (uart_num) {
+    case 0: Serial0.end(); break;
+#if SOC_UART_NUM > 1
+    case 1: Serial1.end(); break;
+#endif
+#if SOC_UART_NUM > 2
+    case 2: Serial2.end(); break;
+#endif
+#if SOC_UART_NUM > 3
+    case 3: Serial3.end(); break;
+#endif
+#if SOC_UART_NUM > 4
+    case 4: Serial4.end(); break;
+#endif
+#if SOC_UART_NUM > 5
+    case 5: Serial5.end(); break;
+#endif
+    default: log_e("hal_uart_notify_pins_detached: UART%d not handled!", uart_num); break;
+  }
+}
+
 #if USB_SERIAL_IS_DEFINED == 1  // Native USB CDC Event
 // Used by Hardware Serial for USB CDC events
 extern void USBSerialEvent(void) __attribute__((weak));
@@ -483,9 +508,6 @@ void HardwareSerial::end() {
   // including any tasks or debug message channel (log_x()) - but not for IDF log messages!
   _onReceiveCB = NULL;
   _onReceiveErrorCB = NULL;
-  if (uartGetDebug() == _uart_nr) {
-    uartSetDebug(0);
-  }
   _rxFIFOFull = 0;
   uartEnd(_uart_nr);    // fully detach all pins and delete the UART driver
   _destroyEventTask();  // when IDF uart driver is deleted, _eventTask must finish too
diff --git a/cores/esp32/chip-debug-report.cpp b/cores/esp32/chip-debug-report.cpp
@@ -291,7 +291,7 @@ static void printPerimanInfo(void) {
 
 void printBeforeSetupInfo(void) {
 #if ARDUINO_USB_CDC_ON_BOOT
-  Serial.begin(0);
+  Serial.begin();
   Serial.setDebugOutput(true);
   uint8_t t = 0;
   while (!Serial && (t++ < 200)) {
diff --git a/cores/esp32/esp32-hal-uart.c b/cores/esp32/esp32-hal-uart.c
@@ -42,6 +42,9 @@
 static int s_uart_debug_nr = 0;         // UART number for debug output
 #define REF_TICK_BAUDRATE_LIMIT 250000  // this is maximum UART badrate using REF_TICK as clock
 
+/* C prototype for the notifier implemented in HardwareSerial.cpp */
+extern void hal_uart_notify_pins_detached(int uart_num);
+
 struct uart_struct_t {
 
 #if !CONFIG_DISABLE_HAL_LOCKS
@@ -282,29 +285,67 @@ static bool _uartDetachPins(uint8_t uart_num, int8_t rxPin, int8_t txPin, int8_t
 // Peripheral Manager detach callback for each specific UART PIN
 static bool _uartDetachBus_RX(void *busptr) {
   // sanity check - it should never happen
-  assert(busptr && "_uartDetachBus_RX bus NULL pointer.");
+  if (busptr == NULL) {
+    log_e("_uartDetachBus_RX: busptr is NULL");
+    return false;
+  }
   uart_t *bus = (uart_t *)busptr;
+  if (bus->_rxPin < 0) {
+    log_d("_uartDetachBus_RX: RX pin already detached for UART%d", bus->num);
+    return true;
+  }
+  if (bus->_txPin < 0) {  // both rx and tx pins are detached, terminate the uart driver
+    log_d("_uartDetachBus_RX: both RX and TX pins detached for UART%d, terminating driver", bus->num);
+    hal_uart_notify_pins_detached(bus->num);
+    return true;
+  }
   return _uartDetachPins(bus->num, bus->_rxPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
 }
 
 static bool _uartDetachBus_TX(void *busptr) {
   // sanity check - it should never happen
-  assert(busptr && "_uartDetachBus_TX bus NULL pointer.");
+  if (busptr == NULL) {
+    log_e("_uartDetachBus_TX: busptr is NULL");
+    return false;
+  }
   uart_t *bus = (uart_t *)busptr;
+  if (bus->_txPin < 0) {
+    log_d("_uartDetachBus_TX: TX pin already detached for UART%d", bus->num);
+    return true;
+  }
+  if (bus->_rxPin < 0) {  // both rx and tx pins are detached, terminate the uart driver
+    log_d("_uartDetachBus_TX: both RX and TX pins detached for UART%d, terminating driver", bus->num);
+    hal_uart_notify_pins_detached(bus->num);
+    return true;
+  }
   return _uartDetachPins(bus->num, UART_PIN_NO_CHANGE, bus->_txPin, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE);
 }
 
 static bool _uartDetachBus_CTS(void *busptr) {
   // sanity check - it should never happen
-  assert(busptr && "_uartDetachBus_CTS bus NULL pointer.");
+  if (busptr == NULL) {
+    log_e("_uartDetachBus_CTS: busptr is NULL");
+    return false;
+  }
   uart_t *bus = (uart_t *)busptr;
+  if (bus->_ctsPin < 0) {
+    log_d("_uartDetachBus_CTS: CTS pin already detached for UART%d", bus->num);
+    return true;
+  }
   return _uartDetachPins(bus->num, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, bus->_ctsPin, UART_PIN_NO_CHANGE);
 }
 
 static bool _uartDetachBus_RTS(void *busptr) {
   // sanity check - it should never happen
-  assert(busptr && "_uartDetachBus_RTS bus NULL pointer.");
+  if (busptr == NULL) {
+    log_e("_uartDetachBus_RTS: busptr is NULL");
+    return false;
+  }
   uart_t *bus = (uart_t *)busptr;
+  if (bus->_rtsPin < 0) {
+    log_d("_uartDetachBus_RTS: RTS pin already detached for UART%d", bus->num);
+    return true;
+  }
   return _uartDetachPins(bus->num, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, bus->_rtsPin);
 }
 
@@ -629,6 +670,16 @@ bool uartSetPins(uint8_t uart_num, int8_t rxPin, int8_t txPin, int8_t ctsPin, in
   //log_v("setting UART%d pins: prev->new RX(%d->%d) TX(%d->%d) CTS(%d->%d) RTS(%d->%d)", uart_num,
   //        uart->_rxPin, rxPin, uart->_txPin, txPin, uart->_ctsPin, ctsPin, uart->_rtsPin, rtsPin); vTaskDelay(10);
 
+  // mute bus detaching callbacks to avoid terminating the UART driver when both RX and TX pins are detached
+  peripheral_bus_deinit_cb_t rxDeinit = perimanGetBusDeinit(ESP32_BUS_TYPE_UART_RX);
+  peripheral_bus_deinit_cb_t txDeinit = perimanGetBusDeinit(ESP32_BUS_TYPE_UART_TX);
+  peripheral_bus_deinit_cb_t ctsDeinit = perimanGetBusDeinit(ESP32_BUS_TYPE_UART_CTS);
+  peripheral_bus_deinit_cb_t rtsDeinit = perimanGetBusDeinit(ESP32_BUS_TYPE_UART_RTS);
+  perimanClearBusDeinit(ESP32_BUS_TYPE_UART_RX);
+  perimanClearBusDeinit(ESP32_BUS_TYPE_UART_TX);
+  perimanClearBusDeinit(ESP32_BUS_TYPE_UART_CTS);
+  perimanClearBusDeinit(ESP32_BUS_TYPE_UART_RTS);
+
   // First step: detaches all previous UART pins
   bool rxPinChanged = rxPin >= 0 && rxPin != uart->_rxPin;
   if (rxPinChanged) {
@@ -660,6 +711,21 @@ bool uartSetPins(uint8_t uart_num, int8_t rxPin, int8_t txPin, int8_t ctsPin, in
   if (rtsPinChanged) {
     retCode &= _uartAttachPins(uart->num, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, UART_PIN_NO_CHANGE, rtsPin);
   }
+
+  // restore bus detaching callbacks
+  if (rxDeinit != NULL) {
+    perimanSetBusDeinit(ESP32_BUS_TYPE_UART_RX, rxDeinit);
+  }
+  if (txDeinit != NULL) {
+    perimanSetBusDeinit(ESP32_BUS_TYPE_UART_TX, txDeinit);
+  }
+  if (ctsDeinit != NULL) {
+    perimanSetBusDeinit(ESP32_BUS_TYPE_UART_CTS, ctsDeinit);
+  }
+  if (rtsDeinit != NULL) {
+    perimanSetBusDeinit(ESP32_BUS_TYPE_UART_RTS, rtsDeinit);
+  }
+
   UART_MUTEX_UNLOCK();
 
   if (!retCode) {
@@ -986,6 +1052,9 @@ void uartEnd(uint8_t uart_num) {
   if (uart_is_driver_installed(uart_num)) {
     uart_driver_delete(uart_num);
   }
+  if (uartGetDebug() == uart_num) {
+    uartSetDebug(0);
+  }
   UART_MUTEX_UNLOCK();
 }
 
diff --git a/docs/en/api/serial.rst b/docs/en/api/serial.rst
@@ -23,6 +23,10 @@ with additional features for advanced use cases.
 * **Event callbacks**: Receive and error event callbacks
 * **Configurable buffers**: Adjustable RX and TX buffer sizes
 
+.. note::
+   In case that both pins, RX and TX are detached from UART, the driver will be stopped.
+   Detaching may occur when, for instance, starting another peripheral using RX and TX pins, such as Wire.begin(RX0, TX0).
+
 UART Availability
 -----------------
 
diff --git a/tests/validation/uart/uart.ino b/tests/validation/uart/uart.ino
@@ -377,19 +377,23 @@ void change_pins_test(void) {
     UARTTestConfig &config = *uart_test_configs[0];
     // pinMode will force enabling the internal pullup resistor (IDF 5.3.2 Change)
     pinMode(NEW_RX1, INPUT_PULLUP);
-    config.serial.setPins(NEW_RX1, NEW_TX1);
+    // Detaching both pins will result in stopping the UART driver
+    // Only detach one of the pins
+    config.serial.setPins(NEW_RX1, /*NEW_TX1*/ -1);
     TEST_ASSERT_EQUAL(NEW_RX1, uart_get_RxPin(config.uart_num));
-    TEST_ASSERT_EQUAL(NEW_TX1, uart_get_TxPin(config.uart_num));
+    //TEST_ASSERT_EQUAL(NEW_TX1, uart_get_TxPin(config.uart_num));
 
     uart_internal_loopback(config.uart_num, NEW_RX1);
     config.transmit_and_check_msg("using new pins");
   } else {
     for (int i = 0; i < TEST_UART_NUM; i++) {
       UARTTestConfig &config = *uart_test_configs[i];
       UARTTestConfig &next_uart = *uart_test_configs[(i + 1) % TEST_UART_NUM];
-      config.serial.setPins(next_uart.default_rx_pin, next_uart.default_tx_pin);
+      // Detaching both pins will result in stopping the UART driver
+      // Only detach one of the pins
+      config.serial.setPins(next_uart.default_rx_pin, /*next_uart.default_tx_pin*/ -1);
       TEST_ASSERT_EQUAL(uart_get_RxPin(config.uart_num), next_uart.default_rx_pin);
-      TEST_ASSERT_EQUAL(uart_get_TxPin(config.uart_num), next_uart.default_tx_pin);
+      //TEST_ASSERT_EQUAL(uart_get_TxPin(config.uart_num), next_uart.default_tx_pin);
 
       uart_internal_loopback(config.uart_num, next_uart.default_rx_pin);
       config.transmit_and_check_msg("using new pins");
@@ -450,7 +454,9 @@ void periman_test(void) {
 
   for (auto *ref : uart_test_configs) {
     UARTTestConfig &config = *ref;
-    Wire.begin(config.default_rx_pin, config.default_tx_pin);
+    // Detaching both pins will result in stopping the UART driver
+    // Only detach one of the pins
+    Wire.begin(config.default_rx_pin, /*config.default_tx_pin*/ -1);
     config.recv_msg = "";
 
     log_d("Trying to send message using UART%d with I2C enabled", config.uart_num);
