feat(gpio): Add GPIO PWM write support and implement for c_board#34
Merged
feat(gpio): Add GPIO PWM write support and implement for c_board#34
Conversation
- Define GPIO digital/analog write protocol (header, serializer, deserializer) with read variants reserved for future work. - Expose gpio_digital_transmit / gpio_analog_transmit in host CBoard agent. - Map c_board channels 1–7 to TIM1 CH1–4 and TIM8 CH1–3 PWM outputs at 50 Hz.
Member
Author
|
@coderabbitai review |
|
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 (1)
Walkthrough在仓库中引入对 GPIO 的端到端支持：新增数据视图、协议位域、序列化/反序列化处理、固件端 PWM/GPIO 管理及定时器（TIM1/TIM8）配置，并在主机与固件之间添加相应的传输/回调接口与集成点。 Changes
Sequence Diagram(s)sequenceDiagram
participant Host as Host (应用)
participant Serializer as Serializer
participant USB as USB/Vendor
participant Deser as Deserializer
participant Vendor as Vendor 回调
participant GPIO as 固件 GPIO 管理器
participant PWM as PWM 硬件
Host->>Serializer: 构造并发送 GPIO 字段 (Digital/Analog)
Serializer->>USB: 发送字节流
USB->>Deser: 接收并调用 process_stream (FieldId::kGpio)
Deser->>Deser: 解析 GpioHeader (PayloadType, Channel)
alt DigitalWrite
Deser->>Vendor: gpio_digital_deserialized_callback(GpioDigitalDataView)
else AnalogWrite
Deser->>Vendor: gpio_analog_deserialized_callback(GpioAnalogDataView)
end
Vendor->>GPIO: handle_digital_write / handle_analog_write
GPIO->>GPIO: 验证通道，计算 CCR 值
GPIO->>PWM: 更新 CCR 寄存器（输出 PWM）
PWM-->>GPIO: PWM 输出生效
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 分钟 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 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 |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
firmware/c_board/app/src/gpio/gpio.cpp (1)
1-9: Include 顺序可优化根据项目的 Google C++ Include Style 规范，建议调整顺序：对应头文件 → C 系统头文件 → C++ 标准库头文件 → 其他库头文件 → 项目头文件。
建议的 include 顺序
`#include` "firmware/c_board/app/src/gpio/gpio.hpp" -#include <cstdint> - `#include` <main.h> `#include` <stm32f4xx_hal_gpio.h> +#include <cstdint> + `#include` "firmware/c_board/app/src/spi/bmi088/accel.hpp" `#include` "firmware/c_board/app/src/spi/bmi088/gyro.hpp"根据 learnings: "In the librmcs project, enforce Google C++ Include Style in all C++ source files."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/src/gpio/gpio.cpp` around lines 1 - 9, Reorder the includes in gpio.cpp to follow Google C++ style: keep the corresponding header "firmware/c_board/app/src/gpio/gpio.hpp" first, then C system headers (e.g., <main.h>), then C++ standard headers (e.g., <cstdint>), then other library headers (e.g., <stm32f4xx_hal_gpio.h>), and finally project headers such as "firmware/c_board/app/src/spi/bmi088/accel.hpp" and "firmware/c_board/app/src/spi/bmi088/gyro.hpp"; adjust the include block accordingly so that gpio.hpp, <main.h>, <cstdint>, <stm32f4xx_hal_gpio.h>, then accel.hpp and gyro.hpp appear in that order.
🤖 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/app/src/gpio/gpio.cpp`:
- Around line 1-9: Reorder the includes in gpio.cpp to follow Google C++ style:
keep the corresponding header "firmware/c_board/app/src/gpio/gpio.hpp" first,
then C system headers (e.g., <main.h>), then C++ standard headers (e.g.,
<cstdint>), then other library headers (e.g., <stm32f4xx_hal_gpio.h>), and
finally project headers such as "firmware/c_board/app/src/spi/bmi088/accel.hpp"
and "firmware/c_board/app/src/spi/bmi088/gyro.hpp"; adjust the include block
accordingly so that gpio.hpp, <main.h>, <cstdint>, <stm32f4xx_hal_gpio.h>, then
accel.hpp and gyro.hpp appear in that order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94761686-d16f-4d01-93da-d18dcccb3ac4
📒 Files selected for processing (18)
core/include/librmcs/data/datas.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/gpio/gpio.cppfirmware/c_board/app/src/gpio/gpio.hppfirmware/c_board/app/src/usb/vendor.hppfirmware/c_board/bsp/cubemx/Core/Inc/main.hfirmware/c_board/bsp/cubemx/Core/Inc/tim.hfirmware/c_board/bsp/cubemx/Core/Src/gpio.cfirmware/c_board/bsp/cubemx/Core/Src/tim.cfirmware/c_board/bsp/cubemx/rmcs_slave.iocfirmware/rmcs_board/src/usb/vendor.hpphost/include/librmcs/agent/c_board.hpphost/include/librmcs/protocol/handler.hpphost/src/protocol/handler.cpp
There was a problem hiding this comment.
🧹 Nitpick comments (1)
firmware/c_board/app/src/gpio/gpio.hpp (1)
52-58:kCounterPeriod硬编码值与定时器配置耦合，建议添加注释说明依赖关系。
kCounterPeriod = 60000需要与.ioc中 TIM1/TIM8 的Period值（59999）保持同步，即kCounterPeriod = Period + 1。当前值是正确的，但如果定时器配置更改，这里需要手动同步更新。建议添加注释说明此依赖关系，或考虑从定时器寄存器动态获取 ARR 值：
📝 建议添加注释
private: - static constexpr uint32_t kCounterPeriod = 60000; + // Must match TIM1/TIM8 ARR + 1 (configured in rmcs_slave.ioc as Period = 59999) + static constexpr uint32_t kCounterPeriod = 60000;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/src/gpio/gpio.hpp` around lines 52 - 58, kCounterPeriod is hardcoded to 60000 but is coupled to the TIM1/TIM8 Period (59999) so it must equal TIMx->ARR + 1; update the code to either: 1) add a clear comment next to kCounterPeriod stating "must equal TIMx ARR + 1 (e.g. .ioc Period 59999 -> kCounterPeriod 60000)" so future maintainers know the dependency, or 2) remove the constexpr and read the ARR value from the timer register at init (store as a non-constexpr member used by duty_to_compare) so it stays in sync; reference kCounterPeriod and duty_to_compare to locate the change.
🤖 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/app/src/gpio/gpio.hpp`:
- Around line 52-58: kCounterPeriod is hardcoded to 60000 but is coupled to the
TIM1/TIM8 Period (59999) so it must equal TIMx->ARR + 1; update the code to
either: 1) add a clear comment next to kCounterPeriod stating "must equal TIMx
ARR + 1 (e.g. .ioc Period 59999 -> kCounterPeriod 60000)" so future maintainers
know the dependency, or 2) remove the constexpr and read the ARR value from the
timer register at init (store as a non-constexpr member used by duty_to_compare)
so it stays in sync; reference kCounterPeriod and duty_to_compare to locate the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d31e2d34-135f-4c79-838b-c4cd1394ff5a
📒 Files selected for processing (18)
core/include/librmcs/data/datas.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/gpio/gpio.cppfirmware/c_board/app/src/gpio/gpio.hppfirmware/c_board/app/src/usb/vendor.hppfirmware/c_board/bsp/cubemx/Core/Inc/main.hfirmware/c_board/bsp/cubemx/Core/Inc/tim.hfirmware/c_board/bsp/cubemx/Core/Src/gpio.cfirmware/c_board/bsp/cubemx/Core/Src/tim.cfirmware/c_board/bsp/cubemx/rmcs_slave.iocfirmware/rmcs_board/src/usb/vendor.hpphost/include/librmcs/agent/c_board.hpphost/include/librmcs/protocol/handler.hpphost/src/protocol/handler.cpp
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.
GPIO PWM 写入支持与 c_board 实现（更新）
功能概述
本 PR 为库增加了端到端的 GPIO 写入支持（数字与模拟/PWM），包含协议定义、序列化/反序列化接口、主机侧传输封装以及 c_board 固件上的 PWM 驱动实现（通道映射 TIM1 CH1–4 与 TIM8 CH1–3，共 7 通道，工作于 50 Hz）。
核心变更
协议层（core）
主机 API（host）
c_board 固件实现（firmware/c_board）
HAL / 硬件配置
rmcs_board
设计要点与兼容性