From 5a05444bada5ab86aa269ca3aa84ff10f1e758c2 Mon Sep 17 00:00:00 2001 From: Eric Katzfey <53063038+katzfey@users.noreply.github.com> Date: Mon, 10 Mar 2025 15:59:52 -0700 Subject: [PATCH] QuRT logging improvements (#24462) - Made general improvements to the QURT platform message logging so that module name is printed both in mini-dm and on apps side terminal --- .../common/include/px4_platform_common/log.h | 32 +++++++++---------- platforms/qurt/include/qurt_log.h | 21 ++++++++---- platforms/qurt/src/px4/SerialImpl.cpp | 2 ++ platforms/qurt/src/px4/drv_hrt.cpp | 2 ++ platforms/qurt/src/px4/main.cpp | 2 ++ platforms/qurt/src/px4/tasks.cpp | 4 ++- platforms/qurt/unresolved_symbols.c | 2 ++ src/drivers/qshell/qurt/qshell.cpp | 2 +- .../muorb/apps/uORBAppsProtobufChannel.cpp | 4 +-- 9 files changed, 45 insertions(+), 26 deletions(-) diff --git a/platforms/common/include/px4_platform_common/log.h b/platforms/common/include/px4_platform_common/log.h index 0b5d878ed7..895679c0bb 100644 --- a/platforms/common/include/px4_platform_common/log.h +++ b/platforms/common/include/px4_platform_common/log.h @@ -70,33 +70,33 @@ __END_DECLS /**************************************************************************** * Messages that should never be filtered or compiled out ****************************************************************************/ -#define PX4_INFO(FMT, ...) qurt_log(_PX4_LOG_LEVEL_INFO, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_INFO(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_INFO, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) #define PX4_INFO_RAW(FMT, ...) qurt_log_raw(FMT, ##__VA_ARGS__) #if defined(TRACE_BUILD) /**************************************************************************** * Extremely Verbose settings for a Trace build ****************************************************************************/ -#define PX4_PANIC(FMT, ...) qurt_log(_PX4_LOG_LEVEL_PANIC, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_ERR(FMT, ...) qurt_log(_PX4_LOG_LEVEL_ERROR, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_WARN(FMT, ...) qurt_log(_PX4_LOG_LEVEL_WARN, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_DEBUG(FMT, ...) qurt_log(_PX4_LOG_LEVEL_DEBUG, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_PANIC(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_PANIC, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_ERR(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_ERROR, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_WARN(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_WARN, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_DEBUG(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_DEBUG, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) #elif defined(DEBUG_BUILD) /**************************************************************************** * Verbose settings for a Debug build ****************************************************************************/ -#define PX4_PANIC(FMT, ...) qurt_log(_PX4_LOG_LEVEL_PANIC, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_ERR(FMT, ...) qurt_log(_PX4_LOG_LEVEL_ERROR, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_WARN(FMT, ...) qurt_log(_PX4_LOG_LEVEL_WARN, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_DEBUG(FMT, ...) qurt_log(_PX4_LOG_LEVEL_DEBUG, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_PANIC(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_PANIC, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_ERR(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_ERROR, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_WARN(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_WARN, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_DEBUG(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_DEBUG, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) #elif defined(RELEASE_BUILD) /**************************************************************************** * Non-verbose settings for a Release build to minimize strings in build ****************************************************************************/ -#define PX4_PANIC(FMT, ...) qurt_log(_PX4_LOG_LEVEL_PANIC, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_ERR(FMT, ...) qurt_log(_PX4_LOG_LEVEL_ERROR, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_PANIC(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_PANIC, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_ERR(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_ERROR, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) #define PX4_WARN(FMT, ...) __px4_log_omit(_PX4_LOG_LEVEL_WARN, FMT, ##__VA_ARGS__) #define PX4_DEBUG(FMT, ...) __px4_log_omit(_PX4_LOG_LEVEL_DEBUG, FMT, ##__VA_ARGS__) @@ -104,14 +104,14 @@ __END_DECLS /**************************************************************************** * Medium verbose settings for a default build ****************************************************************************/ -#define PX4_PANIC(FMT, ...) qurt_log(_PX4_LOG_LEVEL_PANIC, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_ERR(FMT, ...) qurt_log(_PX4_LOG_LEVEL_ERROR, __FILE__, __LINE__, FMT, ##__VA_ARGS__) -#define PX4_WARN(FMT, ...) qurt_log(_PX4_LOG_LEVEL_WARN, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_PANIC(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_PANIC, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_ERR(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_ERROR, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) +#define PX4_WARN(FMT, ...) qurt_log_module(_PX4_LOG_LEVEL_WARN, MODULE_NAME, __FILE__, __LINE__, FMT, ##__VA_ARGS__) #define PX4_DEBUG(FMT, ...) __px4_log_omit(_PX4_LOG_LEVEL_DEBUG, FMT, ##__VA_ARGS__) #endif -#define PX4_LOG_NAMED(name, FMT, ...) qurt_log( _PX4_LOG_LEVEL_INFO, __FILE__, __LINE__, "%s " FMT, name, ##__VA_ARGS__) -#define PX4_LOG_NAMED_COND(name, cond, FMT, ...) if( cond ) qurt_log( _PX4_LOG_LEVEL_INFO, __FILE__, __LINE__, "%s " FMT, name, ##__VA_ARGS__) +#define PX4_LOG_NAMED(name, FMT, ...) qurt_log_module( _PX4_LOG_LEVEL_INFO, MODULE_NAME, __FILE__, __LINE__, "%s " FMT, name, ##__VA_ARGS__) +#define PX4_LOG_NAMED_COND(name, cond, FMT, ...) if( cond ) qurt_log_module( _PX4_LOG_LEVEL_INFO, MODULE_NAME, __FILE__, __LINE__, "%s " FMT, name, ##__VA_ARGS__) #else diff --git a/platforms/qurt/include/qurt_log.h b/platforms/qurt/include/qurt_log.h index 86c12f4024..7c573600e4 100644 --- a/platforms/qurt/include/qurt_log.h +++ b/platforms/qurt/include/qurt_log.h @@ -37,6 +37,10 @@ #include #include +#define BASE_BUFFER_SIZE 256 +#define MAX_MODULE_NAME_SIZE 32 +#define MODULE_BUFFER_SIZE (BASE_BUFFER_SIZE + MAX_MODULE_NAME_SIZE) + __BEGIN_DECLS extern void qurt_log_to_apps(int level, const char *message); @@ -44,22 +48,27 @@ extern void qurt_log_to_apps(int level, const char *message); // Defining hap_debug void HAP_debug(const char *msg, int level, const char *filename, int line); -static __inline void qurt_log(int level, const char *file, int line, - const char *format, ...) +static __inline void qurt_log_module(int level, const char *module, const char *file, int line, + const char *format, ...) { - char buf[256]; + char buf[BASE_BUFFER_SIZE]; va_list args; va_start(args, format); vsnprintf(buf, sizeof(buf), format, args); va_end(args); - HAP_debug(buf, level, file, line); - qurt_log_to_apps(level, buf); + char module_buf[MODULE_BUFFER_SIZE]; + snprintf(module_buf, MAX_MODULE_NAME_SIZE, "[%s] ", module); + strcat(module_buf, buf); + + HAP_debug(module_buf, level, file, line); + + qurt_log_to_apps(level, module_buf); } static __inline void qurt_log_raw(const char *format, ...) { - char buf[256]; + char buf[BASE_BUFFER_SIZE]; va_list args; va_start(args, format); vsnprintf(buf, sizeof(buf), format, args); diff --git a/platforms/qurt/src/px4/SerialImpl.cpp b/platforms/qurt/src/px4/SerialImpl.cpp index 73f8ce8cc4..7225f32dce 100644 --- a/platforms/qurt/src/px4/SerialImpl.cpp +++ b/platforms/qurt/src/px4/SerialImpl.cpp @@ -37,6 +37,8 @@ #include #include +#define MODULE_NAME "SerialImpl" + namespace device { diff --git a/platforms/qurt/src/px4/drv_hrt.cpp b/platforms/qurt/src/px4/drv_hrt.cpp index 2ef1520832..6d5318d0ed 100644 --- a/platforms/qurt/src/px4/drv_hrt.cpp +++ b/platforms/qurt/src/px4/drv_hrt.cpp @@ -43,6 +43,8 @@ #include "hrt_work.h" +#define MODULE_NAME "drv_hrt" + static constexpr unsigned HRT_INTERVAL_MIN = 50; static constexpr unsigned HRT_INTERVAL_MAX = 50000000; diff --git a/platforms/qurt/src/px4/main.cpp b/platforms/qurt/src/px4/main.cpp index 1155576552..48fc3d8002 100644 --- a/platforms/qurt/src/px4/main.cpp +++ b/platforms/qurt/src/px4/main.cpp @@ -44,6 +44,8 @@ #include "apps.h" +#define MODULE_NAME "main" + #define MAX_ARGS 8 // max number of whitespace separated args after app name __BEGIN_DECLS diff --git a/platforms/qurt/src/px4/tasks.cpp b/platforms/qurt/src/px4/tasks.cpp index 529b234edc..112fb15452 100644 --- a/platforms/qurt/src/px4/tasks.cpp +++ b/platforms/qurt/src/px4/tasks.cpp @@ -41,6 +41,8 @@ #include #include "hrt_work.h" +#define MODULE_NAME "tasks" + #define PX4_TASK_STACK_SIZE 8192 #define PX4_TASK_MAX_NAME_LENGTH 32 #define PX4_TASK_MAX_ARGC 32 @@ -129,7 +131,7 @@ static px4_task_t px4_task_spawn_internal(const char *name, int priority, px4_ma int task_index = 0; char *p = (char *)argv; - PX4_INFO("Creating pthread %s\n", name); + PX4_INFO("Creating pthread %s", name); if (task_mutex_initialized == false) { task_mutex_initialized = true; diff --git a/platforms/qurt/unresolved_symbols.c b/platforms/qurt/unresolved_symbols.c index 820ea1f4c4..c0c6bee577 100644 --- a/platforms/qurt/unresolved_symbols.c +++ b/platforms/qurt/unresolved_symbols.c @@ -4,6 +4,8 @@ #include #include +#define MODULE_NAME "unresolved_symbols" + __attribute__((visibility("default"))) void free(void *ptr) { qurt_free(ptr); diff --git a/src/drivers/qshell/qurt/qshell.cpp b/src/drivers/qshell/qurt/qshell.cpp index 7c5d795ba8..63d78885d3 100644 --- a/src/drivers/qshell/qurt/qshell.cpp +++ b/src/drivers/qshell/qurt/qshell.cpp @@ -160,7 +160,7 @@ int QShell::run_cmd(const std::vector &appargs) while (i < appargs.size() && appargs[i].c_str()[0] != '\0') { arg[i] = (char *)appargs[i].c_str(); - PX4_INFO(" arg%d = '%s'\n", i, arg[i]); + PX4_INFO(" arg%d = '%s'", i, arg[i]); ++i; } diff --git a/src/modules/muorb/apps/uORBAppsProtobufChannel.cpp b/src/modules/muorb/apps/uORBAppsProtobufChannel.cpp index 8669d0650d..437e328156 100644 --- a/src/modules/muorb/apps/uORBAppsProtobufChannel.cpp +++ b/src/modules/muorb/apps/uORBAppsProtobufChannel.cpp @@ -56,10 +56,10 @@ void uORB::AppsProtobufChannel::ReceiveCallback(const char *topic, if (_Debug) { PX4_INFO("Got Receive callback for topic %s", topic); } if (strcmp(topic, "slpi_debug") == 0) { - PX4_INFO("SLPI: %s", (const char *) data); + PX4_INFO("%s", (const char *) data); } else if (strcmp(topic, "slpi_error") == 0) { - PX4_ERR("SLPI: %s", (const char *) data); + PX4_ERR("%s", (const char *) data); } else if (IS_MUORB_TEST(topic)) { // Validate the test data received