Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Walkthrough新增 I2C0 端到端支持：数据模型、协议常量、序列化/反序列化、主机代理扩展、STM32/HPM 平台固件驱动（DMA/中断/恢复/上行发布），并伴随构建、CubeMX 与开发容器配置变更。 Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,220,255,0.5)
participant USB as USB 下行
participant Deser as 反序列化器
participant HostCB as 主机回调/Handler
end
rect rgba(200,255,200,0.5)
participant App as I2C 驱动 (固件)
participant DMA as DMA/HAL
participant Dev as I2C 设备
end
USB->>Deser: 收到 I2C 下行帧
Deser->>HostCB: i2c_*_deserialized_callback(...)
HostCB->>App: route to board I2C handler (handle_downlink_*)
App->>DMA: start DMA transfer / HAL 非阻塞调用
DMA->>Dev: 总线传输（SCL/SDA）
Dev-->>DMA: 返回数据 / ACK / NACK
DMA-->>App: 完成或错误回调
App->>HostCB: publish read result 或 error 上行
HostCB->>USB: 序列化并发送 USB 上行
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 分钟 Possibly related PRs
兔子的诗
🚥 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 docstrings
🧪 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 |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
firmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hpp (1)
42-47:⚠️ Potential issue | 🟡 Minor缓冲区满时缺少指示钩子，与 c_board 行为不一致。
c_board 版本在
allocate()缓冲区满时调用led::led->uplink_buffer_full()，而此处仅留 TODO 并静默返回空 span。这会导致 rmcs_board 上行丢包时用户/开发者无任何可观测信号，调试困难。建议至少接一个平台无关的钩子（弱符号函数或回调），即便初期为空实现也便于后续替换。基于 learnings（qzhhhi, PR 32）：
allocate()缓冲区满应由本层直接指示（LED/log），不应向上层传播kBadAlloc；此分层设计要求 rmcs_board 也在本层提供指示入口，而非留空 TODO。需要我草拟一个与 c_board 对齐的弱钩子接口（例如
librmcs::firmware::usb::on_uplink_buffer_full()）吗？🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hpp` around lines 42 - 47, The allocate() path currently silently returns an empty span when writeable == 0, missing the buffer-full indication present in c_board; add a platform-agnostic weak hook that this function calls before returning (e.g. declare a weak function like librmcs::firmware::usb::on_uplink_buffer_full() or a static callback invoked from allocate()), and invoke it in the branch where writeable == 0 (around the TODO) so rmcs_board can provide an LED/logging implementation while keeping a no-op default in the library.
🧹 Nitpick comments (7)
firmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hpp (1)
92-92: 内存序常量风格与文件其余部分不一致。本文件其余处统一使用 C++20 枚举式
std::memory_order::relaxed/std::memory_order::release，但此两处使用了旧式宏std::memory_order_release。功能等价，仅为风格一致性。♻️ 建议统一写法
- std::atomic_signal_fence(std::memory_order_release); + std::atomic_signal_fence(std::memory_order::release);（line 92 与 line 119 各一处）
Also applies to: 119-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hpp` at line 92, Replace the old-style macro memory order constants with the C++20 enum-style form for consistency: locate the two calls to std::atomic_signal_fence that currently use std::memory_order_release (lines around the occurrences) and change them to use std::memory_order::release (and similarly change any std::memory_order_relaxed to std::memory_order::relaxed if present); ensure you update both occurrences (the one at the std::atomic_signal_fence call near line 92 and the other near line 119) so the file consistently uses std::memory_order::release/::relaxed style.firmware/c_board/bsp/cubemx/Core/Src/dma.c (1)
47-61: DMA1_Stream2/7 的中断优先级与 USB OTG 优先级需要协调。DMA1_Stream2_IRQn（I2C2 RX）和 DMA1_Stream7_IRQn（I2C2 TX）的抢占优先级均为 1，而 OTG_FS_IRQn 优先级为 0，这意味着 USB OTG 中断可以抢占 I2C2 DMA 完成处理。同时，I2C2_EV_IRQn 和 I2C2_ER_IRQn 也被设为优先级 1。由于 DMA 完成回调会驱动 I2C 状态机，建议确认是否应将 DMA1_Stream2/7 的优先级调整至低于 USB OTG（例如优先级 4 或 6，与其它 DMA 流一致），以确保 I2C2 数据传输路径不被 USB 处理中断抢占，避免 TinyUSB 出现掉包或状态不一致。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bsp/cubemx/Core/Src/dma.c` around lines 47 - 61, DMA1_Stream2_IRQn and DMA1_Stream7_IRQn are currently set to priority 1 which allows USB OTG (priority 0) to preempt I2C2 DMA handlers and can cause I2C state issues; change the HAL_NVIC_SetPriority calls for DMA1_Stream2_IRQn and DMA1_Stream7_IRQn to a lower priority value (e.g., 4 or 6 to match other DMA streams) so they cannot be preempted by OTG_FS_IRQn, and ensure this change is consistent with I2C2_EV_IRQn / I2C2_ER_IRQn priority assignments.firmware/rmcs_board/boards/pro/board.c (1)
173-185: 对i2c_config_t config进行零初始化。当前代码只显式初始化了
i2c_mode和is_10bit_addressing两个字段。由于i2c_config_t定义在外部 HPM SDK 中，如果该结构体有其他成员在i2c_init_master()中被读取，未初始化的字段将传入驱动，导致未定义行为。使用{0}零初始化成本极低，是 C 编程最佳实践，也便于应对未来 SDK 版本中结构体定义的变化。建议修正
bool board_init_i2c(I2C_Type* ptr) { - i2c_config_t config; + i2c_config_t config = {0}; const uint32_t freq = board_init_i2c_clock(ptr);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/rmcs_board/boards/pro/board.c` around lines 173 - 185, Zero-initialize the i2c_config_t before setting individual fields in board_init_i2c so any unspecified members don't remain indeterminate; locate the i2c_config_t config declaration in board_init_i2c and replace its uninitialized declaration with a zero-initialized one (e.g., initialize config with {0} or use memset) then keep setting config.i2c_mode and config.is_10bit_addressing as before before calling i2c_init_master.core/src/protocol/deserializer.cpp (1)
313-314:slave_address > 0x7F为结构上不可达分支（仅作为防御层保留）
I2cHeader::SlaveAddress定义为BitfieldMember<8, 7>，bitfield 提取出的值上界即0x7F，故slave_address > 0x7F永远为 false，分支是死代码。根据历史学习笔记，此校验是刻意作为“多层防御”保留的一环（协议层兜底），所以不建议删除；但位宽变更时该断言会静默失效。可选增强：把防御由运行期改为编译期约束，位宽变化时会直接编译报错，意图与效果更清晰。
♻️ 可选：转为 static_assert
- if (slave_address > 0x7F) [[unlikely]] - co_return false; + static_assert( + I2cHeader::SlaveAddress::kBitWidth <= 7, + "slave_address bitfield width must keep values within 7-bit I2C address range");根据学习笔记
Learnt from: gqsdjhh PR#43 core/src/protocol/deserializer.cpp:313—— 此 0x7F 校验是协议层兜底的多层防御之一，本建议不改变其作为“防御点”的定位，仅把运行期兜底替换为编译期约束。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/src/protocol/deserializer.cpp` around lines 313 - 314, The runtime check "if (slave_address > 0x7F)" is dead because I2cHeader::SlaveAddress is defined as BitfieldMember<8, 7> so the extracted value cannot exceed 0x7F; replace the runtime guard with a compile-time assertion to preserve the defensive intent: add a static_assert that validates the bitfield width/maximum (e.g. that BitfieldMember<8,7>::max_value == 0x7F or that the bit-width used by I2cHeader::SlaveAddress is 7) near the I2cHeader/BitfieldMember definitions and remove the unreachable runtime branch in deserializer.cpp so changes to BitfieldMember widths will produce a compile error instead of silently disabling the check.firmware/rmcs_board/app/src/usb/vendor.hpp (1)
161-171:i2c_read_result/i2c_error回调静默丢弃的风格建议
can_deserialized_callback/uart_deserialized_callback等对未知DataId均走assert_failed_always()；此处对read_result与error则不分 id 一律静默丢弃。从语义上，host 一般不应向固件下行kReadResult/kError，收到属于协议滥用。当前实现与error_callback()改为“丢弃而非 trap”的整体策略兼容，但相对其它回调的default: assert_failed_always()略显不一致。建议二选一以便意图更明确：
- 若确定“任何方向的下行 read_result/error 都视为可忽略流量”，在函数体加一行注释明确这是按设计的静默丢弃（与
error_callback中的注释风格一致）；- 若希望与 can/uart 保持一致，对未知 id 使用
core::utility::assert_failed_always()，对合法 id 仍(void)。不阻塞合并。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/rmcs_board/app/src/usb/vendor.hpp` around lines 161 - 171, The two I2C callbacks i2c_read_result_deserialized_callback and i2c_error_deserialized_callback currently silently drop all ids; pick one consistent intent and make it explicit: either (A) mark silent-drop as deliberate by adding a one-line comment inside each function stating "deliberate silent discard of downstream kReadResult/kError" (matching error_callback's comment style) while keeping the (void) casts, or (B) make them strict like can_deserialized_callback by calling core::utility::assert_failed_always() for unknown/unsupported ids while still keeping (void)id/(void)data for any handled ids; update both i2c_read_result_deserialized_callback and i2c_error_deserialized_callback accordingly so the intent is clear and consistent with other callbacks.firmware/rmcs_board/boards/lite/board.c (1)
172-184: 建议对i2c_config_t进行零初始化以防御 SDK 字段扩展当前代码在
i2c_init_master前显式设置i2c_mode与is_10bit_addressing两个字段。HPM SDK 后续若在i2c_config_t中新增字段，未初始化的字节会被读取并可能引发隐患。做一次零初始化成本为零且可提前规避该风险。建议的改动
bool board_init_i2c(I2C_Type* ptr) { - i2c_config_t config; + i2c_config_t config = {}; const uint32_t freq = board_init_i2c_clock(ptr);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/rmcs_board/boards/lite/board.c` around lines 172 - 184, The i2c_config_t config in board_init_i2c is only partially initialized and may read uninitialized bytes if the SDK adds fields; zero-initialize config (e.g., set to all zeros or use memset/aggregate initialization) before assigning config.i2c_mode and config.is_10bit_addressing so that i2c_init_master always receives a fully-initialized structure.firmware/rmcs_board/app/src/i2c/i2c.hpp (1)
720-727: 如果BOARD_APP_I2C_BASE被定义为指针，constinit分支会编译失败。当前代码能成功编译，因为
BOARD_APP_I2C_BASE在项目中未被定义，故总是走#ifndef分支使用HPM_I2C0_BASE。但如果未来某块板级头将BOARD_APP_I2C_BASE定义为 typed pointer（如I2C_Type* BOARD_APP_I2C_BASE = HPM_I2C0），则reinterpret_cast<uintptr_t>(BOARD_APP_I2C_BASE)在consteval上下文中无法作为编译期常量表达式被求值，导致编译失败。建议采用下列方案之一避免此隐患：
- 要求板级头提供整数型 base-address 宏（如
#define BOARD_APP_I2C0_BASE 0x...），而非指针- 在
Lazy或I2c构造阶段引入中间转换层，允许接受指针但在初始化时（非consteval阶段）转换为uintptr_t
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/devcontainer.json:
- Line 28: Update the postCreateCommand: remove the directory existence checks
`[ -d ~/.codex ]` and `[ -d ~/.claude ]` so the codex and claude install
branches run based on whether the executables are present (the `command -v
codex` / `command -v claude` checks remain), and enable pipefail so failures in
pipelines are propagated (apply `set -o pipefail` or use `bash -o pipefail` for
the shell invocation that runs the curl | bash pipeline). Target the
postCreateCommand entry that contains the codex and claude install logic and the
`curl ... | bash` pipeline.
---
Outside diff comments:
In `@firmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hpp`:
- Around line 42-47: The allocate() path currently silently returns an empty
span when writeable == 0, missing the buffer-full indication present in c_board;
add a platform-agnostic weak hook that this function calls before returning
(e.g. declare a weak function like
librmcs::firmware::usb::on_uplink_buffer_full() or a static callback invoked
from allocate()), and invoke it in the branch where writeable == 0 (around the
TODO) so rmcs_board can provide an LED/logging implementation while keeping a
no-op default in the library.
---
Nitpick comments:
In `@core/src/protocol/deserializer.cpp`:
- Around line 313-314: The runtime check "if (slave_address > 0x7F)" is dead
because I2cHeader::SlaveAddress is defined as BitfieldMember<8, 7> so the
extracted value cannot exceed 0x7F; replace the runtime guard with a
compile-time assertion to preserve the defensive intent: add a static_assert
that validates the bitfield width/maximum (e.g. that
BitfieldMember<8,7>::max_value == 0x7F or that the bit-width used by
I2cHeader::SlaveAddress is 7) near the I2cHeader/BitfieldMember definitions and
remove the unreachable runtime branch in deserializer.cpp so changes to
BitfieldMember widths will produce a compile error instead of silently disabling
the check.
In `@firmware/c_board/bsp/cubemx/Core/Src/dma.c`:
- Around line 47-61: DMA1_Stream2_IRQn and DMA1_Stream7_IRQn are currently set
to priority 1 which allows USB OTG (priority 0) to preempt I2C2 DMA handlers and
can cause I2C state issues; change the HAL_NVIC_SetPriority calls for
DMA1_Stream2_IRQn and DMA1_Stream7_IRQn to a lower priority value (e.g., 4 or 6
to match other DMA streams) so they cannot be preempted by OTG_FS_IRQn, and
ensure this change is consistent with I2C2_EV_IRQn / I2C2_ER_IRQn priority
assignments.
In `@firmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hpp`:
- Line 92: Replace the old-style macro memory order constants with the C++20
enum-style form for consistency: locate the two calls to
std::atomic_signal_fence that currently use std::memory_order_release (lines
around the occurrences) and change them to use std::memory_order::release (and
similarly change any std::memory_order_relaxed to std::memory_order::relaxed if
present); ensure you update both occurrences (the one at the
std::atomic_signal_fence call near line 92 and the other near line 119) so the
file consistently uses std::memory_order::release/::relaxed style.
In `@firmware/rmcs_board/app/src/usb/vendor.hpp`:
- Around line 161-171: The two I2C callbacks
i2c_read_result_deserialized_callback and i2c_error_deserialized_callback
currently silently drop all ids; pick one consistent intent and make it
explicit: either (A) mark silent-drop as deliberate by adding a one-line comment
inside each function stating "deliberate silent discard of downstream
kReadResult/kError" (matching error_callback's comment style) while keeping the
(void) casts, or (B) make them strict like can_deserialized_callback by calling
core::utility::assert_failed_always() for unknown/unsupported ids while still
keeping (void)id/(void)data for any handled ids; update both
i2c_read_result_deserialized_callback and i2c_error_deserialized_callback
accordingly so the intent is clear and consistent with other callbacks.
In `@firmware/rmcs_board/boards/lite/board.c`:
- Around line 172-184: The i2c_config_t config in board_init_i2c is only
partially initialized and may read uninitialized bytes if the SDK adds fields;
zero-initialize config (e.g., set to all zeros or use memset/aggregate
initialization) before assigning config.i2c_mode and config.is_10bit_addressing
so that i2c_init_master always receives a fully-initialized structure.
In `@firmware/rmcs_board/boards/pro/board.c`:
- Around line 173-185: Zero-initialize the i2c_config_t before setting
individual fields in board_init_i2c so any unspecified members don't remain
indeterminate; locate the i2c_config_t config declaration in board_init_i2c and
replace its uninitialized declaration with a zero-initialized one (e.g.,
initialize config with {0} or use memset) then keep setting config.i2c_mode and
config.is_10bit_addressing as before before calling i2c_init_master.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c11d5a41-d2be-4cda-95db-a5f05abf0df4
📒 Files selected for processing (40)
.codex.devcontainer/devcontainer.json.gitignorecore/include/librmcs/data/datas.hppcore/include/librmcs/protocol/i2c.hppcore/src/protocol/deserializer.cppcore/src/protocol/deserializer.hppcore/src/protocol/protocol.hppcore/src/protocol/serializer.hppfirmware/c_board/app/src/app.cppfirmware/c_board/app/src/i2c/i2c.cppfirmware/c_board/app/src/i2c/i2c.hppfirmware/c_board/app/src/usb/interrupt_safe_buffer.hppfirmware/c_board/app/src/usb/vendor.hppfirmware/c_board/bsp/cubemx/Core/Inc/i2c.hfirmware/c_board/bsp/cubemx/Core/Inc/stm32f4xx_hal_conf.hfirmware/c_board/bsp/cubemx/Core/Inc/stm32f4xx_it.hfirmware/c_board/bsp/cubemx/Core/Src/dma.cfirmware/c_board/bsp/cubemx/Core/Src/gpio.cfirmware/c_board/bsp/cubemx/Core/Src/i2c.cfirmware/c_board/bsp/cubemx/Core/Src/main.cfirmware/c_board/bsp/cubemx/Core/Src/stm32f4xx_it.cfirmware/c_board/bsp/cubemx/cmake/stm32cubemx/CMakeLists.txtfirmware/c_board/bsp/cubemx/rmcs_slave.iocfirmware/rmcs_board/app/CMakeLists.txtfirmware/rmcs_board/app/src/app.cppfirmware/rmcs_board/app/src/i2c/i2c.cppfirmware/rmcs_board/app/src/i2c/i2c.hppfirmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hppfirmware/rmcs_board/app/src/usb/vendor.hppfirmware/rmcs_board/boards/lite/board.cfirmware/rmcs_board/boards/lite/board.hfirmware/rmcs_board/boards/pro/board.cfirmware/rmcs_board/boards/pro/board.hhost/include/librmcs/agent/c_board.hpphost/include/librmcs/agent/detail/i2c0_common.hpphost/include/librmcs/agent/rmcs_board_lite.hpphost/include/librmcs/agent/rmcs_board_pro.hpphost/include/librmcs/protocol/handler.hpphost/src/protocol/handler.cpp
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
firmware/rmcs_board/boards/pro/board.c (1)
173-184:board_init_i2c流程本身合理，但总线清失败时未做状态清理时序为
clock → pins → bus_clear → master init，在board_i2c_bus_clear失败时直接return false，此时clock_i2c0已被加入 group、PA18/PA19 也已切到 I2C 功能但控制器未初始化。上层I2c::try_initialize会反复重试board_init_i2c，每次都会再调用clock_add_to_group（幂等）和重写 IOC（幂等），行为上是可接受的。不过若 SCL 被从机持续拉低（
i2c_get_line_scl_status == false立即return false），这里永远不会触发i2c_gen_reset_signal，也不会对控制器做任何恢复尝试。考虑在 SCL 低的分支里也先尝试一次i2c_gen_reset_signal再判定，可覆盖上电瞬间 SCL 尚未稳定的边缘场景。非阻塞重试建议，不是必须。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/rmcs_board/boards/pro/board.c` around lines 173 - 184, board_init_i2c currently returns false immediately when board_i2c_bus_clear fails, leaving clock/pins configured and not attempting controller recovery; modify board_init_i2c to, when board_i2c_bus_clear reports failure due to SCL held low (use i2c_get_line_scl_status to detect), call i2c_gen_reset_signal once to try to recover the bus and then re-run board_i2c_bus_clear (non-blocking single retry) before giving up, keeping existing clock_add_to_group and board_init_i2c_pins behavior and only returning false if the reset+retry also fails.
🤖 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/rmcs_board/boards/pro/board.c`:
- Around line 125-136: board_init_i2c_pins() sets
IOC_PAD_FUNC_CTL_LOOP_BACK_MASK for PA18 (SCL) but not PA19 (SDA), causing
i2c_get_line_sda_status() in board_i2c_bus_clear() to read SDA unreliably;
update the PA19 FUNC_CTL assignment in board_init_i2c_pins (both in pro and lite
board.c) to include IOC_PAD_FUNC_CTL_LOOP_BACK_MASK alongside
IOC_PA19_FUNC_CTL_I2C0_SDA so SDA is driven/sensed symmetrically with SCL (i.e.,
change the IOC_PAD[IOC_PAD_PA19].FUNC_CTL value to OR in
IOC_PAD_FUNC_CTL_LOOP_BACK_MASK).
---
Nitpick comments:
In `@firmware/rmcs_board/boards/pro/board.c`:
- Around line 173-184: board_init_i2c currently returns false immediately when
board_i2c_bus_clear fails, leaving clock/pins configured and not attempting
controller recovery; modify board_init_i2c to, when board_i2c_bus_clear reports
failure due to SCL held low (use i2c_get_line_scl_status to detect), call
i2c_gen_reset_signal once to try to recover the bus and then re-run
board_i2c_bus_clear (non-blocking single retry) before giving up, keeping
existing clock_add_to_group and board_init_i2c_pins behavior and only returning
false if the reset+retry also fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 59077b22-ac42-483a-9886-3118b42662b6
📒 Files selected for processing (12)
.devcontainer/devcontainer.jsoncore/src/protocol/deserializer.cppcore/src/protocol/protocol.hppfirmware/c_board/app/src/usb/vendor.hppfirmware/rmcs_board/app/src/i2c/i2c.hppfirmware/rmcs_board/app/src/usb/interrupt_safe_buffer.hppfirmware/rmcs_board/app/src/usb/uplink_buffer_hook.cppfirmware/rmcs_board/app/src/usb/vendor.hppfirmware/rmcs_board/boards/lite/board.cfirmware/rmcs_board/boards/lite/board.hfirmware/rmcs_board/boards/pro/board.cfirmware/rmcs_board/boards/pro/board.h
✅ Files skipped from review due to trivial changes (1)
- core/src/protocol/deserializer.cpp
🚧 Files skipped from review as they are similar to previous changes (6)
- firmware/rmcs_board/boards/lite/board.h
- .devcontainer/devcontainer.json
- firmware/rmcs_board/app/src/usb/vendor.hpp
- firmware/rmcs_board/boards/lite/board.c
- firmware/c_board/app/src/usb/vendor.hpp
- firmware/rmcs_board/boards/pro/board.h
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 free
to 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.
Summary
This PR introduces I2C0 as a first-class protocol channel and then hardens it across core protocol, host SDK, and firmware (
c_board+rmcs_board).Commit range:
74caf44(feat(i2c)) ->18b590f(18 commits).Core protocol
DataId::kI2c0,I2cDataView,I2cReadConfigView, andI2cErrorView.core/include/librmcs/protocol/i2c.hppand aligned all size checks to the shared limit.Host SDK
Handler::PacketBuilder:write_i2c,write_i2c_read_config, andwrite_i2c_read_result.i2c0_writeandi2c0_readforCBoard,RmcsBoardLite, andRmcsBoardPro.kBadAlloc).agent/detail/i2c0_common.hpp.Firmware: c_board (STM32)
I2C0transport over physicalI2C2.Firmware: rmcs_board (HPM)
hpm_sdkreference to restorehpm5300evkboard definitions needed by this series.Behavior changes
slave_address,reg_address,data_length,is_read).Misc
.devcontainer/devcontainer.json: deterministic startup and opt-in user tool bootstrap..gitignore: ignorefirmware/rmcs_board/build-lite/.I2C0 端到端传输支持与稳定性加强（概述）
本 PR 将 I2C0 提升为一等协议通道，并在核心协议、Host SDK、以及两块固件（c_board：STM32F4，rmcs_board：HPM）中实现并加固了端到端支持，包含严格验证、错误上下文保留、DMA/中断驱动传输与 USB 背压处理。提交范围：74caf44 -> 18b590f（18 个提交）；分支：dev/i2c → main；状态：open。作者请求 reviewer：@coderabbitai。CodeRabbit 触发了自动回复，无其他审查讨论或批准。
核心协议层
Host SDK（协议与 Agent）
c_board 固件（STM32F4 — 逻辑 I2C0 映射到物理 I2C2）
rmcs_board 固件（HPM — 非阻塞 DMA）
行为变更与兼容性要点
其他杂项
影响与风险提示（建议审查重点）
总体评价：该 PR 系统性地将 I2C0 引入为一等通道并在多层面实现了鲁棒性与回退机制，功能完整且考虑了主机/固件交互的异常情形；因改动范围大，建议分区审查并进行硬件回归测试。