Memfault Firmware SDK 1.4.0 (Build 4099)

Memfault Inc · Memfault Inc · commit 66d696654a23 · 2023-10-24T19:22:45.000Z
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,5 +1,42 @@
 # Memfault Firmware SDK Changelog
 
+## [1.4.0] - 2023-10-23
+
+#### :rocket: New Features
+
+- ESP-IDF:
+
+  - Implement support for a "zero-config" integration on ESP-IDF. This change
+    makes the template + platform config files optional. See the
+    [ESP32 Integration Guide](https://mflt.io/esp-tutorial) section on
+    configuration files for details.
+
+### :chart_with_upwards_trend: Improvements
+
+- ESP-IDF:
+
+  - Fix a build issue when overriding the default device-info implementation,
+    `CONFIG_MEMFAULT_DEFAULT_GET_DEVICE_INFO=n`. This was a regression in
+    **1.3.3**.
+
+  - Add a new Kconfig flag, `MEMFAULT_COREDUMP_STORAGE_MAX_SIZE`, which can be
+    used to set the Memfault SDK's built-in
+    [ESP-IDF coredump storage implementation](sdk/embedded/ports/esp_idf/memfault/common/memfault_platform_coredump.c)
+    to artificially limit the maximum coredump storage size. This is useful for
+    situations where the default `memfault_platform_coredump_get_regions()`
+    function is still desirable, but the coredump maximum size needs to be
+    limited (eg for bandwidth reasons).
+
+  - Switch to using `ESP_DRAM_LOGE` when logging an error from Memfault's
+    FreeRTOS task tracking when the number of tasks created on the system
+    exceeds `MEMFAULT_PLATFORM_MAX_TRACKED_TASKS` (default of 16).
+
+- General:
+
+  - Remove the setting of the Memfault Firmware SDK version into a string metric
+    on system boot. This value is now automatically extracted by the Memfault
+    backend from the symbol file, and does not need to be set by devices.
+
 ## [1.3.5] - 2023-10-14
 
 ### :chart_with_upwards_trend: Improvements
diff --git a/VERSION b/VERSION
@@ -1,3 +1,3 @@
-BUILD ID: 3944
-GIT COMMIT: 429f30930
-VERSION: 1.3.5
+BUILD ID: 4099
+GIT COMMIT: bb1ba895f
+VERSION: 1.4.0
diff --git a/components/include/memfault/metrics/heartbeat_config.def b/components/include/memfault/metrics/heartbeat_config.def
@@ -2,9 +2,6 @@
 //! default health statistics for the fleet. All other metrics are provided by the user
 //! of the sdk via the MEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE file
 
-// Memfault SDK Version String
-#include "memfault/version.h"
-MEMFAULT_METRICS_STRING_KEY_DEFINE(MemfaultSdkMetric_sdk_version, sizeof(MEMFAULT_SDK_VERSION_STR) - 1)
 // The time since the last heartbeat was collected in ms
 MEMFAULT_METRICS_KEY_DEFINE(MemfaultSdkMetric_IntervalMs, kMemfaultMetricType_Timer)
 // The number of reboots that have taken place since the last heartbeat was collected
diff --git a/components/include/memfault/version.h b/components/include/memfault/version.h
@@ -19,8 +19,8 @@ typedef struct {
   uint8_t patch;
 } sMfltSdkVersion;
 
-#define MEMFAULT_SDK_VERSION   { .major = 1, .minor = 3, .patch = 5 }
-#define MEMFAULT_SDK_VERSION_STR "1.3.5"
+#define MEMFAULT_SDK_VERSION   { .major = 1, .minor = 4, .patch = 0 }
+#define MEMFAULT_SDK_VERSION_STR "1.4.0"
 
 #ifdef __cplusplus
 }
diff --git a/components/metrics/src/memfault_metrics.c b/components/metrics/src/memfault_metrics.c
@@ -904,9 +904,6 @@ int memfault_metrics_boot(const sMemfaultEventStorageImpl *storage_impl,
     return rv;
   }
 
