Skip to content

feat(firmware, spi): Add SPI framework and BMI088 IMU support - #9

Merged
qzhhhi merged 1 commit into
dev/v3from
feature/firmware-spi
Jan 3, 2026
Merged

feat(firmware, spi): Add SPI framework and BMI088 IMU support#9
qzhhhi merged 1 commit into
dev/v3from
feature/firmware-spi

Conversation

@qzhhhi

@qzhhhiqzhhhi commented Jan 3, 2026

Copy link
Copy Markdown
Member
  • Add generic SPI module abstraction and SPI2 ISR-based transfer completion
  • Add BMI088 accelerometer/gyroscope drivers with data-ready interrupt readout and USB uplink serialization
  • Add GPIO configuration + ISRs for BMI088 INT pins and hook initialization into App startup
  • Add debug-only lazy assertion helper to support expensive/conditional checks

Notes:

  • SPI transfer currently supports FIFO-sized transactions only (DMA TODO)

SPI框架和BMI088 IMU驱动集成

概述

本PR引入通用SPI子系统并集成BMI088加速度计与陀螺仪驱动,采用SPI2中断回调完成传输,添加GPIO中断处理与应用层初始化,同时新增一个调试时的延迟断言工具以支持昂贵或条件化检查。

主要改动

1. 调试工具增强(core/src/utility/assert.hpp)

  • 新增模板化的延迟断言函数 assert_debug_lazy:接受一个nothrow可调用的谓词,在Debug构建中通过 std::invoke 求值并断言结果,Release构建中为无操作,从而避免运行时开销。

2. SPI框架实现(firmware/src/spi/)

  • 新增 ChipSelectPin 结构体表示基于GPIO的片选配置。
  • 新增抽象接口 ISpiModule(用于设备驱动继承),包含纯虚回调 transmit_receive_completed_callback(size_t)。
  • 新增具体管理类 Spi:
    • 提供 Lazy 初始化别名与静态 spi2 实例(HPM_SPI2_BASE,IRQn_SPI2,10 Mbps)。
    • 原子锁(std::atomic_flag)用于传输序列化(try_lock/unlock)。
    • FIFO 基础的同步 transmit_receive 路径(支持 ≤ kHardwareFifoSize),超出则留作 DMA TODO。
    • begin_transfer / finish_transfer 管理 CS 控制与传输状态。
    • 公共对齐的 tx_buffer / rx_buffer(L1 缓存行对齐)。
  • SPI2 中断服务(firmware/src/spi/spi.cpp):
    • 添加 spi2_isr,读取中断标志、在 end 标志时调用当前模块的 transmit_receive_completed_callback,并清除中断状态;无标志时快速返回([[unlikely]])。

3. BMI088 驱动(firmware/src/spi/bmi088/)

  • accel.hpp(Accelerometer):
    • 完整的BMI088加速度计类,支持量程与数据速率配置、软复位/WHO_AM_I 校验、数据就绪中断配置与回调(data_ready_callback),通过 SPI 发起读并在完成时解析数据,上链至 USB serializer。
    • 提供 prepare_tx_buffer_* 辅助和静态 inline Lazy 实例绑定 spi2 与指定 CS。
  • gyro.hpp(Gyroscope):
    • 完整的BMI088陀螺仪类,支持量程与带宽配置,初始化包含寄存器复位、Who Am I 验证、重试-读回确认模式,data_ready_callback 触发读取并在完成时序列化上链。
    • 提供静态 inline Lazy 实例绑定 spi2 与指定 CS。

4. GPIO 中断处理(firmware/src/gpio/)

  • 新增头文件与实现:声明并实现 void init_bmi088_interrupts()。
  • 为加速度计与陀螺仪分别配置 GPIO 引脚、中断触发(下降沿/active-low)、中断优先级与使能,并通过 SDK_DECLARE_EXT_ISR_M 绑定 ISR。
  • ISR 回调在触发时调用对应 spi::bmi088 设备对象的 data_ready_callback。

5. 应用初始化(firmware/src/app.cpp)

  • 在 App 构造流程中添加 BMI088 驱动与中断初始化(在 CAN 初始化之后、USB 初始化之前调用),确保传感器和中断在 USB 上链前就绪。

