Fix(uart): no global serial creates linking error#12412
Conversation
NO_GLOBAL_SERIAL may be defined and hal_uart_notify_pins_detached() may not exist creating problems with the linker.
👋 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 ...
|
There was a problem hiding this comment.
Pull request overview
Fixes a link failure when building without global Serial instances by making the UART “pins detached” notifier optional (weak) and guarding its invocation.
Changes:
- Declares
hal_uart_notify_pins_detached()as a weak symbol in the UART HAL header. - Removes the non-weak prototype from the C file to avoid undefined references.
- Guards notifier calls in RX/TX detach paths to safely no-op when not implemented.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cores/esp32/esp32-hal-uart.h | Declares the optional notifier as a weak symbol so missing implementations don’t break linking. |
| cores/esp32/esp32-hal-uart.c | Calls the notifier only when present, preventing link errors when Serial globals are disabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test Results101 files 101 suites 31m 26s ⏱️ Results for commit c09954f. ♻️ This comment has been updated with latest results. |
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]
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
lucasssvaz
left a comment
There was a problem hiding this comment.
Looks good. Please take a look at the copilot comments
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
I'll change the way it is done.... not using Serial0, Serial1, Serial2, etc. This will solve the issue for any case for good. |
Added functions to register and unregister HardwareSerial objects, allowing for better management of UART instances.
Removed weak prototype for hal_uart_notify_pins_detached function.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
cores/esp32/HardwareSerial.cpp:179
- The early
returnin the destructor when_uart_nr >= SOC_UART_NUMskips freeing_lock(and any other teardown after this block). Since the constructor may have created_lockeven for an invalid UART number, this can leak the semaphore. Consider removing the early return and instead conditionally skipping only the UART-specificend()/uart_unregister()parts, while still deleting_lockwhen present.
HardwareSerial::~HardwareSerial() {
if (_uart_nr >= SOC_UART_NUM) {
return;
}
end(); // explicit Full UART termination
uart_unregister(_uart_nr);
#if !CONFIG_DISABLE_HAL_LOCKS
if (_lock != NULL) {
vSemaphoreDelete(_lock);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@SuGlider please fix the conflict |
Done. |
|
Whoops, looks like a |
Description of Change
If an Arduino Sketch if compiled with NO_GLOBAL_INSTANCES or NO_GLOBAL_SERIAL defined, it will fail with a linking error:
Test Scenarios
Compile any Serial exemple adding the build_opt.h file with
-DNO_GLOBAL_INSTANCESoptionbuild_opt.h file:
Testing Sketch:
Related links
related to #12402