-  rv = memfault_metrics_heartbeat_set_string(MEMFAULT_METRICS_KEY(MemfaultSdkMetric_sdk_version),
-                                             MEMFAULT_SDK_VERSION_STR);
-
   rv = prv_init_unexpected_reboot_metric();
   if (rv != 0) {
     return rv;
diff --git a/ports/esp_idf/memfault/CMakeLists.txt b/ports/esp_idf/memfault/CMakeLists.txt
@@ -188,15 +188,50 @@ if (CONFIG_MEMFAULT_FREERTOS_TASK_RUNTIME_STATS)
 endif()
 
 component_compile_options(-DMEMFAULT_ESP_HTTP_CLIENT_ENABLE=$ENV{MEMFAULT_ESP_HTTP_CLIENT_ENABLE})
+
+# FreeRTOS error logging redirect for ESP-IDF compatibility.
+component_compile_options(
+  -DMEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG_INCLUDE="memfault_platform_freertos_error_log.h"
+)
+
 # Set the heartbeat config file to use the ESP-IDF port file, which will bring
 # in the user's heartbeat config automatically. Set it as a global compiler
 # option so it properly affects both component compilation and when the metric
 # keys are used in the application.
+#
+# Set the ESP-IDF specific Memfault platform config header as well.
 if (IDF_VERSION_MAJOR VERSION_EQUAL 3)
   # On pre-v4 of ESP-IDF, insert the compiler flag to the global CMAKE_C_FLAGS
-  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DMEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE=\\\"memfault_esp_metrics_heartbeat_config.def\\\"" CACHE STRING "Global C Flags" FORCE)
+  set(
+    memfault_c_flags
+    "-DMEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE=\\\"memfault_esp_metrics_heartbeat_config.def\\\""
+    "-DMEMFAULT_PLATFORM_CONFIG_FILE=\\\"memfault_esp_idf_port_config.h\\\""
+    "-DMEMFAULT_TRACE_REASON_USER_DEFS_FILE=\\\"memfault_trace_reason_esp_idf_port_config.def\\\""
+  )
+  # convert from list to string before setting
+  list(JOIN memfault_c_flags " " memfault_c_flags)
+  set(CMAKE_C_FLAGS
+  "${CMAKE_C_FLAGS} ${memfault_c_flags}"
+  CACHE STRING
+  "Global C Flags"
+  FORCE
+  )
 else()
-  idf_build_set_property(COMPILE_OPTIONS -DMEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE="memfault_esp_metrics_heartbeat_config.def" APPEND)
+  # set these as compile_options, not compile_definitions; pre-v5 of ESP-IDF
+  # required the -D prefix, post-v5 auto strips it, but for safety, set them as
+  # plain options not "definitions"
+  list(APPEND compile_options
+    "-DMEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE=\"memfault_esp_metrics_heartbeat_config.def\""
+    "-DMEMFAULT_PLATFORM_CONFIG_FILE=\"memfault_esp_idf_port_config.h\""
+    "-DMEMFAULT_TRACE_REASON_USER_DEFS_FILE=\"memfault_trace_reason_esp_idf_port_config.def\""
+  )
+  idf_build_set_property(
+    COMPILE_OPTIONS
+    "${compile_options}"
+    APPEND
+  )
+  # idf_build_get_property(bleh COMPILE_OPTIONS)
+  # message(FATAL_ERROR "COMPILE_OPTIONS=${bleh}")
 endif()
 
 # We will intercept the panic handlers enabled by CONFIG_ESP32_ENABLE_COREDUMP_TO_FLASH
diff --git a/ports/esp_idf/memfault/Kconfig b/ports/esp_idf/memfault/Kconfig
@@ -32,6 +32,20 @@ menu "Memfault"
             been shipped and the partition table cannot be modified, an
             OTA slot can be used instead.
 
+    config MEMFAULT_COREDUMP_STORAGE_MAX_SIZE
+        int "The maximum size of a coredump (0=partition max size)"
+        default 0
+        help
+            The maximum size of a coredump. If a coredump is larger than this
+            size, it will be truncated to this size. Must be aligned to
+            SPI_FLASH_SEC_SIZE. When set to =0 (the default), Memfault will
+            attempt to capture the entire RAM region of the device, up to the
+            maximum size of the `coredump` region. If it's desirable to
+            artificially reduce the utilized space in the `coredump` partition,
+            setting a non-zero value will cause the Memfault ESP-IDF port's
+            'memfault_platform_coredump_storage_get_info()' function to return
+            the threshold value set here as the size of the partition..
+
     config MEMFAULT_AUTOMATIC_INIT
         bool "[DEPRECATED] Automatically initialize the SDK when the system is booted"
         default n
