Skip to content
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

In esp32-hal-log, direct calls to ESP_LOG_x macros is more efficient … #5081

Merged
merged 1 commit into from
Apr 19, 2021

Conversation

jfbuggen
Copy link
Contributor

…than using intermediate function log_to_esp

As indicated in #4845 (comment)_ it is more efficient to call directly the ESP LOG macros. This spares a function call, a 512b buffer and a call to vsnprintf. No change in functionality.

…than using intermediate function log_to_esp

As indicated in espressif#4845 (comment) it is more efficient to call directly the ESP LOG macros. This spares a function call, a 512b buffer and a call to vsnprintf. No change in functionality.
@me-no-dev me-no-dev merged commit 55b8f67 into espressif:master Apr 19, 2021
@me-no-dev
Copy link
Member

thanks :)

@coratron
Copy link

coratron commented Nov 28, 2021

Hi, thanks for addressing this. I found a couple of issues
@jfbuggen

  1. there is a bug on line 161 which breaks compilation if log levels are set to ERROR or higher:
    #define log_e(format, ...) do {log_to_esp(TAG, ESP_LOG_ERROR, format, ##__VA_ARGS__);}while(0)
    should be changed to
    #define log_e(format, ...) do {ESP_LOG_LEVEL_LOCAL(ESP_LOG_ERROR, TAG, format, ##__VA_ARGS__);}while(0)

[Note: it seems to have been right at the time of PR but somehow broken afterwards]

  1. Also, not sure why the ARDUHAL_LOG_LEVEL_NONE is formatted as ARDUHAL_LOG_LEVEL_ERROR . Perhaps not an issue but intentional?

  2. Main issue Added possibility to use ESP32-IDF log insted of redefined one #4845 was trying to address is ability to use ESP log instead of Arduino so that esp_log_set_vprintf could be used. This is now broken. Not 100% sure it is due to this change or something else in IDF. See below:

Example code flow:

//function used to reroute the logs
int websocket_printf(char * message, va_list args)
{
  //Serial.printf("%s\r\n", message); //this line is commented but the system crashes anyway
}

esp_log_level_set("*", ESP_LOG_ERROR);
esp_log_set_vprintf((vprintf_like_t)&websocket_printf);
log_e("%s\r\n", "hi");

This results in the following error
0x4014b063:0x3ffb27500 in s_log_level_get_and_unlock at /home/runner/work/esp32-arduino-lib-builder/esp32-arduino-lib-builder/esp-idf/components/log/log.c:162

If the esp_log_set_vprintf() line is commented out then the logs work via serial normally (if point 1. above is fixed first)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants