feat(firmware/c_board): Replace DWT delay helper with TIM2/TIM9 timer service#31
Merged
feat(firmware/c_board): Replace DWT delay helper with TIM2/TIM9 timer service#31
Conversation
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough新增基于硬件定时器的 Timer 类与全局实例，删除基于 DWT 的 delay API；调整启动顺序以先初始化 TIM9/TIM2 并初始化 timer 实例；将 HAL_Delay 与 BMI088 驱动的延时调用迁移到新的 timer 接口；添加 TIM2/TIM9 的 CubeMX 配置与驱动实现。 Changes
Sequence Diagram(s)sequenceDiagram
participant App as App (startup)
participant MX as MX_TIM9_Init / MX_TIM2_Init
participant HAL as HAL / TIM peripherals
participant Timer as timer::Timer
participant BMI as BMI088 driver
App->>MX: 调用 MX_TIM9_Init()
App->>MX: 调用 MX_TIM2_Init()
MX->>HAL: 配置并启动 TIM9/TIM2（时钟/同步/从模式）
App->>Timer: 初始化 timer::timer
Timer->>HAL: HAL_TIM_Base_Start(kTimerHigh/kTimerLow)
BMI->>Timer: 调用 timer::timer->spin_wait(1ms)
Timer-->>BMI: 等待结束并返回
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 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.
🧹 Nitpick comments (2)
firmware/c_board/bsp/cubemx/rmcs_slave.ioc (1)
348-349: 把 TIM5 的 ITR 虚拟配置从.ioc里撤掉。这两行会让 CubeMX 在
firmware/c_board/bsp/cubemx/Core/Src/tim.c里给现有 TIM5 PWM 定时器再生成一段HAL_TIM_SlaveConfigSynchro()配置。当前 48 位 timepoint 只依赖 TIM2/TIM9，这个 TIM5 触发源配置没有功能收益，却扩大了 LED 定时器的回归面。建议只保留 TIM2/TIM9 的虚拟时钟配置。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bsp/cubemx/rmcs_slave.ioc` around lines 348 - 349, 删除 .ioc 文件中 VP_TIM5_VS_ClockSourceITR.Mode=TriggerSource_ITR0 和 VP_TIM5_VS_ClockSourceITR.Signal=TIM5_VS_ClockSourceITR 这两行，避免 CubeMX 在 Core/Src/tim.c 对现有 TIM5 PWM 定时器再生成 HAL_TIM_SlaveConfigSynchro() 的从属触发配置；保留仅针对 TIM2/TIM9 的虚拟时钟配置并在修改后用 CubeMX 重新生成代码以验证 TIM5 触发配置不再被写入。firmware/c_board/app/src/timer/timer.hpp (1)
22-22: 建议添加时钟频率计算的注释说明。
168'000'000 / 2 / 21的计算依赖于 STM32 时钟树配置（系统时钟、APB 分频、定时器预分频），建议添加注释说明推导过程，便于维护时与 CubeMX 配置保持同步。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/src/timer/timer.hpp` at line 22, 常量 kClockFrequency 的数值来源不直观；在 timer.hpp 为该行添加一行注释，说明推导：168'000'000 是系统主时钟频率，除以 APB 分频 (2) 得到定时器总线输入（或按当前时钟树配置的分频），再除以 软件/硬件定时器预分频 21 得到实际定时器计数频率；并指出该注释应随 CubeMX/时钟树配置变更同步更新。这样在后续维护时能快速对应到系统时钟、APB 分频和定时器预分频（kClockFrequency 的来源）。
🤖 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/timer/timer.hpp`:
- Line 22: 常量 kClockFrequency 的数值来源不直观；在 timer.hpp 为该行添加一行注释，说明推导：168'000'000
是系统主时钟频率，除以 APB 分频 (2) 得到定时器总线输入（或按当前时钟树配置的分频），再除以 软件/硬件定时器预分频 21
得到实际定时器计数频率；并指出该注释应随 CubeMX/时钟树配置变更同步更新。这样在后续维护时能快速对应到系统时钟、APB
分频和定时器预分频（kClockFrequency 的来源）。
In `@firmware/c_board/bsp/cubemx/rmcs_slave.ioc`:
- Around line 348-349: 删除 .ioc 文件中
VP_TIM5_VS_ClockSourceITR.Mode=TriggerSource_ITR0 和
VP_TIM5_VS_ClockSourceITR.Signal=TIM5_VS_ClockSourceITR 这两行，避免 CubeMX 在
Core/Src/tim.c 对现有 TIM5 PWM 定时器再生成 HAL_TIM_SlaveConfigSynchro() 的从属触发配置；保留仅针对
TIM2/TIM9 的虚拟时钟配置并在修改后用 CubeMX 重新生成代码以验证 TIM5 触发配置不再被写入。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 99c15462-2d79-494e-b9d3-afe314b1342f
📒 Files selected for processing (10)
firmware/c_board/app/src/app.cppfirmware/c_board/app/src/spi/bmi088/accel.hppfirmware/c_board/app/src/spi/bmi088/base.hppfirmware/c_board/app/src/spi/bmi088/gyro.hppfirmware/c_board/app/src/timer/delay.cppfirmware/c_board/app/src/timer/delay.hppfirmware/c_board/app/src/timer/timer.hppfirmware/c_board/bsp/cubemx/Core/Inc/tim.hfirmware/c_board/bsp/cubemx/Core/Src/tim.cfirmware/c_board/bsp/cubemx/rmcs_slave.ioc
💤 Files with no reviewable changes (1)
- firmware/c_board/app/src/timer/delay.hpp
… service - Add 48-bit timepoint based on chained TIM2/TIM9 counters. - Add delay_async APIs for non-blocking delay checks. - Route HAL_Delay and BMI088 delays through timer::timer. - Initialize TIM2/TIM9 during app startup and regenerate CubeMX TIM config. - Remove legacy DWT-based delay.hpp.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
firmware/c_board/bsp/cubemx/rmcs_slave.ioc (1)
265-265: 把 CubeMX 的函数排序也同步成先TIM9再TIM2。
firmware/c_board/app/src/app.cppLine 38-41 已经把这两个初始化写成硬依赖，但这里的functionlistsort仍然把MX_TIM2_Init排在MX_TIM9_Init前面。现在主流程虽然是手写的，后续重新生成代码或按.ioc顺序维护时，还是很容易把这个依赖关系回退坏。♻️ 建议修改
-ProjectManager.functionlistsort=1-SystemClock_Config-RCC-false-HAL-false,2-MX_GPIO_Init-GPIO-false-HAL-true,3-MX_DMA_Init-DMA-false-HAL-true,4-MX_SPI1_Init-SPI1-false-HAL-true,5-MX_CAN1_Init-CAN1-false-HAL-true,6-MX_CAN2_Init-CAN2-false-HAL-true,7-MX_USART1_UART_Init-USART1-false-HAL-true,8-MX_USART3_UART_Init-USART3-false-HAL-true,9-MX_USART6_UART_Init-USART6-false-HAL-true,10-MX_TIM5_Init-TIM5-false-HAL-true,11-MX_USB_OTG_FS_PCD_Init-USB_OTG_FS-false-HAL-true,12-MX_TIM2_Init-TIM2-false-HAL-true,13-MX_TIM9_Init-TIM9-false-HAL-true +ProjectManager.functionlistsort=1-SystemClock_Config-RCC-false-HAL-false,2-MX_GPIO_Init-GPIO-false-HAL-true,3-MX_DMA_Init-DMA-false-HAL-true,4-MX_SPI1_Init-SPI1-false-HAL-true,5-MX_CAN1_Init-CAN1-false-HAL-true,6-MX_CAN2_Init-CAN2-false-HAL-true,7-MX_USART1_UART_Init-USART1-false-HAL-true,8-MX_USART3_UART_Init-USART3-false-HAL-true,9-MX_USART6_UART_Init-USART6-false-HAL-true,10-MX_TIM5_Init-TIM5-false-HAL-true,11-MX_USB_OTG_FS_PCD_Init-USB_OTG_FS-false-HAL-true,12-MX_TIM9_Init-TIM9-false-HAL-true,13-MX_TIM2_Init-TIM2-false-HAL-true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/bsp/cubemx/rmcs_slave.ioc` at line 265, 更新 ProjectManager.functionlistsort 中的函数顺序，使 MX_TIM9_Init 排在 MX_TIM2_Init 之前以匹配 app.cpp 对 TIM9->TIM2 的硬依赖关系；具体操作是在 functionlistsort 字段中将条目 "12-MX_TIM2_Init-TIM2-..." 和 "13-MX_TIM9_Init-TIM9-..." 互换顺序（使 MX_TIM9_Init 的条目编号和位置在 MX_TIM2_Init 之前），确保 ProjectManager.functionlistsort 中的顺序与 MX_TIM9_Init 和 MX_TIM2_Init 的依赖顺序一致以防止后续代码生成回退出错。firmware/c_board/app/src/timer/timer.hpp (1)
22-23: 避免把 TIM2 时基参数硬编码两份。这里的
168'000'000 / 2 / 21和firmware/c_board/bsp/cubemx/Core/Src/tim.c:45-65/firmware/c_board/bsp/cubemx/rmcs_slave.iocLine 317-320 是重复来源。后续只要改了 APB1 分频或 TIM2 预分频，这里的std::chrono换算就会静默失真，所有 delay 都会跑偏。建议至少在Timer()里加一个运行时一致性断言，或者把频率收敛到单一来源。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@firmware/c_board/app/src/timer/timer.hpp` around lines 22 - 23, The code currently hardcodes kClockFrequency and TickPeriod which duplicates TIM2 configuration; change to a single source or add a runtime consistency check: either (A) centralize the TIM2 clock/prescaler into one header/constant used by both the CubeMX-generated TIM2 init and timer.hpp (remove the literal 168'000'000 / 2 / 21), or (B) keep the compile-time value but add a runtime assertion in Timer::Timer() that computes the effective TIM2 tick rate from the RCC/APB1 prescaler and the TIM2->PSC (or via HAL_RCC_GetPCLK1Freq and TIM2->PSC) and compare it to constexpr kClockFrequency (allowing a small tolerance) and error/log/abort on mismatch; reference kClockFrequency, TickPeriod and Timer::Timer() when making the change.
🤖 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/timer/timer.hpp`:
- Around line 81-98: 在 delay 模板函数中，当前用 InputDuration remaining（基于输入单位）每轮减去的是
max_segment 的截断值但实际等待的是更大的 max_segment_ticks，导致累积超时；把剩余量统一按实际 ticks/FullDuration
跟踪：将传入的 delay 先转换为以 ticks 为单位的 FullDuration 或整型剩余 ticks（使用
duration_to_ticks_checked_ceil/count_to_u64_checked），在循环中每次从该剩余 ticks 中减去真正调用
delay_basic 的 max_segment_ticks（或其 ticks 值），最后对不足一段的剩余 ticks 调用
delay_basic；定位符：delay, InputDuration, remaining, max_segment_ticks, max_segment,
delay_basic, FullDuration, duration_to_ticks_checked_ceil, count_to_u64_checked。
- Around line 120-130: The helpers duration_to_ticks_checked_32 and
duration_to_ticks_checked_48 (and the underlying duration_to_ticks_checked
template they call) currently truncate durations to whole ticks causing
non-multiple-of-tick waits to under-wait; change the conversion to perform
ceiling rounding (i.e., round up to the next tick) so that any fractional tick
yields at least one full tick, while preserving the existing overflow/max-check
behavior against kMaxDelay32Ticks and kMaxDelay48Ticks; locate
duration_to_ticks_checked<max_ticks>, adjust its math to use an upward rounding
strategy (or std::chrono::ceil) and ensure the return types and overflow checks
remain correct for duration_to_ticks_checked_32 and
duration_to_ticks_checked_48.
---
Nitpick comments:
In `@firmware/c_board/app/src/timer/timer.hpp`:
- Around line 22-23: The code currently hardcodes kClockFrequency and TickPeriod
which duplicates TIM2 configuration; change to a single source or add a runtime
consistency check: either (A) centralize the TIM2 clock/prescaler into one
header/constant used by both the CubeMX-generated TIM2 init and timer.hpp
(remove the literal 168'000'000 / 2 / 21), or (B) keep the compile-time value
but add a runtime assertion in Timer::Timer() that computes the effective TIM2
tick rate from the RCC/APB1 prescaler and the TIM2->PSC (or via
HAL_RCC_GetPCLK1Freq and TIM2->PSC) and compare it to constexpr kClockFrequency
(allowing a small tolerance) and error/log/abort on mismatch; reference
kClockFrequency, TickPeriod and Timer::Timer() when making the change.
In `@firmware/c_board/bsp/cubemx/rmcs_slave.ioc`:
- Line 265: 更新 ProjectManager.functionlistsort 中的函数顺序，使 MX_TIM9_Init 排在
MX_TIM2_Init 之前以匹配 app.cpp 对 TIM9->TIM2 的硬依赖关系；具体操作是在 functionlistsort 字段中将条目
"12-MX_TIM2_Init-TIM2-..." 和 "13-MX_TIM9_Init-TIM9-..." 互换顺序（使 MX_TIM9_Init
的条目编号和位置在 MX_TIM2_Init 之前），确保 ProjectManager.functionlistsort 中的顺序与 MX_TIM9_Init
和 MX_TIM2_Init 的依赖顺序一致以防止后续代码生成回退出错。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 081908e0-1072-4526-aa5a-7c1488423fb2
📒 Files selected for processing (10)
firmware/c_board/app/src/app.cppfirmware/c_board/app/src/spi/bmi088/accel.hppfirmware/c_board/app/src/spi/bmi088/base.hppfirmware/c_board/app/src/spi/bmi088/gyro.hppfirmware/c_board/app/src/timer/delay.cppfirmware/c_board/app/src/timer/delay.hppfirmware/c_board/app/src/timer/timer.hppfirmware/c_board/bsp/cubemx/Core/Inc/tim.hfirmware/c_board/bsp/cubemx/Core/Src/tim.cfirmware/c_board/bsp/cubemx/rmcs_slave.ioc
💤 Files with no reviewable changes (1)
- firmware/c_board/app/src/timer/delay.hpp
🚧 Files skipped from review as they are similar to previous changes (2)
- firmware/c_board/bsp/cubemx/Core/Src/tim.c
- firmware/c_board/app/src/spi/bmi088/accel.hpp
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/timer/delay.cpp`:
- Around line 14-15: Formatting issue: run clang-format on
firmware/c_board/app/src/timer/delay.cpp to fix the lint failure at the call to
timer::timer->spin_wait(timer::Timer::to_duration48_checked(std::chrono::milliseconds(delay)));;
apply the project's clang-format configuration (or run the repo's format script)
so the call and nested function arguments are wrapped/indented according to
style, then commit the formatted file so CI lint passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db00ee13-9c6e-44e1-a014-54ee1b2cfea1
📒 Files selected for processing (5)
firmware/c_board/app/src/spi/bmi088/accel.hppfirmware/c_board/app/src/spi/bmi088/base.hppfirmware/c_board/app/src/spi/bmi088/gyro.hppfirmware/c_board/app/src/timer/delay.cppfirmware/c_board/app/src/timer/timer.hpp
🚧 Files skipped from review as they are similar to previous changes (2)
- firmware/c_board/app/src/spi/bmi088/accel.hpp
- firmware/c_board/app/src/spi/bmi088/base.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.
固件延迟计时器服务重构（更新）
概述
本 PR 用基于链式 TIM2/TIM9 计数的 Timer 服务替换了原先基于 DWT 的 busy-wait 延迟实现，新增 48 位时间点与非阻塞延迟检查接口（delay_async 风格的 check_expired），并将 HAL_Delay 及 BMI088 驱动中的延迟调用路由到新的 timer 服务；同时在应用启动时初始化 TIM9/TIM2 并更新了 CubeMX 的 TIM 配置，移除了旧的 DWT delay.hpp。
主要改动
新增 Timer 实现：firmware/c_board/app/src/timer/timer.hpp
启动/初始化调整：firmware/c_board/app/src/app.cpp
CubeMX / HAL 定时器变更
移除与重路由
对公开/导出符号的影响
兼容性与行为注意
影响范围