Memfault Firmware SDK 1.6.0 (Build 5464)

Author: Memfault Inc

CHANGELOG.md

@@ -1,5 +1,64 @@
 # Memfault Firmware SDK Changelog
 
+## [1.6.0] - 2024-01-09
+
+### :chart_with_upwards_trend: Improvements
+
+- ESP-IDF:
+
+  - In the [ESP32 example app](examples/esp32), disable the shell history cache
+    for the FatFS shell command handler, which saves some ~20kB of heap memory.
+
+  - Also in the [ESP32 example app](examples/esp32), add a new built-in metric
+    `sync_successful` which is set to `1` when a successful Memfault OTA
+    check-in is performed. Primarily added for demonstration purposes for the
+    metric. See more information about Memfault Core Metrics
+    [here](https://docs.memfault.com/docs/platform/memfault-core-metrics).
+
+- General:
+
+  - Enhanced the Self-Test component with the following new features:
+
+    - Check that other components are initialized correctly (boot test)
+    - Check that exporting data over the console is supported (primiarly
+      checking that the output buffer is large enough)
+    - Print information about the coredump regions that would be collected
+      during a crash, for diagnostic purposes
+
+- nRF-Connect SDK:
+
+  - Fix a use-after-free problem in the Memfault FOTA helper code
+    ([`ports/zephyr/ncs/src/memfault_fota.c`](ports/zephyr/ncs/src/memfault_fota.c)),
+    where the allocated Memfault OTA download URL was freed too early. This
+    issue was introduced in Memfault SDK v1.5.0, where support for FOTA on
+    nRF-Connect SDK v2.4+ was improved.
+
+- Zephyr:
+
+  - Fix a concurrent access bug in the Memfault Zephyr Logging Backend. Only
+    affected configurations with `CONFIG_LOG_MODE_IMMEDIATE=y`. In certain cases
+    the logging `ctx` could be corrupted, causing unpredictable behavior.
+    Replace the synchronization approach with an atomic primitive and correct a
+    potential concurrency issue in the log panic handler.
+
+  - Fix a build warning for certain Zephyr configurations (when using
+    `CONFIG_NEWLIB_LIBC=y`) due to a missing declaration for `strnlen`. This
+    warning was introduced with the Memfault Self-Test additions in Memfault SDK
+    v1.5.2.
+
+### :boom: Breaking Changes
+
+- General:
+
+  - The Battery Metrics platform API has been consolidated into a single
+    function,
+    `int memfault_platform_get_stateofcharge(sMfltPlatformBatterySoc *soc)`,
+    where the platform data is loaded into the `soc` struct. This should
+    simplify the platform implementation, and enables platforms to return a
+    non-zero value to indicate state-of-charge is unknown. See the new API in
+    the header file
+    [`memfault/metrics/platform/battery.h`](components/include/memfault/metrics/platform/battery.h)
+
 ## [1.5.2] - 2023-12-12
 
 ### :chart_with_upwards_trend: Improvements

Dockerfile

@@ -0,0 +1,41 @@
+FROM ubuntu:22.04
+
+
+# Install dependencies like wget
+RUN apt-get update && apt-get install -y --no-install-recommends \
+    ca-certificates \
+    gnupg \
+    wget \
+    && rm -rf /var/lib/apt/lists/*
+
+# # Add llvm apt src
+# RUN cat > /etc/apt/sources.list.d/llvm.sources <<EOF
+# Types: deb
+# URIs: https://example.com/apt
+# Suites: stable
+# Components: main
+# Signed-By:
+# $(wget -O- https://apt.llvm.org/llvm-snapshot.gpg.key | sed -e 's/^$/./' -e 's/^/ /')
+# EOF
+
+# RUN chmod 644 /etc/apt/sources.list.d/llvm.sources
+
+# RUN wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add - \
+#     && echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy main" >> /etc/apt/sources.list.d/llvm.list \
+#     echo deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy main\
+#      >> /etc/apt/sources.list.d/llvm.list
+# echo deb-src http://apt.llvm.org/jammy/ llvm-toolchain-jammy main\
+#      >> /etc/apt/sources.list.d/llvm.list
+#     && apt-get update
+
+# RUN apt-get update && apt-get install -y --no-install-recommends \
+#     clang-17 \
+#     llvm-17 \
+#     && rm -rf /var/lib/apt/lists/*
+
+
+RUN \
+    echo deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main >> /etc/apt/sources.list.d/llvm.list && \
+    echo deb-src http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main >> /etc/apt/sources.list.d/llvm.list && \
+    wget -q -O - http://apt.llvm.org/llvm-snapshot.gpg.key|apt-key add - && \
+    apt-get update && apt-get install -y clang-17 llvm-17

VERSION

@@ -1,3 +1,3 @@
-BUILD ID: 5100
-GIT COMMIT: 6c3661b0e
-VERSION: 1.5.2
+BUILD ID: 5464
+GIT COMMIT: 96c12257a
+VERSION: 1.6.0

components/core/src/memfault_event_storage.c

@@ -467,3 +467,23 @@ size_t memfault_event_storage_bytes_free(void) {
 
   return bytes_free;
 }
+
+bool memfault_event_storage_booted(void) {
+  // The event storage component does not have any internal state we can check to see if
+  // memfault_events_storage_boot was called. As an indirect method, we can check the value of the
+  // circular buffer used internally. If the buffer structure has been initialized (storage pointer
+  // is not null), then we can assume that memfault_events_storage_boot was called.
+  // This check breaks the circular buffer API contract but is intentional for simplicity/efficiency
+  return (s_event_storage.storage != NULL);
+}
+
+// Resets the internal structures of the event storage component. This function is to only be used
+// in event storage unit tests. Declaration here to silence -Wmissing-prototypes
+void memfault_event_storage_reset(void);
+void memfault_event_storage_reset(void) {
+  // Delete the circular buffer and read/write state
+  // NB: storage implementation is const so cannot be reset
+  memset(&s_event_storage, 0, sizeof(s_event_storage));
+  s_event_storage_write_state = (sMemfaultEventStorageWriteState){ 0 };
+  s_event_storage_read_state = (sMemfaultEventStorageReadState){ 0 };
+}

components/core/src/memfault_log.c

@@ -483,3 +483,7 @@ void memfault_log_reset(void) {
     .enabled = false,
   };
 }
+
+bool memfault_log_booted(void) {
+  return s_memfault_ram_logger.enabled;
+}

components/core/src/memfault_ram_reboot_info_tracking.c

@@ -274,3 +274,8 @@ void memfault_reboot_tracking_clear_reboot_reason(void) {
     .is_valid = false,
   };
 }
+
+bool memfault_reboot_tracking_booted(void) {
+  return ((s_mflt_reboot_info != NULL) &&
+          (s_mflt_reboot_info->magic == MEMFAULT_REBOOT_INFO_MAGIC));
+}

components/core/src/memfault_self_test.c

@@ -5,16 +5,23 @@
 //!
 //! @brief
 //! Memfault SDK self test functions to verify SDK integration
+
 #include <ctype.h>
+#include <inttypes.h>
 #include <stdbool.h>
 #include <string.h>
 
 #include "memfault/core/build_info.h"
+#include "memfault/core/data_export.h"
 #include "memfault/core/debug_log.h"
+#include "memfault/core/event_storage.h"
 #include "memfault/core/math.h"
 #include "memfault/core/platform/device_info.h"
+#include "memfault/core/reboot_tracking.h"
 #include "memfault/core/self_test.h"
-#include "memfault_reboot_tracking_private.h"
+#include "memfault/core/trace_event.h"
+#include "memfault/panics/coredump.h"
+#include "memfault/panics/coredump_impl.h"
 #include "memfault_self_test_private.h"
 
 typedef enum {
@@ -112,7 +119,7 @@ static uint32_t prv_validate_device_info(void) {
 }
 
 static uint32_t prv_validate_build_id(void) {
-  char build_id_str[(MEMFAULT_BUILD_ID_LEN * 2) + 1] = {0};
+  char build_id_str[(MEMFAULT_BUILD_ID_LEN * 2) + 1] = { 0 };
   uint32_t result = 0;
 
   if (!memfault_build_id_get_string(build_id_str, MEMFAULT_ARRAY_SIZE(build_id_str))) {
@@ -123,8 +130,7 @@ static uint32_t prv_validate_build_id(void) {
 }
 
 static void prv_device_info_test_describe(uint32_t results) {
-  MEMFAULT_LOG_INFO("Device Info Test Results");
-  MEMFAULT_LOG_INFO("------------------------");
+  MEMFAULT_SELF_TEST_PRINT_HEADER("Device Info Test");
 
   if (results == 0) {
     MEMFAULT_LOG_INFO("All fields valid");
@@ -138,6 +144,7 @@ static void prv_device_info_test_describe(uint32_t results) {
       MEMFAULT_LOG_ERROR("%s invalid", s_device_info_field_names[i]);
     }
   }
+  MEMFAULT_LOG_INFO(MEMFAULT_SELF_TEST_END_OUTPUT);
 }
 
 uint32_t memfault_self_test_device_info_test(void) {
@@ -151,7 +158,123 @@ uint32_t memfault_self_test_device_info_test(void) {
   return results;
 }
 
+typedef enum MfltBootComponent {
+  kMfltBootComponent_EventStorage = 0,
+  kMfltBootComponent_Logging,
+  kMfltBootComponent_RebootTracking,
+  kMfltBootComponent_TraceEvent,
+  kMfltBootComponent_NumTypes,
+} eMfltBootComponent;
+
+static const struct {
+  char *component_name;
+  bool (*booted)(void);
+} s_boot_components[kMfltBootComponent_NumTypes] = {
+  [kMfltBootComponent_EventStorage] =
+    {
+      .component_name = "Event Storage",
+      memfault_event_storage_booted,
+    },
+  [kMfltBootComponent_Logging] =
+    {
+      .component_name = "Logging",
+      memfault_log_booted,
+    },
+  [kMfltBootComponent_RebootTracking] =
+    {
+      .component_name = "Reboot Tracking",
+      memfault_reboot_tracking_booted,
+    },
+  [kMfltBootComponent_TraceEvent] =
+    {
+      .component_name = "Trace Event",
+      memfault_trace_event_booted,
+    },
+};
+
+void memfault_self_test_component_boot_test(void) {
+  MEMFAULT_SELF_TEST_PRINT_HEADER("Component Boot Test");
+  MEMFAULT_LOG_INFO("%-16s|%8s|", "Component", "Booted?");
+  MEMFAULT_LOG_INFO("-----------------------------");
+  for (size_t i = 0; i < MEMFAULT_ARRAY_SIZE(s_boot_components); i++) {
+    bool booted = s_boot_components[i].booted();
+    MEMFAULT_LOG_INFO("%-16s|%8s|", s_boot_components[i].component_name, booted ? "yes" : "no");
+  }
+  MEMFAULT_LOG_INFO(MEMFAULT_SELF_TEST_END_OUTPUT);
+}
+
+void memfault_self_test_data_export_test(void) {
+  char test_line[MEMFAULT_DATA_EXPORT_BASE64_CHUNK_MAX_LEN] = { 0 };
+  // Set every char to '+' except for the last char before null terminator
+  memset(test_line, '+', MEMFAULT_ARRAY_SIZE(test_line) - 2);
+  // Last character before null should be '1'
+  test_line[MEMFAULT_DATA_EXPORT_BASE64_CHUNK_MAX_LEN - 2] = '1';
+
+  MEMFAULT_SELF_TEST_PRINT_HEADER("Data Export Line Test");
+  MEMFAULT_LOG_INFO("Printing %u characters, confirm line ends with '1' and is not split",
+                    (unsigned)(MEMFAULT_DATA_EXPORT_BASE64_CHUNK_MAX_LEN - 1));
+  MEMFAULT_LOG_INFO("%s", test_line);
+  MEMFAULT_LOG_INFO(MEMFAULT_SELF_TEST_END_OUTPUT);
+}
+
+static void prv_print_region_group_info(const char *group_name, const sMfltCoredumpRegion *regions,
+                                        const size_t num_regions) {
+  MEMFAULT_LOG_INFO("Coredump group: %s", group_name);
+  MEMFAULT_LOG_INFO("-----------------------------");
+  MEMFAULT_LOG_INFO("%10s|%10s|%6s|", "Address", "Length", "Type");
+  MEMFAULT_LOG_INFO("-----------------------------");
+  for (size_t i = 0; i < num_regions; i++) {
+    sMfltCoredumpRegion region = regions[i];
+    MEMFAULT_LOG_INFO("0x%08" PRIxPTR "|%10" PRIu32 "|%6u|", (uintptr_t)region.region_start,
+                      region.region_size, region.type);
+  }
+  MEMFAULT_LOG_INFO("-----------------------------");
+}
+
+uint32_t memfault_self_test_coredump_regions_test(void) {
+  uint32_t result = 0;
+  const sMfltCoredumpRegion *platform_regions = NULL;
+  size_t num_platform_regions = 0;
+
+  // Set stack address to current sp and unknown reboot reason
+  // These are dummy values so we can get as close as possible to the regions
+  // that memfault_platform_coredump_get_regions will return
+  sCoredumpCrashInfo info = {
+    .stack_address = &num_platform_regions,
+    .trace_reason = kMfltRebootReason_Unknown,
+  };
+
+  // Call memfault_platform_coredump_get_regions
+  platform_regions = memfault_platform_coredump_get_regions(&info, &num_platform_regions);
+
+  // Get arch and SDK regions
+  size_t num_arch_regions = 0;
+  const sMfltCoredumpRegion *arch_regions = memfault_coredump_get_arch_regions(&num_arch_regions);
+  size_t num_sdk_regions = 0;
+  const sMfltCoredumpRegion *sdk_regions = memfault_coredump_get_sdk_regions(&num_sdk_regions);
+
+  MEMFAULT_SELF_TEST_PRINT_HEADER("Coredump Regions Test");
+  if (num_platform_regions == 0) {
+    MEMFAULT_LOG_ERROR("Number of platform regions = 0");
+    result = 1;
+  }
+
+  if (platform_regions == NULL) {
+    MEMFAULT_LOG_ERROR("Platform regions was NULL");
+    result = 1;
+  }
+
+  prv_print_region_group_info("Platform Regions", platform_regions, num_platform_regions);
+  prv_print_region_group_info("Arch Regions", arch_regions, num_arch_regions);
+  prv_print_region_group_info("SDK Regions", sdk_regions, num_sdk_regions);
+  MEMFAULT_LOG_INFO(MEMFAULT_SELF_TEST_END_OUTPUT);
+  return result;
+}
+
 int memfault_self_test_run(void) {
   // Run each test one at a time and return result of all runs OR'd together
+  memfault_self_test_component_boot_test();
+  memfault_self_test_data_export_test();
+  memfault_self_test_coredump_regions_test();
   return (memfault_self_test_device_info_test() != 0);
 }

components/core/src/memfault_self_test_private.h

@@ -11,6 +11,18 @@
 extern "C" {
 #endif
 
+//! Macro for beginning of test string
+#define MEMFAULT_SELF_TEST_END_OUTPUT "=============================\n"
+//! Macro for end of test string
+#define MEMFAULT_SELF_TEST_BEGIN_OUTPUT "============================="
+
+#define MEMFAULT_SELF_TEST_PRINT_HEADER(test_name)      \
+  do {                                                  \
+    MEMFAULT_LOG_INFO(MEMFAULT_SELF_TEST_BEGIN_OUTPUT); \
+    MEMFAULT_LOG_INFO(test_name);                       \
+    MEMFAULT_LOG_INFO(MEMFAULT_SELF_TEST_BEGIN_OUTPUT); \
+  } while (0)
+
 // Checks if char fits regex: [-a-zA-z0-0_]
 bool memfault_self_test_valid_device_serial(unsigned char c);
 
@@ -23,6 +35,15 @@ bool memfault_self_test_valid_sw_version(unsigned char c);
 //! Runs device info and build ID test
 uint32_t memfault_self_test_device_info_test(void);
 
+//! Runs component boot test
+void memfault_self_test_component_boot_test(void);
+
+//! Data export test
+void memfault_self_test_data_export_test(void);
+
+//! Runs coredump regions test
+uint32_t memfault_self_test_coredump_regions_test(void);
+
 #ifdef __cplusplus
 }
 #endif