Skip to content

Commit 0da7502

Browse files
ajlennoncursoragent
andcommitted
fix: address Preloop review findings for RAM staging
Gate Zephyr RAM staging behind Kconfig, use mender_malloc, add unit-tested helpers, and fix the ESP-IDF partition-label cmake mismatch plus log truncation. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 7381211 commit 0da7502

13 files changed

Lines changed: 368 additions & 63 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
---
2+
## Unreleased
3+
4+
* Zephyr `zephyr-image` update module: optional RAM staging before secondary-slot
5+
write (XIP-safe on FlexSPI), with `CONFIG_MENDER_ZEPHYR_IMAGE_RAM_STAGE` and
6+
`CONFIG_MENDER_ZEPHYR_IMAGE_RAM_STAGE_MAX_BYTES`, `mender_malloc` staging
7+
buffer, direct-to-flash fallback, and unit tests for the staging helpers.
8+
* ESP-IDF: fix `MENDER_STORAGE_PARTITION_LABEL` compile definition mismatch in
9+
`component.cmake`; grow log formatting beyond a fixed 256-byte stack buffer.
10+
211
## 1.0.0 - 2026-04-17
312

413
* The first stable release

README.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
> Machine SoT: [`DD_PIN`](./DD_PIN) · tag `dd-pin-64c10fa` ·
66
> [`scripts/check-consumer-pin.sh`](./scripts/check-consumer-pin.sh) ·
77
> [`scripts/bump-consumer-pins.sh`](./scripts/bump-consumer-pins.sh).
8+
>
9+
> Pin scripts accept `MENDER_MCU_PIN_BRANCH`, `DD_ROOT`, and
10+
> `MENDER_MCU_CONSUMERS` overrides (see script headers).
811
912

1013
## Overview
@@ -32,6 +35,20 @@ The decision to fork the original mender-mcu-client was made to:
3235
* **Provide Official Support**: Ensure that the project receives the necessary attention and
3336
resources from Northern.tech to meet the needs of the community and enterprise users.
3437

38+
## Zephyr RAM staging (optional)
39+
40+
When `CONFIG_MENDER_ZEPHYR_IMAGE_UPDATE_MODULE` is enabled, the default
41+
`CONFIG_MENDER_ZEPHYR_IMAGE_RAM_STAGE` path accumulates the full artifact payload
42+
in RAM (via `mender_malloc`) during download, then writes the secondary slot in
43+
one pass at close. That avoids FlexSPI XIP stalls when the running image, OTA
44+
slot, and MCUboot share the same NOR (for example i.MX RT).
45+
46+
- Disable with `CONFIG_MENDER_ZEPHYR_IMAGE_RAM_STAGE=n` for direct-to-flash writes.
47+
- Cap staging with `CONFIG_MENDER_ZEPHYR_IMAGE_RAM_STAGE_MAX_BYTES` (0 = no cap).
48+
- If allocation fails or the image exceeds the cap, the module falls back to
49+
writing each chunk directly to flash. MCUboot still verifies the image on
50+
boot; staging does not bypass signature checks.
51+
3552
## Get started
3653

