[Zephyr] Silabs Zephyr OTA Support#72696
Conversation
90c9daa to
31afbb9
Compare
There was a problem hiding this comment.
Code Review
This pull request implements Matter Over-The-Air (OTA) software update support for Silicon Labs Zephyr targets using MCUboot, introducing post-build CMake scripts, configuration overlays, and board-specific partition layouts. Feedback on these changes focuses on improving build portability by replacing platform-specific commands like 'cp' and 'dd' with portable alternatives, resolving the configured signing key path dynamically, explicitly defining storage partitions in device tree overlays to prevent potential flash corruption, reverting unrelated changes in NXP platform files, and correcting a duplicate step number in the README.
I am having trouble creating individual review comments. Click here to see my feedback.
examples/platform/nxp/common/ota_requestor/source/OTARequestorInitiatorCommon.cpp (30)
This PR implements Zephyr OTA support for Silabs. Modifying NXP platform files to remove comments seems unrelated and should be reverted to avoid unnecessary changes in other platforms' code.
// Set the global instance of the OTA requestor core componentReferences
- When performing code moves, avoid introducing unrelated changes, refactorings, or fixes. Such modifications should be deferred to subsequent pull requests to maintain a clear and focused review process.
config/silabs/app/zephyr-post-build.cmake (41-42)
The signing key path is hardcoded to "${ZEPHYR_BASE}/../bootloader/mcuboot/root-ec-p256.pem", which ignores any custom key file configured via CONFIG_MCUBOOT_SIGNATURE_KEY_FILE. It should resolve CONFIG_MCUBOOT_SIGNATURE_KEY_FILE to an absolute path instead.
if(IS_ABSOLUTE "${CONFIG_MCUBOOT_SIGNATURE_KEY_FILE}")
set(MCUBOOT_SIGNING_KEY "${CONFIG_MCUBOOT_SIGNATURE_KEY_FILE}")
else()
get_filename_component(MCUBOOT_SIGNING_KEY
"${ZEPHYR_BASE}/../${CONFIG_MCUBOOT_SIGNATURE_KEY_FILE}" ABSOLUTE)
endif()
config/silabs/app/zephyr-post-build.cmake (54-55)
Using cp is Unix-specific and will fail on Windows build hosts. Use cmake -E copy instead for a platform-independent file copy.
COMMAND
${CMAKE_COMMAND} -E copy ${ZEPHYR_OUTPUT_DIR}/../build_mcuboot/zephyr/zephyr.bin ${ZEPHYR_OUTPUT_DIR}/zephyr.mcuboot.bin
config/silabs/app/zephyr-post-build.cmake (59-69)
Using dd for merging binaries makes the build non-portable and will fail on Windows development hosts. Since Python is guaranteed to be available in a Zephyr environment, you can use a portable Python command to merge the bootloader and application binaries. This also allows you to seek directly to ${mcuboot_size} (which is already in bytes), eliminating the need for BLOCK_SIZE and boot_blocks math.
dt_nodelabel(dts_partition_path NODELABEL "boot_partition")
dt_reg_size(mcuboot_size PATH ${dts_partition_path})
add_custom_command(
OUTPUT ${ZEPHYR_OUTPUT_DIR}/zephyr_full.bin
DEPENDS build_mcuboot ${ZEPHYR_OUTPUT_DIR}/${ZEPHYR_OUTPUT_NAME}.bin
COMMAND ${PYTHON_EXECUTABLE} -c "import sys; f = open(sys.argv[1], 'wb'); f.write(open(sys.argv[2], 'rb').read()); f.seek(int(sys.argv[3])); f.write(open(sys.argv[4], 'rb').read()); f.close()"
${ZEPHYR_OUTPUT_DIR}/zephyr_full.bin
${ZEPHYR_OUTPUT_DIR}/zephyr.mcuboot.bin
${mcuboot_size}
${ZEPHYR_OUTPUT_DIR}/${ZEPHYR_OUTPUT_NAME}.bin
)
examples/lighting-app/silabs/zephyr/boards/xg24_rb4187c.overlay (27-33)
The settings_partition and storage_partition have been removed from the overlay. Matter typically requires a larger storage partition (e.g., 40 KB) for NVS to store fabric and security credentials safely without running out of space. Relying on the board's default partition layout can lead to insufficient space or unexpected overlaps with the new slot0_partition and factory_partition boundaries. It is highly recommended to explicitly define the storage_partition (and/or settings_partition) in this overlay.
examples/lighting-app/silabs/zephyr/boards/xg26_rb4118a.overlay (23-34)
The storage_partition (used for Matter NVS persistent storage) is not explicitly defined or relocated in this overlay. Since slot0_partition, slot1_partition, and factory_partition have been resized and shifted, the board's default storage_partition location might now overlap with these partitions, leading to silent flash corruption at runtime. It is highly recommended to explicitly define and place the storage_partition at a safe, non-overlapping offset (e.g., starting at 0x318000 after the factory_partition).
examples/lighting-app/silabs/zephyr/README.md (340)
There is a duplicate step number 3. in the list. This step should be numbered 4., and the subsequent steps should be incremented accordingly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #72696 +/- ##
==========================================
+ Coverage 56.74% 56.76% +0.01%
==========================================
Files 1630 1634 +4
Lines 112290 112660 +370
Branches 13114 13143 +29
==========================================
+ Hits 63719 63948 +229
- Misses 48571 48712 +141 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
PR #72696: Size comparison from 44fb474 to 31afbb9 Full report (41 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, psoc6, qpg, realtek, stm32, telink)
|
Summary
Implements Zephyr OTA support to the Silabs Zephyr Lighting app.
Specifically tested for MG24, but other Silabs platforms should work with minimal modifications to their configs or overlays.
Changes in src/platform/Zephyr were due to the use of deprecated APIs
This implementation on the 4187C uses external flash to store the OTA image.
Related issues
N/A
Testing
Tested 2 subsequent OTAs on 4187C. (Initial flash using commander for v1 -> OTA to v2 -> OTA to v3)