@@ -40,29 +54,47 @@ menu "Memfault"
             `memfault_boot`. This option is deprecated and not supported.
             A build error will be generated when enabling this option.
 
-    menuconfig MEMFAULT_DEFAULT_GET_DEVICE_INFO
+    menu "Builtin device info implementation"
+
+    config MEMFAULT_DEFAULT_GET_DEVICE_INFO
         bool "Enables the default memfault_platform_get_device_info() implementation"
         default y
         help
             Enable a default implementation of
             memfault_platform_get_device_info(). Disable this config to
             implement a custom version of the function.
 
-    if MEMFAULT_DEFAULT_GET_DEVICE_INFO
-
     config MEMFAULT_DEVICE_INFO_HARDWARE_VERSION
-        string "Hardware version used in the default device info implementation"
+        string "Hardware version used in memfault_esp_port_get_device_info()"
         default "$(IDF_TARGET)-proto"
 
     config MEMFAULT_DEVICE_INFO_SOFTWARE_VERSION
-        string "Software version used in the default device info implementation"
+        string "Software version used in memfault_esp_port_get_device_info()"
         default "1.0.0-dev"
 
     config MEMFAULT_DEVICE_INFO_SOFTWARE_TYPE
-        string "Software type used in the default device info implementation"
+        string "Software type used in memfault_esp_port_get_device_info()"
         default "esp32-main"
 
-    endif # MEMFAULT_DEFAULT_GET_DEVICE_INFO
+    endmenu  # Builtin device info implementation
+
+    config MEMFAULT_PLATFORM_CONFIG_FILE
+        string "Name of the Memfault platform config file"
+        default "memfault_platform_config.h"
+        help
+            The name of the file which contains the Memfault platform config
+            options. This file must be included in the project's include path.
+
+    config MEMFAULT_USER_CONFIG_SILENT_FAIL
+        bool "Continue with build even if user configuration of Memfault SDK is missing"
+        default y
+        help
+          When enabled, __has_include is used to conditionally include the three Memfault
+          configuration files to a port if they exist instead of failing to compile if
+          they do not:
+          memfault_platform_config.h
+          memfault_metrics_heartbeat_config.def
+          memfault_trace_reason_user_config.def
 
     config MEMFAULT_HTTP_CLIENT_TIMEOUT_MS
         int "The HTTP client timeout in milliseconds"
diff --git a/ports/esp_idf/memfault/common/memfault_platform_coredump.c b/ports/esp_idf/memfault/common/memfault_platform_coredump.c
@@ -298,12 +298,26 @@ void memfault_platform_coredump_storage_get_info(sMfltCoredumpStorageInfo *info)
   // sanity check that the memory holding the info is populated and not corrupted
   const esp_partition_t *core_part = prv_validate_and_get_core_partition();
   if (core_part == NULL) {
-    *info = (sMfltCoredumpStorageInfo) { 0 };
+    *info = (sMfltCoredumpStorageInfo){0};
     return;
   }
 
-  *info  = (sMfltCoredumpStorageInfo) {
-    .size = core_part->size,
+#if defined(CONFIG_MEMFAULT_COREDUMP_STORAGE_LIMIT_SIZE)
+  // confirm MAX_SIZE is aligned to SPI_FLASH_SEC_SIZE
+  MEMFAULT_STATIC_ASSERT(
+    (CONFIG_MEMFAULT_COREDUMP_STORAGE_MAX_SIZE % SPI_FLASH_SEC_SIZE == 0),
+    "Error, check CONFIG_MEMFAULT_COREDUMP_STORAGE_MAX_SIZE value: " MEMFAULT_EXPAND_AND_QUOTE(
+      CONFIG_MEMFAULT_COREDUMP_STORAGE_MAX_SIZE));
+#endif
+
+  *info = (sMfltCoredumpStorageInfo) {
+    .size =
+#if CONFIG_MEMFAULT_COREDUMP_STORAGE_MAX_SIZE > 0
+      MEMFAULT_MIN(core_part->size, CONFIG_MEMFAULT_COREDUMP_STORAGE_MAX_SIZE)
+#else
+      core_part->size
+#endif
+      ,
     .sector_size = SPI_FLASH_SEC_SIZE,
   };
 }
