-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
feat(logging): Arduino log redirection #11159
base: master
Are you sure you want to change the base?
Conversation
👋 Hello mathieucarbou, 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 ...
|
be8de0a
to
8c7fcac
Compare
@mathieucarbou - I remember that the reason for that is because ESP32|ESP32-S2|ESP32-S3 (Xtensa SoC) had a problem with We can review if this is still valid. |
@mathieucarbou - What SoC do you use with your projects? |
I am currently testing with an esp32dev board, but the project has to also work for some other boards such as s2, s3, wt32_eth01, etc. This is a feature that will only be activated in debug mode to grab the boot logs from the ESP since users cannot connect easily to a USB computer. I did the modification locally in the c file and tested with the esp32dev board and it works but I don't know the possible impact of this on other boards. I was also intrigued by this comment in the header:
So this is my understanding that putc1 (so a call to ets_printf) should already use ets_write_char_uart behind ? |
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.
I'll retest log output within all SoC using just ets_printf()
to verify if it works for all possible Serial ports (UART/CDC).
Let set this PR on hold until the results are out.
Test Results 76 files 76 suites 12m 40s ⏱️ Results for commit 8c7fcac. ♻️ This comment has been updated with latest results. |
Thanks a lot! |
Therefore, if your software needs to redirect log output, it could just change the function pointed by
|
That's exactly my understanding also.
That's what does not work. If you mean: if your software needs to redirect log output when using then I agree with you. But this is not the use case explained above. What I want and need is to redirect all calls to log_e, log_w, log_i, etc Basically, all Arduino logs, to a custom function. And for that, Arduino would need to call every time |
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]
|
ets_sys.h
supports log redirection whenets_printf()
is used and some functions are set withets_install_putc1()
orets_install_putc1()
.I need to add a custom
ets_install_putc1()
handler to redirect arduino logs, but sadly the implementation inesp32-hal-uart.c
is bypassing theets_printf()
internal mechanism and is directly callingets_write_char_uart()
instead .arduino-esp32/cores/esp32/esp32-hal-uart.c
Lines 1123 to 1132 in 6c04a93
Why ? I really was not able to figure out...
But this piece of code prevents everybody from redirecting Arduino logs, except with the boards in the macro conditions listed above...
Since I do not know the logic why
ets_printf()
is not called in all cases, I am proposing to add a macro:ARDUINO_LOG_FORCE_ETS_PRINTF=1
which, when set, will make sure all the logs go throughets_printf()
.The user setting this macro will then be responsible to:
ets_install_putc1(ets_write_char_uart)
ets_install_putc1(my_custom_character_log_callback)
with his custom function to redirect the logs.Use case example:
With the macro def and the code above Arduino logs with be printed:
outputs: