examples: zephyr_example: refactor (ESF-270) - #167
Conversation
👋 Hello marekmatej, we appreciate your contribution to this project! Click to see more instructions ...
Review and merge process you can expect ...
|
950c559 to
c7af97d
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors the Zephyr integration for the ESP Serial Flasher library as an 'External project module', improving its integration with Zephyr RTOS projects. The refactoring introduces a more modular structure using Zephyr's iterable sections for binary firmware management, adds a new interactive shell sample, and updates the existing example to use the improved integration.
Changes:
- Restructured Zephyr module with new build system (subsys directory) and devicetree bindings
- Implemented binary firmware storage using Zephyr's iterable sections pattern
- Added new interactive shell sample with comprehensive ESP Serial Flasher commands
- Updated existing Zephyr example to use simplified API and devicetree-based configuration
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 35 comments.
Show a summary per file
| File | Description |
|---|---|
| zephyr/module.yml | Updated module name and structure to include DTS root, samples, and tests directories |
| zephyr/Kconfig | Added new config options for shell and static resources support |
| zephyr/subsys/esp_loader/CMakeLists.txt | New modular build system replacing old zephyr/CMakeLists.txt |
| zephyr/subsys/esp_loader/bin_images_sections.cmake | CMake function to convert binary files to C arrays using iterable sections |
| zephyr/subsys/esp_loader/include/bin_image.h | Header defining bin_image structure and DEFINE_IMAGE macro for iterable sections |
| zephyr/subsys/esp_loader/sections-rom.ld | Linker script for iterable sections support |
| zephyr/dts/bindings/misc/esp-loader/espressif,esp-loader.yaml | Devicetree binding for ESP loader node configuration |
| port/zephyr_port.h | Simplified API removing loader_zephyr_config_t structure |
| port/zephyr_port.c | Refactored to use devicetree directly and added SYS_INIT for automatic initialization |
| zephyr/samples/shell/ | New interactive shell sample with comprehensive ESP Serial Flasher commands |
| examples/zephyr_example/ | Updated to use new devicetree-based configuration and iterable sections |
| zephyr/CMakeLists.txt | Deleted - functionality moved to subsys/esp_loader/CMakeLists.txt |
| # Compute MD5 hash of the file | ||
| execute_process(COMMAND md5sum ${bin} OUTPUT_VARIABLE md5_output) | ||
| string(REGEX REPLACE " .*" "" md5_hash ${md5_output}) |
There was a problem hiding this comment.
The md5sum command may not be portable across all platforms. On macOS, the command is md5 -q instead of md5sum, and on Windows, it may not be available at all. Consider using CMake's built-in file(MD5 ...) command instead for better cross-platform compatibility: file(MD5 ${bin} md5_hash).
| # Compute MD5 hash of the file | |
| execute_process(COMMAND md5sum ${bin} OUTPUT_VARIABLE md5_output) | |
| string(REGEX REPLACE " .*" "" md5_hash ${md5_output}) | |
| # Compute MD5 hash of the file (use CMake's built-in MD5 for portability) | |
| file(MD5 ${bin} md5_hash) |
|
Thanks for the improvements for zephyr port. Sorry it took me so long to add some comments to it. I am not that familiar with zephyr, so I was trying to study it a bit, also other responsibilities were there. Regarding the implementation. I think that the shell is really cool, but should not be part of the library. It duplicates code from the examples_common and adds unnecessary maintenance burden and is out of scope of the library. That is the first thing. Second, there are some breaking changes in the port, for example There are also other small changes, but I will address them individually. Overall thanks for this, looks nice. |
Dzarda7
left a comment
There was a problem hiding this comment.
The whole iterable_section is great addition and I would like to include binaries directly without converting it to array, but it should also be part of the example, not the library as we do not know what source user wants to use.
| #include <esp_loader_io.h> | ||
|
|
||
| #include <zephyr/logging/log.h> | ||
| LOG_MODULE_REGISTER(esf, CONFIG_ESP_SERIAL_FLASHER_LOG_LEVEL); |
There was a problem hiding this comment.
CONFIG_ESP_SERIAL_FLASHER_LOG_LEVEL is nice improvement, but inconsistent with other ports and documentation which uses SERIAL_FLASHER_DEBUG_TRACE
| - `bootloader_0x0.bin` - ESP32 bootloader binary flashed at offset 0x0 | ||
| - `partition-table_0x8000.bin` - Partition table configuration at offset 0x8000 | ||
| - `app_0x10000.bin` - Main application binary at offset 0x10000 |
There was a problem hiding this comment.
I believe this change is unnecessary and also not true as bootloader will be flashed at address based on the target.
a7b519b to
7f83e8f
Compare
4ee73e2 to
88f31ca
Compare
Update Zephyr port to better integrate with dts and use of off-tree subsystem. Update example accordingly and add shell commands for manual target access. Signed-off-by: Marek Matej <marek.matej@espressif.com>
Fix the board name to the supported name. Use latter Zephyr version and actually supported boards. Update packages for subsystems and hals. Signed-off-by: Marek Matej <marek.matej@espressif.com>
88f31ca to
2b1e0e0
Compare
|
This was done elsewhere and already merged 174f3ea. |
Description
Improve the Zephyr integration as an 'External project module'. The Zephyr portion of the integration describes the module and the way to add it to a new project.
Use the Zephyr's
iterable sectionsto store and use the binary firmware images.Add a new example in the Zephyr integration folder, including an interactive shell.
Update the existing Zephyr example to reflect the improved module integration.
Motivation
This work improves the ESP Serial Flasher integration in Zephyr so it can be easily added to applications on various supported platforms and vendors.
Related
zephyr_compile_definitionszephyrproject-rtos/zephyr#98647Testing
After you add the module as described in the Zephyr integration module and
west update:Build the original sample:
west build -b esp32s3_devkitc//procpu ../esp-serial-flasher/examples/zephyr_example -pBuild new shell sample:
west build -b esp32s3_devkitc//procpu ../esp-serial-flasher/zephyr/samples/shell -p