diff --git a/ports/esp_idf/memfault/config/memfault_esp_idf_port_config.h b/ports/esp_idf/memfault/config/memfault_esp_idf_port_config.h
@@ -0,0 +1,35 @@
+#pragma once
+
+//! @file
+//!
+//! Copyright (c) Memfault, Inc.
+//! See License.txt for details
+//
+// ESP-IDF specific Memfault configs. This file has the following purposes:
+// 1. provide a way to set Memfault configs (default_config.h overrides) from
+//    ESP-IDF Kconfig flags
+// 2. remove the requirement for a user to provide "memfault_platform_config.h"
+//    themselves, if they don't need to override any default options
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "sdkconfig.h"
+
+// Pick up any user configuration overrides
+#if CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL
+
+  #if __has_include(CONFIG_MEMFAULT_PLATFORM_CONFIG_FILE)
+    #include CONFIG_MEMFAULT_PLATFORM_CONFIG_FILE
+  #endif
+
+#else
+
+  #include CONFIG_MEMFAULT_PLATFORM_CONFIG_FILE
+
+#endif /* CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL */
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/ports/esp_idf/memfault/config/memfault_esp_metrics_heartbeat_config.def b/ports/esp_idf/memfault/config/memfault_esp_metrics_heartbeat_config.def
@@ -3,7 +3,6 @@
 // Required to pull in Kconfig-generated definitions
 #include "sdkconfig.h"
 
-
 #if CONFIG_MEMFAULT_LWIP_METRICS
 #include "memfault_lwip_metrics_heartbeat_config.def"
 #endif  // CONFIG_MEMFAULT_LWIP_METRICS
@@ -24,5 +23,14 @@ MEMFAULT_METRICS_KEY_DEFINE(wifi_connected_time_ms, kMemfaultMetricType_Timer)
 MEMFAULT_METRICS_KEY_DEFINE(wifi_disconnect_count, kMemfaultMetricType_Unsigned)
 #endif // CONFIG_MEMFAULT_ESP_WIFI_METRICS
 
-// Include user metrics
+#if CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL
+
+# if __has_include("memfault_metrics_heartbeat_config.def")
+#   include "memfault_metrics_heartbeat_config.def"
+# endif
+
+#else
+
 #include "memfault_metrics_heartbeat_config.def"
+
+#endif /* CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL */
diff --git a/ports/esp_idf/memfault/config/memfault_platform_freertos_error_log.h b/ports/esp_idf/memfault/config/memfault_platform_freertos_error_log.h
@@ -0,0 +1,27 @@
+#pragma once
+
+//! @file
+//!
+//! Copyright (c) Memfault, Inc.
+//! See License.txt for details
+//!
+//! Copyright (c) Memfault, Inc.
+//! See License.txt for details
+//!
+//! Implement a special MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG() macro used
+//! when logging from the FreeRTOS task creation function.
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "esp_log.h"
+
+// Use the ESP-IDF logging 'ESP_DRAM_LOGx' log variant instead of the default
+// 'MEMFAULT_LOG_ERROR' macro. This should be safe to call in interrupt context
+// or when the logging subsystem has not been initialized (early init).
+#define MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG(...) ESP_DRAM_LOGE("mflt", __VA_ARGS__)
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/ports/esp_idf/memfault/config/memfault_trace_reason_esp_idf_port_config.def b/ports/esp_idf/memfault/config/memfault_trace_reason_esp_idf_port_config.def
@@ -0,0 +1,13 @@
+#include "sdkconfig.h"
+
+#if CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL
+
+# if __has_include("memfault_trace_reason_user_config.def")
+#   include "memfault_trace_reason_user_config.def"
+# endif
+
+#else
+
+#include "memfault_trace_reason_user_config.def"
+
+#endif /* CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL */
diff --git a/ports/freertos/src/memfault_freertos_ram_regions.c b/ports/freertos/src/memfault_freertos_ram_regions.c
@@ -122,12 +122,28 @@
   #endif
 #endif
 