关键特性与设计要点

  • 中断驱动的读取路径:数据就绪中断触发异步 SPI 读取,减少轮询开销。
  • SPI 传输序列化:使用原子标志避免并发传输冲突;中断回调负责完成/解锁逻辑。
  • USB 上链:传感器数据通过 Serializer 接口序列化并上传主机。
  • 驱动配置稳健性:对关键寄存器写入采用重试与读回确认。
  • 性能优化:TX/RX 缓冲区按 L1 缓存行对齐,便于将来 DMA 支持。

限制与待办

  • 当前 SPI 仅支持 FIFO 大小(≤ kHardwareFifoSize)的事务,同步路径;对更大传输需补充 DMA 支持(标注为 TODO)。
  • 增加了新的全局/静态设备实例与中断入口,代码审查时需注意初始化顺序和中断优先级对系统其他外设的影响。

@coderabbitai

coderabbitaiBot commented Jan 3, 2026

Copy link
Copy Markdown

Walkthrough

引入 BMI088 加速度计与陀螺仪的 SPI 驱动与 GPIO 中断集成,并在核心断言工具中新增一个仅在调试模式下评估的惰性断言模板函数 assert_debug_lazy

Changes

Cohort / File(s)Change summary
核心断言工具
core/src/utility/assert.hpp
新增 assert_debug_lazy 模板声明与实现,添加相关 type_traits/utility/includes;在非发布构建中接受 nothrow-invocable 谓词并通过 std::invoke 验证,发布构建为空操作。
应用初始化
firmware/src/app.cpp
在 App 构造中加入 BMI088 加速度计与陀螺仪驱动初始化调用并启用中断(在 CAN 初始化后、USB 初始化前)。
GPIO 中断处理
firmware/src/gpio/gpio.hpp, firmware/src/gpio/gpio.cpp
新增 init_bmi088_interrupts() 声明与实现;配置对应 GPIO 引脚/中断并通过 SDK_DECLARE_EXT_ISR_M 注册两个 ISR,ISR 调用对应 spi::bmi088 传感器的 data_ready_callback()
SPI 驱动框架
firmware/src/spi/spi.hpp, firmware/src/spi/spi.cpp
新增 Spi 类、ISpiModule 抽象接口与 ChipSelectPin 类型;实现传输锁定、同步/阻塞传输路径、完成回调及 SPI2 静态实例;新增 spi2_isr() 中断处理文件。
BMI088 传感器驱动
firmware/src/spi/bmi088/accel.hpp, firmware/src/spi/bmi088/gyro.hpp
分别新增 AccelerometerGyroscope 类:完整 SPI 初始化序列、寄存器读写、数据就绪回调、传输完成处理与上传序列化接口,并在命名空间内提供 inline 懒初始化实例。

Sequence Diagram(s)

sequenceDiagram
autonumber
participant GPIO as GPIO
participant ISR as ISR
participant Sensor as BMI088 Sensor (Accel/Gyro)
participant SPI as Spi (驱动)
participant USB as USB Serializer
Note over GPIO,ISR `#DDEBF7`: 中断触发
GPIO->>ISR: 数据准备中断
ISR->>Sensor: data_ready_callback()
Note over Sensor `#F7F6D8`: 组装读寄存器请求
Sensor->>SPI: transmit_receive(module, size)
Note over SPI `#E8F5E9`: 开始传输 (CS 拉低、写入/读出 FIFO)
SPI->>SPI: begin_transfer()
SPI->>SPI: 处理硬件传输
SPI->>Sensor: transmit_receive_completed_callback(size)
Note over Sensor,USB `#FFF4E6`: 解析并序列化数据
Sensor->>USB: handle_uplink(serializer, data)
USB->>USB: 序列化并上传
Sensor->>SPI: unlock() / 释放 CS
Loading

Estimated code review effort

🎯 4 (复杂) | ⏱️ ~60 分钟

Possibly related PRs

诗作

🐰 我是条爱跳的兔子,来报喜:
传感器醒来拍着脚,GPIO 敲门急又俏,
SPI 列队把数据拉,USB 微笑把信跑,
调试断言惰性算,代码夜里也会跳! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 2.78% which is insufficient. The required threshold is 80.00%.You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedPR标题准确概括了主要变更:添加SPI框架和BMI088 IMU支持,这与raw_summary中涵盖的SPI抽象、BMI088驱动程序和GPIO中断处理完全一致。
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qzhhhi

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
firmware/src/spi/spi.hpp (1)

