From 6e6c7c2d10c4f7c3291938b10656a18ded20f30a Mon Sep 17 00:00:00 2001 From: Rashmi Vaidya Date: Tue, 7 May 2024 17:38:12 -0700 Subject: [PATCH 1/2] ESP-IDF Log Backend - unrevert original commit Signed-off-by: Rashmi Vaidya --- .../components/golioth_sdk/CMakeLists.txt | 1 + port/esp_idf/components/golioth_sdk/Kconfig | 9 +- port/esp_idf/golioth_log_esp_idf.c | 113 ++++++++++++++++++ port/esp_idf/golioth_log_esp_idf.h | 11 ++ port/esp_idf/golioth_port_config.h | 54 ++++----- port/freertos/golioth_sys_freertos.c | 4 - port/modus_toolbox/golioth_sys_modus.c | 10 ++ 7 files changed, 169 insertions(+), 33 deletions(-) create mode 100644 port/esp_idf/golioth_log_esp_idf.c create mode 100644 port/esp_idf/golioth_log_esp_idf.h create mode 100644 port/modus_toolbox/golioth_sys_modus.c diff --git a/port/esp_idf/components/golioth_sdk/CMakeLists.txt b/port/esp_idf/components/golioth_sdk/CMakeLists.txt index ffe3ddc1d..1563403cd 100644 --- a/port/esp_idf/components/golioth_sdk/CMakeLists.txt +++ b/port/esp_idf/components/golioth_sdk/CMakeLists.txt @@ -29,6 +29,7 @@ idf_component_register( SRCS "${sdk_port}/freertos/golioth_sys_freertos.c" "${sdk_port}/esp_idf/fw_update_esp_idf.c" + "${sdk_port}/esp_idf/golioth_log_esp_idf.c" "${sdk_src}/golioth_status.c" "${sdk_src}/coap_client.c" "${sdk_src}/coap_client_libcoap.c" diff --git a/port/esp_idf/components/golioth_sdk/Kconfig b/port/esp_idf/components/golioth_sdk/Kconfig index ea954d8cc..e51b6a01b 100644 --- a/port/esp_idf/components/golioth_sdk/Kconfig +++ b/port/esp_idf/components/golioth_sdk/Kconfig @@ -8,4 +8,11 @@ menu "Golioth SDK Configuration" rsource '../../../../src/Kconfig' -endmenu # Golioth SDK Configuration +config GOLIOTH_ESPLOG_MAX_BUFF_SIZE + int "Maximum log buffer size allowed for ESP log backend" + default 128 + help + Maximum log buffer size allowed for ESP log backend to be sent to + Golioth cloud + +endmenu # Golioth SDK Configuration diff --git a/port/esp_idf/golioth_log_esp_idf.c b/port/esp_idf/golioth_log_esp_idf.c new file mode 100644 index 000000000..4003f51b6 --- /dev/null +++ b/port/esp_idf/golioth_log_esp_idf.c @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2024 Golioth, Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include +#include "golioth_log_esp_idf.h" +#include +#include +#include +#include +#include "string.h" + +static vprintf_like_t original_log_func; + +static struct golioth_client *_client = NULL; + +static int golioth_log_espidf(const char *format, va_list args) +{ + + const char *new_format = format; + const char *tag_arg = NULL; + + if (!CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD) + { + return -1; + } + + if (!_client) + { + return -1; + } + + va_list new_args; + va_copy(new_args, args); + + // Since the format contains time (%lu) and tag (%s) values that need not + // be sent to the cloud like so : "[0;32mI (%lu) %s: Actual Log %d [0m", + // we remove the string upto colon : and update the args list + // Also, the 7th character in the string tells the log level as + // E = Error, W = Warning, I = info, D = Debug, V = Verbose + char log_level = format[7]; + const char *colon_pos = strchr(format, ':'); + if (colon_pos != NULL) + { + new_format = colon_pos + 2; // Skip ':' and space + + // Move va_list to correspond to the new format + va_arg(new_args, unsigned long); // Skip %lu + tag_arg = va_arg(new_args, const char *); // Extract tag value + } + + // Temporarily buffer to store the message + char msg_buffer[CONFIG_GOLIOTH_ESPLOG_MAX_BUFF_SIZE] = {0}; + + vsnprintf(msg_buffer, CONFIG_GOLIOTH_ESPLOG_MAX_BUFF_SIZE, new_format, new_args); + + // Insert a null char 5 places behind to remove ESC[0m at the end of the string + msg_buffer[strlen(msg_buffer) - 5] = '\0'; + // Here strlen(msg_buffer) - 5 will not go out of bounds because the format will + // always have at least 5 characters, even with an empty string + + printf("%s\n", msg_buffer); + + // Log to Golioth asynchronously. + switch (log_level) + { + case 'E': + golioth_log_error_async(_client, tag_arg, msg_buffer, NULL, NULL); + break; + case 'W': + golioth_log_warn_async(_client, tag_arg, msg_buffer, NULL, NULL); + break; + case 'I': + golioth_log_info_async(_client, tag_arg, msg_buffer, NULL, NULL); + break; + case 'V': // fallthrough + case 'D': + golioth_log_debug_async(_client, tag_arg, msg_buffer, NULL, NULL); + break; + default: + break; + } + + return 0; +} + +vprintf_like_t golioth_log_set_esp_vprintf(void *client) +{ + _client = client; + return esp_log_set_vprintf(golioth_log_espidf); +} + +vprintf_like_t golioth_log_reset_esp_vprintf(vprintf_like_t original_log_func) +{ + return esp_log_set_vprintf(original_log_func); +} + +void golioth_sys_client_connected(void *client) +{ + if (CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD == 1) + { + original_log_func = golioth_log_set_esp_vprintf(client); + } +} + +void golioth_sys_client_disconnected(void *client) +{ + if (CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD == 1) + { + golioth_log_reset_esp_vprintf(original_log_func); + } +} diff --git a/port/esp_idf/golioth_log_esp_idf.h b/port/esp_idf/golioth_log_esp_idf.h new file mode 100644 index 000000000..1ea06cd9a --- /dev/null +++ b/port/esp_idf/golioth_log_esp_idf.h @@ -0,0 +1,11 @@ +/* + * Copyright (c) 2024 Golioth, Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#pragma once + +#include + +vprintf_like_t golioth_log_set_esp_vprintf(void *client); +vprintf_like_t golioth_log_reset_esp_vprintf(vprintf_like_t original_log_func); diff --git a/port/esp_idf/golioth_port_config.h b/port/esp_idf/golioth_port_config.h index 415009558..0dcb89f12 100644 --- a/port/esp_idf/golioth_port_config.h +++ b/port/esp_idf/golioth_port_config.h @@ -34,33 +34,31 @@ } \ } while (0) -// TODO - should we hook into the ESP logging backend via esp_log_set_vprintf? -// This would enable all logs to be sent to Golioth (not just the GLTH_LOGX logs). - -#define GLTH_LOGX(COLOR, LEVEL, LEVEL_STR, TAG, ...) \ - do { \ - if ((LEVEL) <= golioth_debug_get_log_level()) { \ - uint64_t now_ms = golioth_sys_now_ms(); \ - switch (LEVEL) { \ - case GOLIOTH_DEBUG_LOG_LEVEL_ERROR: \ - ESP_LOGE(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_WARN: \ - ESP_LOGW(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_INFO: \ - ESP_LOGI(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_DEBUG: \ - ESP_LOGD(TAG, __VA_ARGS__); \ - break; \ +#define GLTH_LOGX(COLOR, LEVEL, LEVEL_STR, TAG, ...) \ + do \ + { \ + if ((LEVEL) <= golioth_debug_get_log_level()) \ + { \ + switch (LEVEL) \ + { \ + case GOLIOTH_DEBUG_LOG_LEVEL_ERROR: \ + ESP_LOGE(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_WARN: \ + ESP_LOGW(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_INFO: \ + ESP_LOGI(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_DEBUG: \ + ESP_LOGD(TAG, __VA_ARGS__); \ + break; \ case GOLIOTH_DEBUG_LOG_LEVEL_VERBOSE: \ - ESP_LOGV(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_NONE: \ - default: \ - break; \ - } \ - golioth_debug_printf(now_ms, LEVEL, TAG, __VA_ARGS__); \ - } \ + ESP_LOGV(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_NONE: \ + default: \ + break; \ + } \ + } \ } while (0) diff --git a/port/freertos/golioth_sys_freertos.c b/port/freertos/golioth_sys_freertos.c index ab9c43777..a62c93178 100644 --- a/port/freertos/golioth_sys_freertos.c +++ b/port/freertos/golioth_sys_freertos.c @@ -148,7 +148,3 @@ void golioth_sys_thread_destroy(golioth_sys_thread_t thread) { /*-------------------------------------------------- * Misc *------------------------------------------------*/ - -void golioth_sys_client_connected(void* client) {} - -void golioth_sys_client_disconnected(void* client) {} diff --git a/port/modus_toolbox/golioth_sys_modus.c b/port/modus_toolbox/golioth_sys_modus.c new file mode 100644 index 000000000..aa0300acd --- /dev/null +++ b/port/modus_toolbox/golioth_sys_modus.c @@ -0,0 +1,10 @@ +/* + * Copyright (c) 2024 Golioth, Inc. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include + +void golioth_sys_client_connected(void *client) {} + +void golioth_sys_client_disconnected(void *client) {} From 1f87b8eaf33d310ad01c940fbd53cc8ffe4ea5c2 Mon Sep 17 00:00:00 2001 From: Rashmi Vaidya Date: Tue, 23 Apr 2024 10:31:18 -0700 Subject: [PATCH 2/2] Improved ESP Log Backend with cloud logging option default to 0 Signed-off-by: Rashmi Vaidya --- examples/esp_idf/hello/sdkconfig.defaults | 3 ++ port/esp_idf/components/golioth_sdk/Kconfig | 7 +++ port/esp_idf/golioth_log_esp_idf.c | 56 +++++++++---------- port/esp_idf/golioth_port_config.h | 59 +++++++++++---------- 4 files changed, 67 insertions(+), 58 deletions(-) diff --git a/examples/esp_idf/hello/sdkconfig.defaults b/examples/esp_idf/hello/sdkconfig.defaults index a05c4423c..89a9a0cc9 100644 --- a/examples/esp_idf/hello/sdkconfig.defaults +++ b/examples/esp_idf/hello/sdkconfig.defaults @@ -14,6 +14,9 @@ CONFIG_COMPILER_OPTIMIZATION_SIZE=y CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD=1 CONFIG_GOLIOTH_COAP_REQUEST_QUEUE_MAX_ITEMS=20 +# Uncomment to automatically upload ESP_LOGX statements to Golioth Logs service +# CONFIG_GOLIOTH_ESPLOG_AUTO_LOG_TO_CLOUD=1 + # To enable hardcode credentials, set to y CONFIG_GOLIOTH_SAMPLE_HARDCODED_CREDENTIALS=n diff --git a/port/esp_idf/components/golioth_sdk/Kconfig b/port/esp_idf/components/golioth_sdk/Kconfig index e51b6a01b..844e01cc3 100644 --- a/port/esp_idf/components/golioth_sdk/Kconfig +++ b/port/esp_idf/components/golioth_sdk/Kconfig @@ -8,6 +8,13 @@ menu "Golioth SDK Configuration" rsource '../../../../src/Kconfig' +config GOLIOTH_ESPLOG_AUTO_LOG_TO_CLOUD + int "Option to send logs to cloud" + default 0 + help + If set to 1, ESP_LOGX statements will automatically upload to + Golioth cloud + config GOLIOTH_ESPLOG_MAX_BUFF_SIZE int "Maximum log buffer size allowed for ESP log backend" default 128 diff --git a/port/esp_idf/golioth_log_esp_idf.c b/port/esp_idf/golioth_log_esp_idf.c index 4003f51b6..ef88a5561 100644 --- a/port/esp_idf/golioth_log_esp_idf.c +++ b/port/esp_idf/golioth_log_esp_idf.c @@ -17,10 +17,6 @@ static struct golioth_client *_client = NULL; static int golioth_log_espidf(const char *format, va_list args) { - - const char *new_format = format; - const char *tag_arg = NULL; - if (!CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD) { return -1; @@ -31,52 +27,50 @@ static int golioth_log_espidf(const char *format, va_list args) return -1; } - va_list new_args; - va_copy(new_args, args); - - // Since the format contains time (%lu) and tag (%s) values that need not - // be sent to the cloud like so : "[0;32mI (%lu) %s: Actual Log %d [0m", - // we remove the string upto colon : and update the args list - // Also, the 7th character in the string tells the log level as + // The 7th character in the format tells the log level as // E = Error, W = Warning, I = info, D = Debug, V = Verbose char log_level = format[7]; - const char *colon_pos = strchr(format, ':'); - if (colon_pos != NULL) - { - new_format = colon_pos + 2; // Skip ':' and space - - // Move va_list to correspond to the new format - va_arg(new_args, unsigned long); // Skip %lu - tag_arg = va_arg(new_args, const char *); // Extract tag value - } - // Temporarily buffer to store the message + // Temporary buffer to store the message char msg_buffer[CONFIG_GOLIOTH_ESPLOG_MAX_BUFF_SIZE] = {0}; + vsnprintf(msg_buffer, CONFIG_GOLIOTH_ESPLOG_MAX_BUFF_SIZE, format, args); + printf("%s", msg_buffer); - vsnprintf(msg_buffer, CONFIG_GOLIOTH_ESPLOG_MAX_BUFF_SIZE, new_format, new_args); - + // The formatted string looks like this: "[0;32mI (%lu) %s: Actual Log %d [0m" + // The start and end characters convey formatting and color information + // This information should not be sent to the cloud. // Insert a null char 5 places behind to remove ESC[0m at the end of the string - msg_buffer[strlen(msg_buffer) - 5] = '\0'; // Here strlen(msg_buffer) - 5 will not go out of bounds because the format will // always have at least 5 characters, even with an empty string + msg_buffer[strlen(msg_buffer) - 5] = '\0'; - printf("%s\n", msg_buffer); + // Find ) and : characters in the string for extracting tag + char tag_str[64] = {0}; + const char *tag = strchr(msg_buffer, ')'); + const char *msg_str = strchr(msg_buffer, ':'); + if (tag != NULL && msg_str != NULL) + { + tag = tag + 1; // Skip space after ) + strncpy(tag_str, tag, msg_str - tag); + // Ignore everything before ':' + msg_str = msg_str + 2; // Skip ':' and space + } // Log to Golioth asynchronously. switch (log_level) { case 'E': - golioth_log_error_async(_client, tag_arg, msg_buffer, NULL, NULL); + golioth_log_error_async(_client, tag_str, msg_str, NULL, NULL); break; case 'W': - golioth_log_warn_async(_client, tag_arg, msg_buffer, NULL, NULL); + golioth_log_warn_async(_client, tag_str, msg_str, NULL, NULL); break; case 'I': - golioth_log_info_async(_client, tag_arg, msg_buffer, NULL, NULL); + golioth_log_info_async(_client, tag_str, msg_str, NULL, NULL); break; case 'V': // fallthrough case 'D': - golioth_log_debug_async(_client, tag_arg, msg_buffer, NULL, NULL); + golioth_log_debug_async(_client, tag_str, msg_str, NULL, NULL); break; default: break; @@ -98,7 +92,7 @@ vprintf_like_t golioth_log_reset_esp_vprintf(vprintf_like_t original_log_func) void golioth_sys_client_connected(void *client) { - if (CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD == 1) + if (CONFIG_GOLIOTH_ESPLOG_AUTO_LOG_TO_CLOUD == 1) { original_log_func = golioth_log_set_esp_vprintf(client); } @@ -106,7 +100,7 @@ void golioth_sys_client_connected(void *client) void golioth_sys_client_disconnected(void *client) { - if (CONFIG_GOLIOTH_AUTO_LOG_TO_CLOUD == 1) + if (CONFIG_GOLIOTH_ESPLOG_AUTO_LOG_TO_CLOUD == 1) { golioth_log_reset_esp_vprintf(original_log_func); } diff --git a/port/esp_idf/golioth_port_config.h b/port/esp_idf/golioth_port_config.h index 0dcb89f12..efdfc5a94 100644 --- a/port/esp_idf/golioth_port_config.h +++ b/port/esp_idf/golioth_port_config.h @@ -34,31 +34,36 @@ } \ } while (0) -#define GLTH_LOGX(COLOR, LEVEL, LEVEL_STR, TAG, ...) \ - do \ - { \ - if ((LEVEL) <= golioth_debug_get_log_level()) \ - { \ - switch (LEVEL) \ - { \ - case GOLIOTH_DEBUG_LOG_LEVEL_ERROR: \ - ESP_LOGE(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_WARN: \ - ESP_LOGW(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_INFO: \ - ESP_LOGI(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_DEBUG: \ - ESP_LOGD(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_VERBOSE: \ - ESP_LOGV(TAG, __VA_ARGS__); \ - break; \ - case GOLIOTH_DEBUG_LOG_LEVEL_NONE: \ - default: \ - break; \ - } \ - } \ +#define GLTH_LOGX(COLOR, LEVEL, LEVEL_STR, TAG, ...) \ + do \ + { \ + if ((LEVEL) <= golioth_debug_get_log_level()) \ + { \ + switch (LEVEL) \ + { \ + case GOLIOTH_DEBUG_LOG_LEVEL_ERROR: \ + ESP_LOGE(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_WARN: \ + ESP_LOGW(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_INFO: \ + ESP_LOGI(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_DEBUG: \ + ESP_LOGD(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_VERBOSE: \ + ESP_LOGV(TAG, __VA_ARGS__); \ + break; \ + case GOLIOTH_DEBUG_LOG_LEVEL_NONE: \ + default: \ + break; \ + } \ + if (!CONFIG_GOLIOTH_ESPLOG_AUTO_LOG_TO_CLOUD) \ + { \ + uint64_t now_ms = golioth_sys_now_ms(); \ + golioth_debug_printf(now_ms, LEVEL, TAG, __VA_ARGS__); \ + } \ + } \ } while (0)