feat: add build two target and serial flasher example#26
Conversation
WalkthroughThis PR introduces a complete build-two-target example for ESP-IDF that demonstrates configuring and building one target (ESP32-C6) as a binary, embedding it, and using a second host target as a serial flasher to program the embedded binary to the slave device. Changes
Sequence DiagramsequenceDiagram
participant CMake as CMake Build System
participant Host as Host (ESP32-P4)
participant Slave as Slave (ESP32-C6)
rect rgb(200, 220, 255)
Note over CMake: Build Configuration Phase
CMake->>CMake: Build example_c6 target
CMake->>CMake: Generate merged-binary.bin
CMake->>Host: Embed merged-binary.bin into Host app
end
rect rgb(220, 200, 255)
Note over Host,Slave: Runtime Execution
Host->>Host: app_main() initializes serial flasher
Host->>Slave: Connect via UART
Host->>Slave: Establish connection & detect target
Host->>Slave: Negotiate baud rate (or fallback)
Host->>Slave: Erase flash region
Host->>Host: Stream merged-binary.bin in chunks
Host->>Slave: Program chunks to flash memory
Host->>Host: Update progress indicator
Host->>Slave: Reset target
Slave->>Slave: Boot and run app_main()
Host->>Host: Spawn slave_monitor task
Slave-->>Host: Serial output (Hello, World!...)
Host->>Host: Forward slave output to console
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
examples/system/build_two_target/main/CMakeLists.txt (1)
1-9:if(TARGET build_example_c6)block likely never fires due to CMake ordering
main/CMakeLists.txtis processed as part ofproject(build_two_target)in the top-level CMakeLists, while thebuild_example_c6custom target is only created after thatproject()call. That means, on first configure,if(TARGET build_example_c6)will evaluate to false, soadd_dependencies(${COMPONENT_LIB} build_example_c6)is never applied, despite the comment saying it should enforce the dependency. (cmake.org)You already make
build_example_c6anALLtarget and generatespiffs/merged-binary.binvia anadd_custom_command, so this doesn’t currently break the build, but it does make the intent here misleading.Consider one of these cleanups:
- Move the dependency wiring into the top-level
examples/system/build_two_target/CMakeLists.txt, afteradd_custom_target(build_example_c6 ...), e.g.:-# Ensure that merged-binary.bin has been generated before building the main component -# Add a dependency to make sure example_c6 is built before building the main component -if(TARGET build_example_c6) - add_dependencies(${COMPONENT_LIB} build_example_c6) -endif() +if(TARGET build_example_c6 AND TARGET ${COMPONENT_LIB}) + add_dependencies(${COMPONENT_LIB} build_example_c6) +endif()
- Or, alternatively, define
build_example_c6earlier in the top-level CMake so the existingif(TARGET ...)here becomes true on first configure.examples/system/build_two_target/CMakeLists.txt (1)
5-28: Make theidf.pycustom command more portable and robustThe overall flow (build
example_c6, runidf.py merge-bin, then copymerged-binary.binintospiffs/) is good, but the current implementation has a couple of portability/robustness issues:
COMMAND bash -c "cd ... && idf.py ..."assumes a POSIX shell andidf.pyonPATH, which will be brittle on Windows and in some IDF setups.- Embedding
cdand a long shell pipeline in a single string also makes quoting/space-in-path issues more likely.You can achieve the same effect in a more CMake/IDF‑idiomatic and cross‑platform way by leveraging
${IDF_PY}(set by IDF’sproject.cmake) and separateCOMMANDs, e.g.:-add_custom_command( - OUTPUT ${CMAKE_CURRENT_SOURCE_DIR}/spiffs/merged-binary.bin - COMMAND bash -c "cd ${CMAKE_CURRENT_SOURCE_DIR}/example_c6 && idf.py set-target esp32c6 && idf.py build && idf.py merge-bin" - COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_SOURCE_DIR}/spiffs - COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_CURRENT_SOURCE_DIR}/example_c6/build/merged-binary.bin ${CMAKE_CURRENT_SOURCE_DIR}/spiffs/merged-binary.bin - WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} - COMMENT "Building example_c6 with target esp32c6 and copying merged-binary.bin to spiffs" - VERBATIM -) +add_custom_command( + OUTPUT ${CMAKE_CURRENT_SOURCE_DIR}/spiffs/merged-binary.bin + COMMAND ${IDF_PY} -C ${CMAKE_CURRENT_SOURCE_DIR}/example_c6 set-target esp32c6 + COMMAND ${IDF_PY} -C ${CMAKE_CURRENT_SOURCE_DIR}/example_c6 build + COMMAND ${IDF_PY} -C ${CMAKE_CURRENT_SOURCE_DIR}/example_c6 merge-bin + COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_CURRENT_SOURCE_DIR}/spiffs + COMMAND ${CMAKE_COMMAND} -E copy_if_different + ${CMAKE_CURRENT_SOURCE_DIR}/example_c6/build/merged-binary.bin + ${CMAKE_CURRENT_SOURCE_DIR}/spiffs/merged-binary.bin + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + COMMENT "Building example_c6 with target esp32c6 and copying merged-binary.bin to spiffs" + VERBATIM +)This keeps the logic the same but avoids hard‑coding a shell and improves behavior on non‑POSIX hosts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
examples/system/build_two_target/spiffs/merged-binary.binis excluded by!**/*.bin
📒 Files selected for processing (8)
examples/system/build_two_target/CMakeLists.txt(1 hunks)examples/system/build_two_target/example_c6/CMakeLists.txt(1 hunks)examples/system/build_two_target/example_c6/main/CMakeLists.txt(1 hunks)examples/system/build_two_target/example_c6/main/example_c6.c(1 hunks)examples/system/build_two_target/main/CMakeLists.txt(1 hunks)examples/system/build_two_target/main/build_two_target.c(1 hunks)examples/system/build_two_target/main/idf_component.yml(1 hunks)examples/system/build_two_target/sdkconfig.defaults(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
examples/system/build_two_target/example_c6/main/example_c6.c (1)
examples/system/build_two_target/main/build_two_target.c (1)
app_main(46-163)
examples/system/build_two_target/main/build_two_target.c (1)
examples/system/build_two_target/example_c6/main/example_c6.c (1)
app_main(5-11)
🪛 Clang (14.0.6)
examples/system/build_two_target/example_c6/main/example_c6.c
[error] 1-1: 'stdio.h' file not found
(clang-diagnostic-error)
examples/system/build_two_target/main/build_two_target.c
[error] 1-1: 'sys/param.h' file not found
(clang-diagnostic-error)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
examples/system/build_two_target/example_c6/main/example_c6.c (1)
5-10: Simple target app looks goodThe example
app_mainis minimal, usesvTaskDelaycorrectly, and is fine as a slave “Hello, World!” payload.examples/system/build_two_target/example_c6/main/CMakeLists.txt (1)
1-2: Component registration is idiomatic
idf_component_registersetup matches the single‑source component and local includes; nothing to change here.examples/system/build_two_target/example_c6/CMakeLists.txt (1)
1-6: Boilerplate CMake is correctThe IDF project boilerplate is in the expected order and should work fine for the standalone
example_c6project.examples/system/build_two_target/sdkconfig.defaults (1)
1-5: Defaults are coherent with the exampleTarget and flash size defaults are consistent and minimal; this is fine for a sample. Just ensure your actual host board matches esp32p4 + 16MB flash.
examples/system/build_two_target/main/idf_component.yml (1)
1-17: Component manifest is well-formedYAML structure and minimal
idfversion declaration look correct for use with the Component Manager.
| #include <sys/param.h> | ||
| #include <string.h> | ||
| #include "esp_err.h" | ||
| #include "esp_log.h" | ||
| #include "driver/uart.h" | ||
| #include "driver/gpio.h" | ||
| #include "esp32_port.h" | ||
| #include "esp_loader.h" | ||
|
|
||
| static const char *TAG = "serial_flasher"; | ||
| extern const uint8_t merged_binary_bin_start[] asm("_binary_merged_binary_bin_start"); | ||
| extern const uint8_t merged_binary_bin_end[] asm("_binary_merged_binary_bin_end"); | ||
|
|
||
| static const char *get_error_string(const esp_loader_error_t error) | ||
| { | ||
| const char *mapping[ESP_LOADER_ERROR_INVALID_RESPONSE + 1] = { | ||
| "NONE", "UNKNOWN", "TIMEOUT", "IMAGE SIZE", | ||
| "INVALID MD5", "INVALID PARAMETER", "INVALID TARGET", | ||
| "UNSUPPORTED CHIP", "UNSUPPORTED FUNCTION", "INVALID RESPONSE" | ||
| }; | ||
|
|
||
| assert(error <= ESP_LOADER_ERROR_INVALID_RESPONSE); | ||
|
|
||
| return mapping[error]; | ||
| } |
There was a problem hiding this comment.
Add explicit headers for assert, FreeRTOS, and printf
This file uses assert(), vTaskDelay(), xTaskCreate(), portTICK_PERIOD_MS, and printf() but doesn’t include the corresponding headers locally. That relies on transitive includes from other IDF headers, which is fragile and will often fail with -Werror=implicit-function-declaration in stricter builds.
To make this unit self‑contained and robust, add the explicit headers near the top:
-#include <sys/param.h>
-#include <string.h>
-#include "esp_err.h"
-#include "esp_log.h"
-#include "driver/uart.h"
-#include "driver/gpio.h"
-#include "esp32_port.h"
-#include "esp_loader.h"
+#include <sys/param.h>
+#include <string.h>
+#include <stdio.h>
+#include <assert.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "esp_err.h"
+#include "esp_log.h"
+#include "driver/uart.h"
+#include "driver/gpio.h"
+#include "esp32_port.h"
+#include "esp_loader.h"This avoids any dependence on indirect includes and keeps the example more portable across IDF versions and toolchains.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #include <sys/param.h> | |
| #include <string.h> | |
| #include "esp_err.h" | |
| #include "esp_log.h" | |
| #include "driver/uart.h" | |
| #include "driver/gpio.h" | |
| #include "esp32_port.h" | |
| #include "esp_loader.h" | |
| static const char *TAG = "serial_flasher"; | |
| extern const uint8_t merged_binary_bin_start[] asm("_binary_merged_binary_bin_start"); | |
| extern const uint8_t merged_binary_bin_end[] asm("_binary_merged_binary_bin_end"); | |
| static const char *get_error_string(const esp_loader_error_t error) | |
| { | |
| const char *mapping[ESP_LOADER_ERROR_INVALID_RESPONSE + 1] = { | |
| "NONE", "UNKNOWN", "TIMEOUT", "IMAGE SIZE", | |
| "INVALID MD5", "INVALID PARAMETER", "INVALID TARGET", | |
| "UNSUPPORTED CHIP", "UNSUPPORTED FUNCTION", "INVALID RESPONSE" | |
| }; | |
| assert(error <= ESP_LOADER_ERROR_INVALID_RESPONSE); | |
| return mapping[error]; | |
| } | |
| #include <sys/param.h> | |
| #include <string.h> | |
| #include <stdio.h> | |
| #include <assert.h> | |
| #include "freertos/FreeRTOS.h" | |
| #include "freertos/task.h" | |
| #include "esp_err.h" | |
| #include "esp_log.h" | |
| #include "driver/uart.h" | |
| #include "driver/gpio.h" | |
| #include "esp32_port.h" | |
| #include "esp_loader.h" | |
| static const char *TAG = "serial_flasher"; | |
| extern const uint8_t merged_binary_bin_start[] asm("_binary_merged_binary_bin_start"); | |
| extern const uint8_t merged_binary_bin_end[] asm("_binary_merged_binary_bin_end"); | |
| static const char *get_error_string(const esp_loader_error_t error) | |
| { | |
| const char *mapping[ESP_LOADER_ERROR_INVALID_RESPONSE + 1] = { | |
| "NONE", "UNKNOWN", "TIMEOUT", "IMAGE SIZE", | |
| "INVALID MD5", "INVALID PARAMETER", "INVALID TARGET", | |
| "UNSUPPORTED CHIP", "UNSUPPORTED FUNCTION", "INVALID RESPONSE" | |
| }; | |
| assert(error <= ESP_LOADER_ERROR_INVALID_RESPONSE); | |
| return mapping[error]; | |
| } |
🧰 Tools
🪛 Clang (14.0.6)
[error] 1-1: 'sys/param.h' file not found
(clang-diagnostic-error)
🤖 Prompt for AI Agents
In examples/system/build_two_target/main/build_two_target.c around lines 1 to
25, the file uses assert(), vTaskDelay(), xTaskCreate(), portTICK_PERIOD_MS and
printf() but lacks their direct headers; add explicit includes near the top:
<assert.h>, <stdio.h>, "freertos/FreeRTOS.h", and "freertos/task.h" so the file
is self-contained and no longer depends on transitive IDF headers.
| // Max line size | ||
| #define BUF_LEN 128 | ||
| static uint8_t buf[BUF_LEN] = {0}; | ||
|
|
||
| void slave_monitor(void *arg) | ||
| { | ||
| #if (HIGHER_BAUDRATE != 115200) | ||
| uart_flush_input(UART_NUM_1); | ||
| uart_flush(UART_NUM_1); | ||
| uart_set_baudrate(UART_NUM_1, 115200); | ||
| #endif | ||
| while (1) { | ||
| int rxBytes = uart_read_bytes(UART_NUM_1, buf, BUF_LEN, 100 / portTICK_PERIOD_MS); | ||
| buf[rxBytes] = '\0'; | ||
| printf("%s", buf); | ||
| } |
There was a problem hiding this comment.
Fix buffer underflow/overflow in slave_monitor when handling UART reads
slave_monitor currently assumes uart_read_bytes always returns a value in [0, BUF_LEN - 1]:
int rxBytes = uart_read_bytes(UART_NUM_1, buf, BUF_LEN, 100 / portTICK_PERIOD_MS);
buf[rxBytes] = '\0';
printf("%s", buf);However, per ESP‑IDF docs, uart_read_bytes returns -1 on error and otherwise a value in [0, length] (inclusive). (docs.espressif.com)
That means:
- If
rxBytes == -1,buf[rxBytes]underflows and corrupts memory. - If
rxBytes == BUF_LEN,buf[rxBytes]writes one byte past the end of the buffer. - If
rxBytes == 0, you end up writingbuf[0] = '\0'and printing an empty string, which is harmless but still doesn’t guard against the error case.
This is a real memory‑safety bug that can crash or destabilize the host.
A safe pattern is to reserve space for the terminator and only null‑terminate when you’ve read a positive number of bytes within range:
-// Max line size
-#define BUF_LEN 128
-static uint8_t buf[BUF_LEN] = {0};
+// Max line size (not counting terminator)
+#define BUF_LEN 128
+static uint8_t buf[BUF_LEN + 1] = {0};
...
- while (1) {
- int rxBytes = uart_read_bytes(UART_NUM_1, buf, BUF_LEN, 100 / portTICK_PERIOD_MS);
- buf[rxBytes] = '\0';
- printf("%s", buf);
- }
+ while (1) {
+ int rxBytes = uart_read_bytes(UART_NUM_1, buf, BUF_LEN, 100 / portTICK_PERIOD_MS);
+ if (rxBytes > 0 && rxBytes <= BUF_LEN) {
+ buf[rxBytes] = '\0';
+ printf("%s", (char *)buf);
+ }
+ // Optionally handle rxBytes < 0 (error) with a log if you want visibility.
+ }This guards both the negative error case and the full‑buffer case while keeping behavior unchanged for normal reads.
🤖 Prompt for AI Agents
In examples/system/build_two_target/main/build_two_target.c around lines 27-42,
the code writes buf[rxBytes] without validating rxBytes which can be -1 or equal
to BUF_LEN; change the logic to check the return: if rxBytes <= 0 handle the
error/empty read (e.g., continue or set buf[0] = '\0' and skip printing),
otherwise if rxBytes >= BUF_LEN clamp rxBytes to BUF_LEN-1, then null-terminate
with buf[rxBytes] = '\0' and call printf; this prevents underflow and overflow
while preserving behavior for normal reads.
1 participant
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.