82-84: 内存序可能需要更强的保证。

当前 locking_ 使用 memory_order::relaxed。由于 unlock() 在 ISR(通过 transmit_receive_completed_callback)中调用,而 try_lock() 在主线程中调用,relaxed 序可能导致主线程观察到过期的锁状态。

建议考虑:

  • unlock() 使用 memory_order::release
  • try_lock() 使用 memory_order::acquire

不过,如果所有 SPI 调用都发生在同一优先级的中断上下文中,或者架构保证了可见性(例如单核且中断总是更高优先级),则当前实现可能是安全的。

🔎 建议的修改
- bool locking() { return locking_.test(std::memory_order::relaxed); }+ bool locking() { return locking_.test(std::memory_order::acquire); }- bool try_lock() { return !locking_.test_and_set(std::memory_order::relaxed); }+ bool try_lock() { return !locking_.test_and_set(std::memory_order::acquire); }
void unlock() {
core::utility::assert_debug_lazy([&]() noexcept { return locking(); });
- locking_.clear(std::memory_order::relaxed);+ locking_.clear(std::memory_order::release);
}
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 28fc007 and 208bf76.

📒 Files selected for processing (8)
  • core/src/utility/assert.hpp
  • firmware/src/app.cpp
  • firmware/src/gpio/gpio.cpp
  • firmware/src/gpio/gpio.hpp
  • firmware/src/spi/bmi088/accel.hpp
  • firmware/src/spi/bmi088/gyro.hpp
  • firmware/src/spi/spi.cpp
  • firmware/src/spi/spi.hpp
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-20T05:28:48.614Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: core/src/utility/assert.hpp:13-15
Timestamp: 2025-12-20T05:28:48.614Z
Learning: Require include directives in all header files to place system/library headers (e.g., #include <...>) before project headers (e.g., #include "..."). This ordering reduces cross-library macro interference (such as #undef assert in core/src/utility/assert.hpp) and improves portability. Apply this convention to every .hpp in the repository (e.g., core/src/utility/assert.hpp and other headers).

Applied to files:

  • firmware/src/spi/bmi088/accel.hpp
  • firmware/src/spi/bmi088/gyro.hpp
  • firmware/src/gpio/gpio.hpp
  • firmware/src/spi/spi.hpp
  • core/src/utility/assert.hpp
📚 Learning: 2025-12-29T06:42:42.597Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 6
File: firmware/src/utility/lazy.hpp:22-22
Timestamp: 2025-12-29T06:42:42.597Z
Learning: In bare-metal firmware, objects with static storage duration that are lazily initialized should not rely on non-trivial destructors, because the system runs continuously until reset. If you have global/lazy-initialized objects in firmware, prefer empty or trivial destructors (or rely on startup/hardware reset) to avoid teardown issues. This guideline applies to firmware code across the repository, e.g., modules under firmware/ including header or implementation files that declare such global objects.

Applied to files:

  • firmware/src/spi/bmi088/accel.hpp
  • firmware/src/spi/bmi088/gyro.hpp
  • firmware/src/gpio/gpio.hpp
  • firmware/src/spi/spi.hpp
📚 Learning: 2025-12-20T05:31:53.309Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/CMakeLists.txt:61-66
Timestamp: 2025-12-20T05:31:53.309Z
Learning: In the librmcs project, Debug builds intentionally enable high optimization levels (-O3 for GCC/Clang, /O2 for MSVC) to expose undefined behavior early while keeping assertions active. This is a deliberate design choice where catching UB takes priority over interactive debugging convenience.

Applied to files:

  • core/src/utility/assert.hpp
🧬 Code graph analysis (2)
firmware/src/spi/bmi088/gyro.hpp (3)
core/src/utility/assert.hpp (4)
  • assert_debug (36-44)
  • assert_debug (36-37)
  • assert_always (30-34)
  • assert_always (30-31)
firmware/src/spi/bmi088/accel.hpp (13)
  • address (139-145)
  • address (139-139)
  • address (147-153)
  • address (147-147)
  • address (162-166)
  • address (162-162)
  • address (168-172)
  • address (168-168)
  • read (103-103)
  • size (155-160)
  • size (155-155)
  • serializer (174-179)
  • serializer (174-174)
firmware/src/spi/spi.hpp (1)
  • size (32-32)
firmware/src/app.cpp (2)
firmware/src/gpio/gpio.cpp (2)
  • init_bmi088_interrupts (46-49)
  • init_bmi088_interrupts (46-46)
firmware/src/gpio/gpio.hpp (1)
  • init_bmi088_interrupts (5-5)
🪛 Cppcheck (2.19.0)
firmware/src/gpio/gpio.cpp

[error] 51-51: There is an unknown macro here somewhere. Configuration is required. If SDK_DECLARE_EXT_ISR_M is a macro then please configure it.

(unknownMacro)

firmware/src/spi/spi.cpp

[error] 10-10: There is an unknown macro here somewhere. Configuration is required. If SDK_DECLARE_EXT_ISR_M is a macro then please configure it.

(unknownMacro)

🔇 Additional comments (13)
core/src/utility/assert.hpp (2)

4-5: 必要的头文件引入

为新增的 assert_debug_lazy 函数添加了必要的头文件:

  • <type_traits> 用于约束检查
  • <utility> 用于完美转发

头文件顺序符合项目规范(系统库头文件在前)。


7-9: 合理的条件编译优化

仅在调试构建中引入 <functional> 头文件是正确的,因为 std::invoke 仅在 assert_debug_lazy 的调试分支中使用。这减少了发布构建的编译依赖。

firmware/src/app.cpp (1)

29-31: 初始化顺序正确。

传感器驱动在启用 GPIO 中断之前完成初始化,确保了 ISR 回调时传感器对象已准备就绪。整个初始化过程在 InterruptLockGuard 保护下进行,避免了竞态条件。

firmware/src/gpio/gpio.hpp (1)

1-7: LGTM!

头文件结构简洁,API 表面最小化,命名空间使用正确。

firmware/src/spi/spi.cpp (1)

10-22: ISR 实现正确。

中断处理逻辑清晰:检查标志位、调用回调、清除标志。[[unlikely]] 属性对无中断标志的情况是合理的优化提示。

关于 Cppcheck 报告的 SDK_DECLARE_EXT_ISR_M 宏警告是误报,这是 SDK 提供的标准 ISR 注册宏。

firmware/src/gpio/gpio.cpp (1)

51-63: GPIO ISR 实现正确。

使用 gpio_check_clear_interrupt_flag 进行原子性检查并清除标志,避免了虚假中断回调。ISR 实现简洁高效。

firmware/src/spi/bmi088/gyro.hpp (2)

105-105: data_ready_callback 在 SPI 锁定失败时静默丢弃读取。

当前实现中,如果 read()try_lock() 失败而返回 false,该次数据采样将被静默丢弃。对于 IMU 这类高频数据流,偶尔丢失一个样本通常是可接受的。但如果需要调试或监控丢失情况,可以考虑添加计数器或日志。


149-154: 回调实现正确。

std::launder 的使用正确处理了类型双关。数据偏移量 rx_buffer + 1 与陀螺仪 SPI 协议一致(无需虚拟字节,与加速度计的 +2 偏移不同)。

firmware/src/spi/bmi088/accel.hpp (2)

70-72: 加速度计 SPI 模式切换处理正确。

根据 BMI088 数据手册,加速度计在上电后默认为 I2C 模式,需要进行一次虚拟读取来切换到 SPI 模式。这个处理是正确的。


155-160: 回调与缓冲区偏移正确。

加速度计读取需要额外的虚拟字节(BMI088 加速度计 SPI 协议要求),因此数据偏移为 rx_buffer + 2,与陀螺仪的 rx_buffer + 1 不同。这是正确的实现。

firmware/src/spi/spi.hpp (3)

146-160: finish_transfer 实现正确。

通过在阻塞传输期间禁用中断(第 108 行),避免了与 ISR 的并发访问。CS 引脚的激活/去激活逻辑正确处理了 active_low 配置。


99-104: FIFO 传输路径正确实现。

当前 BMI088 传输大小(7-8 字节)在 kHardwareFifoSize(8 字节)限制内。DMA 路径使用 assert_failed_always() 明确标记为未实现,这是合理的占位符。


25-35: 接口设计合理。

ISpiModule 提供了清晰的回调机制,friend class Spi 允许访问 CS 引脚配置。在裸机固件环境中不使用动态分配,缺少虚析构函数是可接受的。

Comment threadcore/src/utility/assert.hpp
- Add generic SPI module abstraction and SPI2 ISR-based transfer completion
- Add BMI088 accelerometer/gyroscope drivers with data-ready interrupt readout and USB uplink serialization
- Add GPIO configuration + ISRs for BMI088 INT pins and hook initialization into App startup
- Add debug-only lazy assertion helper to support expensive/conditional checks
Notes:
- SPI transfer currently supports FIFO-sized transactions only (DMA TODO)
@qzhhhi
qzhhhiforce-pushed the feature/firmware-spi branch from 208bf76 to efa5a10CompareJanuary 3, 2026 15:09
@qzhhhi

Copy link
Copy Markdown
MemberAuthor

@coderabbitai review

@coderabbitai

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
firmware/src/spi/spi.hpp (2)

119-123: 注释掉的断言应明确处理

第 120 行的断言被注释但未删除:

// core::utility::assert_debug(module_);

由于第 121 行通过 if (auto module = finish_transfer()) 处理了空指针情况,此断言可能不再必要。建议:

  • 如果断言确实不需要,请删除注释行
  • 如果断言用于调试目的,请解释为何注释掉并考虑使用条件编译
🔎 建议的处理方式

方案 1:删除不需要的断言

- // core::utility::assert_debug(module_);
if (auto module = finish_transfer())

方案 2:如果需要保留用于调试,添加说明

- // core::utility::assert_debug(module_);+ // Note: module_ nullptr is valid when called spuriously; handled by if-check below
if (auto module = finish_transfer())

133-134: 公开缓冲区是有意设计,但可考虑封装改进

第 133-134 行的 tx_bufferrx_buffer 被声明为公开成员,允许外部模块直接访问。从相关代码片段(firmware/src/spi/bmi088/accel.hpp 第 156 行和 gyro.hpp 第 150 行)可以看到,这是有意为之,用于直接硬件缓冲区访问:

auto& data = *std::launder(reinterpret_cast<Data*>(spi_.rx_buffer + 2));

当前设计是功能性的,但暴露了实现细节。

可选改进:考虑提供访问器方法来封装缓冲区访问,例如:

std::byte* get_tx_buffer() { return tx_buffer; }
const std::byte* get_rx_buffer() const { return rx_buffer; }

这样可以在不改变功能的情况下改善封装性,但鉴于裸机固件的性能要求,当前的直接访问也是可以接受的。

firmware/src/spi/bmi088/accel.hpp (1)

19-182: 考虑减少与陀螺仪驱动的代码重复。

accel.hppgyro.hpp 之间存在显著的结构相似性:

  • 构造函数中的初始化模式(带重试的阻塞读写 lambda)
  • write()read() 方法
  • 缓冲区准备方法
  • handle_uplink 模式

虽然两个设备具有不同的 SPI 协议(加速度计有 2 个哑字节,陀螺仪有 1 个)和不同的寄存器映射,但可以考虑以下重构:

  • 提取共享的初始化辅助工具
  • 使用模板或 CRTP 模式减少重复
  • 创建通用的 BMI088 设备基类

当前代码清晰且可维护,这个重构可以在后续优化中考虑。

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 208bf76 and efa5a10.

📒 Files selected for processing (8)
  • core/src/utility/assert.hpp
  • firmware/src/app.cpp
  • firmware/src/gpio/gpio.cpp
  • firmware/src/gpio/gpio.hpp
  • firmware/src/spi/bmi088/accel.hpp
  • firmware/src/spi/bmi088/gyro.hpp
  • firmware/src/spi/spi.cpp
  • firmware/src/spi/spi.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • firmware/src/gpio/gpio.hpp
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-20T05:28:48.614Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: core/src/utility/assert.hpp:13-15
Timestamp: 2025-12-20T05:28:48.614Z
Learning: Require include directives in all header files to place system/library headers (e.g., #include <...>) before project headers (e.g., #include "..."). This ordering reduces cross-library macro interference (such as #undef assert in core/src/utility/assert.hpp) and improves portability. Apply this convention to every .hpp in the repository (e.g., core/src/utility/assert.hpp and other headers).

Applied to files:

  • firmware/src/spi/bmi088/gyro.hpp
  • firmware/src/spi/bmi088/accel.hpp
  • firmware/src/spi/spi.hpp
  • core/src/utility/assert.hpp
📚 Learning: 2025-12-29T06:42:42.597Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 6
File: firmware/src/utility/lazy.hpp:22-22
Timestamp: 2025-12-29T06:42:42.597Z
Learning: In bare-metal firmware, objects with static storage duration that are lazily initialized should not rely on non-trivial destructors, because the system runs continuously until reset. If you have global/lazy-initialized objects in firmware, prefer empty or trivial destructors (or rely on startup/hardware reset) to avoid teardown issues. This guideline applies to firmware code across the repository, e.g., modules under firmware/ including header or implementation files that declare such global objects.

Applied to files:

  • firmware/src/spi/bmi088/gyro.hpp
  • firmware/src/spi/bmi088/accel.hpp
  • firmware/src/spi/spi.hpp
🧬 Code graph analysis (3)
firmware/src/spi/bmi088/accel.hpp (3)
core/src/utility/assert.hpp (4)
  • assert_debug (36-44)
  • assert_debug (36-37)
  • assert_always (30-34)
  • assert_always (30-31)
firmware/src/spi/bmi088/gyro.hpp (13)
  • address (133-139)
  • address (133-133)
  • address (141-147)
  • address (141-141)
  • address (156-160)
  • address (156-156)
  • address (162-166)
  • address (162-162)
  • read (105-105)
  • size (149-154)
  • size (149-149)
  • serializer (168-173)
  • serializer (168-168)
firmware/src/spi/spi.hpp (1)
  • size (32-32)
firmware/src/spi/spi.hpp (3)
firmware/src/spi/bmi088/accel.hpp (2)
  • size (155-160)
  • size (155-155)
firmware/src/spi/bmi088/gyro.hpp (2)
  • size (149-154)
  • size (149-149)
core/src/utility/assert.hpp (6)
  • assert_always (30-34)
  • assert_always (30-31)
  • assert_debug_lazy (48-56)
  • assert_debug_lazy (48-49)
  • assert_debug (36-44)
  • assert_debug (36-37)
firmware/src/app.cpp (2)
firmware/src/gpio/gpio.cpp (2)
  • init_bmi088_interrupts (46-49)
  • init_bmi088_interrupts (46-46)
firmware/src/gpio/gpio.hpp (1)
  • init_bmi088_interrupts (5-5)
🪛 Cppcheck (2.19.0)
firmware/src/spi/spi.cpp

[error] 10-10: There is an unknown macro here somewhere. Configuration is required. If SDK_DECLARE_EXT_ISR_M is a macro then please configure it.

(unknownMacro)

firmware/src/gpio/gpio.cpp

[error] 51-51: There is an unknown macro here somewhere. Configuration is required. If SDK_DECLARE_EXT_ISR_M is a macro then please configure it.

(unknownMacro)

🔇 Additional comments (20)
core/src/utility/assert.hpp (2)

4-9: 新增的头文件引入正确支持惰性断言功能

为支持新的 assert_debug_lazy 模板函数,正确添加了必要的标准库头文件:

  • <type_traits> 用于约束检查
  • <utility> 用于完美转发
  • <functional> 仅在调试构建中引入,用于 std::invoke

条件编译的引入策略能够在发布版本中最小化依赖,设计合理。


46-56: 已修正参数传递方式,惰性断言设计优秀

相比于之前的审查,参数 location 已从按值传递修正为 const std::source_location&,现在与文件中其他断言函数(第 16、21、31、37 行)保持一致。修正正确。

新增的 assert_debug_lazy 功能设计合理,适用于开销较大的调试检查:

  • 约束条件 std::is_nothrow_invocable_r_v<bool, Condition&&> 确保类型安全
  • 使用 std::invoke 和完美转发正确实现惰性求值
  • 发布版本中完全优化掉参数求值,无运行时开销

基于先前的审查意见,此问题已得到解决。

firmware/src/spi/spi.hpp (5)

1-15: 头文件引入顺序符合规范

头文件引入顺序正确遵循了编码规范:标准库头文件(第 3-5 行)→ HPM SDK 头文件(第 7-10 行)→ 项目头文件(第 12-14 行)。这种顺序能够减少跨库宏干扰并提高可移植性。

基于先前的学习经验,此做法符合仓库约定。


41-78: 构造函数实现正确,assert_debug_lazy 使用恰当

构造函数正确初始化了 SPI 硬件配置,包括时钟、格式、控制参数和中断。

第 74-77 行对 assert_debug_lazy 的使用是一个很好的示例:

  • 用于验证硬件 FIFO 大小的开销较大的检查
  • Lambda 表达式正确标记为 noexcept
  • 仅在调试构建中执行,发布版本无开销

80-84: 析构函数删除正确,锁机制简单但需注意内存序

第 80 行删除析构函数符合裸机固件的最佳实践,因为系统持续运行直到硬件重置。

第 82-84 行的锁管理使用 std::atomic_flag 实现了简单的自旋锁机制:

  • locking() 检查锁状态而不修改
  • try_lock() 使用 test_and_set 尝试获取锁

当前使用 relaxed 内存序,在单核系统中可接受,但如果目标平台支持多核或将来扩展到多核场景,可能需要更强的内存序保证。

基于裸机固件学习经验。

如果目标平台可能运行在多核环境,请验证 relaxed 内存序是否提供足够的同步保证。


173-173: 静态实例初始化正确使用 constinit 和 Lazy 包装器

第 173 行使用 inline constinitLazy 包装器初始化 spi2 实例,这是裸机固件中静态存储期对象的正确做法:

  • constinit 确保常量初始化
  • Lazy 包装器延迟构造直到首次访问
  • 配合第 80 行删除的析构函数,避免了非平凡析构器问题

基于裸机固件学习经验,此实现符合最佳实践。


86-105: DMA 支持仍待实现,但当前 8 字节 FIFO 限制不会影响现有设备

函数正确实现了 SPI 传输流程,但需要注意的是:

虽然 kMaxTransferSize 被设置为 HPM_L1C_CACHELINE_SIZE(通常为 64 字节),但硬件 FIFO 仅支持 8 字节。第 103 行对于超过 8 字节的传输会触发断言失败,DMA 支持标记为 TODO。

不过,对 BMI088 的实际使用分析表明,这不构成当前的限制问题:

  • 陀螺仪 (gyro):最大传输 7 字节(1 字节命令 + 6 字节数据)
  • 加速度计 (accel):最大传输 8 字节(2 字节命令 + 6 字节数据)

所有单寄存器读写操作均为 2-3 字节。因此,现有代码的所有使用都恰好适配 8 字节的硬件 FIFO。

DMA 支持仍然是后续改进工作(支持更大传输或未来设备),但对当前 BMI088 设备驱动无影响。

Likely an incorrect or invalid review comment.

firmware/src/app.cpp (2)

3-5: 新增的头文件引入支持 BMI088 集成

为集成 BMI088 加速度计和陀螺仪,正确添加了必要的头文件:

  • gpio/gpio.hpp:GPIO 中断配置
  • spi/bmi088/accel.hpp:加速度计驱动
  • spi/bmi088/gyro.hpp:陀螺仪驱动

29-31: BMI088 初始化顺序正确

第 29-31 行的初始化顺序符合逻辑:

  1. 先初始化加速度计和陀螺仪驱动(第 29-30 行)
  2. 再启用 BMI088 中断(第 31 行)

这种顺序能够确保传感器就绪后再启用中断,避免在初始化期间产生伪中断。初始化位置(CAN 之后、USB 之前)也是合理的。

firmware/src/spi/spi.cpp (1)

10-22: SPI2 中断服务例程实现正确

中断处理逻辑清晰且正确:

  • 第 15-16 行的提前返回(带 [[unlikely]] 属性)优化了无中断标志的情况
  • 第 18-19 行在检测到传输结束中断时调用回调
  • 第 21 行清除中断标志以确认中断

流程顺序正确:先处理回调,再清除标志,避免了竞态条件。

注:第 10 行的静态分析提示关于 SDK_DECLARE_EXT_ISR_M 未知宏是误报,这是 HPM SDK 提供的供应商宏。

firmware/src/gpio/gpio.cpp (2)

14-40: BMI088 中断配置实现正确且一致

第 17-27 行和第 29-40 行分别配置陀螺仪和加速度计的 GPIO 中断,遵循一致的模式:

  1. 设置引脚功能(IOC 配置)
  2. 分配 GPIO 控制器
  3. 配置为输入模式
  4. 配置中断触发方式(下降沿)
  5. 清除中断标志
  6. 使能引脚中断和 IRQ

注意事项:

  • 陀螺仪使用 PB15(第 14、18 行)
  • 加速度计使用 PY00(第 15、30-31 行),需要同时配置 IOC 和 PIOC
  • 两者均配置为下降沿触发(active-low)
  • IRQ 优先级均为 1

配置逻辑正确且文档化良好(第 14-15 行的常量定义包含引脚说明)。


46-63: 中断初始化和 ISR 实现正确

第 46-49 行的 init_bmi088_interrupts 函数正确调用两个配置函数来初始化 BMI088 中断。

第 51-56 行和第 58-63 行的 ISR 实现正确:

  • 使用 gpio_check_clear_interrupt_flag 原子地检查并清除中断标志
  • 仅在标志确实被设置时调用相应传感器的 data_ready_callback()
  • 避免了伪中断和竞态条件

ISR 流程简洁高效,符合裸机固件的最佳实践。

注:第 51 和 58 行的静态分析提示关于 SDK_DECLARE_EXT_ISR_M 未知宏是误报,这是 HPM SDK 提供的供应商宏。

firmware/src/spi/bmi088/gyro.hpp (4)

3-15: Include 顺序正确!

系统/库头文件在项目头文件之前,符合代码规范。这种顺序可以减少跨库宏干扰并提高可移植性。


19-39: 类声明结构良好!

使用 final 关键字防止继承,枚举类型定义清晰,Lazy 别名的使用符合延迟初始化模式。


149-154: 回调函数中的缓冲区处理正确。

使用 std::launderreinterpret_cast 进行类型转换是正确的做法,符合 C++ 严格别名规则。rx_buffer + 1 的偏移量正确处理了陀螺仪 SPI 协议的 1 字节哑元。

注意:assert_debug 在 Release 构建中会变成性能提示([[assume]]),如果大小验证失败将导致未定义行为。考虑到这是硬件通信,如果协议实现正确应该不会失败,这种用法是可接受的。


178-178: 全局实例声明正确。

使用 inlineLazy 包装器进行延迟初始化是适合裸机固件的模式。Gyroscope 类型具有平凡析构函数(仅包含引用成员),符合固件中静态存储期对象的要求。

基于 learnings,裸机固件中的静态存储期对象应避免依赖非平凡析构函数,因为系统持续运行直到复位。

firmware/src/spi/bmi088/accel.hpp (4)

3-15: Include 顺序正确!

与陀螺仪驱动一致,系统/库头文件在项目头文件之前,符合代码规范。


70-72: 加速度计 SPI 模式切换的哑读操作正确。

根据 BMI088 数据手册,加速度计需要进行哑读操作以从 I2C 模式切换到 SPI 模式。这个实现是正确的。


155-160: 加速度计回调函数中的缓冲区偏移正确。

加速度计 SPI 协议使用 2 个哑字节(与陀螺仪的 1 个不同),因此 rx_buffer + 2sizeof(Data) + 2 的验证是正确的。这与 prepare_tx_buffer_read 中的 read_size + 2 返回值保持一致。


184-185: 全局实例声明正确。

与陀螺仪驱动类似,使用 inlineLazy 包装器进行延迟初始化,使用不同的 GPIO 片选引脚(pin 14)。Accelerometer 类型具有平凡析构函数,符合固件静态存储期对象的要求。

基于 learnings,这种模式适合裸机固件。

Comment threadfirmware/src/spi/bmi088/accel.hpp
Comment threadfirmware/src/spi/bmi088/gyro.hpp
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant

@qzhhhi