diff --git a/CHANGES.md b/CHANGES.md index dcb92bb7e..fa205bbdc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,30 @@ +### Changes between Memfault SDK 0.42.0 and 0.41.2 - Mar 22, 2023 + +#### :chart_with_upwards_trend: Improvements + +- Zephyr: + + - Add option to capture full thread stacks for classifying stack overflows and + determining stack high watermarks. This feature is enabled by setting + `CONFIG_MEMFAULT_COREDUMP_FULL_THREAD_STACKS=y` + - Remove usage of the `zephyr.h` header in preparation for Zephyr v3.4.0 + +- `memfault_gdb.py`: + - Add support for exporting data from GCC 12 compiled symbol files + - Add arguments to override device serial ID, software type, software version, + and hardware revision + +#### :boom: Breaking Changes + +- Metrics: + - Integer type metrics (signed/unsigned) will reset to NULL when not set + during a heartbeat interval. This NULL value will be discarded by Memfault + when received. The previous behavior was to reset to 0 which makes + discarding values difficult as 0 is a valid value for these types. For more + info please see the + [Metrics](https://docs.memfault.com/docs/mcu/metrics-api#setting-metric-values) + docs. + ### Changes between Memfault SDK 0.41.2 and SDK 0.41.1 - Mar 10, 2023 #### :chart_with_upwards_trend: Improvements diff --git a/VERSION b/VERSION index 8d32e44b3..608ec32cf 100644 --- a/VERSION +++ b/VERSION @@ -1,2 +1,2 @@ -BUILD ID: 1696 -GIT COMMIT: e44b190c9 +BUILD ID: 1778 +GIT COMMIT: 539b8ab42 diff --git a/components/include/memfault/core/compiler_gcc.h b/components/include/memfault/core/compiler_gcc.h index e6bcd5ff2..7bc1f241c 100644 --- a/components/include/memfault/core/compiler_gcc.h +++ b/components/include/memfault/core/compiler_gcc.h @@ -26,7 +26,11 @@ extern "C" { #if defined(__clang__) #define MEMFAULT_NO_OPT __attribute__((optnone)) #else -#define MEMFAULT_NO_OPT __attribute__((optimize("O0"))) +// GCC 12 changed to no longer enable the var-tracking debug option when +// compiling without optimizations. Explicitly enable it so we always get a nice +// variable loading experience in GDB, even when the function prologue hasn't +// run yet. This is required when using the memfault_gdb.py hook to upload data. +#define MEMFAULT_NO_OPT __attribute__((optimize("O0", "var-tracking", "var-tracking-assignments"))) #endif #define MEMFAULT_ALIGNED(x) __attribute__((aligned(x))) diff --git a/components/include/memfault/core/math.h b/components/include/memfault/core/math.h index 79ad57e10..efd465e4d 100644 --- a/components/include/memfault/core/math.h +++ b/components/include/memfault/core/math.h @@ -18,6 +18,7 @@ extern "C" { #define MEMFAULT_MAX(a , b) (((a) > (b)) ? (a) : (b)) #define MEMFAULT_FLOOR(a, align) (((a) / (align)) * (align)) #define MEMFAULT_ABS(a) (((a) < 0) ? -(a) : (a)) +#define MEMFAULT_CEIL_DIV(x, y) (((x) + (y)-1) / (y)) #ifdef __cplusplus } diff --git a/components/include/memfault/metrics/metrics.h b/components/include/memfault/metrics/metrics.h index c855bdc65..9b533e5a9 100644 --- a/components/include/memfault/metrics/metrics.h +++ b/components/include/memfault/metrics/metrics.h @@ -116,7 +116,10 @@ typedef struct MemfaultMetricsBootInfo { } sMemfaultMetricBootInfo; //! Initializes the metric events API. -//! All heartbeat values will be initialized to 0. +//! All heartbeat values will be initialized to their reset values. +//! Integer types will be reset to unset/null. +//! Timer metrics will be reset to 0. +//! String metrics will be reset to empty strings. //! @param storage_impl The storage location to serialize metrics out to //! @param boot_info Info added to metrics to facilitate computing aggregate statistics in //! the Memfault cloud @@ -126,6 +129,9 @@ int memfault_metrics_boot(const sMemfaultEventStorageImpl *storage_impl, const sMemfaultMetricBootInfo *boot_info); //! Set the value of a signed integer metric. +//! +//! Integer metrics that are unset during a heartbeat interval +//! are sent as null and dropped when received. //! @param key The key of the metric. @see MEMFAULT_METRICS_KEY //! @param value The new value to set for the metric //! @return 0 on success, else error code @@ -162,7 +168,7 @@ int memfault_metrics_heartbeat_timer_stop(MemfaultMetricId key); //! @param key The key of the metric. @see MEMFAULT_METRICS_KEY //! @param inc The amount to increment the metric by //! @return 0 on success, else error code -//! @note The metric must be of type kMemfaultMetricType_Counter +//! @note The metric must be of type kMemfaultMetricType_Unsigned or kMemfaultMetricType_Signed int memfault_metrics_heartbeat_add(MemfaultMetricId key, int32_t amount); //! For debugging purposes: prints the current heartbeat values using diff --git a/components/include/memfault/metrics/utils.h b/components/include/memfault/metrics/utils.h index cc0c4c662..4c5fbf5dd 100644 --- a/components/include/memfault/metrics/utils.h +++ b/components/include/memfault/metrics/utils.h @@ -11,6 +11,8 @@ //! @note A user of the Memfault SDK should _never_ call any //! of these routines directly +#include + #include "memfault/metrics/metrics.h" #ifdef __cplusplus @@ -26,6 +28,7 @@ union MemfaultMetricValue { typedef struct { MemfaultMetricId key; eMemfaultMetricType type; + bool is_set; union MemfaultMetricValue val; } sMemfaultMetricInfo; diff --git a/components/include/memfault/util/cbor.h b/components/include/memfault/util/cbor.h index bdd39087b..a2458eab3 100644 --- a/components/include/memfault/util/cbor.h +++ b/components/include/memfault/util/cbor.h @@ -155,6 +155,11 @@ bool memfault_cbor_encode_uint64_as_double(sMemfaultCborEncoder *encoder, uint64 //! @return true on success, false otherwise bool memfault_cbor_encode_long_signed_integer(sMemfaultCborEncoder *encoder, int64_t value); +//! Encode a CBOR null value +//! +//! @param encoder The encoder context to use +//! @return true on success, false otherwise +bool memfault_cbor_encode_null(sMemfaultCborEncoder *encoder); //! NOTE: For internal use only, included in the header so it's easy for a caller to statically //! allocate the structure diff --git a/components/include/memfault/version.h b/components/include/memfault/version.h index 324107514..c216c7c65 100644 --- a/components/include/memfault/version.h +++ b/components/include/memfault/version.h @@ -19,7 +19,7 @@ typedef struct { uint8_t patch; } sMfltSdkVersion; -#define MEMFAULT_SDK_VERSION { .major = 0, .minor = 41, .patch = 2 } +#define MEMFAULT_SDK_VERSION { .major = 0, .minor = 42, .patch = 0 } #ifdef __cplusplus } diff --git a/components/metrics/src/memfault_metrics.c b/components/metrics/src/memfault_metrics.c index ffade5951..d27201d41 100644 --- a/components/metrics/src/memfault_metrics.c +++ b/components/metrics/src/memfault_metrics.c @@ -38,6 +38,7 @@ MEMFAULT_DISABLE_WARNING("-Wunused-macros") #define MEMFAULT_METRICS_TYPE_NO_CHANGE (-4) #define MEMFAULT_METRICS_STORAGE_TOO_SMALL (-5) #define MEMFAULT_METRICS_TIMER_BOOT_FAILED (-6) +#define MEMFAULT_METRICS_VALUE_NOT_SET (-7) typedef struct MemfaultMetricKVPair { MemfaultMetricId key; @@ -90,7 +91,7 @@ MEMFAULT_STATIC_ASSERT(MEMFAULT_ARRAY_SIZE(s_memfault_heartbeat_keys) != 0, #define MEMFAULT_METRICS_TIMER_VAL_MAX 0x80000000 typedef struct MemfaultMetricValueMetadata { - bool is_running:1; + bool is_running : 1; // We'll use 32 bits since the rollover time is ~25 days which is much much greater than a // reasonable heartbeat interval. This let's us track whether or not the timer is running in the // top bit @@ -100,6 +101,7 @@ typedef struct MemfaultMetricValueMetadata { typedef struct MemfaultMetricValueInfo { union MemfaultMetricValue *valuep; sMemfaultMetricValueMetadata *meta_datap; + bool is_set; } sMemfaultMetricValueInfo; @@ -180,6 +182,21 @@ static union MemfaultMetricValue s_memfault_heartbeat_values[] = { #undef MEMFAULT_METRICS_STRING_KEY_DEFINE }; +// Value Set flag data structures and definitions +// MEMFAULT_IS_SET_FLAGS_PER_BYTE must be a power of 2 +// MEMFAULT_IS_SET_FLAGS_DIVIDER must be equal to log2(MEMFAULT_IS_SET_FLAGS_PER_BYTE) +#define MEMFAULT_IS_SET_FLAGS_PER_BYTE 8 +#define MEMFAULT_IS_SET_FLAGS_DIVIDER 3 + +// Create a byte array to contain an is-set flag for each entry in s_memfault_heartbeat_values +static uint8_t s_memfault_heatbeat_value_is_set_flags[MEMFAULT_CEIL_DIV( + MEMFAULT_ARRAY_SIZE(s_memfault_heartbeat_values), MEMFAULT_IS_SET_FLAGS_PER_BYTE)]; + +MEMFAULT_STATIC_ASSERT( + MEMFAULT_ARRAY_SIZE(s_memfault_heatbeat_value_is_set_flags) >= + (MEMFAULT_ARRAY_SIZE(s_memfault_heartbeat_values) / MEMFAULT_IS_SET_FLAGS_PER_BYTE), + "Mismatch between s_memfault_heatbeat_value_is_set_flags and s_memfault_heartbeat_values"); + // String value lookup table. Const- the pointers do not change at runtime, so // this table can be stored in ROM and save a little RAM. #define MEMFAULT_METRICS_KEY_DEFINE(key_name, value_type) @@ -250,6 +267,12 @@ static const int s_metric_timer_metadata_mapping[] = { #undef MEMFAULT_METRICS_STRING_KEY_DEFINE }; +// Helper macros to convert between the various metrics indices +#define MEMFAULT_METRICS_ID_TO_KEY(id) ((size_t)(id)._impl) +#define MEMFAULT_METRICS_KEY_TO_KV_INDEX(key) (s_memfault_heartbeat_key_to_valueindex[(key)]) +#define MEMFAULT_METRICS_ID_TO_KV_INDEX(id) \ + (MEMFAULT_METRICS_KEY_TO_KV_INDEX(MEMFAULT_METRICS_ID_TO_KEY(id))) + static struct { const sMemfaultEventStorageImpl *storage_impl; } s_memfault_metrics_ctx; @@ -280,16 +303,36 @@ static sMemfaultMetricValueMetadata *prv_find_timer_metadatap(eMfltMetricsIndex return &s_memfault_heartbeat_timer_values_metadata[timer_index]; } -static eMemfaultMetricType prv_find_value_for_key(MemfaultMetricId key, +//! Helper function to read/write is_set bits for the provided metric +//! +//! @param id Metric ID to select corresponding is_set field +//! @param write Boolean to control whether to write 1 to is_set +//! @return Returns the value of metric's is_set field. The updated value is returned if write = +//! true +static bool prv_read_write_is_value_set(MemfaultMetricId id, bool write) { + // Shift the kv index by MEMFAULT_IS_SET_FLAGS_DIVIDER to select byte within + // s_memfault_heartbeat_value_is_set_flags + size_t byte_index = MEMFAULT_METRICS_ID_TO_KV_INDEX(id) >> MEMFAULT_IS_SET_FLAGS_DIVIDER; + // Modulo the kv index by MEMFAULT_IS_SET_FLAGS_PER_BYTE to get bit of the selected byte + size_t bit_index = MEMFAULT_METRICS_ID_TO_KV_INDEX(id) % MEMFAULT_IS_SET_FLAGS_PER_BYTE; + + if (write) { + s_memfault_heatbeat_value_is_set_flags[byte_index] |= (1 << bit_index); + } + + return (s_memfault_heatbeat_value_is_set_flags[byte_index] >> bit_index) & 0x01; +} + +static eMemfaultMetricType prv_find_value_for_key(MemfaultMetricId id, sMemfaultMetricValueInfo *value_info_out) { - const size_t idx = (size_t)key._impl; + const size_t idx = MEMFAULT_METRICS_ID_TO_KEY(id); if (idx >= MEMFAULT_ARRAY_SIZE(s_memfault_heartbeat_keys)) { *value_info_out = (sMemfaultMetricValueInfo){0}; return kMemfaultMetricType_NumTypes; } // get the index for the value matching this key. - eMfltMetricKeyToValueIndex key_index = s_memfault_heartbeat_key_to_valueindex[idx]; + eMfltMetricKeyToValueIndex key_index = MEMFAULT_METRICS_KEY_TO_KV_INDEX(idx); // for scalar types, this will be the returned value pointer. non-scalars // will be handled in the switch below union MemfaultMetricValue *value_ptr = &s_memfault_heartbeat_values[key_index]; @@ -321,6 +364,10 @@ static eMemfaultMetricType prv_find_value_for_key(MemfaultMetricId key, .meta_datap = prv_find_timer_metadatap((eMfltMetricsIndex)idx), }; + if (key_type == kMemfaultMetricType_Unsigned || key_type == kMemfaultMetricType_Signed) { + value_info_out->is_set = prv_read_write_is_value_set(id, false); + } + return key_type; } @@ -374,6 +421,7 @@ static int prv_find_and_set_value_for_key( } *value_info.valuep = *new_value; + prv_read_write_is_value_set(key, true); return 0; } @@ -516,6 +564,7 @@ static bool prv_tally_and_update_timer_cb(MEMFAULT_UNUSED void *ctx, static void prv_reset_metrics(void) { // reset all scalar metric values memset(s_memfault_heartbeat_values, 0, sizeof(s_memfault_heartbeat_values)); + memset(s_memfault_heatbeat_value_is_set_flags, 0, sizeof(s_memfault_heatbeat_value_is_set_flags)); // reset all string metric values. -1 to skip the last, stub entry in the // table @@ -525,6 +574,13 @@ static void prv_reset_metrics(void) { ((char *)s_memfault_heartbeat_string_values[i].ptr)[0] = 0; } } + + // Set MemfaultSdkMetric_UnexpectedRebootDidOccur to 0 at the end of each reset. + // This must be explicitly set because we use 0 to track number of operational hours + // Without setting to 0, this defaults to null and will not be counted in the sum + // of total operational hours + memfault_metrics_heartbeat_set_unsigned( + MEMFAULT_METRICS_KEY(MemfaultSdkMetric_UnexpectedRebootDidOccur), 0); } static void prv_heartbeat_timer_update(void) { @@ -608,6 +664,11 @@ static int prv_find_key_of_type(MemfaultMetricId key, eMemfaultMetricType expect if (type != expected_type) { return MEMFAULT_METRICS_TYPE_INCOMPATIBLE; } + if ((type == kMemfaultMetricType_Signed || type == kMemfaultMetricType_Unsigned) && + !(value_info.is_set)) { + return MEMFAULT_METRICS_VALUE_NOT_SET; + } + *value_out = value_info.valuep; return 0; } @@ -707,7 +768,8 @@ static bool prv_metrics_heartbeat_iterate_cb(void *ctx, sMemfaultMetricInfo info = { .key = key_info->key, .type = key_info->type, - .val = *value_info->valuep + .val = *value_info->valuep, + .is_set = value_info->is_set, }; return ctx_info->user_cb(ctx_info->user_ctx, &info); } @@ -748,12 +810,22 @@ static bool prv_heartbeat_debug_print(MEMFAULT_UNUSED void *ctx, const char *key_name = s_idx_to_metric_name[key->_impl]; switch (metric_info->type) { - case kMemfaultMetricType_Unsigned: case kMemfaultMetricType_Timer: MEMFAULT_LOG_DEBUG(" %s: %" PRIu32, key_name, value->u32); break; + case kMemfaultMetricType_Unsigned: + if (metric_info->is_set) { + MEMFAULT_LOG_DEBUG(" %s: %" PRIu32, key_name, value->u32); + } else { + MEMFAULT_LOG_DEBUG(" %s: null", key_name); + } + break; case kMemfaultMetricType_Signed: - MEMFAULT_LOG_DEBUG(" %s: %" PRIi32, key_name, value->i32); + if (metric_info->is_set) { + MEMFAULT_LOG_DEBUG(" %s: %" PRIi32, key_name, value->i32); + } else { + MEMFAULT_LOG_DEBUG(" %s: null", key_name); + } break; case kMemfaultMetricType_String: MEMFAULT_LOG_DEBUG(" %s: \"%s\"", key_name, (const char *)value->ptr); diff --git a/components/metrics/src/memfault_metrics_serializer.c b/components/metrics/src/memfault_metrics_serializer.c index ec3c501c0..2dd36983e 100644 --- a/components/metrics/src/memfault_metrics_serializer.c +++ b/components/metrics/src/memfault_metrics_serializer.c @@ -30,21 +30,41 @@ typedef struct { bool encode_success; } sMemfaultSerializerState; +static bool prv_metric_heartbeat_write_integer(sMemfaultSerializerState *state, + sMemfaultCborEncoder *encoder, + const sMemfaultMetricInfo *metric_info) { + if (!metric_info->is_set && !state->compute_worst_case_size) { + return memfault_cbor_encode_null(encoder); + } + + if (metric_info->type == kMemfaultMetricType_Unsigned) { + const uint32_t value = state->compute_worst_case_size ? UINT32_MAX : metric_info->val.u32; + return memfault_cbor_encode_unsigned_integer(encoder, value); + } + + if (metric_info->type == kMemfaultMetricType_Signed) { + const int32_t value = state->compute_worst_case_size ? INT32_MIN : metric_info->val.i32; + return memfault_cbor_encode_signed_integer(encoder, value); + } + + // Should be unreachable + return false; +} + static bool prv_metric_heartbeat_writer(void *ctx, const sMemfaultMetricInfo *metric_info) { sMemfaultSerializerState *state = (sMemfaultSerializerState *)ctx; sMemfaultCborEncoder *encoder = &state->encoder; // encode the value switch (metric_info->type) { - case kMemfaultMetricType_Timer: - case kMemfaultMetricType_Unsigned: { + case kMemfaultMetricType_Timer: { const uint32_t value = state->compute_worst_case_size ? UINT32_MAX : metric_info->val.u32; state->encode_success = memfault_cbor_encode_unsigned_integer(encoder, value); break; } + case kMemfaultMetricType_Unsigned: case kMemfaultMetricType_Signed: { - const int32_t value = state->compute_worst_case_size ? INT32_MIN : metric_info->val.i32; - state->encode_success = memfault_cbor_encode_signed_integer(encoder, value); + state->encode_success = prv_metric_heartbeat_write_integer(state, encoder, metric_info); break; } case kMemfaultMetricType_String: { diff --git a/components/util/src/memfault_minimal_cbor.c b/components/util/src/memfault_minimal_cbor.c index cc26b3fd5..95a91f52d 100644 --- a/components/util/src/memfault_minimal_cbor.c +++ b/components/util/src/memfault_minimal_cbor.c @@ -22,10 +22,23 @@ typedef enum CborMajorType { kCborMajorType_SimpleType = 7, } eCborMajorType; +// Definitions for Additional Info for Simple Values can be found here: +// https://www.rfc-editor.org/rfc/rfc7049#section-2.3 +typedef enum CborAddInfoSimpleVals { + kCborAddInfoSimpleVals_False = 20, + kCborAddInfoSimpleVals_True = 21, + kCborAddInfoSimpleVals_Null = 22, + kCborAddInfoSimpleVals_Undefined = 23, +} eCborAddInfoSimpleVals; + // A CBOR payload is composed of a stream of "data items" // The main type of each "data item" is known as the CBOR Major Type // and populates the upper 3 bits of the first byte of each "data item" -#define CBOR_SERIALIZE_MAJOR_TYPE(mt) ((uint8_t) ((mt & 0x7) << 5)) +#define CBOR_SERIALIZE_MAJOR_TYPE(mt) ((uint8_t)((mt & 0x7) << 5)) + +// CBOR NULL is formed by using a major type of SimpleValue and the assigned value of null +#define CBOR_NULL \ + (CBOR_SERIALIZE_MAJOR_TYPE(kCborMajorType_SimpleType) | kCborAddInfoSimpleVals_Null) void memfault_cbor_encoder_init(sMemfaultCborEncoder *encoder, MemfaultCborWriteCallback write_cb, void *write_cb_ctx, size_t buf_len) { @@ -184,3 +197,8 @@ bool memfault_cbor_encode_array_begin( sMemfaultCborEncoder *encoder, size_t num_elements) { return prv_encode_unsigned_integer(encoder, kCborMajorType_Array, num_elements); } + +bool memfault_cbor_encode_null(sMemfaultCborEncoder *encoder) { + uint8_t tmp_buf[] = {CBOR_NULL}; + return prv_add_to_result_buffer(encoder, tmp_buf, sizeof(tmp_buf)); +} 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 index d9d8bf3dc..83e34961c 100644 --- a/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/main.c +++ b/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/main.c @@ -10,12 +10,9 @@ //! (memfault_nrfconnect_port_install_root_certs()) //! 4. Start of the LTE modem & AT interface -#include "memfault_demo_app.h" - -#include - -#include #include +#include +#include #include #include "memfault/core/build_info.h" @@ -25,6 +22,7 @@ #include "memfault/http/http_client.h" #include "memfault/nrfconnect_port/http.h" #include "memfault/ports/ncs/version.h" +#include "memfault_demo_app.h" // nRF Connect SDK < 1.3 #if (NCS_VERSION_MAJOR == 1) && (NCS_VERSION_MINOR < 3) diff --git a/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/watchdog.c b/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/watchdog.c index 91920d305..b333b9f25 100644 --- a/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/watchdog.c +++ b/examples/nrf-connect-sdk/nrf9160/memfault_demo_app/src/watchdog.c @@ -10,11 +10,10 @@ #include #include +#include #include -#include #include "memfault/components.h" - #include "memfault/ports/zephyr/version.h" //! Note: The timeout must be large enough to give us enough time to capture a coredump diff --git a/examples/zephyr/qemu/qemu-app/src/main.c b/examples/zephyr/qemu/qemu-app/src/main.c index 53c778f31..955201f3c 100644 --- a/examples/zephyr/qemu/qemu-app/src/main.c +++ b/examples/zephyr/qemu/qemu-app/src/main.c @@ -8,9 +8,9 @@ #include #include #include -#include +#include #include -#include +#include #include "memfault/components.h" diff --git a/examples/zephyr/stm32l4_disco/apps/memfault_demo_app/src/main.c b/examples/zephyr/stm32l4_disco/apps/memfault_demo_app/src/main.c index a24626d65..aaddefea3 100644 --- a/examples/zephyr/stm32l4_disco/apps/memfault_demo_app/src/main.c +++ b/examples/zephyr/stm32l4_disco/apps/memfault_demo_app/src/main.c @@ -3,7 +3,7 @@ //! Copyright (c) Memfault, Inc. //! See License.txt for details -#include +#include #include LOG_MODULE_REGISTER(mflt_app, LOG_LEVEL_DBG); diff --git a/ports/zephyr/Kconfig b/ports/zephyr/Kconfig index 4593caad1..ca7488cff 100644 --- a/ports/zephyr/Kconfig +++ b/ports/zephyr/Kconfig @@ -234,6 +234,18 @@ config MEMFAULT_COREDUMP_STACK_SIZE_TO_COLLECT The larger the size, the more stack frames Memfault can recover for tasks. The default setting typically allows for 4 or more frames to be recovered. +config MEMFAULT_COREDUMP_FULL_THREAD_STACKS + bool "Collect full thread stacks in coredumps" + default n + depends on THREAD_STACK_INFO + help + When enabled, Memfault will collect the full thread stack in + coredumps. This will likely increase the size of coredumps, and there + will no longer be a strict ceiling on the coredump task region sizes. + This is provided as an alternative when capturing all of RAM is not + viable but full thread stack analysis (watermarking, stack overflow + tagging) is desired. + config MEMFAULT_COREDUMP_MAX_TRACKED_TASKS int "Maximum amount of tasks to collect in a coredump" default 32 diff --git a/ports/zephyr/common/memfault_http_periodic_upload.c b/ports/zephyr/common/memfault_http_periodic_upload.c index 212632ab7..c99645e05 100644 --- a/ports/zephyr/common/memfault_http_periodic_upload.c +++ b/ports/zephyr/common/memfault_http_periodic_upload.c @@ -3,15 +3,13 @@ //! Copyright (c) Memfault, Inc. //! See License.txt for details -#include "memfault/ports/zephyr/http.h" - -#include "memfault/core/data_packetizer.h" -#include "memfault/core/debug_log.h" - #include #include #include -#include + +#include "memfault/core/data_packetizer.h" +#include "memfault/core/debug_log.h" +#include "memfault/ports/zephyr/http.h" #if CONFIG_MEMFAULT_HTTP_PERIODIC_UPLOAD_USE_DEDICATED_WORKQUEUE static K_THREAD_STACK_DEFINE( diff --git a/ports/zephyr/common/memfault_platform_coredump_regions.c b/ports/zephyr/common/memfault_platform_coredump_regions.c index c0c809482..a62e085f7 100644 --- a/ports/zephyr/common/memfault_platform_coredump_regions.c +++ b/ports/zephyr/common/memfault_platform_coredump_regions.c @@ -7,13 +7,11 @@ //! The default regions to collect on Zephyr when a crash takes place. //! Function is defined as weak so an end user can override it. -#include "memfault/panics/platform/coredump.h" -#include "memfault/panics/arch/arm/cortex_m.h" - -#include #include #include +#include "memfault/panics/arch/arm/cortex_m.h" +#include "memfault/panics/platform/coredump.h" #include "memfault/ports/zephyr/version.h" #if MEMFAULT_ZEPHYR_VERSION_GT(2, 1) diff --git a/ports/zephyr/common/memfault_platform_http.c b/ports/zephyr/common/memfault_platform_http.c index 8e1cfefc3..d305d9be3 100644 --- a/ports/zephyr/common/memfault_platform_http.c +++ b/ports/zephyr/common/memfault_platform_http.c @@ -11,7 +11,6 @@ #include #include #include -#include #include "memfault/core/compiler.h" #include "memfault/core/data_packetizer.h" diff --git a/ports/zephyr/common/memfault_platform_metrics.c b/ports/zephyr/common/memfault_platform_metrics.c index b6c29227c..2866b8bd9 100644 --- a/ports/zephyr/common/memfault_platform_metrics.c +++ b/ports/zephyr/common/memfault_platform_metrics.c @@ -3,8 +3,7 @@ //! Copyright (c) Memfault, Inc. //! See License.txt for details -#include - +#include #include #include "memfault/metrics/metrics.h" diff --git a/ports/zephyr/common/memfault_software_watchdog.c b/ports/zephyr/common/memfault_software_watchdog.c index 7a0b35fbb..842243b56 100644 --- a/ports/zephyr/common/memfault_software_watchdog.c +++ b/ports/zephyr/common/memfault_software_watchdog.c @@ -8,11 +8,11 @@ //! The implementation uses the k_timer_ module. When the timer expires, //! the implementation will assert so a coredump of the system state is captured. +#include + #include "memfault/panics/assert.h" #include "memfault/ports/watchdog.h" -#include - static void prv_software_watchdog_timeout(struct k_timer *dummy) { MEMFAULT_SOFTWARE_WATCHDOG(); } diff --git a/ports/zephyr/common/memfault_zephyr_ram_regions.c b/ports/zephyr/common/memfault_zephyr_ram_regions.c index 618c3c3df..d13040b61 100644 --- a/ports/zephyr/common/memfault_zephyr_ram_regions.c +++ b/ports/zephyr/common/memfault_zephyr_ram_regions.c @@ -6,14 +6,12 @@ //! Implements convenience APIs that can be used when building the set of //! RAM regions to collect as part of a coredump. See header for more details/ -#include "memfault/ports/zephyr/coredump.h" - -#include #include #include #include #include "memfault/components.h" +#include "memfault/ports/zephyr/coredump.h" #include "memfault/ports/zephyr/version.h" // Use this config flag to manually select the old data region names @@ -129,19 +127,34 @@ size_t memfault_zephyr_get_task_regions(sMfltCoredumpRegion *regions, size_t num continue; } - void *sp = (void*)thread->callee_saved.psp; + // When capturing full thread stacks, also include the active thread. Note + // that the active stack may already be partially collected in a previous + // region, so we might be duplicating it here; it's a little wasteful, but + // it's good to always prioritize the currently running stack, in case the + // coredump is truncated due to lack of space. +#if !CONFIG_MEMFAULT_COREDUMP_FULL_THREAD_STACKS if ((uintptr_t)_kernel.cpus[0].current == (uintptr_t)thread) { - // thread context is only valid when task is _not_ running so we skip collecting it + // when collecting partial stacks, thread context is only valid when task is _not_ running so we skip collecting it continue; } +#endif + + void *sp = (void *)thread->callee_saved.psp; #if defined(CONFIG_THREAD_STACK_INFO) // We know where the top of the stack is. Use that information to shrink // the area we need to collect if less than CONFIG_MEMFAULT_COREDUMP_STACK_SIZE_TO_COLLECT // is in use const uint32_t stack_top = thread->stack_info.start + thread->stack_info.size; + + #if CONFIG_MEMFAULT_COREDUMP_FULL_THREAD_STACKS + // Capture the entire stack for this thread + size_t stack_size_to_collect = thread->stack_info.size; + sp = thread->stack_info.start; + #else size_t stack_size_to_collect = MEMFAULT_MIN(stack_top - (uint32_t)sp, CONFIG_MEMFAULT_COREDUMP_STACK_SIZE_TO_COLLECT); + #endif #else size_t stack_size_to_collect = CONFIG_MEMFAULT_COREDUMP_STACK_SIZE_TO_COLLECT; #endif diff --git a/requirements.txt b/requirements.txt index 483572f64..a479804d0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,3 +8,5 @@ colorama==0.4.3 fastcov==1.14 pytest==7.0.1 pytest-xdist==3.0.2 +pytest-mock==3.10.0 +snapshottest==0.6.0 diff --git a/scripts/fw_build_id.py b/scripts/fw_build_id.py index 726d49e56..bddabc77b 100644 --- a/scripts/fw_build_id.py +++ b/scripts/fw_build_id.py @@ -18,9 +18,9 @@ # Released SDK: sys.path.insert(0, bundled_mflt_build_id_src_dir) -from mflt_build_id import * # noqa: E402,F401,F403,M900 +from mflt_build_id import * # noqa: E402, F403 if __name__ == "__main__": - from mflt_build_id import main # noqa: M900 + from mflt_build_id import main main() diff --git a/scripts/memfault_gdb.py b/scripts/memfault_gdb.py index 95f56e208..cebc9f0b9 100644 --- a/scripts/memfault_gdb.py +++ b/scripts/memfault_gdb.py @@ -553,7 +553,6 @@ def _try_read_register(arch, frame, lookup_name, register_list, analytics_props, _add_reg_collection_error_analytic( arch, analytics_props, lookup_name, traceback.format_exc() ) - pass def lookup_registers_from_list(arch, info_reg_all_list, analytics_props): @@ -612,7 +611,6 @@ def _search_list_for_alt_name(reg, found_registers): check_and_patch_reglist_for_fault(register_list, analytics_props) except Exception: analytics_props["fault_register_recover_error"] = {"traceback": traceback.format_exc()} - pass return register_list @@ -640,11 +638,11 @@ class MemfaultCoredumpBlockType(object): # (IntEnum): # trying to be python2.7 class MemfaultCoredumpWriter(object): - def __init__(self, arch): - self.device_serial = "DEMOSERIALNUMBER" - self.software_version = "1.0.0" - self.software_type = "main" - self.hardware_revision = "DEVBOARD" + def __init__(self, arch, device_serial, software_type, software_version, hardware_revision): + self.device_serial = device_serial + self.software_version = software_version + self.software_type = software_type + self.hardware_revision = hardware_revision self.trace_reason = 5 # Debugger Halted self.regs = {} self.sections = [] @@ -1185,7 +1183,7 @@ def stop(self): if status != 202: print("Chunk Post Failed with Http Status Code {}".format(status)) print("Reason: {}".format(reason)) - print("ERROR: Disabling Memfault GDB Chunk Handler and Halting" "") + print("ERROR: Disabling Memfault GDB Chunk Handler and Halting") self.enabled = False return True @@ -1319,6 +1317,12 @@ def parse_args(self, unicode_args): class MemfaultCoredump(MemfaultGdbCommand): """Captures a coredump from the target and uploads it to Memfault for analysis""" + ALPHANUM_SLUG_DOTS_COLON_REGEX = r"^[-a-zA-Z0-9_\.\+:]+$" + ALPHANUM_SLUG_DOTS_COLON_SPACES_PARENS_SLASH_REGEX = r"^[-a-zA-Z0-9_\.\+: \(\)\[\]/]+$" + DEFAULT_CORE_DUMP_HARDWARE_REVISION = "DEVBOARD" + DEFAULT_CORE_DUMP_SERIAL_NUMBER = "DEMOSERIALNUMBER" + DEFAULT_CORE_DUMP_SOFTWARE_TYPE = "main" + DEFAULT_CORE_DUMP_SOFTWARE_VERSION = "1.0.0" GDB_CMD = "memfault coredump" def _check_permission(self, analytics_props): @@ -1382,7 +1386,7 @@ def _invoke(self, unicode_args, from_tty, analytics_props, config): ANALYTICS.error("Missing login or key") return - cd_writer, elf_fn = self.build_coredump_writer(parsed_args.region, analytics_props) + cd_writer, elf_fn = self.build_coredump_writer(parsed_args, analytics_props) if not cd_writer: return @@ -1390,7 +1394,7 @@ def _invoke(self, unicode_args, from_tty, analytics_props, config): # TODO: try calling memfault_platform_get_device_info() and use returned info if present # Populate the version based on hash of the .elf for now: - software_version = "1.0.0-md5+{}".format(elf_hash[:8]) + software_version = cd_writer.software_version + "-md5+{}".format(elf_hash[:8]) cd_writer.software_version = software_version if not parsed_args.no_symbols: @@ -1464,6 +1468,54 @@ def _auto_int(x): nargs=2, action="append", ) + + def _character_check(regex, name): + def _check_inner(input): + pattern = re.compile(regex) + if not pattern.match(input): + raise argparse.ArgumentTypeError( + "Invalid characters in {}: {}.".format(name, input) + ) + + return input + + return _check_inner + + parser.add_argument( + "--device-serial", + type=_character_check(self.ALPHANUM_SLUG_DOTS_COLON_REGEX, "device serial"), + help="Overrides the device serial that will be reported in the core dump. (default: {})".format( + self.DEFAULT_CORE_DUMP_SERIAL_NUMBER + ), + default=self.DEFAULT_CORE_DUMP_SERIAL_NUMBER, + ) + parser.add_argument( + "--software-type", + type=_character_check(self.ALPHANUM_SLUG_DOTS_COLON_REGEX, "software type"), + help="Overrides the software type that will be reported in the core dump. (default: {})".format( + self.DEFAULT_CORE_DUMP_SOFTWARE_TYPE + ), + default=self.DEFAULT_CORE_DUMP_SOFTWARE_TYPE, + ) + parser.add_argument( + "--software-version", + type=_character_check( + self.ALPHANUM_SLUG_DOTS_COLON_SPACES_PARENS_SLASH_REGEX, "software version" + ), + help="Overrides the software version that will be reported in the core dump. (default: {})".format( + self.DEFAULT_CORE_DUMP_SOFTWARE_VERSION + ), + default=self.DEFAULT_CORE_DUMP_SOFTWARE_VERSION, + ) + parser.add_argument( + "--hardware-revision", + type=_character_check(self.ALPHANUM_SLUG_DOTS_COLON_REGEX, "hardware revision"), + help="Overrides the hardware revision that will be reported in the core dump. (default: {})".format( + self.DEFAULT_CORE_DUMP_HARDWARE_REVISION + ), + default=self.DEFAULT_CORE_DUMP_HARDWARE_REVISION, + ) + return populate_config_args_and_parse_args(parser, unicode_args, config) @staticmethod @@ -1477,7 +1529,7 @@ def _get_arch(current_arch, analytics_props): analytics_props["xtensa_target"] = target return None - def build_coredump_writer(self, regions, analytics_props): + def build_coredump_writer(self, parsed_args, analytics_props): # inferior.architecture() is a relatively new API, so let's use "show arch" instead: show_arch_output = gdb.execute("show arch", to_string=True).lower() current_arch_matches = re.search("currently ([^)]+)", show_arch_output) @@ -1551,11 +1603,18 @@ def build_coredump_writer(self, regions, analytics_props): "Hints: did you compile with -g or similar flags? did you inadvertently strip the binary?" ) - cd_writer = MemfaultCoredumpWriter(arch) + cd_writer = MemfaultCoredumpWriter( + arch, + parsed_args.device_serial, + parsed_args.software_type, + parsed_args.software_version, + parsed_args.hardware_revision, + ) cd_writer.regs = arch.get_current_registers(thread, analytics_props) arch.add_platform_specific_sections(cd_writer, inferior, analytics_props) + regions = parsed_args.region if regions is None: print( "No capturing regions were specified; will default to capturing the first 1MB for each used RAM address range." diff --git a/scripts/mflt-build-id/poetry.lock b/scripts/mflt-build-id/poetry.lock index bbe15590d..4acfaf729 100644 --- a/scripts/mflt-build-id/poetry.lock +++ b/scripts/mflt-build-id/poetry.lock @@ -327,5 +327,5 @@ testing = ["func-timeout", "jaraco.itertools", "pytest (>=4.6)", "pytest-black ( [metadata] lock-version = "2.0" -python-versions = "^3.6" -content-hash = "b227eaeff5463c7432b2ba97ceee6f6a27baef6e2ba1cc464a3f3eccc52fe5ec" +python-versions = "^3.6" # must match memfault-cli +content-hash = "18edcdf9c11c872ce59414cec53aa8ef6751816e00161c2566cf0f82eb4853da" diff --git a/scripts/mflt-build-id/pyproject.toml b/scripts/mflt-build-id/pyproject.toml index 4ad20d414..fedf9759c 100644 --- a/scripts/mflt-build-id/pyproject.toml +++ b/scripts/mflt-build-id/pyproject.toml @@ -14,7 +14,7 @@ mflt_build_id = 'mflt_build_id:main' [tool.poetry.dependencies] python = "^3.6" # must match memfault-cli -pyelftools = ">=0.26,<=0.28" +pyelftools = ">=0.26,<=0.29" [tool.poetry.dev-dependencies] pytest = "^6" diff --git a/scripts/tests_embedded_scripts/snapshots/__init__.py b/scripts/tests_embedded_scripts/snapshots/__init__.py new file mode 100644 index 000000000..d62d72023 --- /dev/null +++ b/scripts/tests_embedded_scripts/snapshots/__init__.py @@ -0,0 +1,5 @@ +# +# Copyright (c) Memfault, Inc. +# See License.txt for details +# + diff --git a/scripts/tests_embedded_scripts/snapshots/snap_test_memfault_gdb.py b/scripts/tests_embedded_scripts/snapshots/snap_test_memfault_gdb.py new file mode 100644 index 000000000..84e5aa611 --- /dev/null +++ b/scripts/tests_embedded_scripts/snapshots/snap_test_memfault_gdb.py @@ -0,0 +1,24 @@ +# +# Copyright (c) Memfault, Inc. +# See License.txt for details +# + +# -*- coding: utf-8 -*- +# snapshottest: v1 - https://goo.gl/zC4yUc +from __future__ import unicode_literals + +from snapshottest import Snapshot + +snapshots = Snapshot() + +snapshots[ + "test_coredump_command_with_login_no_existing_release_or_symbols[None--fixture2.bin] 1" +] = "434f524501000000e906100000000000000000005400000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000001000000044454d4f53455249414c4e554d4245520a0000000000000012000000312e302e302d6d64352b62363162326436630b00000000000000040000006d61696e040000000000000008000000444556424f415244070000000000000004000000280000000500000000000000040000000500000001000000fcffffff04000000a288485d0100000004e000e004000000414141410100000010e000e010000000414141414141414141414141414141410100000000ed00e08f000000414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414101000000fced00e004000000414141410100000000e100e0000500004141414141414141414141414141414141414141414141" + +snapshots[ + "test_coredump_command_with_login_no_existing_release_or_symbols[None--r 0x600000 8 -r 0x800000 4-fixture3.bin] 1" +] = "434f5245010000000107000000000000000000005400000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000001000000044454d4f53455249414c4e554d4245520a0000000000000012000000312e302e302d6d64352b62363162326436630b00000000000000040000006d61696e040000000000000008000000444556424f415244070000000000000004000000280000000500000000000000040000000500000001000000fcffffff04000000a288485d0100000004e000e004000000414141410100000010e000e010000000414141414141414141414141414141410100000000ed00e08f000000414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414141414101000000fced00e004000000414141410100000000e100e0000500004141414141414141414141414141414141414141414141" + +snapshots[ + "test_coredump_writer 1" +] = "434f52450100000002010000000000000000000054000000000000000101010102020202030303030404040405050505060606060707070708080808090909090a0a0a0a0b0b0b0b0c0c0c0c0d0d0d0d0e0e0e0e0f0f0f0f101010101111111112121212131313131414141402000000000000000d0000006465766963655f73657269616c0a0000000000000005000000312e322e330b00000000000000040000006d61696e0400000000000000090000006764622d70726f746f070000000000000004000000280000000500000000000000040000000500000001000000fcffffff04000000a288485d01000000040000000b00000068656c6c6f20776f726c64" diff --git a/scripts/tests_embedded_scripts/test_memfault_gdb.py b/scripts/tests_embedded_scripts/test_memfault_gdb.py index dc8b46d60..110a9e36c 100644 --- a/scripts/tests_embedded_scripts/test_memfault_gdb.py +++ b/scripts/tests_embedded_scripts/test_memfault_gdb.py @@ -22,7 +22,7 @@ sys.path.insert(0, scripts_dir) -from memfault_gdb import ( # noqa: E402 M900 +from memfault_gdb import ( # noqa: E402 ArmCortexMCoredumpArch, MemfaultConfig, MemfaultCoredump, @@ -221,7 +221,6 @@ def _settings_coredump_allowed(test_config): def test_parse_maintenance_info_sections_no_file(): - fn, sections = parse_maintenance_info_sections( """Remote serial target in gdb-specific protocol: Debugging a target over a serial line. @@ -232,7 +231,6 @@ def test_parse_maintenance_info_sections_no_file(): def test_parse_maintenance_info_sections_with_file(maintenance_info_sections_fixture, fake_elf): - fn, sections = parse_maintenance_info_sections(maintenance_info_sections_fixture) assert fn == str(fake_elf) assert len(sections) == 35 @@ -255,7 +253,6 @@ def test_read_current_registers(mocker, info_reg_all_fixture): def test_should_capture_section(): - # Never capture .text, even when NOT marked READONLY: assert not should_capture_section(Section(0, 10, ".text", read_only=False)) @@ -273,7 +270,6 @@ def test_should_capture_section(): def test_armv7_get_used_ram_base_addresses(): - sections = ( Section(0x30000000, 10, "", read_only=True), Section(0x70000000, 10, "", read_only=True), @@ -291,7 +287,6 @@ def test_armv7_get_used_ram_base_addresses(): def test_read_memory_until_error_no_error(): - read_size = 1024 size = 1024 * 1024 inferior = MagicMock() @@ -301,7 +296,6 @@ def test_read_memory_until_error_no_error(): def test_read_memory_until_error_after_10k(): - read_size = 1024 size = 1024 * 1024 inferior = MagicMock() @@ -320,12 +314,14 @@ def _read_memory_raise_after_10k(_addr, _size): def test_coredump_writer(snapshot): - arch = ArmCortexMCoredumpArch() - cd_writer = MemfaultCoredumpWriter(arch) - cd_writer.device_serial = "device_serial" - cd_writer.firmware_version = "1.2.3" - cd_writer.hardware_revision = "gdb-proto" + device_serial = "device_serial" + software_type = "main" + software_version = "1.2.3" + hardware_revision = "gdb-proto" + cd_writer = MemfaultCoredumpWriter( + arch, device_serial, software_type, software_version, hardware_revision + ) cd_writer.trace_reason = 5 cd_writer.regs = [ { @@ -363,7 +359,6 @@ def test_coredump_writer(snapshot): def test_http_basic_auth(): - headers = add_basic_auth("martijn@memfault.com", "open_sesame", {"Foo": "Bar"}) assert headers == { "Foo": "Bar", @@ -379,7 +374,7 @@ def _gdb_for_coredump(mocker, maintenance_info_sections_fixture, _settings_cored def _gdb_execute(cmd, to_string): if cmd == "maintenance info sections": return maintenance_info_sections_fixture - if cmd == "info reg all": + if cmd == "info all-registers": return "r0\t0\n" if cmd == "info threads": return "" @@ -467,6 +462,65 @@ def test_coredump_command_not_allowing( assert "Aborting" in stdout +@pytest.mark.usefixtures("_gdb_for_coredump") +def test_coredump_all_overrides(http_expect_request, test_config, mocker): + """Test coredump command with all overrides set""" + + # Verify coredump is uploaded + test_config.ingress_uri = "https://custom-ingress.memfault.com" + http_expect_request( + "https://custom-ingress.memfault.com/api/v0/upload/coredump", + "POST", + ANY, + {"Content-Type": "application/octet-stream", "Memfault-Project-Key": TEST_PROJECT_KEY}, + 200, + {}, + ) + + writer = MagicMock() + mocker.patch("memfault_gdb.MemfaultCoredumpWriter", writer) + + hardware_revision = "TESTREVISION" + software_version = "TESTVERSION" + software_type = "TESTTYPE" + device_serial = "TESTSERIAL" + + cmd = MemfaultCoredump() + cmd.invoke( + "--project-key {} --hardware-revision {} --software-version {} --software-type {} --device-serial {}".format( + TEST_PROJECT_KEY, hardware_revision, software_version, software_type, device_serial + ), + True, + ) + + # Verify that all args were successfully extracted and passed to writer + writer.assert_called_once_with( + ANY, device_serial, software_type, software_version, hardware_revision + ) + + +@pytest.mark.parametrize( + ("cmd_option", "cmd_type"), + [ + ("--hardware-revision", "hardware revision"), + ("--software-version", "software version"), + ("--software-type", "software type"), + ("--device-serial", "device serial"), + ], +) +def test_coredump_override_invalid_input(capsys, cmd_option, cmd_type): + """Test coredump command with invalid input for each override""" + + invalid_input = 'inv"lid' + + cmd = MemfaultCoredump() + cmd.invoke("--project-key {} {} {}".format(TEST_PROJECT_KEY, cmd_option, invalid_input), True) + + # Verify an error is thrown for the invalid input + stderr = capsys.readouterr().err + assert "Invalid characters in {}: {}".format(cmd_type, invalid_input) in stderr + + @pytest.mark.parametrize( ("expected_result", "extra_cmd_args", "expected_coredump_fn"), [ @@ -576,7 +630,6 @@ def _check_request_body(body_bytes): def test_login_command_simple(http_expect_request, test_config): - http_expect_request( "https://api.memfault.com/auth/me", "GET", None, TEST_AUTH_HEADERS, 200, {"id": 123} ) @@ -592,7 +645,6 @@ def test_login_command_simple(http_expect_request, test_config): def test_login_command_with_all_options(http_expect_request, test_config): - test_api_uri = "http://dev-api.memfault.com:8000" test_ingress_uri = "http://dev-ingress.memfault.com" @@ -640,7 +692,6 @@ def test_login_command_with_all_options(http_expect_request, test_config): def test_has_uploaded_symbols( http_expect_request, expected_result, resp_status, resp_body, test_config_with_login ): - test_software_type = "main" test_software_version = "1.0.0" http_expect_request( @@ -664,7 +715,6 @@ def test_has_uploaded_symbols( def test_post_chunk_data(http_expect_request): - base_uri = "https://example.chunks.memfault.com" chunk_data = bytearray([1, 2, 3, 4]) device_serial = "GDB_TESTSERIAL" diff --git a/tasks/__init__.py b/tasks/__init__.py index 0ee0ba971..b319542c8 100644 --- a/tasks/__init__.py +++ b/tasks/__init__.py @@ -150,7 +150,6 @@ def fw_sdk_unit_test( ) def build_all_demos(ctx): """Builds all demo apps (for CI purposes)""" - pass ci = Collection("~ci") diff --git a/tests/src/test_memfault_heartbeat_metrics.cpp b/tests/src/test_memfault_heartbeat_metrics.cpp index 1315ae180..a9209fe24 100644 --- a/tests/src/test_memfault_heartbeat_metrics.cpp +++ b/tests/src/test_memfault_heartbeat_metrics.cpp @@ -474,8 +474,8 @@ TEST(MemfaultHeartbeatMetrics, Test_HeartbeatCollection) { mock().checkExpectations(); // values should all be reset - memfault_metrics_heartbeat_read_signed(keyi32, &vali32); - memfault_metrics_heartbeat_read_unsigned(keyu32, &valu32); - LONGS_EQUAL(vali32, 0); - LONGS_EQUAL(valu32, 0); + rv = memfault_metrics_heartbeat_read_signed(keyi32, &vali32); + CHECK(rv != 0); + rv = memfault_metrics_heartbeat_read_unsigned(keyu32, &valu32); + CHECK(rv != 0); } diff --git a/tests/src/test_memfault_heartbeat_metrics_debug.cpp b/tests/src/test_memfault_heartbeat_metrics_debug.cpp index 5c73e6293..8ec85cc12 100644 --- a/tests/src/test_memfault_heartbeat_metrics_debug.cpp +++ b/tests/src/test_memfault_heartbeat_metrics_debug.cpp @@ -104,8 +104,8 @@ TEST(MemfaultHeartbeatMetricsDebug, Test_DebugPrints) { " MemfaultSdkMetric_IntervalMs: 0", " MemfaultSdkMetric_UnexpectedRebootCount: 1", " MemfaultSdkMetric_UnexpectedRebootDidOccur: 1", - " test_key_unsigned: 0", - " test_key_signed: 0", + " test_key_unsigned: null", + " test_key_signed: null", " test_key_timer: 0", " test_key_string: \"\"", }; @@ -139,10 +139,10 @@ TEST(MemfaultHeartbeatMetricsDebug, Test_DebugPrints) { const char *heartbeat_debug_print_reset[] = { "Heartbeat keys/values:", " MemfaultSdkMetric_IntervalMs: 0", - " MemfaultSdkMetric_UnexpectedRebootCount: 0", + " MemfaultSdkMetric_UnexpectedRebootCount: null", " MemfaultSdkMetric_UnexpectedRebootDidOccur: 0", - " test_key_unsigned: 0", - " test_key_signed: 0", + " test_key_unsigned: null", + " test_key_signed: null", " test_key_timer: 0", " test_key_string: \"\"", }; diff --git a/tests/src/test_memfault_metrics_serializer.cpp b/tests/src/test_memfault_metrics_serializer.cpp index f1c45dcd3..54bcf3d48 100644 --- a/tests/src/test_memfault_metrics_serializer.cpp +++ b/tests/src/test_memfault_metrics_serializer.cpp @@ -19,7 +19,7 @@ #include "memfault/metrics/utils.h" static const sMemfaultEventStorageImpl *s_fake_event_storage_impl; -#define FAKE_EVENT_STORAGE_SIZE 54 +#define FAKE_EVENT_STORAGE_SIZE 56 TEST_GROUP(MemfaultMetricsSerializer){ void setup() { @@ -35,7 +35,8 @@ TEST_GROUP(MemfaultMetricsSerializer){ // // For the purposes of our serialization test, we will -// just serialize 1 of each supported type +// just serialize 1 of each supported type plus two unset +// integers (signed + unsigned) for 6 total metrics // void memfault_metrics_heartbeat_iterate(MemfaultMetricIteratorCallback cb, void *ctx) { @@ -44,10 +45,20 @@ void memfault_metrics_heartbeat_iterate(MemfaultMetricIteratorCallback cb, void info.type = kMemfaultMetricType_Unsigned; info.val.u32 = 1000; + info.is_set = true; + cb(ctx, &info); + + // Test an unset unsigned metric + info.is_set = false; cb(ctx, &info); info.type = kMemfaultMetricType_Signed; info.val.i32 = -1000; + info.is_set = true; + cb(ctx, &info); + + // Test an unset signed metric + info.is_set = false; cb(ctx, &info); info.type = kMemfaultMetricType_Timer; @@ -68,7 +79,9 @@ size_t memfault_metrics_heartbeat_get_num_metrics(void) { // if this fails, it means we need to add add a report for the new type // to the fake "memfault_metrics_heartbeat_iterate" LONGS_EQUAL(kMemfaultMetricType_NumTypes, 4); - return kMemfaultMetricType_NumTypes; + + // Additionally we test that unset integers (signed + unsigned) are set to null + return kMemfaultMetricType_NumTypes + 2; } TEST(MemfaultMetricsSerializer, Test_MemfaultMetricSerialize) { @@ -84,18 +97,14 @@ TEST(MemfaultMetricsSerializer, Test_MemfaultMetricSerialize) { // "9": "1.2.3", // "6": "evt_24", // "4": { - // "1": [ 1000, -1000, 1234, "123456789abcde" ] + // "1": [ 1000, null, -1000, null, 1234, "123456789abcde" ] // } // } const uint8_t expected_serialization[] = { - 0xa6, - 0x02, 0x01, - 0x03, 0x01, - 0x0a, 0x64, 'm', 'a', 'i', 'n', - 0x09, 0x65, '1', '.', '2', '.', '3', - 0x06, 0x66, 'e', 'v', 't', '_', '2', '4', - 0x04, 0xa1, 0x01, 0x84, 0x19, 0x03, 0xe8, 0x39, 0x03, 0xe7, 0x19, 0x04, 0xd2, 0x6e, - '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', + 0xa6, 0x02, 0x01, 0x03, 0x01, 0x0a, 0x64, 'm', 'a', 'i', 'n', 0x09, 0x65, '1', + '.', '2', '.', '3', 0x06, 0x66, 'e', 'v', 't', '_', '2', '4', 0x04, 0xa1, + 0x01, 0x86, 0x19, 0x03, 0xe8, 0xf6, 0x39, 0x03, 0xe7, 0xf6, 0x19, 0x04, 0xd2, 0x6e, + '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', }; fake_event_storage_assert_contents_match(expected_serialization, sizeof(expected_serialization)); @@ -103,7 +112,7 @@ TEST(MemfaultMetricsSerializer, Test_MemfaultMetricSerialize) { TEST(MemfaultMetricsSerializer, Test_MemfaultMetricSerializeWorstCaseSize) { const size_t worst_case_storage = memfault_metrics_heartbeat_compute_worst_case_storage_size(); - LONGS_EQUAL(60, worst_case_storage); + LONGS_EQUAL(70, worst_case_storage); } TEST(MemfaultMetricsSerializer, Test_MemfaultMetricSerializeOutOfSpace) { diff --git a/tests/src/test_memfault_minimal_cbor.cpp b/tests/src/test_memfault_minimal_cbor.cpp index 3b78bcfe3..9fafcc42f 100644 --- a/tests/src/test_memfault_minimal_cbor.cpp +++ b/tests/src/test_memfault_minimal_cbor.cpp @@ -478,3 +478,14 @@ TEST(MemfaultMinimalCbor, Test_BufTooSmall) { const bool success = memfault_cbor_encode_string(&encoder, "a"); CHECK(!success); } + +TEST(MemfaultMinimalCbor, Test_Null) { + sMemfaultCborEncoder encoder; + uint8_t result[1]; + memfault_cbor_encoder_init(&encoder, prv_write_cb, result, MEMFAULT_ARRAY_SIZE(result)); + + const bool success = memfault_cbor_encode_null(&encoder); + CHECK(success); + + LONGS_EQUAL(0xF6, result[0]); +}