-//! The maximum number
+//! Allow overriding the MEMFAULT_LOG_ERROR() call when the task registry is
+//! full and a new task is created. Some systems are unable to issue logs from
+//! the task creation calling context (i.e the task is created before the
+//! logging system is initialized).
+//!
+//! For example, for ESP-IDF, it might be appropriate to instead use:
+//! #define MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG(...) ESP_EARLY_LOGE("mflt", __VA_ARGS__)
+//!
+//! The overriding is done in a file included from here, in case additional
+//! headers are needed
+#if defined(MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG_INCLUDE)
+  #include MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG_INCLUDE
+#else
+  #define MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG MEMFAULT_LOG_ERROR
+#endif
+
+//! This array holds the tracked TCB references
 static void *s_task_tcbs[MEMFAULT_PLATFORM_MAX_TRACKED_TASKS];
 #define EMPTY_SLOT 0
 
 static bool prv_find_slot(size_t *idx, void *desired_tcb) {
-  for (size_t i = 0; i < MEMFAULT_PLATFORM_MAX_TRACKED_TASKS; i++) {
+  for (size_t i = 0; i < MEMFAULT_ARRAY_SIZE(s_task_tcbs); i++) {
     if (s_task_tcbs[i] == desired_tcb) {
       *idx = i;
       return true;
@@ -151,7 +167,8 @@ void memfault_freertos_trace_task_create(void *tcb) {
   }
 
   if (!slot_found) {
-    MEMFAULT_LOG_ERROR("Task registry full (%d)", MEMFAULT_PLATFORM_MAX_TRACKED_TASKS);
+    MEMFAULT_FREERTOS_REGISTRY_FULL_ERROR_LOG(
+      "Task registry full (" MEMFAULT_EXPAND_AND_QUOTE(MEMFAULT_PLATFORM_MAX_TRACKED_TASKS) ")");
   }
 }
 
diff --git a/scripts/mflt-build-id/pyproject.toml b/scripts/mflt-build-id/pyproject.toml
@@ -17,7 +17,7 @@ python = "^3.6" # must match memfault-cli
 pyelftools = ">=0.26,<=0.29"
 
 [tool.poetry.dev-dependencies]
-pytest = "^6"
+pytest = ">=6"
 pytest-cov = "^2"
 pytest-timeout = "^2.0"
 wasmer = "0.3.0"
diff --git a/scripts/tests_embedded_scripts/gdb_fake.py b/scripts/tests_embedded_scripts/gdb_fake.py
diff --git a/tests/src/test_memfault_heartbeat_metrics.cpp b/tests/src/test_memfault_heartbeat_metrics.cpp
diff --git a/tests/src/test_memfault_heartbeat_metrics_debug.cpp b/tests/src/test_memfault_heartbeat_metrics_debug.cpp
diff --git a/tests/src/test_memfault_heartbeat_metrics_nocustom.cpp b/tests/src/test_memfault_heartbeat_metrics_nocustom.cpp

Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,8 @@ typedef struct {`
`19`	`19`	`uint8_t patch;`
`20`	`20`	`} sMfltSdkVersion;`
`21`	`21`
`22`		`-#define MEMFAULT_SDK_VERSION { .major = 1, .minor = 3, .patch = 5 }`
`23`		`-#define MEMFAULT_SDK_VERSION_STR "1.3.5"`
	`22`	`+#define MEMFAULT_SDK_VERSION { .major = 1, .minor = 4, .patch = 0 }`
	`23`	`+#define MEMFAULT_SDK_VERSION_STR "1.4.0"`
`24`	`24`
`25`	`25`	`#ifdef __cplusplus`
`26`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -904,9 +904,6 @@ int memfault_metrics_boot(const sMemfaultEventStorageImpl *storage_impl,`
`904`	`904`	`return rv;`
`905`	`905`	`}`
`906`	`906`
`907`		`- rv = memfault_metrics_heartbeat_set_string(MEMFAULT_METRICS_KEY(MemfaultSdkMetric_sdk_version),`
`908`		`- MEMFAULT_SDK_VERSION_STR);`
`909`		`-`
`910`	`907`	`rv = prv_init_unexpected_reboot_metric();`
`911`	`908`	`if (rv != 0) {`
`912`	`909`	`return rv;`