Memfault Firmware SDK 0.25.0 (Build 300504)

Memfault Inc · Memfault Inc · commit 4ec7a0c61f9c · 2021-08-30T19:55:37.000-04:00
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,3 +1,34 @@
+### Changes between Memfault SDK 0.25.0 and SDK 0.24.2 - August 30, 2021
+
+#### :chart_with_upwards_trend: Improvements
+
+- Added a workaround to
+  [`event_storage.h`](components/include/memfault/core/event_storage.h)
+  to prevent compilation errors when using
+  [the unity test framework](http://www.throwtheswitch.org/unity) to generate
+  mocks.
+- Updated [`makefiles/MemfaultWorker.mk`](makefiles/MemfaultWorker.mk) to use
+  `sort` to guarantee a deterministic file list order irrespestive of
+  [make version](https://savannah.gnu.org/bugs/index.php?52076). A consistent
+  order is useful for
+  [reproducible builds](https://mflt.io/reproducible-builds).
+- Make use of `__has_include()` in Zephy port to remove the requirement of
+  always needing to create`memfault_platform_config.h`,
+  `memfault_metrics_heartbeat_config.def`, &
+  `memfault_trace_reason_user_config.def` for a build to compile. To force a
+  compile failure instead when any of these files do not exist, a user can set
+  [`CONFIG_MEMFAULT_USER_CONFIG_SILENT_FAIL=n`](ports/zephyr/Kconfig)
+
+#### :house: Internal
+
+- The current version of the Memfault Firmware SDK can now be accessed
+  programmatically from the
+  [`memfault/version.h`](components/include/memfault/version.h).
+- Improved HTTP util parser when dealing with malformed status codes
+- Updated
+  [nRF91 sample test app](examples/nrf-connect-sdk/nrf9160/memfault_demo_app) to
+  be compatible with nRF Connect SDK 1.6
+
 ### Changes between Memfault SDK 0.24.2 and SDK 0.24.1 - August 17, 2021
 
 #### :chart_with_upwards_trend: Improvements
diff --git a/VERSION b/VERSION
@@ -1,2 +1,2 @@
-BUILD ID: 294635
-GIT COMMIT: 0276687c3
+BUILD ID: 300504
+GIT COMMIT: b6e67d617
diff --git a/components/core/src/memfault_build_id.c b/components/core/src/memfault_build_id.c
@@ -16,14 +16,15 @@
 
 #if MEMFAULT_USE_GNU_BUILD_ID
 
-// Note: This variables is emitted by the linker script
+// Note: This variable is emitted by the linker script
 extern uint8_t __start_gnu_build_id_start[];
 
 MEMFAULT_BUILD_ID_QUALIFIER sMemfaultBuildIdStorage g_memfault_build_id = {
   .type = kMemfaultBuildIdType_GnuBuildIdSha1,
   .len = sizeof(sMemfaultElfNoteSection),
   .short_len = MEMFAULT_EVENT_INCLUDED_BUILD_ID_SIZE_BYTES,
   .storage = __start_gnu_build_id_start,
+  .sdk_version = MEMFAULT_SDK_VERSION,
 };
 #else
 
@@ -35,5 +36,6 @@ MEMFAULT_BUILD_ID_QUALIFIER sMemfaultBuildIdStorage g_memfault_build_id = {
   .len = sizeof(g_memfault_sdk_derived_build_id),
   .short_len = MEMFAULT_EVENT_INCLUDED_BUILD_ID_SIZE_BYTES,
   .storage = g_memfault_sdk_derived_build_id,
+  .sdk_version = MEMFAULT_SDK_VERSION,
 };
 #endif
diff --git a/components/core/src/memfault_build_id_private.h b/components/core/src/memfault_build_id_private.h
@@ -9,9 +9,11 @@
 //! Internal file that should never be included by a consumer of the SDK. See
 //! "memfault/core/build_info.h" for details on how to leverage the build id.
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include "memfault/core/compiler.h"
+#include "memfault/version.h"
 
 #ifdef __cplusplus
 extern "C" {
@@ -38,8 +40,13 @@ typedef struct {
   uint8_t short_len;
   uint8_t rsvd;
   const void *storage;
+  const sMfltSdkVersion sdk_version;
 } sMemfaultBuildIdStorage;
 
+MEMFAULT_STATIC_ASSERT(((offsetof(sMemfaultBuildIdStorage, type) == 0) &&
+                        (offsetof(sMemfaultBuildIdStorage, short_len) == 2)),
+                       "be sure to update fw_build_id.py!");
+
 #if defined(MEMFAULT_UNITTEST)
 //! NB: For unit tests we want to be able to instrument the data in the test
 //! so we drop the `const` qualifier
diff --git a/components/core/src/memfault_data_packetizer.c b/components/core/src/memfault_data_packetizer.c
@@ -126,7 +126,7 @@ typedef MEMFAULT_PACKED_STRUCT {
 
 static sMfltTransportState s_mflt_packetizer_state;
 
-static eMfltDataSourceMask s_active_data_sources = kMfltDataSourceMask_All;
+static uint32_t s_active_data_sources = kMfltDataSourceMask_All;
 
 void memfault_packetizer_set_active_sources(uint32_t mask) {
   memfault_packetizer_abort();
diff --git a/components/http/src/memfault_http_utils.c b/components/http/src/memfault_http_utils.c
@@ -8,6 +8,7 @@
 
 #include "memfault/http/utils.h"
 
+#include <limits.h>
 #include <stdio.h>
 #include <string.h>
 
@@ -245,6 +246,18 @@ static int prv_str_to_dec(const char *buf, size_t buf_len, int *value_out) {
     }
 
     int digit = c - '0';
+
+    // there's no limit to the size of a Content-Length value per specification:
+    // https://datatracker.ietf.org/doc/html/rfc7230#section-3.3.2
+    //
+    // status code is required to be 3 digits per:
+    // https://datatracker.ietf.org/doc/html/rfc7230#section-3.1.2
+    //
+    // any value that we can't fit in our variable is an error
+    if ((INT_MAX / 10) < (result + digit)) {
+      return -1; // result will overflow
+    }
+
     result = (result * 10) + digit;
   }
 
diff --git a/components/include/memfault/core/data_packetizer.h b/components/include/memfault/core/data_packetizer.h
@@ -153,13 +153,13 @@ typedef enum {
 //! @note This API can be used to prioritize the data source drained from the packetizer.
 //! This can be useful for use cases such as:
 //!  - Devices with multi connectivity toplogies (i.e BLE & WiFi) For example, in this situation a
-//!    user in this situation could choose to only enable Event and Log transfer when connected to
-//!    BLE and enable all sources when connected to WiFi
-//!  - Devices with extended periods where there is no connection to the internet In this
+//!    user could choose to only enable Event and Log transfer when connected to BLE and enable all
+//!    sources when connected to WiFi.
+//!  - Devices with extended periods where there is no connection to the internet. In this
 //!    situation, an end user may want to store data buffered in RAM (i.e events & logs) on flash
 //!    to minimize the RAM footprint and prevent data from being lost by an unexpected reset.
-//!    If an end user is already using a flash region to save coredumps, pre-encoded chunks
-//!    can of just events can be saved, i.e
+//!    If an end user is already saving coredumps in a dedicated flash region, pre-encoded chunks
+//!    can of just events can be saved as follows:
 //!
 //!     1. Only enable draining of events with the following API call:
 //!        memfault_packetizer_set_active_sources(kMfltDataSourceMask_Event);
diff --git a/components/include/memfault/core/event_storage.h b/components/include/memfault/core/event_storage.h
@@ -22,6 +22,13 @@
 #include <stddef.h>
 #include <stdint.h>
 
+// The unity test framework (http://www.throwtheswitch.org/unity) fails to generate mocks when
+// opaque pointers are used in a header.  To work around, the problem, we pull in the full internal
+// definition for "sMemfaultEventStorageImpl" when the unity framework is being used.
+#if defined(UNITY_INCLUDE_CONFIG_H)
+#include "memfault/core/event_storage_implementation.h"
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
diff --git a/components/include/memfault/version.h b/components/include/memfault/version.h
@@ -2,6 +2,9 @@
 
 //! @file
 //!
+//! Copyright (c) Memfault, Inc.
+//! See License.txt for details
+//!
 //! Contains Memfault SDK version information.
 
 #ifdef __cplusplus
@@ -11,12 +14,12 @@ extern "C" {
 #include <stdint.h>
 
 typedef struct {
-  uint16_t major;
-  uint16_t minor;
-  uint16_t patch;
+  uint8_t major;
+  uint8_t minor;
+  uint8_t patch;
 } sMfltSdkVersion;
 
-#define MEMFAULT_SDK_VERSION   { .major = 0, .minor = 24, .patch = 2 }
+#define MEMFAULT_SDK_VERSION   { .major = 0, .minor = 25, .patch = 0 }
 
 #ifdef __cplusplus
 }
diff --git a/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/CMakeLists.txt b/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/CMakeLists.txt
@@ -12,6 +12,33 @@ if(DEFINED ENV{MEMFAULT_INCLUDE_ZEPHYR_BOILERPLATE})
   include($ENV{ZEPHYR_BASE}/../nrf/cmake/boilerplate.cmake)
 endif()
 
+# Starting in nRF Connect SDK >= 1.3, versions are exposed in
+# a CMake package:
+#   https://github.com/nrfconnect/sdk-nrf/blob/master/share/ncs-package/cmake/NcsConfig.cmake
+#
+# We can pre-load the package before Zephyr to dynamically change Kconfig options based on version
+# which is not something that can be achieved with Zephyr today
+find_package(Ncs HINTS $ENV{ZEPHYR_BASE}/../nrf)
+
+# Conditionally set Kconfig variables based on nRF Connect SDK version
+# (since setting non-existent variables result in a build time error which
+# would break compiling older builds)
+#
+# Zephyrs Kconfig extension pulls variables via the "CMake variable cache"
+# so we set the ones we need here using that:
+#   https://docs.zephyrproject.org/latest/guides/build/kconfig/setting.html#the-initial-configuration
+if (NCS_VERSION_MAJOR)
+  if (${NCS_VERSION_MAJOR} LESS_EQUAL 1 AND ${NCS_VERSION_MINOR} LESS 6 )
+    # Required for logging to work from crash, deprecated in Zephyr included in NCS 1.6
+    set(CONFIG_LOG_IMMEDIATE y CACHE INTERNAL "")
+  elseif (${NCS_VERSION_MAJOR} LESS_EQUAL 1 AND ${NCS_VERSION_MINOR} GREATER_EQUAL 6 )
+    set(CONFIG_MEMFAULT_NCS_PROVISION_CERTIFICATES n CACHE INTERNAL "")
+    set(CONFIG_MEMFAULT_NCS_DEVICE_ID_RUNTIME y CACHE INTERNAL "")
+
+    set(CONFIG_NEWLIB_LIBC y CACHE INTERNAL "")
+  endif()
+endif()
+
 find_package(Zephyr HINTS $ENV{ZEPHYR_BASE})
 project(hello_world)
 
diff --git a/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/prj.conf b/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/prj.conf
@@ -24,9 +24,6 @@ CONFIG_MEMFAULT_ROOT_CERT_STORAGE_NRF9160_MODEM=y
 CONFIG_MEMFAULT_HTTP_ENABLE=y
 CONFIG_MEMFAULT_HTTP_PERIODIC_UPLOAD=y
 
-# Required for logging to work from crash
-CONFIG_LOG_IMMEDIATE=y
-
 # Required for logging backends to work as expected
 CONFIG_LOG=y
 CONFIG_LOG_BACKEND_UART=y
@@ -83,4 +80,8 @@ CONFIG_BSD_LIBRARY_SYS_INIT=n
 CONFIG_LTE_AUTO_INIT_AND_CONNECT=n
 CONFIG_AT_CMD_SYS_INIT=n
 
-CONFIG_WATCHDOG=y
+CONFIG_WATCHDOG=y
+
+# Note: See ./CMakeLists.txt for additional Kconfig variables settings as not all variables can be
+# set in a single prj.conf for backward compatibility with older versions of the nRF Connect SDK /
+# Zephyr
diff --git a/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/main.c b/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/main.c
@@ -5,10 +5,6 @@
 //!
 //! Entry point to Memfault Radio. In this file you will find:
 //!  1. Implementation for memfault_platform_get_device_info()
-//!    - The firmware version uses the Memfault integration with the GNU Build ID
-//!      (https://mflt.io/gnu-build-id) to guarantee dev builds always have a unique
-//!      version
-//!    - Looks up the IMEI and uses that as the device_serial
 //!  2. g_mflt_http_client_config dependency which needs to be filled in with your Project Key
 //!  3. A call to install the Root Certs used by Memfault on the nRF91 modem
 //!     (memfault_nrfconnect_port_install_root_certs())
@@ -74,7 +70,13 @@ static int prv_init_modem_lib(void) {
 static int prv_init_modem_lib(void) {
   return bsdlib_init();
 }
-#else /* nRF Connect SDK >= 1.5 */
+#elif (NCS_VERSION_MAJOR == 1) && (NCS_VERSION_MINOR <= 5)
+#include <modem/nrf_modem_lib.h>
+static int prv_init_modem_lib(void) {
+  return nrf_modem_lib_init(NORMAL_MODE);
+}
+#else /* nRF Connect SDK >= 1.6 */
+#include "memfault_ncs.h"
 #include <modem/nrf_modem_lib.h>
 static int prv_init_modem_lib(void) {
   return nrf_modem_lib_init(NORMAL_MODE);
@@ -85,31 +87,28 @@ static int prv_init_modem_lib(void) {
 #include <dfu/mcuboot.h>
 #endif
 
+// Since the example app manages when the modem starts/stops, we manually configure the device
+// serial even when using nRF Connect SDKs including the Memfault integration (by using
+// CONFIG_MEMFAULT_NCS_DEVICE_ID_RUNTIME)
+#define IMEI_LEN 15
+static char s_device_serial[IMEI_LEN + 1 /* '\0' */] = "unknown";
+
+// A direct Memfault integration was added in 1.6
+#if (NCS_VERSION_MAJOR == 1) && (NCS_VERSION_MINOR < 6)
+
+// Note: Starting with nRF Connect SDK 1.6, there is a direct integration of Memfault
+// and these dependencies are no longer needed!
+// See https://mflt.io/nrf-connect-sdk-lib for more details
+
+static char s_fw_version[16] = "1.0.0";
+
 sMfltHttpClientConfig g_mflt_http_client_config = {
   .api_key = "<YOUR PROJECT KEY HERE>",
 };
 
-static char s_fw_version[16]="1.0.0+";
-
-#define IMEI_LEN 15
-static char s_device_serial[IMEI_LEN + 1 /* '\0' */];
-
 void memfault_platform_get_device_info(sMemfaultDeviceInfo *info) {
   static bool s_init = false;
 
-  if (!s_init) {
-    const size_t version_len = strlen(s_fw_version);
-    // We will use 6 characters of the build id to make our versions unique and
-    // identifiable between releases
-    const size_t build_id_chars = 6 + 1 /* '\0' */;
-
-    const size_t build_id_num_chars =
-        MEMFAULT_MIN(build_id_chars, sizeof(s_fw_version) - version_len - 1);
-
-    memfault_build_id_get_string(&s_fw_version[version_len], build_id_num_chars);
-    s_init = true;
-  }
-
   // platform specific version information
   *info = (sMemfaultDeviceInfo) {
     .device_serial = s_device_serial,
@@ -119,6 +118,13 @@ void memfault_platform_get_device_info(sMemfaultDeviceInfo *info) {
   };
 }
 
+// stub for function implemented in nRF Connect SDK >= 1.6
+static int memfault_ncs_device_id_set(const char *device_id, size_t len) {
+  return 0;
+}
+
+#endif
+
 static int query_modem(const char *cmd, char *buf, size_t buf_len) {
   enum at_cmd_state at_state;
   int ret = at_cmd_write(cmd, buf, buf_len, &at_state);
@@ -143,6 +149,9 @@ static void prv_init_device_info(void) {
   strcat(s_device_serial, imei_buf);
 
   printk("Device Serial: %s\n", s_device_serial);
+
+  // register the device id with memfault port so it is used for reporting
+  memfault_ncs_device_id_set(s_device_serial, IMEI_LEN);
 }
 
 void main(void) {
diff --git a/makefiles/MemfaultWorker.mk b/makefiles/MemfaultWorker.mk
@@ -28,11 +28,18 @@ MEMFAULT_COMPONENTS_DIR := $(MEMFAULT_SDK_ROOT)/components
 
 MEMFAULT_COMPONENTS_INC_FOLDERS := $(MEMFAULT_COMPONENTS_DIR)/include
 
-MEMFAULT_COMPONENTS_SRCS = $(foreach component, $(MEMFAULT_COMPONENTS), $(wildcard $(MEMFAULT_COMPONENTS_DIR)/$(component)/src/*.c))
+MEMFAULT_COMPONENTS_SRCS = \
+  $(foreach component, $(MEMFAULT_COMPONENTS), \
+    $(sort $(wildcard $(MEMFAULT_COMPONENTS_DIR)/$(component)/src/*.c)) \
+  )
 
 ifneq ($(filter demo,$(MEMFAULT_COMPONENTS)),)
 # The demo component is enabled so let's pick up component specific cli commands
-MEMFAULT_COMPONENTS_SRCS += $(foreach component, $(MEMFAULT_COMPONENTS), $(wildcard $(MEMFAULT_COMPONENTS_DIR)/demo/src/$(component)/*.c))
+MEMFAULT_COMPONENTS_SRCS += \
+  $(foreach component, $(MEMFAULT_COMPONENTS), \
+    $(sort $(wildcard $(MEMFAULT_COMPONENTS_DIR)/demo/src/$(component)/*.c)) \
+  )
 endif
 
-MEMFAULT_COMPONENTS_SRCS := $(patsubst %memfault_fault_handling_xtensa.c, , $(MEMFAULT_COMPONENTS_SRCS))
+MEMFAULT_COMPONENTS_SRCS := \
+  $(patsubst %memfault_fault_handling_xtensa.c, , $(MEMFAULT_COMPONENTS_SRCS))
diff --git a/ports/zephyr/CMakeLists.txt b/ports/zephyr/CMakeLists.txt
@@ -38,11 +38,13 @@ if(CONFIG_MEMFAULT)
   # INTERFACE as the scope so users will pickup this define as expected.
   zephyr_compile_definitions(MEMFAULT_PLATFORM_CONFIG_FILE=\"memfault_zephyr_platform_config.h\")
 
-  # We have some port-specific examples for Zepher so pull them in
-  # by overriding this #define. The Zephyr port def file will pull
-  # in the user's file via include directive.
+  # We automatically collect some Zephyr kernel metrics from a custom Zephyr port def file. The
+  # Zephyr port def file will pull in the user's file via include directive.
   zephyr_compile_definitions(MEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE=\"memfault_metrics_heartbeat_zephyr_port_config.def\")
 
+  # Zephyr specific port def for trace errors
+  zephyr_compile_definitions(MEMFAULT_TRACE_REASON_USER_DEFS_FILE=\"memfault_trace_reason_zephyr_port_config.def\")
+
   if(${MEMFAULT_ZEPHYR_PORT_TARGET} MATCHES "^v2\.*")
     add_subdirectory(v2.x)
   endif()
diff --git a/ports/zephyr/Kconfig b/ports/zephyr/Kconfig
@@ -30,6 +30,17 @@ config MEMFAULT_USER_CONFIG_ENABLE
           memfault_metrics_heartbeat_config.def
           memfault_trace_reason_user_config.def
 
+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_COREDUMP_STORAGE_CUSTOM
         bool
         default n
diff --git a/ports/zephyr/common/memfault_http_periodic_upload.c b/ports/zephyr/common/memfault_http_periodic_upload.c
diff --git a/ports/zephyr/config/memfault_metrics_heartbeat_zephyr_port_config.def b/ports/zephyr/config/memfault_metrics_heartbeat_zephyr_port_config.def
diff --git a/ports/zephyr/config/memfault_trace_reason_zephyr_port_config.def b/ports/zephyr/config/memfault_trace_reason_zephyr_port_config.def
diff --git a/ports/zephyr/config/memfault_zephyr_platform_config.h b/ports/zephyr/config/memfault_zephyr_platform_config.h
diff --git a/scripts/mflt-build-id/pyproject.toml b/scripts/mflt-build-id/pyproject.toml
diff --git a/tests/makefiles/Makefile_memfault_http_utils.mk b/tests/makefiles/Makefile_memfault_http_utils.mk
diff --git a/tests/src/test_memfault_http_utils.cpp b/tests/src/test_memfault_http_utils.cpp
