feat(gpio): Add GPIO digital/analog read with configurable sampling modes#35
Merged
feat(gpio): Add GPIO digital/analog read with configurable sampling modes#35
Conversation
Walkthrough此PR在协议与数据层新增 GPIO 读配置与读结果路径，扩展回调接口并重命名若干 GPIO 写接口；固件引入通道化 GPIO 驱动、周期/边沿采样与中断路由；主机/USB 层调整对应序列化/反序列化与接口映射。 Changes
Sequence DiagramsequenceDiagram
participant Host as Host
participant PacketBuilder as PacketBuilder
participant Serializer as Serializer
participant USB as USB/Transport
participant Deserializer as Deserializer
participant DeserializeCb as DeserializeCallback
participant FirmwareGPIO as Firmware GPIO
participant Timer as Timer/EXTI
Host->>PacketBuilder: gpio_digital_read(GpioReadConfigView)
PacketBuilder->>Serializer: write_gpio_digital_read_config(config)
Serializer->>USB: send packet (kDigitalRead)
USB->>Deserializer: receive packet
Deserializer->>DeserializeCb: gpio_digital_read_config_deserialized_callback(config)
DeserializeCb->>FirmwareGPIO: handle_digital_read(config)
alt 周期采样
Timer->>FirmwareGPIO: poll_periodic_input_samples()
FirmwareGPIO->>Serializer: write_gpio_digital_read_result(sample)
Serializer->>USB: send result
USB->>Host: gpio_digital_read_result_callback(result)
else 边沿触发
Timer->>FirmwareGPIO: EXTI interrupt -> handle_input_edge_interrupt(pin)
FirmwareGPIO->>Serializer: write_gpio_digital_read_result(sample)
Serializer->>USB: send result
USB->>Host: gpio_digital_read_result_callback(result)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 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 |
![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
🧹 Nitpick comments (3)
firmware/c_board/bsp/cubemx/Core/Src/stm32f4xx_it.c (1)
463-468: 建议在头文件中添加EXTI15_10_IRQHandler声明。新增的
EXTI15_10_IRQHandler函数实现在stm32f4xx_it.c中，但根据上下文stm32f4xx_it.h中缺少对应的函数声明。虽然启动文件中的弱符号机制确保链接能够正常工作，但为了代码一致性和可维护性，建议在头文件中补充声明。📝 建议在 stm32f4xx_it.h 中添加
在
/* Exported functions prototypes */部分添加：void EXTI15_10_IRQHandler(void);🤖 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/stm32f4xx_it.c` around lines 463 - 468, 在 stm32f4xx_it.c 中新增了中断处理函数 EXTI15_10_IRQHandler，但头文件 stm32f4xx_it.h 中未声明该符号；请在 stm32f4xx_it.h 的 “Exported functions prototypes” 区块添加 void EXTI15_10_IRQHandler(void); 的声明，以保持头/源一致性并提高可维护性（function name: EXTI15_10_IRQHandler，file: stm32f4xx_it.c / stm32f4xx_it.h）。host/include/librmcs/agent/c_board.hpp (1)
59-66: Channel 6 边沿触发限制的错误消息可以更具体当前错误消息
"Invalid GPIO data"未说明 channel 6 为何不支持边沿触发（EXTI 线与 channel 5 冲突）。建议在异常消息中提供更明确的原因，便于用户诊断问题。💡 建议的改进
PacketBuilder& gpio_digital_read(const librmcs::data::GpioReadConfigView& data) { if (data.channel < 1 || data.channel > 7 || (data.channel == 6 && (data.rising_edge || data.falling_edge)) || !builder_.write_gpio_digital_read_config(data)) [[unlikely]] - throw std::invalid_argument{ - "GPIO digital read configuration transmission failed: Invalid GPIO data"}; + throw std::invalid_argument{ + data.channel == 6 && (data.rising_edge || data.falling_edge) + ? "GPIO digital read configuration failed: Channel 6 does not support edge-triggered mode (EXTI conflict with channel 5)" + : "GPIO digital read configuration transmission failed: Invalid GPIO data"}; return *this; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@host/include/librmcs/agent/c_board.hpp` around lines 59 - 66, The thrown std::invalid_argument in gpio_digital_read should provide a specific reason for the failure instead of the generic "Invalid GPIO data"; update the exception message in gpio_digital_read (and where builder_.write_gpio_digital_read_config is checked) to state that channel 6 does not support edge-triggered reads because it shares the EXTI line with channel 5 (e.g., "GPIO digital read failed: channel 6 cannot use rising/falling edge trigger due to EXTI line conflict with channel 5"), so callers immediately see the cause.firmware/c_board/bsp/cubemx/Core/Inc/main.h (1)
271-276: 记录 EXTI 线冲突约束。
CHANNEL5_Pin(PC6) 和CHANNEL6_Pin(PI6) 共享同一 GPIO_PIN_6，在 STM32 上映射到相同的 EXTI6 中断线。PR 描述中提到固件层已处理此冲突，建议在此处添加注释说明该硬件约束，以便后续维护者了解。`#define` CHANNEL6_Pin GPIO_PIN_6 `#define` CHANNEL6_GPIO_Port GPIOI `#define` CHANNEL5_Pin GPIO_PIN_6 `#define` CHANNEL5_GPIO_Port GPIOC +/* Note: CHANNEL5 (PC6) and CHANNEL6 (PI6) share EXTI6 line. + Edge-triggered sampling cannot be enabled on both channels simultaneously. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bsp/cubemx/Core/Inc/main.h` around lines 271 - 276, CHANNEL5_Pin (PC6) and CHANNEL6_Pin (PI6) both map to GPIO_PIN_6 which shares the same EXTI6 interrupt line on STM32; add a concise comment next to the defines for CHANNEL5_Pin and CHANNEL6_Pin in main.h noting the EXTI6 conflict and that the firmware/IRQ routing in the codebase already handles this overlap to guide future maintainers (referencing SYMBOLS: CHANNEL5_Pin, CHANNEL6_Pin, EXTI6).
🤖 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/src/gpio/gpio.hpp`:
- Line 119: kPwmCounterPeriod is set to 60000 which exceeds the timer ARR
(59999) and can produce CCR writes outside the valid [0,ARR] range; change
kPwmCounterPeriod from 60000 to 59999 and update any code that computes or
clamps CCR values (e.g., functions that compute "digital high" or 100% analog
duty and any SetPwmDuty/WriteCompare helpers) to ensure they never write a value
> kPwmCounterPeriod (clamp to kPwmCounterPeriod where needed).
---
Nitpick comments:
In `@firmware/c_board/bsp/cubemx/Core/Inc/main.h`:
- Around line 271-276: CHANNEL5_Pin (PC6) and CHANNEL6_Pin (PI6) both map to
GPIO_PIN_6 which shares the same EXTI6 interrupt line on STM32; add a concise
comment next to the defines for CHANNEL5_Pin and CHANNEL6_Pin in main.h noting
the EXTI6 conflict and that the firmware/IRQ routing in the codebase already
handles this overlap to guide future maintainers (referencing SYMBOLS:
CHANNEL5_Pin, CHANNEL6_Pin, EXTI6).
In `@firmware/c_board/bsp/cubemx/Core/Src/stm32f4xx_it.c`:
- Around line 463-468: 在 stm32f4xx_it.c 中新增了中断处理函数 EXTI15_10_IRQHandler，但头文件
stm32f4xx_it.h 中未声明该符号；请在 stm32f4xx_it.h 的 “Exported functions prototypes” 区块添加
void EXTI15_10_IRQHandler(void); 的声明，以保持头/源一致性并提高可维护性（function name:
EXTI15_10_IRQHandler，file: stm32f4xx_it.c / stm32f4xx_it.h）。
In `@host/include/librmcs/agent/c_board.hpp`:
- Around line 59-66: The thrown std::invalid_argument in gpio_digital_read
should provide a specific reason for the failure instead of the generic "Invalid
GPIO data"; update the exception message in gpio_digital_read (and where
builder_.write_gpio_digital_read_config is checked) to state that channel 6 does
not support edge-triggered reads because it shares the EXTI line with channel 5
(e.g., "GPIO digital read failed: channel 6 cannot use rising/falling edge
trigger due to EXTI line conflict with channel 5"), so callers immediately see
the cause.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3161f3c-28a2-41a5-a16f-54f5a376a07c
📒 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/Src/stm32f4xx_it.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/agent/rmcs_board.hpphost/include/librmcs/protocol/handler.hpphost/src/protocol/handler.cpp
There was a problem hiding this comment.
🧹 Nitpick comments (2)
firmware/c_board/app/src/gpio/gpio.hpp (2)
202-210: 考虑处理缓冲区满的情况。当前仅断言检查
kInvalidArgument，若序列化器返回kBufferFull，采样会被静默丢弃。虽然非阻塞设计是合理的，但丢弃采样可能影响调试。可考虑添加计数器或日志记录丢弃次数。🤖 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 202 - 210, publish_digital_input_sample 中只用断言忽略了序列化器可能返回的 kBufferFull，导致采样被静默丢弃；修改 publish_digital_input_sample 在调用 usb::get_serializer().write_gpio_digital_read_result(...) 后显式处理返回值：保留对 kInvalidArgument 的断言，但对 core::protocol::Serializer::SerializeResult::kBufferFull 增加处理逻辑（例如递增一个丢弃计数器或调用日志函数记录丢弃次数），以便后续可观察到缓冲区满的情况并且在必要时统计或上报。
143-147: EXTI 冲突处理正确。Channel 5 (PC6) 和 Channel 6 (PI6) 共享 EXTI 线 6，代码正确地禁用了 Channel 6 的边沿触发以避免冲突。建议在此限制条件上添加日志或返回错误码，让上层知晓配置被修改。
🤖 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 143 - 147, The code disables edge triggers for channel 6 to avoid EXTI line 6 conflicts but does not notify the caller; update the channel configuration routine (the function that contains the if (channel == 6) check and sets rising_edge/falling_edge—e.g., the channel configure/set_edge_trigger function) to surface this change: either emit a diagnostic (call the project's logging/debug function with a clear message that channel 6 was modified due to EXTI conflict) or change the function to return a status/enum (e.g., SUCCESS vs ERROR_EXTI_CONFLICT or CONFIG_MODIFIED) so callers can detect the modification; ensure you reference and preserve the existing rising_edge and falling_edge assignments when adding the notification.
🤖 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 202-210: publish_digital_input_sample 中只用断言忽略了序列化器可能返回的
kBufferFull，导致采样被静默丢弃；修改 publish_digital_input_sample 在调用
usb::get_serializer().write_gpio_digital_read_result(...) 后显式处理返回值：保留对
kInvalidArgument 的断言，但对 core::protocol::Serializer::SerializeResult::kBufferFull
增加处理逻辑（例如递增一个丢弃计数器或调用日志函数记录丢弃次数），以便后续可观察到缓冲区满的情况并且在必要时统计或上报。
- Around line 143-147: The code disables edge triggers for channel 6 to avoid
EXTI line 6 conflicts but does not notify the caller; update the channel
configuration routine (the function that contains the if (channel == 6) check
and sets rising_edge/falling_edge—e.g., the channel configure/set_edge_trigger
function) to surface this change: either emit a diagnostic (call the project's
logging/debug function with a clear message that channel 6 was modified due to
EXTI conflict) or change the function to return a status/enum (e.g., SUCCESS vs
ERROR_EXTI_CONFLICT or CONFIG_MODIFIED) so callers can detect the modification;
ensure you reference and preserve the existing rising_edge and falling_edge
assignments when adding the notification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 111cd515-60ad-4e6f-9087-c72a391399b7
📒 Files selected for processing (1)
firmware/c_board/app/src/gpio/gpio.hpp
Implement GPIO read support with configurable sampling modes: - ASAP one-shot sampling - Periodic sampling via timer polling - Edge-triggered sampling via EXTI - Configurable pull mode for input channels Extend the GPIO protocol with read config and read result messages: - Add GpioPull and GpioReadConfigView - Add GpioReadConfigPayload bitfield - Add digital and analog read config and result payload types - Extend serializer and deserializer with read-specific handling Update c_board firmware to support GPIO input reads: - Reconfigure GPIO pins dynamically between output and input modes - Poll periodic samples in the main loop - Publish edge-triggered samples from EXTI callbacks - Handle the EXTI line conflict between channels 5 and 6 Update host-side APIs for GPIO read support: - Add gpio_digital_read() to the CBoard agent - Route GPIO read results through DataCallback - Rename GPIO write APIs to explicit write-specific names Analog read protocol support is included in the core layer, but c_board firmware does not implement analog sampling yet. BREAKING CHANGE: Rename GPIO transmit APIs to write-specific names and add GPIO read result callbacks to the host callback interface.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
firmware/c_board/app/src/gpio/gpio.hpp (1)
73-86: 周期采样时间可能产生累积漂移。当采样后将
next_sample_time设置为当前时间now（Line 84），如果主循环执行慢于采样周期，会导致时间漂移。若需要精确的采样频率，考虑使用累加方式：publish_digital_input_sample(channel); - state.next_sample_time = now; + state.next_sample_time += state.period; }但如果当前行为（"自上次采样以来至少经过 period"）是预期的，则可以忽略此建议。
🤖 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 73 - 86, poll_periodic_input_samples updates state.next_sample_time to now which causes cumulative drift when the loop runs late; instead, compute next_sample_time by incrementing the previous next_sample_time by state.period (e.g., state.next_sample_time += state.period) in the poll_periodic_input_samples function so samples stay on the fixed schedule, while still using timer::timer->check_expired(state.next_sample_time, state.period) and timer::timer->timepoint() for comparisons and calling publish_digital_input_sample(channel) when due; keep the late-run handling by advancing next_sample_time in a loop or by using integer multiples of state.period to catch up if now > next_sample_time + period.
🤖 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 73-86: poll_periodic_input_samples updates state.next_sample_time
to now which causes cumulative drift when the loop runs late; instead, compute
next_sample_time by incrementing the previous next_sample_time by state.period
(e.g., state.next_sample_time += state.period) in the
poll_periodic_input_samples function so samples stay on the fixed schedule,
while still using timer::timer->check_expired(state.next_sample_time,
state.period) and timer::timer->timepoint() for comparisons and calling
publish_digital_input_sample(channel) when due; keep the late-run handling by
advancing next_sample_time in a loop or by using integer multiples of
state.period to catch up if now > next_sample_time + period.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1041f415-dafd-4e49-ada6-8683bef79370
📒 Files selected for processing (19)
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/gpio.hfirmware/c_board/bsp/cubemx/Core/Inc/main.hfirmware/c_board/bsp/cubemx/Core/Src/stm32f4xx_it.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/agent/rmcs_board.hpphost/include/librmcs/protocol/handler.hpphost/src/protocol/handler.cpp
✅ Files skipped from review due to trivial changes (3)
- firmware/c_board/bsp/cubemx/Core/Inc/gpio.h
- firmware/c_board/bsp/cubemx/Core/Src/tim.c
- firmware/c_board/bsp/cubemx/rmcs_slave.ioc
🚧 Files skipped from review as they are similar to previous changes (8)
- firmware/c_board/app/src/app.cpp
- firmware/c_board/app/src/gpio/gpio.cpp
- core/include/librmcs/data/datas.hpp
- core/src/protocol/protocol.hpp
- firmware/c_board/bsp/cubemx/Core/Inc/main.h
- core/src/protocol/deserializer.hpp
- host/include/librmcs/agent/c_board.hpp
- core/src/protocol/serializer.hpp
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.
Implement GPIO read functionality supporting multiple trigger modes:
Protocol changes:
Firmware (c_board):
Host SDK:
Note: Protocol layer includes analog read support, but ADC sampling is not implemented in c_board firmware. Only digital read is functional.
GPIO数字/模拟读取功能实现（支持可配置采样模式）
概述
本 PR 在协议层、c_board 固件和主机 SDK（Agent/Handler/PacketBuilder）中添加了 GPIO 读取功能。支持触发模式：立即（ASAP）、周期采样（1 Hz–1 kHz，定时器轮询）和边缘触发（上升沿/下降沿/两者，EXTI 中断）。协议引入读取配置与结果消息，固件实现了数字读取（数字周期/边缘），主机 SDK 暴露读取 API 并将结果通过 DataCallback 回调向上转发。注意：协议包含模拟（ADC）读的支持，但 c_board 固件尚未实现 ADC 采样，当前仅数字读取可用。
主要变更
协议层（protocol/serializer/deserializer）
固件（firmware/c_board）
主机 SDK / Agent / Handler
已知限制与注意事项
影响范围