3754
This guide is based on our Zephyr reference application [mender-mcu-integration](https://github.com/mendersoftware/mender-mcu-integration).

cmake/mender_mcu_sources.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ file(GLOB MENDER_MCU_SOURCES
182182
"${MENDER_MCU_ROOT}/src/core/client.c"
183183
"${MENDER_MCU_ROOT}/src/core/deployment-data.c"
184184
"${MENDER_MCU_ROOT}/src/core/error-counters.c"
185+
"${MENDER_MCU_ROOT}/src/core/image-ram-stage.c"
185186
"${MENDER_MCU_ROOT}/src/core/update-module.c"
186187
"${MENDER_MCU_ROOT}/src/core/utils.c"
187188
"${MENDER_MCU_ROOT}/src/platform/log/${CONFIG_MENDER_PLATFORM_LOG_TYPE}/log.c"

scripts/bump-consumer-pins.sh

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,14 @@
55
# ./scripts/bump-consumer-pins.sh <full-40-char-sha> # dry-run
66
# ./scripts/bump-consumer-pins.sh <full-40-char-sha> --apply # write files
77
#
8-
# Expects sibling checkouts under DD_ROOT (default /data_drive/dd):
9-
# zephyr-rt1186-f1
10-
# zephyr-rt1170-room-display
11-
# zephyr-rt1170-eink
8+
# Env:
9+
# DD_ROOT parent of consumer checkouts (default: /data_drive/dd)
10+
# MENDER_MCU_CONSUMERS override consumer list as "repo|rel[,repo|rel...]"
11+
#
12+
# Default consumers (under DD_ROOT):
13+
# zephyr-rt1186-f1|f1-controller/west.yml
14+
# zephyr-rt1170-room-display|room-display/west.yml
15+
# zephyr-rt1170-eink|mender-mcu-integration/west.yml
1216
#
1317
# Does not git commit/push — review, then commit each repo (and retag dd-pin-*).
1418
set -euo pipefail
@@ -27,15 +31,23 @@ SHORT=${NEW:0:7}
2731
ROOT=$(cd "$(dirname "$0")/.." && pwd)
2832
DD_ROOT=${DD_ROOT:-/data_drive/dd}
2933

30-
CONSUMERS=(
34+
DEFAULT_CONSUMERS=(
3135
"zephyr-rt1186-f1|f1-controller/west.yml"
3236
"zephyr-rt1170-room-display|room-display/west.yml"
3337
"zephyr-rt1170-eink|mender-mcu-integration/west.yml"
3438
)
3539

40+
CONSUMERS=()
41+
if [[ -n "${MENDER_MCU_CONSUMERS:-}" ]]; then
42+
IFS=',' read -r -a CONSUMERS <<<"$MENDER_MCU_CONSUMERS"
43+
else
44+
CONSUMERS=("${DEFAULT_CONSUMERS[@]}")
45+
fi
46+
3647
OLD=$(tr -d '[:space:]' < "$ROOT/DD_PIN" || true)
3748
echo "mender-mcu pin: ${OLD:-"(none)"}$NEW"
3849
echo "tag suggestion: dd-pin-$SHORT (annotate at $NEW)"
50+
echo "DD_ROOT=$DD_ROOT"
3951
echo
4052

4153
replace_revision() {

scripts/check-consumer-pin.sh

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,16 @@
66
# curl -fsSL …/scripts/check-consumer-pin.sh | bash -s -- f1-controller/west.yml
77
#
88
# Env:
9+
# MENDER_MCU_PIN_BRANCH branch hosting DD_PIN / PIN-POLICY (default: feature/zephyr-ram-stage-on-main)
910
# MENDER_MCU_DD_PIN_URL override raw DD_PIN URL
1011
# MENDER_MCU_DD_PIN_FILE read pin from this file instead of URL
12+
# MENDER_MCU_PIN_POLICY_URL override policy URL shown on drift (optional)
1113
set -euo pipefail
1214

1315
WEST_YML=${1:?usage: check-consumer-pin.sh <west.yml>}
14-
PIN_URL=${MENDER_MCU_DD_PIN_URL:-https://raw.githubusercontent.com/DynamicDevices/mender-mcu/feature/zephyr-ram-stage-on-main/DD_PIN}
16+
PIN_BRANCH=${MENDER_MCU_PIN_BRANCH:-feature/zephyr-ram-stage-on-main}
17+
PIN_URL=${MENDER_MCU_DD_PIN_URL:-https://raw.githubusercontent.com/DynamicDevices/mender-mcu/${PIN_BRANCH}/DD_PIN}
18+
POLICY_URL=${MENDER_MCU_PIN_POLICY_URL:-https://github.com/DynamicDevices/mender-mcu/blob/${PIN_BRANCH}/PIN-POLICY.md}
1519

1620
if [[ ! -f "$WEST_YML" ]]; then
1721
echo "error: west.yml not found: $WEST_YML" >&2
@@ -70,7 +74,7 @@ if [[ "$ACTUAL" != "$EXPECTED" ]]; then
7074
echo "error: mender-mcu pin drift" >&2
7175
echo " west.yml ($WEST_YML): $ACTUAL" >&2
7276
echo " DD_PIN ($PIN_SRC): $EXPECTED" >&2
73-
echo " policy: https://github.com/DynamicDevices/mender-mcu/blob/feature/zephyr-ram-stage-on-main/PIN-POLICY.md" >&2
77+
echo " policy: $POLICY_URL" >&2
7478
exit 5
7579
fi
7680

src/core/image-ram-stage.c

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* @file image-ram-stage.c
3+
* @brief Whole-image RAM staging helpers (platform-independent)
4+
*
5+
* Copyright Northern.tech AS
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
#include <string.h>
21+
22+
#include "alloc.h"
23+
#include "image-ram-stage.h"
24+
25+
mender_err_t
26+
mender_image_ram_stage_begin(mender_image_ram_stage_t *stage, size_t size, bool enabled, size_t max_size) {
27+
if (NULL == stage) {
28+
return MENDER_FAIL;
29+
}
30+
31+
mender_image_ram_stage_reset(stage);
32+
33+
if (!enabled || 0 == size) {
34+
return MENDER_OK;
35+
}
36+
if ((max_size > 0) && (size > max_size)) {
37+
return MENDER_OK;
38+
}
39+
40+
stage->buf = mender_malloc(size);
41+
if (NULL == stage->buf) {
42+
return MENDER_OK;
43+
}
44+
stage->capacity = size;
45+
stage->length = 0;
46+
return MENDER_OK;
47+
}
48+
49+
bool
50+
mender_image_ram_stage_active(const mender_image_ram_stage_t *stage) {
51+
return (NULL != stage) && (NULL != stage->buf);
52+
}
53+
54+
mender_err_t
55+
mender_image_ram_stage_write(mender_image_ram_stage_t *stage, const void *data, size_t index, size_t length) {
56+
if ((NULL == stage) || (NULL == stage->buf) || (NULL == data)) {
57+
return MENDER_FAIL;
58+
}
59+
if (index + length > stage->capacity) {
60+
return MENDER_FAIL;
61+
}
62+
memcpy(stage->buf + index, data, length);
63+
if (index + length > stage->length) {
64+
stage->length = index + length;
65+
}
66+
return MENDER_OK;
67+
}
68+
69+
void
70+
mender_image_ram_stage_reset(mender_image_ram_stage_t *stage) {
71+
if (NULL == stage) {
72+
return;
73+
}
74+
if (NULL != stage->buf) {
75+
mender_free(stage->buf);
76+
}
77+
stage->buf = NULL;
78+
stage->capacity = 0;
79+
stage->length = 0;
80+
}

src/include/image-ram-stage.h

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
/**
2+
* @file image-ram-stage.h
3+
* @brief Whole-image RAM staging helpers (platform-independent)
4+
*
5+
* Copyright Northern.tech AS
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
#ifndef __MENDER_IMAGE_RAM_STAGE_PRIV_H__
21+
#define __MENDER_IMAGE_RAM_STAGE_PRIV_H__
22+
23+
#ifdef __cplusplus
24+
extern "C" {
25+
#endif /* __cplusplus */
26+
27+
#include <stdbool.h>
28+
#include <stddef.h>
29+
#include <stdint.h>
30+
31+
#include <mender/utils.h>
32+
33+
typedef struct mender_image_ram_stage {
34+
uint8_t *buf;
35+
size_t capacity;
36+
size_t length;
37+
} mender_image_ram_stage_t;
38+
39+
/**
40+
* @brief Try to allocate a staging buffer for @p size bytes
41+
* @param stage Staging state (must not be NULL)
42+
* @param size Artifact payload size
43+
* @param enabled When false, leave stage inactive (direct-to-flash)
44+
* @param max_size When > 0 and size > max_size, leave stage inactive
45+
* @return MENDER_OK always (inactive stage is a valid outcome); MENDER_FAIL on bad args
46+
*
47+
* Allocation uses mender_malloc. Failure to allocate leaves the stage inactive.
48+
*/
49+
mender_err_t mender_image_ram_stage_begin(mender_image_ram_stage_t *stage, size_t size, bool enabled, size_t max_size);
50+
51+
bool mender_image_ram_stage_active(const mender_image_ram_stage_t *stage);
52+
53+
/**
54+
* @brief Copy a download chunk into the staging buffer
55+
* @return MENDER_OK, or MENDER_FAIL on overflow / inactive stage / bad args
56+
*/
57+
mender_err_t mender_image_ram_stage_write(mender_image_ram_stage_t *stage, const void *data, size_t index, size_t length);
58+
59+
/** Free staging buffer and clear state. */
60+
void mender_image_ram_stage_reset(mender_image_ram_stage_t *stage);
61+
62+
#ifdef __cplusplus
63+
}
64+
#endif /* __cplusplus */
65+
66+
#endif /* __MENDER_IMAGE_RAM_STAGE_PRIV_H__ */

src/platform/log/esp-idf/log.c

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,20 +22,54 @@
2222

2323
#include "esp_log.h"
2424

25+
#include "alloc.h"
2526
#include "log.h"
2627

28+
/* Cap heap-backed messages so a runaway format cannot exhaust memory. */
29+
#define MENDER_ESP_LOG_MAX_MSG 2048
30+
2731
mender_err_t
2832
mender_log_init(void) {
2933
/* Nothing to do */
3034
return MENDER_OK;
3135
}
3236

3337
mender_err_t
34-
mender_log_print(uint8_t level, MENDER_ARG_UNUSED const char *filename, const char *function, int line, char *format, ...) {
35-
char msg[256] = "";
38+
mender_log_print(uint8_t level, const char *filename, const char *function, int line, char *format, ...) {
39+
char stack_msg[256];
40+
char *msg = stack_msg;
41+
char *heap_msg = NULL;
3642
va_list args;
43+
va_list args_copy;
44+
int needed;
45+
3746
va_start(args, format);
38-
vsnprintf(msg, sizeof(msg), format, args);
47+
va_copy(args_copy, args);
48+
needed = vsnprintf(NULL, 0, format, args_copy);
49+
va_end(args_copy);
50+
51+
if (needed < 0) {
52+
va_end(args);
53+
return MENDER_FAIL;
54+
}
55+
56+
if ((size_t)needed + 1 > sizeof(stack_msg)) {
57+
size_t alloc_len = (size_t)needed + 1;
58+
if (alloc_len > MENDER_ESP_LOG_MAX_MSG) {
59+
alloc_len = MENDER_ESP_LOG_MAX_MSG;
60+
}
61+
heap_msg = mender_malloc(alloc_len);
62+
if (NULL != heap_msg) {
63+
msg = heap_msg;
64+
vsnprintf(msg, alloc_len, format, args);
65+
} else {
66+
/* Fall back to truncated stack buffer if heap is exhausted. */
67+
vsnprintf(stack_msg, sizeof(stack_msg), format, args);
68+
msg = stack_msg;
69+
}
70+
} else {
71+
vsnprintf(stack_msg, sizeof(stack_msg), format, args);
72+
}
3973
va_end(args);
4074

4175
esp_log_level_t esp_level;
@@ -55,8 +89,10 @@ mender_log_print(uint8_t level, MENDER_ARG_UNUSED const char *filename, const ch
5589
break;
5690
}
5791

58-
ESP_LOG_LEVEL(esp_level, "mender", "%s:%d: %s", function, line, msg);
92+
/* Include source filename in the tag path so it is not dropped. */
93+
ESP_LOG_LEVEL(esp_level, "mender", "%s:%s:%d: %s", filename ? filename : "?", function, line, msg);
5994

95+
mender_free(heap_msg);
6096
return MENDER_OK;
6197
}
6298

0 commit comments

Comments
 (0)