espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
12.97k stars 7.29k forks source link

Support log redirection #9814

Open yaqwsx opened 3 weeks ago

yaqwsx commented 3 weeks ago

Related area

Core of the framework

Hardware specification

Not relevant

Is your feature request related to a problem?

The logging facilities provided by the Arduino framework are hard-coded to the main serial output. This is often unwanted (e.g. when the persistence of logs is required or the serial interface needs to be free for other purposes).

Describe the solution you'd like

Allow the user to specify a custom sink function for the logs.

Describe alternatives you've considered

I wanted to use the built-in ESP-IDF logging facilities; however, I ran into #9813

Additional context

No response

I have checked existing list of Feature requests and the Contribution Guide

me-no-dev commented 3 weeks ago
#include "rom/ets_sys.h"

static void custom_debug_write_char(char c) {
  //send the char to wherever you want, but be aware that this code might be called from interrupt
}

ets_install_putc1((void (*)(char)) & custom_debug_write_char);
ets_install_putc2(NULL);
yaqwsx commented 3 weeks ago

Thank you for the proposed solution. However, I am afraid of unwanted consequences of the proposed solution - it doesn't redirect the output of logging, but the output of the underlying layer. Thus any code using the ets_ function for output will be affected.

A quick grep of the Arduino framework showed that ets_printf isn't used extensively (just to output the reboot message), however, I have already seen 3rd party out in the wild using it. Thus, I don't find the proposed solution to be exactly clean. On top of that, if there was an interface to supply custom log_v and isr_log_v functions, the logging facility could store the format string and arguments separately (e.g., to save bandwidth or storage). This is not possible in the proposed solution. On top of that, we can't supply separate function to handle isr and non-isr context in you proposed solution.

everslick commented 3 weeks ago

esp_log_set_vprintf(...); in combination of setting -DUSE_ESP_IDF_LOG -DTAG=\"CORE\" in CFLAGS works for most of the logs for me.

igrr commented 3 weeks ago

logging facility could store the format string and arguments separately (e.g., to save bandwidth or storage). This is not possible in the proposed solution. On top of that, we can't supply separate function to handle isr and non-isr context in you proposed solution

The new logging library is in development in ESP-IDF and it should address both points. I think the initial version will appear in ESP-IDF before the end of the year.

yaqwsx commented 3 weeks ago

@igrr: Thank you for the update. Nevertheless, if I understood correctly, the Arduino framework is opinionated, hence they reimplement logging (differently than ESP-IDF). Will this change also propagate to the Arduino framework?