Merge branch 'exter_wrap' into 'main'

diag: Support to use externally wrapped logging APIs See merge request app-frameworks/esp-insights!158
espressif · Aug 8, 2023 · d082711 · d082711
2 parents 56eec5c + db956c1
commit d082711
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 18 deletions.
diff --git a/components/esp_diagnostics/CMakeLists.txt b/components/esp_diagnostics/CMakeLists.txt
@@ -34,7 +34,9 @@ idf_component_register(SRCS "${srcs}"
                     INCLUDE_DIRS "include"
                     PRIV_REQUIRES ${priv_req})
 
-set(WRAP_FUNCTIONS esp_log_write esp_log_writev)
+if(NOT CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP)
+    set(WRAP_FUNCTIONS esp_log_write esp_log_writev)
+endif()
 
 if(CONFIG_LIB_BUILDER_COMPILE)
     list(APPEND WRAP_FUNCTIONS log_printf)

diff --git a/components/esp_diagnostics/Kconfig b/components/esp_diagnostics/Kconfig
@@ -96,4 +96,12 @@ menu "Diagnostics"
         default n
         help
             Enable more advanced network variables
+
+    config DIAG_USE_EXTERNAL_LOG_WRAP
+        bool "Use external log wrapper"
+        default n
+        help
+            Diagnostics component wraps the esp_log_write and esp_log_writev APIs using `--wrap` gcc option.
+            There can be scenario where another component also wants to wrap the logging functions.
+            In that case, enable this option and use the data ingestion APIs esp_diag_log_write and esp_diag_log_writev.
 endmenu
diff --git a/components/esp_diagnostics/include/esp_diagnostics.h b/components/esp_diagnostics/include/esp_diagnostics.h
@@ -298,6 +298,39 @@ uint32_t esp_diag_meta_crc_get(void);
  */
 uint32_t esp_diag_data_size_get_crc(void);
 
+
+/**
+ * @brief Convenience API for ingesting log data into diagnostics when esp_log_writev() is externally wrapped.
+ *        This API should be called from __wrap_esp_log_writev(). \see CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP.
+ *
+ * @param[in] level  Log level
+ * @param[in] tag    Tag of the log
+ * @param[in] format Format of the log
+ * @param[in] v      Variable argument list
+ *
+ * @note The Diagnostics component wraps the esp_log_write() and esp_log_writev() APIs using the `--wrap` GCC option
+ *       to collect logs. If another component intends to wrap the logging APIs, enable the configuration option
+ *       CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP. This will prevent the Diagnostics component from wrapping the logging APIs.
+ *       To enable log diagnostics in such case, call the esp_diag_log_writev() and esp_diag_log_write() APIs within
+ *       their respective externally wrapped APIs.
+ *
+ * @note Avoid calling this API explicitly unless there is an use case as the one described above.
+ */
+void esp_diag_log_writev(esp_log_level_t level, const char *tag, const char *format, va_list v);
+
+/**
+ * @brief Convenience API for ingesting log data into diagnostics when esp_log_write() is externally wrapped.
+ *        This API should be called from __wrap_esp_log_write(). \see CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP.
+ *
+ * @param[in] level  Log level
+ * @param[in] tag    Tag of the log
+ * @param[in] format Format of the log
+ * @param[in] v      variable argument list
+ *
+ * @note Please see notes from \see esp_diag_log_writev()
+ */
+void esp_diag_log_write(esp_log_level_t level, const char *tag, const char *format, va_list v);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/components/esp_diagnostics/src/esp_diagnostics_log_hook.c b/components/esp_diagnostics/src/esp_diagnostics_log_hook.c
@@ -427,6 +427,38 @@ void __wrap_log_printf(const char *format, ...)
     va_end(list);
 }
 #endif
+
+void esp_diag_log_writev(esp_log_level_t level,
+                         const char *tag,
+                         const char *format,
+                         va_list args)
+{
+#ifndef CONFIG_DIAG_LOG_DROP_WIFI_LOGS
+    /* Only collect logs with "wifi" tag */
+    if (strcmp(tag, "wifi") == 0) {
+        uint32_t pc = 0;
+        pc = esp_cpu_process_stack_pc((uint32_t)__builtin_return_address(0));
+        esp_diag_log(level, pc, tag, format, args);
+    }
+#endif /* !CONFIG_DIAG_LOG_DROP_WIFI_LOGS */
+}
+
+void esp_diag_log_write(esp_log_level_t level,
+                        const char *tag,
+                        const char *format,
+                        va_list list)
+{
+#ifndef BOOTLOADER_BUILD
+    /* Logs with "wifi" tag, will be collected in esp_log_writev() */
+    if (strcmp(tag, "wifi") != 0) {
+        uint32_t pc = 0;
+        pc = esp_cpu_process_stack_pc((uint32_t)__builtin_return_address(0));
+        esp_diag_log(level, pc, tag, format, list);
+    }
+#endif
+}
+
+#if !CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP
 /* Wrapping esp_log_write() and esp_log_writev() reduces the
  * changes required in esp_log module to support diagnostics
  */
@@ -440,15 +472,7 @@ void __wrap_esp_log_writev(esp_log_level_t level,
                            const char *format,
                            va_list args)
 {
-#ifndef CONFIG_DIAG_LOG_DROP_WIFI_LOGS
-    /* Only collect logs with "wifi" tag */
-    if (strcmp(tag, "wifi") == 0) {
-        uint32_t pc = 0;
-        pc = esp_cpu_process_stack_pc((uint32_t)__builtin_return_address(0));
-        esp_diag_log(level, pc, tag, format, args);
-    }
-#endif /* !CONFIG_DIAG_LOG_DROP_WIFI_LOGS */
-
+    esp_diag_log_write(level, tag, format, args);
     __real_esp_log_writev(level, tag, format, args);
 }
 
@@ -458,14 +482,8 @@ void __wrap_esp_log_write(esp_log_level_t level,
 {
     va_list list;
     va_start(list, format);
-#ifndef BOOTLOADER_BUILD
-    /* Logs with "wifi" tag, will be collected in esp_log_writev() */
-    if (strcmp(tag, "wifi") != 0) {
-        uint32_t pc = 0;
-        pc = esp_cpu_process_stack_pc((uint32_t)__builtin_return_address(0));
-        esp_diag_log(level, pc, tag, format, list);
-    }
-#endif
+    esp_diag_log_writev(level, tag, format, list);
     esp_log_writev(level, tag, format, list);
     va_end(list);
 }
+#endif // CONFIG_DIAG_USE_EXTERNAL_LOG_WRAP