Uh oh!
There was an error while loading. Please reload this page.
feat(firmware/c_board): Introduce bootloader and DFU flashing pipeline - #28
Merged
Conversation
- Add `firmware/c_board/bootloader` target to implement download, write, and manifest flows via TinyUSB DFU. - Establish image layout and metadata management, featuring a validation chain with CRC32 and SHA256 (`HASH` suffix). - Prioritize app image validation on startup, jumping from the bootloader to the app upon successful verification. - Enable DFU Runtime in the application layer, allowing reboot requests into DFU mode via the `.boot_mailbox`. - Split APP and BOOTLOADER linker scripts, introducing `MAILBOX` and `.flash_sector_cache` sections. - Add post-build processing to generate `c_board_app.bin` and `c_board_app.dfu`. - Update Docker/CI packaging to include bootloader artifacts and switch the default c_board release artifact to `.dfu`. - Update lint targets and add `dfu-util` to the build image dependencies. Note: Rollbacks are not supported once the flashing process begins; a full flash cycle must be completed.
Walkthrough该 PR 在 c_board 上加入完整引导加载程序与 USB DFU 支持，包含引导邮箱、DFU 协议实现、分区写入器、镜像验证（CRC32 + SHA‑256）、构建与打包改动以生成 bootloader 与 DFU 工件，并对应用端添加 DFU 运行时钩子。 Changes
Sequence DiagramssequenceDiagram
participant Host as Host (PC)
participant Device as Device (Bootloader)
participant Flash as Flash 存储
participant Metadata as Metadata
Host->>Device: USB DFU 连接并发送 download blocks
activate Device
Device->>Device: Dfu::download 验证地址/块序与边界
Device->>Flash: Writer.write(分扇区缓冲并编程)
Flash-->>Device: 编程完成
Device->>Metadata: 更新/写入临时状态（flashing）
Device-->>Host: 返回 DFU_STATUS_OK
Host->>Device: 发送 manifest（完成）
Device->>Device: Dfu::manifest 验证向量表与 CRC/SHA
Device->>Metadata: finish_flashing(写就绪元数据)
Metadata-->>Device: 元数据写入确认
Device->>Device: 请求系统重置（pending reset）
deactivate Device
sequenceDiagram
participant App as Application
participant USB as TinyUSB (DFU runtime)
participant Mailbox as BootMailbox (shared)
participant Device as Device (将重启为 Bootloader)
App->>USB: 用户触发 DFU 运行时重启命令
USB->>App: 调用 tud_dfu_runtime_reboot_to_dfu_cb()
App->>Mailbox: boot_mailbox.request_enter_dfu()
Mailbox-->>App: 标记已设置
App->>Device: 执行同步屏障并触发系统重置
Device->>Device: 启动引导加载程序，检查 mailbox
Mailbox-->>Device: 检测到 DFU 请求
Device->>Device: 进入 DFU 模式并初始化 USB DFU 服务
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 分钟 Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
qzhhhi
commented
Mar 2, 2026
MemberAuthor
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
firmware/c_board/bootloader/CMakeLists.txt (1)
32-35: 考虑显式列出源文件以提高构建可靠性。使用
file(GLOB_RECURSE ... CONFIGURE_DEPENDS)虽然方便，但 CMake 官方文档指出CONFIGURE_DEPENDS在某些构建系统中可能无法可靠地检测新增文件。对于源文件相对稳定的嵌入式项目，这通常可以接受，但显式列出源文件（如 TinyUSB 源文件的处理方式）可以避免潜在的构建问题。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bootloader/CMakeLists.txt` around lines 32 - 35, The current use of file(GLOB_RECURSE C_BOARD_BOOTLOADER_SOURCES CONFIGURE_DEPENDS ...) is fragile; replace the glob with an explicit list assigned to C_BOARD_BOOTLOADER_SOURCES (or include a separate generated/list file) enumerating all .c/.cpp bootloader sources (same symbols: C_BOARD_BOOTLOADER_SOURCES and the CMakeLists.txt target that consumes it) and remove CONFIGURE_DEPENDS so new/removed files are tracked reliably by CMake; list each TinyUSB and board-specific source file explicitly and update any add_executable/add_library calls that use C_BOARD_BOOTLOADER_SOURCES accordingly.Dockerfile.build_firmware (1)
24-27: 建议统一 bootloader 产物命名以提升可追溯性。当前 firmware 文件带版本号，而 bootloader 文件不带版本号，后续归档/比对会不一致。
♻️ 建议修改
- cp "${build_dir}/bootloader/${board}_bootloader.elf" \- "/output/librmcs-bootloader-${board}.elf"; \+ cp "${build_dir}/bootloader/${board}_bootloader.elf" \+ "/output/librmcs-bootloader-${board}-${LIBRMCS_PROJECT_VERSION}.elf"; \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile.build_firmware` around lines 24 - 27, The bootloader output file lacks the project version while the firmware file includes ${LIBRMCS_PROJECT_VERSION}, causing inconsistent artifact names; update the cp that copies "${build_dir}/bootloader/${board}_bootloader.elf" to write "/output/librmcs-bootloader-${board}-${LIBRMCS_PROJECT_VERSION}.elf" (matching the firmware pattern), ensuring you reference the same build_dir, board and LIBRMCS_PROJECT_VERSION variables as used for the firmware cp command.firmware/c_board/app/CMakeLists.txt (1)
60-62: 建议显式用 Python 解释器调用脚本，减少环境依赖。当前直接执行脚本路径依赖可执行位与 shebang；改为显式
python3调用更稳健。♻️ 建议修改
- COMMAND ${LIBRMCS_PROJECT_ROOT}/.scripts/append_image_hash+ COMMAND python3 ${LIBRMCS_PROJECT_ROOT}/.scripts/append_image_hash -o $<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu $<TARGET_FILE_DIR:c_board_app>/c_board_app.bin🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/CMakeLists.txt` around lines 60 - 62, 当前 CMake COMMAND 直接调用脚本路径 (${LIBRMCS_PROJECT_ROOT}/.scripts/append_image_hash) 依赖可执行位与 shebang，改为显式使用 Python 解释器来降低环境依赖：在 CMakeLists 中将该 COMMAND 改为调用 `python3`（或 `${PYTHON_EXECUTABLE}` 若已定义）并传入相同参数 so that the script is invoked as `python3 ${LIBRMCS_PROJECT_ROOT}/.scripts/append_image_hash -o $<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu $<TARGET_FILE_DIR:c_board_app>/c_board_app.bin`, ensuring the target names ($<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu and .../c_board_app.bin) remain unchanged.firmware/c_board/bootloader/src/crypto/sha256.hpp (1)
25-25:kSha256BlockSize命名可更精确这里的
32实际是摘要长度（digest size），不是 SHA-256 的消息分组大小（64 字节）。建议改名为kSha256DigestSize，减少语义歧义。♻️ 建议修改
-inline constexpr size_t kSha256BlockSize = 32U;+inline constexpr size_t kSha256DigestSize = 32U;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bootloader/src/crypto/sha256.hpp` at line 25, Rename the misleading constant kSha256BlockSize to kSha256DigestSize and keep its value as 32U; update the declaration in sha256.hpp and replace every usage of kSha256BlockSize across the codebase (e.g., in functions, structs, tests, and any static_asserts) to refer to kSha256DigestSize so the name reflects the digest length (not the 64‑byte message block size) and adjust any related comments/documentation to match.firmware/c_board/app/src/usb/usb_descriptors.hpp (1)
49-50: 建议将 DFU 字符串索引提取为常量。Line 49 与 Line 165 目前都硬编码
4，后续调整字符串表时容易漂移，建议统一为命名常量。♻️ 建议改法
+ static constexpr uint8_t kStringIndexDfuRuntime = 4;+ uint16_t const* get_string_descriptor(uint8_t index, uint16_t langid) { @@ - case 4: str = kDfuRuntimeString; break;+ case kStringIndexDfuRuntime: str = kDfuRuntimeString; break; @@ TUD_DFU_RT_DESCRIPTOR( - kItfNumDfuRuntime, 4, DFU_ATTR_CAN_DOWNLOAD | DFU_ATTR_WILL_DETACH, 1000, 1024),+ kItfNumDfuRuntime,+ kStringIndexDfuRuntime,+ DFU_ATTR_CAN_DOWNLOAD | DFU_ATTR_WILL_DETACH,+ 1000,+ 1024),Also applies to: 164-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/src/usb/usb_descriptors.hpp` around lines 49 - 50, Introduce a named constant for the DFU string index (e.g., DFU_STRING_INDEX or kDfuStringIndex) and replace the hard-coded literal 4 used in the switch case and the other occurrence(s) (the branch that currently uses 4 at both the case in the descriptor resolver and the other check at/around lines 164-165) with that constant; update any comments/uses in the same scope so all references to the DFU string index use the new constant (search for the case 4 and the other 4 occurrence and swap them to the single constant) to avoid drift when the string table changes.firmware/c_board/app/src/app.cpp (1)
24-24: 避免在 VTOR 设置中硬编码应用基址。Line 24 直接写
0x08010000U，后续若链接脚本或分区调整，这里可能静默失配并把中断向量指向错误地址。建议改为共享布局常量（与 bootloader 跳转地址同源），至少先提取为命名常量避免魔法值。♻️ 建议改法（最小改动）
+namespace {+constexpr uint32_t kAppVectorTableAddress = 0x08010000U; // TODO: 改为共享 flash layout 常量+}+ int main() { - SCB->VTOR = 0x08010000U;+ SCB->VTOR = kAppVectorTableAddress; librmcs::firmware::app.init().run(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/src/app.cpp` at line 24, Replace the hardcoded VTOR value (SCB->VTOR = 0x08010000U) with a shared, named symbol used by the bootloader/linker so the vector base tracks layout changes; expose a single source of truth (e.g., APPLICATION_BASE or an extern linker symbol like _app_start/_app_vector) in a header both bootloader and app include, then set SCB->VTOR to that symbol (properly cast to uint32_t) and ensure the symbol aligns to a vector table boundary. This removes the magic literal and keeps bootloader jump address and VTOR in sync.firmware/c_board/bootloader/src/flash/metadata.hpp (1)
54-56: 元数据地址与擦除扇区建议统一由layout.hpp提供。当前地址与 sector 是硬编码，和 linker/layout 存在双源配置风险；后续内存布局调整时容易漏改，导致误擦写。建议把 region/sector 常量集中到
firmware/c_board/bootloader/src/flash/layout.hpp，本文件仅引用。♻️ 建议重构
- static constexpr uintptr_t kMetadataStartAddress = 0x0800C000U;- static constexpr uintptr_t kMetadataEndAddress = 0x08010000U;+ static constexpr uintptr_t kMetadataStartAddress = kMetadataRegionStartAddress;+ static constexpr uintptr_t kMetadataEndAddress = kMetadataRegionEndAddress; @@ - erase.Sector = FLASH_SECTOR_3;+ erase.Sector = kMetadataRegionFlashSector;并在
firmware/c_board/bootloader/src/flash/layout.hpp中维护kMetadataRegionStartAddress/kMetadataRegionEndAddress/kMetadataRegionFlashSector。Also applies to: 184-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bootloader/src/flash/metadata.hpp` around lines 54 - 56, The metadata start/end addresses are hardcoded in kMetadataStartAddress and kMetadataEndAddress; move these constants into the central flash layout and reference them instead: add kMetadataRegionStartAddress, kMetadataRegionEndAddress and kMetadataRegionFlashSector to firmware/c_board/bootloader/src/flash/layout.hpp and replace uses of kMetadataStartAddress/kMetadataEndAddress (and the other hardcoded sector values noted elsewhere) to include layout.hpp and use the new kMetadataRegion* symbols so the region/sector definitions are single-sourced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@firmware/c_board/app/CMakeLists.txt`:
- Around line 63-66: add_custom_command 的 COMMAND 中不能直接使用 shell
重定向；把当前以字面参数传入的重定向修正为通过 shell 执行：将原来调用 dfu-suffix ... -a
$<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu > /dev/null 的片段改为通过 /bin/sh -c
包裹整个命令字符串（包含重定向），即在 add_custom_command 的 COMMAND 中调用 /bin/sh -c "dfu-suffix -v
0xA11C -p 0xD401 -d 0x0100 -a $<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu >
/dev/null"；在 CMakeLists 中定位包含 dfu-suffix 调用的 add_custom_command 并替换为该包装形式以确保重定向被
shell 解析。
In `@firmware/c_board/bootloader/src/flash/validation.hpp`:
- Around line 19-38: The reset handler range check in is_vector_table_valid()
currently allows any address < kAppEndAddress, but the verified image only spans
kAppStartAddress up to kAppStartAddress + size (the region covered by the
CRC/SHA in the other validation code). Update is_vector_table_valid() to
constrain reset_handler_addr to be >= kAppStartAddress and < (kAppStartAddress +
image_size / validated_size) — i.e. use the same image size variable used by the
CRC/SHA routine (the "size" used in the CRC/SHA checks) instead of
kAppEndAddress so the reset vector cannot point into unvalidated/old code;
reference is_vector_table_valid(), kAppStartAddress, reset_handler_addr, and the
CRC/SHA size variable used in the validation block.
In `@firmware/c_board/bootloader/src/flash/writer.hpp`:
- Around line 100-107: The current advance_buffer(uint32_t offset, size_t size)
allows non-contiguous writes that lead to unwritten gaps being left as 0xFF
after the "erase sector + write prefix [0, buffered_size_)" strategy; update the
logic to prevent unordered writes by either (A) enforcing contiguous writes
inside the active sector: check has_active_sector_ and require offset ==
buffered_size_ (or offset <= buffered_size_ only if you intend to allow
overwrite inside the already-buffered range) and assert/error otherwise, or (B)
implement a full-sector read-modify-write path: when a write targets a
non-contiguous offset, read the entire sector into the buffer, merge the
incoming data at the offset, and set buffered_size_ = sector_size (or the merged
highest dirty byte) before erasing+writing; apply the same fix to the related
logic referenced around lines 117-131 that updates buffered_size_ and performs
commit/flush so gaps cannot be left as 0xFF.
- Around line 34-39: The runtime parameter checks in write() currently use
utility::assert_debug (lines checking !data.empty(), address range against
kAppStartAddress/kAppEndAddress, end64 <= kAppEndAddress, and the UINT32_MAX
overflow check) which becomes assume under NDEBUG; replace these with
runtime-safe validation—either call utility::assert_always for invariant
enforcement or (preferably) perform explicit input validation and
return/propagate an error code or boolean from write() when the checks fail so
invalid runtime inputs are handled deterministically instead of being optimized
away.
In `@firmware/c_board/bootloader/src/usb/usb_descriptors.hpp`:
- Around line 65-76: The bootloader's update_serial_string() currently formats
the raw HAL_GetUIDw* values directly, while the app uses mix_uid_entropy()
before formatting, causing inconsistent USB serials after DFU; update
update_serial_string() to call the same mix_uid_entropy() path (or move the
mixing+formatting into a shared helper used by both bootloader and app), e.g.
obtain the UID words via HAL_GetUIDw0/1/2, pass them through mix_uid_entropy()
and then use write_hex_u16(...) to write into serial_string_.data() (cursor
handling remains the same) so both sides use the exact same algorithm for serial
generation.
In `@firmware/c_board/bootloader/src/utility/jump.hpp`:
- Around line 10-12: 在执行跳转前对从镜像读取的向量表条目做最小合法性校验：验证 app_entry 的最低位为 1（确保为 Thumb
函数指针）、验证 app_stack 的对齐（至少 8 字节对齐且不为 0），并可选检查 app_stack 所在于合法 RAM 区间。将这些检查加入设置
VTOR 和切换栈（使用 app_stack、app_entry、app_reset_handler）的逻辑里，若任一检查失败则不修改
VTOR/堆栈并立即返回错误或进入 fail-fast 路径。
---
Nitpick comments:
In `@Dockerfile.build_firmware`:
- Around line 24-27: The bootloader output file lacks the project version while
the firmware file includes ${LIBRMCS_PROJECT_VERSION}, causing inconsistent
artifact names; update the cp that copies
"${build_dir}/bootloader/${board}_bootloader.elf" to write
"/output/librmcs-bootloader-${board}-${LIBRMCS_PROJECT_VERSION}.elf" (matching
the firmware pattern), ensuring you reference the same build_dir, board and
LIBRMCS_PROJECT_VERSION variables as used for the firmware cp command.
In `@firmware/c_board/app/CMakeLists.txt`:
- Around line 60-62: 当前 CMake COMMAND 直接调用脚本路径
(${LIBRMCS_PROJECT_ROOT}/.scripts/append_image_hash) 依赖可执行位与 shebang，改为显式使用
Python 解释器来降低环境依赖：在 CMakeLists 中将该 COMMAND 改为调用 `python3`（或
`${PYTHON_EXECUTABLE}` 若已定义）并传入相同参数 so that the script is invoked as `python3
${LIBRMCS_PROJECT_ROOT}/.scripts/append_image_hash -o
$<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu
$<TARGET_FILE_DIR:c_board_app>/c_board_app.bin`, ensuring the target names
($<TARGET_FILE_DIR:c_board_app>/c_board_app.dfu and .../c_board_app.bin) remain
unchanged.
In `@firmware/c_board/app/src/app.cpp`:
- Line 24: Replace the hardcoded VTOR value (SCB->VTOR = 0x08010000U) with a
shared, named symbol used by the bootloader/linker so the vector base tracks
layout changes; expose a single source of truth (e.g., APPLICATION_BASE or an
extern linker symbol like _app_start/_app_vector) in a header both bootloader
and app include, then set SCB->VTOR to that symbol (properly cast to uint32_t)
and ensure the symbol aligns to a vector table boundary. This removes the magic
literal and keeps bootloader jump address and VTOR in sync.
In `@firmware/c_board/app/src/usb/usb_descriptors.hpp`:
- Around line 49-50: Introduce a named constant for the DFU string index (e.g.,
DFU_STRING_INDEX or kDfuStringIndex) and replace the hard-coded literal 4 used
in the switch case and the other occurrence(s) (the branch that currently uses 4
at both the case in the descriptor resolver and the other check at/around lines
164-165) with that constant; update any comments/uses in the same scope so all
references to the DFU string index use the new constant (search for the case 4
and the other 4 occurrence and swap them to the single constant) to avoid drift
when the string table changes.
In `@firmware/c_board/bootloader/CMakeLists.txt`:
- Around line 32-35: The current use of file(GLOB_RECURSE
C_BOARD_BOOTLOADER_SOURCES CONFIGURE_DEPENDS ...) is fragile; replace the glob
with an explicit list assigned to C_BOARD_BOOTLOADER_SOURCES (or include a
separate generated/list file) enumerating all .c/.cpp bootloader sources (same
symbols: C_BOARD_BOOTLOADER_SOURCES and the CMakeLists.txt target that consumes
it) and remove CONFIGURE_DEPENDS so new/removed files are tracked reliably by
CMake; list each TinyUSB and board-specific source file explicitly and update
any add_executable/add_library calls that use C_BOARD_BOOTLOADER_SOURCES
accordingly.
In `@firmware/c_board/bootloader/src/crypto/sha256.hpp`:
- Line 25: Rename the misleading constant kSha256BlockSize to kSha256DigestSize
and keep its value as 32U; update the declaration in sha256.hpp and replace
every usage of kSha256BlockSize across the codebase (e.g., in functions,
structs, tests, and any static_asserts) to refer to kSha256DigestSize so the
name reflects the digest length (not the 64‑byte message block size) and adjust
any related comments/documentation to match.
In `@firmware/c_board/bootloader/src/flash/metadata.hpp`:
- Around line 54-56: The metadata start/end addresses are hardcoded in
kMetadataStartAddress and kMetadataEndAddress; move these constants into the
central flash layout and reference them instead: add
kMetadataRegionStartAddress, kMetadataRegionEndAddress and
kMetadataRegionFlashSector to firmware/c_board/bootloader/src/flash/layout.hpp
and replace uses of kMetadataStartAddress/kMetadataEndAddress (and the other
hardcoded sector values noted elsewhere) to include layout.hpp and use the new
kMetadataRegion* symbols so the region/sector definitions are single-sourced.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.github/workflows/release-package.yml.scripts/append_image_hash.scripts/lint-targets.ymlDockerfileDockerfile.build_firmwarefirmware/c_board/CMakeLists.txtfirmware/c_board/app/CMakeLists.txtfirmware/c_board/app/include/tusb_config.hfirmware/c_board/app/src/app.cppfirmware/c_board/app/src/usb/usb_descriptors.hppfirmware/c_board/app/src/usb/vendor.cppfirmware/c_board/app/src/utility/boot_mailbox.hppfirmware/c_board/bootloader/CMakeLists.txtfirmware/c_board/bootloader/include/tusb_config.hfirmware/c_board/bootloader/src/crypto/sha256.hppfirmware/c_board/bootloader/src/flash/layout.hppfirmware/c_board/bootloader/src/flash/metadata.hppfirmware/c_board/bootloader/src/flash/unlock_guard.hppfirmware/c_board/bootloader/src/flash/validation.hppfirmware/c_board/bootloader/src/flash/writer.hppfirmware/c_board/bootloader/src/main.cppfirmware/c_board/bootloader/src/usb/dfu.cppfirmware/c_board/bootloader/src/usb/dfu.hppfirmware/c_board/bootloader/src/usb/usb_descriptors.cppfirmware/c_board/bootloader/src/usb/usb_descriptors.hppfirmware/c_board/bootloader/src/utility/assert.hppfirmware/c_board/bootloader/src/utility/boot_mailbox.hppfirmware/c_board/bootloader/src/utility/jump.hppfirmware/c_board/bsp/cubemx/Core/Inc/main.hfirmware/c_board/bsp/cubemx/STM32F407XX_APP.ldfirmware/c_board/bsp/cubemx/STM32F407XX_BOOTLOADER.ld
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
firmware/c_board/bootloader/src/usb/usb_descriptors.hpp (1)
73-78: 建议补充序列号写入后的游标一致性断言。这里的游标推进依赖
serial_string_模板格式（含-分隔位）。建议在循环后增加内部契约断言，防止后续模板改动时出现静默越界。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bootloader/src/usb/usb_descriptors.hpp` around lines 73 - 78, Add an internal contract assertion after the loop that validates the cursor did not overrun and ended where the serial-string template expects: compute a pointer to the buffer end (e.g., auto* end = serial_string_.data() + serial_string_.size()) and then assert(cursor <= end) and, if you can compute the exact expected position from the template, assert(cursor == expected_end); reference the existing symbols serial_string_, uid and write_hex_u16 when adding the check to the same scope as the loop so template changes will trigger the assertion rather than silent bounds errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@firmware/c_board/bootloader/src/usb/usb_descriptors.hpp`:
- Around line 73-78: Add an internal contract assertion after the loop that
validates the cursor did not overrun and ended where the serial-string template
expects: compute a pointer to the buffer end (e.g., auto* end =
serial_string_.data() + serial_string_.size()) and then assert(cursor <= end)
and, if you can compute the exact expected position from the template,
assert(cursor == expected_end); reference the existing symbols serial_string_,
uid and write_hex_u16 when adding the check to the same scope as the loop so
template changes will trigger the assertion rather than silent bounds errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
firmware/c_board/bootloadertarget to implement download, write, and manifest flows via TinyUSB DFU.HASHsuffix)..boot_mailbox.MAILBOXand.flash_sector_cachesections.c_board_app.binandc_board_app.dfu..dfu.dfu-utilto the build image dependencies.Note: Rollbacks are not supported once the flashing process begins; a full flash cycle must be completed.
引入 Bootloader 与 DFU 刷写管道
概述
本 PR 为 c_board 固件引入完整的 Bootloader 与 USB DFU（Device Firmware Update）刷写流水线，包含镜像布局与元数据管理、校验链（CRC32 + SHA‑256 HASH 后缀）、Boot Mailbox 协议以便应用请求进入 DFU、以及构建/CI 发布工件变更（将 .dfu 作为默认发布固件）。
Bootloader（firmware/c_board/bootloader）
应用层变更（app）
后构建处理与发行工件
链接脚本与内存布局
安全性与校验
关键参数与实现细节（要点）
需注意/待评估项