LibRMCS v3 is READY! #11
Conversation
Walkthrough本次变更引入完整固件—主机通信协议栈及固件设备驱动：新增 LIFO 协程框架、协议（头/序列化/反序列化）、USB vendor 接口、CAN/SPI/UART 驱动、DMA 缓冲机制与主机端 USB 传输/处理，同时删除旧示例与若干遗留设备/客户端实现。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant FirmwareApp as 固件 App
participant CAN as CAN 驱动 (ISR)
participant Serializer as Serializer (firmware)
participant USB as TinyUSB/Vendor
participant HostTransport as 主机 USB 传输 (libusb)
participant HostHandler as 主机 Handler
participant Deserializer as Deserializer (host)
participant Callback as IDataCallback (host client)
Note over FirmwareApp,CAN: 硬件中断路径（上行）
CAN->>FirmwareApp: ISR -> handle_uplink(DataView)
FirmwareApp->>Serializer: write_can(view)
Serializer->>USB: allocate & fill batch
USB->>HostTransport: USB transmit (vendor endpoint)
Note over HostTransport,HostHandler: 主机接收并反序列化
HostTransport->>HostHandler: receive(buffer)
HostHandler->>Deserializer: feed(buffer)
Deserializer->>Deserializer: process_stream()/process_can_field()
Deserializer->>Callback: can_deserialized_callback(id, view)
Note over Callback,HostHandler: 主机发起下行
Callback->>HostHandler: start_transmit()->PacketBuilder
HostHandler->>Serializer: write_can(...)
Serializer->>HostTransport: transmit buffer
HostTransport->>USB: USB packet -> 固件
Estimated code review effort🎯 4 (复杂) | ⏱️ ~60 分钟 Possibly related PRs
诗歌
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 14
🤖 Fix all issues with AI agents
In @core/include/librmcs/data/datas.hpp:
- Around line 57-71: The method signatures in IDataCallback incorrectly prefix
types with the local namespace `data::` (causing `librmcs::data::data`
resolution); update the signatures in class IDataCallback (can_receive_callback,
uart_receive_callback, accelerometer_receive_callback,
gyroscope_receive_callback) to use the types directly as CanDataView,
UartDataView, AccelerometerDataView, GyroscopeDataView (i.e., remove the
erroneous `data::` qualifier) so they resolve to the types declared in
librmcs::data.
In @core/src/utility/assert.hpp:
- Around line 36-44: The use of C++23 attribute [[assume(condition)]] in
assert_debug is not portable; update assert_debug to detect support and fall
back to compiler intrinsics: use __has_cpp_attribute(assume) to use [[assume]],
otherwise use compiler-specific builtins like __builtin_assume(condition) for
GCC/Clang or __assume(condition) for MSVC, and if none exist keep the existing
(void)location; apply this via conditional compilation around the assert_debug
body (keeping the NDEBUG branch behavior) and reference the existing symbols
assert_debug and assert_always when making the change.
In @core/src/utility/bitfield.hpp:
- Around line 25-34: BitfieldMember currently allows signed ValueType but
read_bits does not perform sign extension, so sub-width signed fields (e.g.,
BitfieldMember<0,12,int16_t>) decode incorrectly; update the implementation used
by read_bits to detect if Member::ValueType is signed and Member::bit_width <
(sizeof(Member::ValueType)*8) then perform proper sign-extension after
extracting the raw unsigned bitfield (e.g., if the sign bit (1 << (bit_width-1))
is set, set the high bits of the ValueType to 1); reference BitfieldMember,
read_bits, and usages such as ImuAccelerometerPayload/ImuGyroscopePayload when
locating code, and also adjust or add a clarifying comment/docstring about
supporting signed sub-width fields (or alternatively add a static_assert to
forbid signed ValueType if you prefer to restrict instead of adding sign
extension).
In @core/src/utility/uncopyable.hpp:
- Around line 5-12: The class name is misspelled: rename class Uncopyble to
Uncopyable and update all internal declarations (constructors, copy/move
operations: Uncopyble(), Uncopyble(const Uncopyble&), Uncopyble& operator=(const
Uncopyble&), Uncopyble(Uncopyble&&), Uncopyble& operator=(Uncopyble&&)) to use
Uncopyable so the type and its member signatures match; then update all usages,
forward declarations and includes across the codebase to reference Uncopyable to
keep the public API consistent.
In @firmware/bsp/hpm_sdk:
- Line 1: 子模块 firmware/bsp/hpm_sdk 当前在 superproject 中记录的提交
afb0634497795100ff6f2bcf155a8a31eeec1b61 在远程仓库
https://github.com/Alliance-Algorithm/hpm_sdk.git 不存在；请检查并修复子模块指针：在 hpm_sdk
远程仓库中确认一个有效的目标提交或分支（例如通过列出远程 refs 并选取一个存在的 commit/branch），然后在 superproject 中将
firmware/bsp/hpm_sdk 的 gitlink 更新为该有效提交（替换掉 afb0634... 的无效引用），确保更新后的子模块
URL/remote 配置正确并提交 superproject 的子模块变更到主分支，以便后续 clone/submodule init 能正常拉取。
In @firmware/src/can/can.hpp:
- Around line 49-67: handle_downlink currently ignores the return value of
mcan_transmit_via_txfifo_nonblocking, so TX-FIFO-full or other send failures are
silently dropped; update handle_downlink to check the function's return (e.g.,
the mcan return type/result code) and handle failures explicitly: on success
proceed as now, on TX-FIFO-full either retry a bounded number of times, enqueue
the frame to a software transmit queue, or log/error-report the failure (use
existing logging or an error counter/flag) and avoid silent loss. Locate
mcan_transmit_via_txfifo_nonblocking call in handle_downlink and add
return-value handling and a concrete failure path (retry/enqueue/log) consistent
with surrounding error handling policies.
In @firmware/src/gpio/gpio.cpp:
- Around line 1-10: The include order violates the project rule: system/library
headers (cstdint, board.h, hpm_gpio_drv.h, hpm_gpiom_drv.h) must appear before
project headers; reorder the top of gpio.cpp so the standard/system headers
(cstdint then board.h, hpm_gpio_drv.h, hpm_gpiom_drv.h) come first and then
include the local project headers (gpio.hpp, accel.hpp, gyro.hpp) to avoid macro
conflicts and follow the expected include layering.
In @firmware/src/usb/usb_descriptors.cpp:
- Around line 1-4: The system header <cstdint> should be included before the
project header "firmware/src/usb/usb_descriptors.hpp"; edit usb_descriptors.cpp
to swap the include order so <cstdint> comes first, keeping only necessary
includes and preserving file-level includes for functions/types used by
usb_descriptors.hpp (e.g., any types referenced in that file).
In @firmware/src/utility/lazy.hpp:
- Around line 3-12: Header is not self-contained: it uses std::construct_at,
std::destroy_at and std::addressof but does not include <memory>, risking build
failures when transitive includes change; fix by adding #include <memory> to the
top of firmware/src/utility/lazy.hpp (so usages of
std::construct_at/std::destroy_at/std::addressof compile reliably), and make the
same change for the other occurrence noted around lines 66-69.
In @host/CMakeLists.txt:
- Line 130: The Debian and RPM package homepage variables are inconsistent with
the PR target repo; update both CPack variables (CPACK_DEBIAN_PACKAGE_HOMEPAGE
and CPACK_RPM_PACKAGE_HOMEPAGE) to the correct GitHub URL (choose and use the
single agreed URL, e.g., https://github.com/Alliance-Algorithm/librmcs) so both
set(...) calls reference the same homepage string.
- Around line 74-76: The CMakeLists currently hardcodes /usr/include/libusb-1.0
and links usb-1.0 for UNIX; replace this with a pkg-config based find to make
libusb discovery flexible: use pkg_check_modules or find_package(PkgConfig) +
pkg_check_modules(LIBUSB REQUIRED libusb-1.0) (or pkg_get_variable), then add
include dirs and link libraries via target_include_directories(${PROJECT_NAME}
SYSTEM PRIVATE ${LIBUSB_INCLUDE_DIRS}) and target_link_libraries(${PROJECT_NAME}
PUBLIC ${LIBUSB_LIBRARIES}) (also propagate ${LIBUSB_CFLAGS_OTHER} if needed);
keep the existing PROJECT_NAME reference so the target settings apply to the
same target.
- Line 1: Update the cmake_minimum_required call from 3.24 to 3.15 in
CMakeLists.txt (change the cmake_minimum_required(VERSION 3.24) line to 3.15) so
the project requires CMake 3.15; leave commented test code that references
DOWNLOAD_EXTRACT_TIMESTAMP as-is but add a short inline TODO if those tests are
ever re-enabled to re-evaluate the minimum CMake version because
DOWNLOAD_EXTRACT_TIMESTAMP may require newer CMake, and ensure
CMAKE_MSVC_RUNTIME_LIBRARY usage remains compatible with 3.15.
In @host/src/protocol/handler.cpp:
- Around line 99-111: The function process_result can fall through after
core::utility::assert_failed_debug() and cause undefined behavior in release
builds; ensure every control path returns a bool by adding an explicit return
(e.g., return false) after the assert_failed_debug() call or mark the path as
unreachable (e.g., using [[unreachable]] or __builtin_unreachable()) so that
process_result always returns a value for all Serializer::SerializeResult cases.
🧹 Nitpick comments (39)
host/src/utility/cross_os.hpp (1)
27-33: 建议将宏定义移到命名空间外或使用更具体的名称。虽然在命名空间块内定义宏在语法上是有效的，但预处理器宏始终是全局的，不受命名空间约束。在命名空间内定义宏可能会让人误以为它属于该命名空间，但实际上
ALWAYS_INLINE是全局可见的。此外，这个通用名称可能与其他库中的相同宏定义产生冲突。建议考虑以下方案之一：
- 将宏定义移到命名空间外（在
namespace声明之前）- 使用更具体的名称，如
LIBRMCS_ALWAYS_INLINE，以避免潜在的命名冲突♻️ 建议的重构方案
方案 1：将宏定义移到命名空间外
#pragma once +#if defined(_MSC_VER) +# define ALWAYS_INLINE __forceinline +#elif defined(__GNUC__) || defined(__clang__) +# define ALWAYS_INLINE __attribute__((always_inline)) inline +#else +# define ALWAYS_INLINE inline +#endif + namespace librmcs::host::utility { #ifdef _MSC_VER @@ -23,13 +30,6 @@ return false; #endif } -#if defined(_MSC_VER) -# define ALWAYS_INLINE __forceinline -#elif defined(__GNUC__) || defined(__clang__) -# define ALWAYS_INLINE __attribute__((always_inline)) inline -#else -# define ALWAYS_INLINE inline -#endif } // namespace librmcs::host::utility方案 2：使用更具体的宏名称
#if defined(_MSC_VER) -# define ALWAYS_INLINE __forceinline +# define LIBRMCS_ALWAYS_INLINE __forceinline #elif defined(__GNUC__) || defined(__clang__) -# define ALWAYS_INLINE __attribute__((always_inline)) inline +# define LIBRMCS_ALWAYS_INLINE __attribute__((always_inline)) inline #else -# define ALWAYS_INLINE inline +# define LIBRMCS_ALWAYS_INLINE inline #endifhost/src/utility/final_action.hpp (3)
7-14: 建议给FinalAction加[[nodiscard]]，避免误写成临时对象导致“立刻执行”
scope guard 如果被无意写成临时变量（如FinalAction{...};），会在语句末立刻析构并执行 action，属于常见踩坑点；[[nodiscard]]能显著降低误用概率。建议修改
template <typename Functor> requires requires(Functor& action) { { action() } noexcept; -} struct FinalAction { +} struct [[nodiscard]] FinalAction { constexpr explicit FinalAction(Functor action) : enabled_(true) , action_{std::move(action)} {}
26-31:disable()建议标注constexpr noexcept并去掉多余分号；可选：成员顺序/默认初始化优化
void disable() { ... };末尾分号虽不一定会炸，但属于噪音；同时该函数不抛异常，建议显式noexcept。另外可用成员默认值减少初始化样板，并可考虑把action_放前面减少 padding（取决于 Functor）。建议修改
- void disable() { enabled_ = false; }; + constexpr void disable() noexcept { enabled_ = false; } private: - bool enabled_; Functor action_; + bool enabled_{true}; };
15-19: 确认“不可移动”是有意设计；否则建议支持 move 以提升可用性
现在FinalAction既不可拷贝也不可移动，会限制一些常见用法（如条件构造后再赋值、从 helper 返回等）。如果不是刻意约束，建议实现 move：转移enabled_并把源对象 disable，保证 action 只执行一次。host/src/utility/assert.cpp (1)
1-4:<iostream>头文件可能不需要。
std::println来自<print>头文件，而std::cerr在 C++23 中通过<print>已经可用（作为输出目标）。如果编译器支持，可以考虑移除<iostream>。♻️ 建议的修改
#include <exception> -#include <iostream> #include <print> #include <source_location>firmware/src/utility/assert.cpp (1)
1-3: Include 顺序与项目约定不符。根据项目约定，系统/库头文件（
#include <...>）应该放在项目头文件（#include "..."）之前，以减少跨库宏干扰。Based on learnings.♻️ 建议的修改
+#include <source_location> + #include "core/src/utility/assert.hpp" - -#include <source_location>core/src/utility/assert.hpp (1)
15-18:constexpr标记可能产生误导。
assert_failed_always被标记为constexpr，但它调用了非constexpr的assert_func。虽然这在语法上是合法的（constexpr函数可以在运行时调用非constexpr函数），但这意味着该函数实际上永远不能在编译时求值。如果这是有意为之（例如为了与其他断言函数保持 API 一致性），可以忽略此建议。host/src/logging/logging.hpp (2)
3-3:<cstdio>头文件似乎未使用。代码中没有使用 C 风格的 I/O 函数（如
printf、fprintf等）。所有输出都通过std::print、std::println和std::cerr完成。♻️ 建议的修改
#pragma once -#include <cstdio> - #include <format>
122-138: 输出方法略有不一致。格式化版本使用
std::println，而原始消息版本使用std::cerr << msg << '\n'。考虑统一使用std::println以保持一致性：♻️ 建议的修改
template <typename T> void log_internal(Level level, const T& msg) { if (!should_log(level)) return; print_prefix(level); - std::cerr << msg << '\n'; + std::println(std::cerr, "{}", msg); }host/CMakePresets.json (1)
4-11: 考虑添加更多构建配置预设。当前仅提供了一个名为 "linux" 的 Debug 构建预设。建议添加以下预设以提高灵活性：
- Release 构建预设
- RelWithDebInfo 构建预设
- 考虑为不同平台（如 Windows、macOS）添加对应的预设
这样可以让开发者更方便地在不同构建配置之间切换。
📋 建议的预设扩展
{ "version": 2, "configurePresets": [ { "name": "linux", "generator": "Ninja", "binaryDir": "${sourceDir}/build", "cacheVariables": { "CMAKE_BUILD_TYPE": "Debug" } + }, + { + "name": "linux-release", + "generator": "Ninja", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release" + } } ] }host/CMakeLists.txt (3)
25-25: 调试符号-g在所有构建类型中都会生成。在第 25 行中，
-g选项被无条件添加到非 MSVC 编译器。这会导致即使在 Release 构建中也生成调试符号，可能增加二进制文件大小。考虑仅在 Debug 和 RelWithDebInfo 配置中添加此选项。♻️ 建议的修改
else() # GCC/Clang - add_compile_options(-g -Wall -Wextra -Wpedantic -fvisibility=hidden) + add_compile_options(-Wall -Wextra -Wpedantic -fvisibility=hidden) + add_compile_options($<$<OR:$<CONFIG:Debug>,$<CONFIG:RelWithDebInfo>>:-g>) endif()
32-37: 避免使用 GLOB_RECURSE 收集源文件。CMake 官方文档不推荐使用
GLOB或GLOB_RECURSE来收集源文件，因为当添加或删除文件时，CMake 不会自动重新配置。这可能导致构建系统状态不一致。建议显式列出源文件或使用
target_sources()命令。CMake GLOB_RECURSE best practices alternatives
59-59: 私有包含父目录可能暴露过多内容。将父目录
${CMAKE_CURRENT_SOURCE_DIR}/..作为私有包含目录可能会暴露不必要的头文件。建议仅包含确实需要的特定子目录。core/src/protocol/constant.hpp (1)
7-7: 考虑为魔术数字添加文档注释常量值 1023（1024 - 1）看起来是有意设计的。建议添加 Doxygen 注释说明为何选择这个特定值，以及它在协议缓冲区设计中的作用（例如，是否为帧头、校验码等预留了空间）。
📝 建议的文档注释
+/// @brief 协议缓冲区大小（字节） +/// +/// 设置为 1023 以便为协议帧头、校验等预留空间 static constexpr std::size_t kProtocolBufferSize = 1023;firmware/CMakePresets.json (1)
5-11: 考虑为预设添加描述字段建议为每个预设添加
"description"字段，说明其用途，提升可维护性。📝 建议的改进
{ "name": "debug", + "description": "调试构建（固件调试）", "generator": "Ninja", "binaryDir": "${sourceDir}/build", "cacheVariables": { "HOST_DEBUGGER": "OFF", "CMAKE_BUILD_TYPE": "debug" } }firmware/.clangd (1)
2-11: 硬编码的绝对路径降低了可移植性所有工具链路径都硬编码为
/opt/riscv32-none-elf的绝对路径。这会导致：
- 不同开发者的工具链安装位置不同时无法使用
- CI/CD 环境需要严格匹配此路径结构
建议考虑以下方案之一：
- 使用环境变量（如
${env:RISCV_TOOLCHAIN_PATH}）- 在项目文档中说明工具链的标准安装路径要求
- 提供脚本自动检测和配置工具链路径
core/src/utility/verify.hpp (1)
1-2: 考虑添加文件级文档注释建议在文件开头添加 Doxygen 注释，说明这些验证宏的用途、使用场景和与断言（assert）的区别。这符合项目在 .clangd 中启用的 Doxygen 注释格式规范。
📝 建议的文档注释
+/// @file verify.hpp +/// @brief 运行时验证宏 +/// +/// 提供轻量级的条件验证机制，用于非致命错误处理。 +/// 与 assert 不同，这些宏在 Release 构建中依然有效，通过返回值传递错误。 #pragma oncefirmware/CMakeLists.txt (1)
55-65: 路径拼接可能产生双斜杠。如果
SOURCE_PATH_MAP已经以斜杠结尾，第 59 行会追加额外的斜杠，可能导致路径映射异常。虽然cmake_path(NORMAL_PATH ...)可能会规范化路径，但更安全的做法是在追加前检查。♻️ 建议修复
- set(SOURCE_PATH_MAP "${SOURCE_PATH_MAP}/") - cmake_path(NORMAL_PATH SOURCE_PATH_MAP) + cmake_path(NORMAL_PATH SOURCE_PATH_MAP) + string(REGEX REPLACE "/$" "" SOURCE_PATH_MAP "${SOURCE_PATH_MAP}") + set(SOURCE_PATH_MAP "${SOURCE_PATH_MAP}/")core/src/utility/stack_allocator.hpp (1)
40-43:deallocate中n参数被忽略。在 release 版本中，
deallocate直接将top_重置为p，完全忽略了n参数。这意味着调用者必须确保按 LIFO 顺序释放。虽然 debug 版本会验证这一点，但在 release 版本中没有任何保护。这是栈分配器的预期行为，但建议添加注释说明这一约定。📝 建议添加注释
constexpr void deallocate(void* p, size_t n) noexcept { + // n is unused: stack allocator resets top_ to p directly. + // Caller must ensure LIFO deallocation order. (void)n; top_ = static_cast<std::byte*>(p); }core/src/coroutine/lifo.hpp (1)
119-121:[[noreturn]]与返回类型的组合说明。
get_return_object_on_allocation_failure()声明为返回LifoTask但标记为[[noreturn]]。这是因为内部调用了assert_failed_debug()永远不会返回。虽然这在技术上是正确的（满足协程规范要求的签名），但可能会让阅读代码的人感到困惑。建议添加注释说明此设计意图。📝 建议添加注释
+ // Required by coroutine nothrow-new customization point. + // Never returns: asserts on allocation failure. [[noreturn]] static LifoTask get_return_object_on_allocation_failure() noexcept { utility::assert_failed_debug(); }firmware/src/can/can.cpp (1)
13-67: ISR 实现逻辑正确四个 CAN ISR 的实现遵循了一致的模式：读取中断标志、处理 RXFIFO0 新消息、清除标志。使用
[[likely]]和[[unlikely]]属性优化了分支预测，中断处理路径清晰。考虑到这是裸机中断处理，代码重复在这种场景下是可以接受的（ISR 需要快速执行且彼此独立）。
♻️ 可选的重构建议：减少代码重复
如果未来需要维护更多 CAN 通道或修改 ISR 逻辑，可以考虑使用模板辅助函数来减少重复：
template<int N> static void can_isr_impl(MCAN_Type* base, auto* can_obj, data::DataId data_id) { uint32_t flags = mcan_get_interrupt_flags(base); if (!flags) [[unlikely]] return; if (flags & MCAN_INT_RXFIFO0_NEW_MSG) [[likely]] can_obj->handle_uplink(data_id, usb::vendor->serializer()); mcan_clear_interrupt_flags(base, flags); } SDK_DECLARE_EXT_ISR_M(IRQn_MCAN0, can0_isr) void can0_isr() { can_isr_impl<0>(HPM_MCAN0, can0, data::DataId::CAN0); }不过当前实现在裸机 ISR 场景下也完全合理。
firmware/src/usb/usb_descriptors.hpp (2)
66-85: 设备描述符定义完整设备描述符正确配置了 USB 设备类、供应商/产品 ID 和字符串索引。供应商 ID
0xa11c与代码库中其他位置保持一致。Line 77 的 TODO 注释提醒需要动态生成产品 ID。如果不同硬件变体需要不同的产品 ID，这是一个合理的改进方向。
需要我帮助实现动态产品 ID 生成的逻辑吗？可以基于硬件版本或配置信息生成唯一的产品 ID。
119-124: 字符串描述符内容合理，序列号待实现字符串描述符包含了：
- 语言 ID（US English）
- 制造商和产品信息
- 产品版本 "v3.0.0alpha" 与 PR 标题一致
Line 123 的注释正确指出序列号应使用芯片 ID。使用唯一的芯片 ID 作为序列号可以帮助识别和调试特定设备。
需要我帮助实现基于芯片 ID 的序列号生成吗？通常可以读取 MCU 的唯一 ID 寄存器并格式化为十六进制字符串。
firmware/src/utility/lazy.hpp (1)
18-23:~Lazy()空析构 +union：建议显式约束T/Args...可平凡析构，避免“未 init 也不析构参数”造成隐性资源泄漏这符合固件“常驻直到复位”的设计取向，但最好用编译期约束/注释把意图钉死，防止后续有人把
std::string/RAII 句柄塞进来。基于 learnings（固件 lazy 全局对象避免依赖非平凡析构）。建议修复
template <typename T, typename... Args> class Lazy { public: + static_assert(std::is_trivially_destructible_v<T>, "Lazy<T> in firmware should be trivially destructible"); + static_assert((std::is_trivially_destructible_v<Args> && ...), "Lazy args should be trivially destructible"); + consteval explicit Lazy(Args... args) : init_status_(InitStatus::UNINITIALIZED) , construction_arguments{std::move(args)...} {} constexpr ~Lazy() {}; // No need to deconstructcore/src/protocol/protocol.hpp (2)
16-49:FieldHeaderExtendedLayout::IdExtended（bit 4, len 8）这类“跨 nibble/跨字节”打包很容易踩坑：建议补一段协议注释或加静态断言/测试用例锁定格式目前看起来是为了配合 EXTEND 后“复用下一字节的高 nibble 作为 flags”的布局；强烈建议在这里写清楚位分配（哪个 nibble 是 id 的高/低位，哪些位保留）。
24-84: FieldHeader 与各具体 Header（CAN/UART/IMU）共享“第一个字节”的隐含约定：建议提炼/注释，降低维护风险例如
FieldHeader::Id用低 4bit，而CanHeaderLayout/UartHeaderLayout/ImuHeader::PayloadType都从 bit4 开始读 flags；这依赖“首字节低 nibble 是 FieldId，高 nibble 是 flags（或 EXTEND 的一部分）”的协议约定。建议显式说明或抽一个公共 layout 复用。core/src/protocol/deserializer.cpp (3)
19-39: EXTEND 路径的consume_peeked_partial(sizeof(FieldHeader))很“魔法”：建议补注释解释为何只消费 1 字节以及它如何与 protocol.hpp 的位布局对应从实现推断：EXTEND 会把“下一字节”当作具体字段 header 的起始字节（让 CAN/UART/IMU 的 flags 仍然位于 bit4..7），所以只消费掉 EXTEND 的第一个字节而保留第二字节供后续解析。建议把这个协议约定写在这里（或写在
protocol.hpp并在这里引用）。
64-109:can_data_length先当 bool（HasCanData）再当长度：建议改名或拆成has_can_data+can_data_length，避免误读现在
can_data_length = header.get<HasCanData>()容易让人以为已经是 DLC/字节数。
115-137: UART 扩展长度校验点合理，但建议把“上限为何等于 pending buffer 大小”写清楚
uart_data_length > sizeof(pending_bytes_buffer_)这条 guard 很关键；建议补一句注释，说明peek_bytes()需要缓存拼包且缓存上限由该 buffer 决定。firmware/src/uart/uart.hpp (1)
81-88: 上行写入只断言!= kInvalidArgument：建议至少处理/统计“buffer 满”等其它失败结果否则满载时可能静默丢包（release 下更隐蔽）。
firmware/src/usb/vendor.hpp (2)
42-60: 回调里default: assert_failed_always()很硬：建议至少在 release 下走“忽略/计数/进入 discard + error_callback”而不是直接炸尤其是协议扩展或噪声输入场景，固件侧可恢复性会更好。
81-86: 建议加一个 debug 断言防止written_size - transmitted_size_意外下溢（防御性）例如
assert_debug(transmitted_size_ <= written_size)，避免未来改动引入难排查问题。Also applies to: 95-100
firmware/src/usb/interrupt_safe_buffer.hpp (3)
25-47:allocate()里把out_的 load 放在循环外依赖“仅 ISR 调用 allocate，且 ISR 期间主线程暂停”的假设：建议把该假设写成注释固定下来否则未来引入 RTOS/多核或把 allocate 挪到线程里会直接变成数据竞争/逻辑错误。基于 learnings。
69-84: 内存序/栅栏：当前atomic_signal_fence明显是为“中断边界”服务；建议补注释说明为何不用 acquire/release（以及一旦并发模型变化需要怎么改）这段非常依赖运行模型，建议把约束写死，避免后续维护者误改。
Also applies to: 80-82
39-43: buffer 满直接返回空 span：TODO 很好，建议后续至少加计数器/丢包指示点便于现场定位现阶段可接受。
host/src/protocol/stream_buffer.hpp (1)
69-93: StreamBuffer 的移动语义不对称移动构造函数已定义（Line 72-79），但移动赋值运算符被删除（Line 80）。这种不对称可能导致使用上的困惑。
如果有意为之（例如因为
transport_是引用无法重新绑定），建议添加注释说明原因。host/src/transport/usb.cpp (2)
196-198: 使用 std::vector 替代原始 new 分配Line 196 使用
new libusb_device_descriptor[device_count]进行分配。虽然有FinalAction确保释放，但使用std::vector会更加安全和惯用。建议使用 std::vector
- auto device_descriptors = new libusb_device_descriptor[device_count]; - utility::FinalAction free_device_descriptors{ - [&device_descriptors]() noexcept { delete[] device_descriptors; }}; + std::vector<libusb_device_descriptor> device_descriptors(device_count);
468-474: 硬编码的端点地址和接口号端点地址（
0x01、0x81）和接口号（0x00）是硬编码的。这假设固件端使用相同的配置。如果未来需要支持不同的设备配置，考虑将这些作为配置参数。firmware/src/uart/rx_buffer.hpp (1)
102-141: try_dequeue 实现逻辑清晰。正确处理了：
- 环形缓冲区的连续读取和跨边界读取两种情况
- 缓存失效操作在读取 DMA 写入的数据之前执行
- 溢出情况下的调试断言和发布版本回退策略
注意 Line 118 的 TODO 提到添加丢包计数器/遥测钩子，这对生产环境的调试会很有帮助。
如需要，我可以帮助实现丢包计数器的追踪逻辑。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (84)
.clang-format.clangd.devcontainer/devcontainer.json.gitmodulesCMakeLists.txtcmake/librmcsConfig.cmake.incore/.gitignorecore/include/librmcs/data/datas.hppcore/setup-clangdcore/src/coroutine/lifo.hppcore/src/protocol/constant.hppcore/src/protocol/deserializer.cppcore/src/protocol/deserializer.hppcore/src/protocol/protocol.hppcore/src/protocol/serializer.hppcore/src/utility/assert.hppcore/src/utility/bitfield.hppcore/src/utility/immovable.hppcore/src/utility/stack_allocator.hppcore/src/utility/uncopyable.hppcore/src/utility/verify.hppexample/0.blank/main.cppexample/1.receive_imu_data/main.cppexample/2.receive_motor_data/main.cppexample/3.closed_loop_motor_velocity/main.cppexample/4.use_encapsulated_motor_class/main.cppexample/5.custom_reduction_ratio/main.cppexample/6.use_encapsulated_pid_class/main.cppexample/7.multithreading/main.cppexample/8.multithreading_advanced/main.cppexample/9.remote_control/main.cppexample/CMakeLists.txtfirmware/.clangdfirmware/CMakeLists.txtfirmware/CMakePresets.jsonfirmware/app.yamlfirmware/bsp/hpm_sdkfirmware/include/tusb_config.hfirmware/src/app.cppfirmware/src/app.hppfirmware/src/can/can.cppfirmware/src/can/can.hppfirmware/src/gpio/gpio.cppfirmware/src/gpio/gpio.hppfirmware/src/spi/bmi088/accel.hppfirmware/src/spi/bmi088/gyro.hppfirmware/src/spi/spi.cppfirmware/src/spi/spi.hppfirmware/src/uart/rx_buffer.hppfirmware/src/uart/tx_buffer.hppfirmware/src/uart/uart.cppfirmware/src/uart/uart.hppfirmware/src/usb/helper.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/usb/usb_descriptors.cppfirmware/src/usb/usb_descriptors.hppfirmware/src/usb/vendor.cppfirmware/src/usb/vendor.hppfirmware/src/utility/assert.cppfirmware/src/utility/interrupt_lock_guard.hppfirmware/src/utility/lazy.hppfirmware/src/utility/ring_buffer.hpphost/CMakeLists.txthost/CMakePresets.jsonhost/include/librmcs/agent/alliance_board.hpphost/include/librmcs/protocol/handler.hpphost/src/logging/logging.hpphost/src/protocol/handler.cpphost/src/protocol/stream_buffer.hpphost/src/transport/transport.hpphost/src/transport/usb.cpphost/src/utility/assert.cpphost/src/utility/cross_os.hpphost/src/utility/final_action.hpphost/src/utility/ring_buffer.hpplibrmcs/client/cboard.hpplibrmcs/device/bmi088.hpplibrmcs/device/dji_motor.hpplibrmcs/device/dr16.hpplibrmcs/device/lk_motor.hpplibrmcs/utility/endian_promise.hpplibrmcs/utility/logging.hpplibrmcs/utility/pid_calculator.hpplibrmcs/utility/ring_buffer.hpp
💤 Files with no reviewable changes (22)
- cmake/librmcsConfig.cmake.in
- example/8.multithreading_advanced/main.cpp
- example/9.remote_control/main.cpp
- CMakeLists.txt
- example/1.receive_imu_data/main.cpp
- example/CMakeLists.txt
- example/5.custom_reduction_ratio/main.cpp
- librmcs/device/dji_motor.hpp
- librmcs/utility/ring_buffer.hpp
- example/2.receive_motor_data/main.cpp
- example/4.use_encapsulated_motor_class/main.cpp
- example/0.blank/main.cpp
- librmcs/device/dr16.hpp
- librmcs/utility/endian_promise.hpp
- librmcs/device/bmi088.hpp
- librmcs/utility/logging.hpp
- librmcs/client/cboard.hpp
- example/6.use_encapsulated_pid_class/main.cpp
- example/7.multithreading/main.cpp
- example/3.closed_loop_motor_velocity/main.cpp
- librmcs/device/lk_motor.hpp
- librmcs/utility/pid_calculator.hpp
🧰 Additional context used
🧠 Learnings (15)
📚 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:
core/src/utility/verify.hppcore/src/protocol/constant.hpphost/include/librmcs/agent/alliance_board.hppfirmware/src/gpio/gpio.hppcore/src/utility/uncopyable.hppfirmware/src/app.hppfirmware/src/usb/usb_descriptors.hppcore/src/utility/immovable.hppcore/src/protocol/protocol.hppfirmware/src/usb/vendor.hpphost/src/utility/final_action.hpphost/src/utility/ring_buffer.hppfirmware/src/uart/tx_buffer.hpphost/src/logging/logging.hpphost/include/librmcs/protocol/handler.hppfirmware/src/utility/lazy.hpphost/src/protocol/stream_buffer.hpphost/src/transport/transport.hpphost/src/utility/cross_os.hppcore/src/utility/assert.hppcore/src/protocol/deserializer.hppfirmware/src/usb/helper.hppfirmware/src/spi/bmi088/gyro.hppfirmware/src/utility/ring_buffer.hppcore/src/utility/bitfield.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/spi/spi.hppcore/src/protocol/serializer.hppfirmware/src/spi/bmi088/accel.hppcore/include/librmcs/data/datas.hppcore/src/utility/stack_allocator.hppcore/src/coroutine/lifo.hppfirmware/src/utility/interrupt_lock_guard.hppfirmware/src/uart/rx_buffer.hppfirmware/src/can/can.hppfirmware/src/uart/uart.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/verify.hppcore/src/utility/assert.hpphost/src/utility/assert.cppfirmware/src/utility/assert.cpp
📚 Learning: 2025-12-20T05:26:36.937Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/include/librmcs/agent/alliance_board.hpp:98-103
Timestamp: 2025-12-20T05:26:36.937Z
Learning: In the librmcs codebase, when overriding virtual functions from a base class in C++, declare overrides with the override specifier and do not repeat virtual on the overriding function. The virtual keyword should only be used on the topmost declaration of a virtual function. This aligns with clang-tidy's modernize-use-override check. Apply this pattern to header files under host/include/librmcs (e.g., host/include/librmcs/**/*.hpp) and ensure all overrides use 'override' without 'virtual'.
Applied to files:
host/include/librmcs/agent/alliance_board.hpphost/include/librmcs/protocol/handler.hpp
📚 Learning: 2025-12-20T05:28:56.619Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: core/src/utility/assert.hpp:13-15
Timestamp: 2025-12-20T05:28:56.619Z
Learning: In the librmcs project, include order convention guarantees that `#include "..."` (project files) always come after `#include <...>` (system/library headers), which ensures that modifications to standard macros (like `#undef assert` in core/src/utility/assert.hpp) won't affect other libraries.
Applied to files:
host/CMakeLists.txthost/src/utility/assert.cppfirmware/src/utility/assert.cpp
📚 Learning: 2026-01-03T15:20:51.624Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 9
File: firmware/src/spi/bmi088/accel.hpp:41-41
Timestamp: 2026-01-03T15:20:51.624Z
Learning: In firmware/src/spi/bmi088/accel.hpp and firmware/src/spi/bmi088/gyro.hpp, the use of assert_debug(spi_.try_lock()) during BMI088 driver initialization is intentional and correct. During the initialization phase, interrupts are disabled and all SPI calls are synchronous, so try_lock() should never fail in correct code. If it returns false, it indicates a logic error that should be caught during development.
Applied to files:
firmware/src/spi/spi.cppfirmware/src/gpio/gpio.hppfirmware/src/gpio/gpio.cppfirmware/src/spi/spi.hppfirmware/src/utility/interrupt_lock_guard.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/gpio/gpio.hppfirmware/src/app.hppfirmware/src/usb/usb_descriptors.hppfirmware/src/usb/vendor.hppfirmware/src/uart/tx_buffer.hppfirmware/src/utility/lazy.hppfirmware/src/usb/helper.hppfirmware/src/spi/bmi088/gyro.hppfirmware/src/utility/ring_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/spi/spi.hppfirmware/src/spi/bmi088/accel.hppfirmware/src/utility/interrupt_lock_guard.hppfirmware/src/uart/rx_buffer.hppfirmware/src/can/can.hppfirmware/src/uart/uart.hpp
📚 Learning: 2026-01-11T11:53:29.752Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 10
File: firmware/src/usb/vendor.hpp:87-93
Timestamp: 2026-01-11T11:53:29.752Z
Learning: 在 librmcs 固件项目中，TinyUSB 被配置为无缓冲模式（CFG_TUD_VENDOR_RX_BUFSIZE = 0, CFG_TUD_VENDOR_TX_BUFSIZE = 0），数据直接发送至 USB 端点。在 `firmware/src/usb/vendor.hpp` 的 `try_transmit()` 中，由于 `target_size` 总是 ≤ `max_packet_size` 且 `device_ready()` 预先检查端点可用性，`tud_vendor_n_write()` 在理想状况下应返回完整的 `target_size`，因此使用 `assert_debug(sent == target_size)` 作为调试断言是合理的。
Applied to files:
firmware/src/usb/usb_descriptors.cppfirmware/include/tusb_config.hfirmware/src/usb/vendor.hppfirmware/src/uart/tx_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/usb/vendor.cpphost/src/transport/usb.cppfirmware/src/uart/rx_buffer.hpp
📚 Learning: 2025-12-26T09:45:56.870Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 6
File: firmware/src/usb/interrupt_safe_buffer.hpp:25-47
Timestamp: 2025-12-26T09:45:56.870Z
Learning: 在 librmcs 固件的 `firmware/src/usb/interrupt_safe_buffer.hpp` 中，`InterruptSafeBuffer::allocate()` 在循环外加载 `out_` 值是有意为之的设计。该缓冲区采用裸机中断安全模型：`allocate()` 仅在 ISR 中调用（通过 CAN ISR 中的 serializer），`pop_batch()` 仅在主线程中调用（通过 `App::run()` 中的 `try_transmit()`）。由于项目不使用 RTOS，ISR 执行时主线程被暂停，因此 `out_` 在 `allocate()` 执行期间保持稳定。
Applied to files:
host/src/utility/ring_buffer.hppfirmware/src/uart/tx_buffer.hppfirmware/src/utility/ring_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/can/can.cppfirmware/src/utility/interrupt_lock_guard.hppfirmware/src/uart/rx_buffer.hppfirmware/src/can/can.hpp
📚 Learning: 2025-12-20T05:52:44.423Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/src/logging/logging.hpp:27-157
Timestamp: 2025-12-20T05:52:44.423Z
Learning: In the librmcs project, the Logger class in host/src/logging/logging.hpp intentionally does not include thread synchronization (std::mutex). The team has decided that interleaved output is acceptable to avoid blocking overhead. A full async logging solution with message queues would be needed for proper thread-safe logging, but this complexity is not desired at the current stage.
Applied to files:
host/src/logging/logging.hpp
📚 Learning: 2025-12-20T06:19:33.594Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/src/logging/logging.hpp:15-23
Timestamp: 2025-12-20T06:19:33.594Z
Learning: In the librmcs project Logger class (host/src/logging/logging.hpp), Level::OFF is designed exclusively as a configuration value for setting logging_level to disable all logging. Passing Level::OFF to logging methods (e.g., log(Level::OFF, "msg")) is illegal by design and intentionally triggers an assertion to catch programming errors. This is not a bug but an intentional safeguard.
Applied to files:
host/src/logging/logging.hpp
📚 Learning: 2025-12-20T04:28:53.374Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 4
File: core/src/protocol/deserializer.cpp:38-58
Timestamp: 2025-12-20T04:28:53.374Z
Learning: In core/src/protocol/deserializer.cpp, for the default switch case handling an unknown FieldId, keep the current behavior: do not eagerly consume any peeked bytes before invoking deserializing_error(). This path intentionally relies on deserializing_error() resetting input_cursor_ to input_end_ and pending_bytes_ to 0, so that the next peek_bytes() suspends the coroutine until finish_transfer() clears the error state. Ensure this rationale is documented with a comment near the default branch and add a targeted test or regression note to prevent future reintroduction of a byte-consumption in this path. If future changes modify this behavior, verify that the coroutine suspension and error-reset semantics remain consistent and that the existing assertion at the relevant line remains valid.
Applied to files:
core/src/protocol/deserializer.cpp
📚 Learning: 2025-12-20T04:28:56.876Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 4
File: core/src/protocol/deserializer.cpp:38-58
Timestamp: 2025-12-20T04:28:56.876Z
Learning: In core/src/protocol/deserializer.cpp, when an unknown FieldId is encountered in the default switch branch, the code does not explicitly consume peeked bytes before calling deserializing_error(). This is intentional: deserializing_error() resets input_cursor_ to input_end_ and pending_bytes_ to 0, causing the next peek_bytes() to suspend the coroutine until finish_transfer() resets the error state. The assertion at line 22 will not fail because the coroutine suspends before re-entering that path.
Applied to files:
core/src/protocol/deserializer.hpp
📚 Learning: 2026-01-03T15:20:44.048Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 9
File: firmware/src/spi/bmi088/accel.hpp:41-41
Timestamp: 2026-01-03T15:20:44.048Z
Learning: For BMI088 SPI driver initialization (accel.hpp and gyro.hpp), use assert_debug(spi_.try_lock()) since interrupts are disabled and SPI calls are synchronous during init; a false result indicates a logic error. This pattern should be applicable to all files in firmware/src/spi/bmi088 with the same shall-be-initialization behavior. Reviewers should verify that try_lock() cannot fail in proper init and that such failures are strictly treated as development-time issues.
Applied to files:
firmware/src/spi/bmi088/gyro.hppfirmware/src/spi/bmi088/accel.hpp
📚 Learning: 2025-12-20T06:17:08.271Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/src/transport/usb.cpp:42-62
Timestamp: 2025-12-20T06:17:08.271Z
Learning: 在 librmcs 项目中,下位机使用 USB vendor 类,因此不需要也不应该调用 libusb_attach_kernel_driver 或 libusb_detach_kernel_driver。
Applied to files:
firmware/src/usb/vendor.cpphost/src/transport/usb.cpp
📚 Learning: 2025-12-20T06:17:50.471Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: core/src/coroutine/lifo.hpp:0-0
Timestamp: 2025-12-20T06:17:50.471Z
Learning: In coroutine result accessors (e.g., LifoTask::result()), prefer returning T& instead of T to express ownership transfer. Update the API so callers use std::move(task.result()) and adjust callers accordingly. Apply this pattern to similar accessors in core/src/coroutine; ensure const-correctness, avoid dangling references, and update any internal copies or move semantics to reflect the new ownership model.
Applied to files:
core/src/coroutine/lifo.hpp
🧬 Code graph analysis (28)
host/include/librmcs/agent/alliance_board.hpp (3)
core/src/protocol/deserializer.hpp (4)
data(24-24)data(26-26)id(20-20)id(22-22)host/src/protocol/handler.cpp (8)
data(47-49)data(47-47)data(51-53)data(51-51)id(35-39)id(35-36)id(41-45)id(41-42)core/include/librmcs/data/datas.hpp (4)
data(66-66)data(68-68)id(62-62)id(64-64)
firmware/src/app.hpp (2)
firmware/src/app.cpp (1)
noreturn(44-50)firmware/src/utility/assert.cpp (1)
noreturn(11-17)
firmware/src/usb/usb_descriptors.cpp (1)
firmware/src/usb/usb_descriptors.hpp (4)
index(25-33)index(25-25)index(35-63)index(35-35)
firmware/src/usb/vendor.hpp (3)
firmware/src/can/can.hpp (2)
data(49-67)data(49-49)firmware/src/uart/uart.hpp (2)
data(41-41)data(41-41)firmware/src/utility/lazy.hpp (3)
assert_debug(44-47)assert_debug(49-52)assert_debug(54-57)
host/src/utility/ring_buffer.hpp (1)
firmware/src/utility/ring_buffer.hpp (18)
RingBuffer(34-34)RingBuffer(34-34)RingBuffer(36-36)RingBuffer(38-38)RingBuffer(45-45)in(53-58)in(66-71)in(94-102)out(79-86)emplace_back_n(119-146)emplace_back_n(119-121)value(187-190)value(187-187)value(195-201)value(195-195)pop_front_n(213-240)pop_front_n(213-213)pop_front_n(257-259)
firmware/src/uart/tx_buffer.hpp (2)
firmware/src/utility/ring_buffer.hpp (4)
in(53-58)in(66-71)in(94-102)out(79-86)host/src/utility/ring_buffer.hpp (4)
in(61-65)in(73-77)in(100-107)out(85-92)
host/src/logging/logging.hpp (1)
core/src/utility/assert.hpp (1)
assert_failed_debug(21-21)
core/src/protocol/deserializer.cpp (4)
core/src/protocol/deserializer.hpp (5)
assert_debug(110-150)assert_debug(156-173)assert_debug(191-199)id(20-20)id(22-22)core/src/utility/assert.hpp (2)
assert_debug(36-44)assert_debug(36-37)core/src/coroutine/lifo.hpp (5)
assert_debug(184-187)assert_debug(189-192)assert_debug(258-258)assert_debug(264-264)static_cast(66-66)core/include/librmcs/data/datas.hpp (2)
id(62-62)id(64-64)
host/src/transport/transport.hpp (2)
core/src/protocol/deserializer.hpp (2)
buffer(43-70)buffer(43-43)host/src/transport/usb.cpp (6)
buffer(77-94)buffer(77-77)buffer(96-102)buffer(96-96)callback(104-112)callback(104-104)
core/src/utility/assert.hpp (2)
core/src/coroutine/lifo.hpp (4)
noreturn(119-121)noreturn(150-150)noreturn(207-209)noreturn(236-236)host/src/utility/assert.cpp (2)
noreturn(8-13)assert_func(8-8)
host/src/utility/assert.cpp (2)
core/src/utility/assert.hpp (2)
noreturn(15-18)noreturn(20-28)core/src/coroutine/lifo.hpp (4)
noreturn(119-121)noreturn(150-150)noreturn(207-209)noreturn(236-236)
core/src/protocol/deserializer.hpp (5)
core/include/librmcs/data/datas.hpp (4)
id(62-62)id(64-64)data(66-66)data(68-68)core/src/protocol/deserializer.cpp (2)
process_stream(15-61)process_stream(15-15)core/src/utility/assert.hpp (2)
assert_debug(36-44)assert_debug(36-37)core/src/coroutine/lifo.hpp (17)
assert_debug(184-187)assert_debug(189-192)assert_debug(258-258)assert_debug(264-264)static_cast(66-66)h(44-46)h(44-44)h(132-140)h(133-133)h(220-228)h(221-221)n(21-21)n(21-21)n(76-87)n(76-76)n(91-93)n(91-91)core/src/protocol/serializer.hpp (15)
field_id(31-74)field_id(31-31)field_id(76-117)field_id(76-78)field_id(170-173)field_id(170-170)field_id(175-178)field_id(175-175)field_id(192-209)field_id(192-192)field_id(211-227)field_id(211-213)field_id(231-242)field_id(231-231)size(21-21)
firmware/src/spi/bmi088/gyro.hpp (2)
firmware/src/spi/bmi088/accel.hpp (13)
address(142-148)address(142-142)address(150-156)address(150-150)address(165-169)address(165-165)address(171-175)address(171-171)read(106-106)size(158-163)size(158-158)serializer(177-182)serializer(177-177)firmware/src/spi/spi.hpp (1)
size(32-32)
firmware/src/utility/ring_buffer.hpp (3)
host/src/utility/ring_buffer.hpp (18)
RingBuffer(26-33)RingBuffer(26-26)RingBuffer(35-35)RingBuffer(37-37)RingBuffer(44-47)in(61-65)in(73-77)in(100-107)out(85-92)emplace_back_n(121-143)emplace_back_n(121-121)value(180-183)value(180-180)value(188-194)value(188-188)pop_front_n(206-232)pop_front_n(206-206)pop_front_n(249-251)core/src/coroutine/lifo.hpp (1)
static_cast(66-66)host/src/transport/usb.cpp (1)
i(370-392)
firmware/src/usb/interrupt_safe_buffer.hpp (2)
host/src/utility/ring_buffer.hpp (1)
mask(53-53)firmware/src/usb/vendor.hpp (4)
data(62-64)data(62-62)data(66-68)data(66-66)
firmware/src/spi/spi.hpp (3)
firmware/src/spi/bmi088/accel.hpp (2)
size(158-163)size(158-158)firmware/src/spi/bmi088/gyro.hpp (2)
size(152-157)size(152-152)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)
host/src/protocol/handler.cpp (5)
host/src/transport/transport.hpp (3)
callback(128-128)buffer(91-91)buffer(105-105)core/src/protocol/deserializer.hpp (9)
buffer(43-70)buffer(43-43)id(20-20)id(22-22)data(24-24)data(26-26)field_id(90-90)field_id(92-92)field_id(94-94)core/include/librmcs/data/datas.hpp (4)
id(62-62)id(64-64)data(66-66)data(68-68)host/src/logging/logging.hpp (2)
get_logger(161-161)get_logger(161-161)core/src/protocol/serializer.hpp (17)
field_id(31-74)field_id(31-31)field_id(76-117)field_id(76-78)field_id(170-173)field_id(170-170)field_id(175-178)field_id(175-175)field_id(192-209)field_id(192-192)field_id(211-227)field_id(211-213)field_id(231-242)view(119-142)view(119-119)view(144-167)view(144-144)
core/src/protocol/serializer.hpp (1)
core/src/utility/bitfield.hpp (2)
Ref(153-154)Ref(153-153)
firmware/src/app.cpp (3)
firmware/src/app.hpp (1)
App(12-12)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)
firmware/src/spi/bmi088/accel.hpp (2)
firmware/src/spi/bmi088/gyro.hpp (13)
address(136-142)address(136-136)address(144-150)address(144-144)address(159-163)address(159-159)address(165-169)address(165-165)read(108-108)size(152-157)size(152-152)serializer(171-176)serializer(171-171)firmware/src/spi/spi.hpp (1)
size(32-32)
core/include/librmcs/data/datas.hpp (3)
core/src/protocol/deserializer.hpp (4)
id(20-20)id(22-22)data(24-24)data(26-26)host/src/protocol/handler.cpp (8)
id(35-39)id(35-36)id(41-45)id(41-42)data(47-49)data(47-47)data(51-53)data(51-51)host/include/librmcs/agent/alliance_board.hpp (14)
id(66-74)id(66-66)id(81-90)id(81-81)data(19-22)data(19-19)data(23-26)data(23-23)data(27-30)data(27-27)data(31-34)data(31-31)data(36-39)data(36-36)
core/src/utility/stack_allocator.hpp (2)
core/src/coroutine/lifo.hpp (10)
n(21-21)n(21-21)n(76-87)n(76-76)n(91-93)n(91-91)p(23-25)p(23-23)p(96-108)p(96-96)core/src/utility/assert.hpp (2)
assert_always(30-34)assert_always(30-31)
firmware/src/usb/vendor.cpp (1)
firmware/src/usb/vendor.hpp (2)
buffer(36-40)buffer(36-36)
core/src/coroutine/lifo.hpp (3)
core/src/utility/stack_allocator.hpp (8)
n(27-38)n(27-27)n(57-69)n(57-57)p(40-43)p(40-40)p(71-82)p(71-71)core/src/utility/assert.hpp (5)
noreturn(15-18)noreturn(20-28)assert_failed_debug(21-21)assert_debug(36-44)assert_debug(36-37)host/src/utility/assert.cpp (1)
noreturn(8-13)
host/src/transport/usb.cpp (3)
host/src/logging/logging.hpp (2)
get_logger(161-161)get_logger(161-161)host/src/utility/ring_buffer.hpp (6)
value(180-183)value(180-180)value(188-194)value(188-188)n(258-269)n(258-258)host/src/transport/transport.hpp (3)
buffer(91-91)buffer(105-105)callback(128-128)
firmware/src/uart/rx_buffer.hpp (1)
firmware/src/uart/tx_buffer.hpp (6)
dmamux_src(146-170)dmamux_src(146-146)src(172-191)src(172-172)src(193-198)src(193-193)
firmware/src/can/can.hpp (1)
firmware/src/utility/lazy.hpp (3)
assert_debug(44-47)assert_debug(49-52)assert_debug(54-57)
firmware/src/uart/uart.hpp (3)
firmware/src/uart/rx_buffer.hpp (4)
try_dequeue(50-50)try_dequeue(53-53)is_idle(102-141)is_idle(102-102)firmware/src/usb/helper.hpp (1)
get_serializer(7-7)firmware/src/usb/vendor.cpp (2)
get_serializer(13-13)get_serializer(13-13)
🪛 Clang (14.0.6)
core/src/protocol/constant.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
host/include/librmcs/agent/alliance_board.hpp
[error] 3-3: 'librmcs/data/datas.hpp' file not found
(clang-diagnostic-error)
firmware/src/spi/spi.cpp
[error] 1-1: 'firmware/src/spi/spi.hpp' file not found
(clang-diagnostic-error)
firmware/src/app.hpp
[error] 3-3: 'core/src/utility/immovable.hpp' file not found
(clang-diagnostic-error)
firmware/src/usb/usb_descriptors.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
core/src/protocol/protocol.hpp
[error] 3-3: 'cstdint' file not found
(clang-diagnostic-error)
firmware/src/gpio/gpio.cpp
[error] 1-1: 'firmware/src/gpio/gpio.hpp' file not found
(clang-diagnostic-error)
firmware/src/usb/usb_descriptors.cpp
[error] 1-1: 'firmware/src/usb/usb_descriptors.hpp' file not found
(clang-diagnostic-error)
firmware/include/tusb_config.h
[error] 39-39: CFG_TUSB_MCU must be defined
(clang-diagnostic-error)
firmware/src/usb/vendor.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
host/src/utility/final_action.hpp
[error] 3-3: 'utility' file not found
(clang-diagnostic-error)
host/src/utility/ring_buffer.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/uart/tx_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
host/src/logging/logging.hpp
[error] 3-3: 'cstdio' file not found
(clang-diagnostic-error)
host/include/librmcs/protocol/handler.hpp
[error] 3-3: 'cstdint' file not found
(clang-diagnostic-error)
firmware/src/utility/lazy.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
host/src/protocol/stream_buffer.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
host/src/transport/transport.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
core/src/utility/assert.hpp
[error] 3-3: 'source_location' file not found
(clang-diagnostic-error)
host/src/utility/assert.cpp
[error] 1-1: 'exception' file not found
(clang-diagnostic-error)
core/src/protocol/deserializer.hpp
[error] 3-3: 'coroutine' file not found
(clang-diagnostic-error)
firmware/src/usb/helper.hpp
[error] 3-3: 'core/src/protocol/serializer.hpp' file not found
(clang-diagnostic-error)
firmware/src/spi/bmi088/gyro.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/utility/ring_buffer.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
core/src/utility/bitfield.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/usb/interrupt_safe_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/src/utility/assert.cpp
[error] 1-1: 'core/src/utility/assert.hpp' file not found
(clang-diagnostic-error)
firmware/src/spi/spi.hpp
[error] 3-3: 'atomic' file not found
(clang-diagnostic-error)
host/src/protocol/handler.cpp
[error] 1-1: 'cstddef' file not found
(clang-diagnostic-error)
core/src/protocol/serializer.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/can/can.cpp
[error] 1-1: 'firmware/src/can/can.hpp' file not found
(clang-diagnostic-error)
firmware/src/app.cpp
[error] 1-1: 'firmware/src/app.hpp' file not found
(clang-diagnostic-error)
firmware/src/spi/bmi088/accel.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/uart/uart.cpp
[error] 1-1: 'firmware/src/uart/uart.hpp' file not found
(clang-diagnostic-error)
core/include/librmcs/data/datas.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
core/src/utility/stack_allocator.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/usb/vendor.cpp
[error] 1-1: 'firmware/src/usb/vendor.hpp' file not found
(clang-diagnostic-error)
core/src/coroutine/lifo.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
host/src/transport/usb.cpp
[error] 1-1: 'cstdint' file not found
(clang-diagnostic-error)
firmware/src/utility/interrupt_lock_guard.hpp
[error] 3-3: 'cstdint' file not found
(clang-diagnostic-error)
firmware/src/uart/rx_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/src/can/can.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/uart/uart.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
🪛 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)
host/src/protocol/handler.cpp
[error] 109-109: Found an exit path from function with non-void return type that has missing return statement
(missingReturn)
firmware/src/can/can.cpp
[error] 13-13: 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/uart/uart.cpp
[error] 8-8: There is an unknown macro here somewhere. Configuration is required. If SDK_DECLARE_EXT_ISR_M is a macro then please configure it.
(unknownMacro)
| template <std::size_t Index, std::size_t BitWidth, typename ValueT = DefaultValueTypeT<BitWidth>> | ||
| requires(std::is_trivial_v<ValueT> && sizeof(ValueT) <= sizeof(std::uint64_t)) | ||
| struct BitfieldMember : BitfieldMemberTag { | ||
| static_assert(BitWidth > 0 && BitWidth <= 64); | ||
|
|
||
| static constexpr std::size_t index = Index; | ||
| static constexpr std::size_t bit_width = BitWidth; | ||
|
|
||
| using ValueType = ValueT; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n core/src/utility/bitfield.hpp | head -100Repository: Alliance-Algorithm/librmcs
Length of output: 4642
🏁 Script executed:
sed -n '100,200p' core/src/utility/bitfield.hppRepository: Alliance-Algorithm/librmcs
Length of output: 3672
🏁 Script executed:
sed -n '200,280p' core/src/utility/bitfield.hppRepository: Alliance-Algorithm/librmcs
Length of output: 1939
🏁 Script executed:
rg -t cpp "BitfieldMember.*int.*>|BitfieldMember.*int8|BitfieldMember.*int16|BitfieldMember.*int32|BitfieldMember.*int64" --max-count 20Repository: Alliance-Algorithm/librmcs
Length of output: 589
🏁 Script executed:
sed -n '1,100p' core/src/protocol/protocol.hpp | head -150Repository: Alliance-Algorithm/librmcs
Length of output: 2619
🏁 Script executed:
find . -type f -name "*.cpp" -o -name "*test*.hpp" | xargs rg -l "BitfieldMember.*int" | head -10Repository: Alliance-Algorithm/librmcs
Length of output: 52
🏁 Script executed:
rg -A5 -B5 "BitfieldMember.*int.*_t>.*get\(\)|\.get.*BitfieldMember.*int" --max-count 10Repository: Alliance-Algorithm/librmcs
Length of output: 52
🏁 Script executed:
find . -type f \( -name "*test*" -o -name "*example*" \) | grep -E "\.(cpp|hpp|h)$" | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 52
🏁 Script executed:
rg "ImuAccelerometerPayload|ImuGyroscopePayload" -A10 -B2 | head -50Repository: Alliance-Algorithm/librmcs
Length of output: 3866
🏁 Script executed:
rg "BitfieldMember.*int" core/src/protocol/protocol.hpp -B2 -A2Repository: Alliance-Algorithm/librmcs
Length of output: 520
🏁 Script executed:
rg "BitfieldMember<.*[0-9]+\s*,\s*([0-9]+)\s*,\s*(int|int8_t|int16_t|int32_t|int64_t)" --replace '$1,$2' | grep -E "int[0-9]" | sort | uniq -cRepository: Alliance-Algorithm/librmcs
Length of output: 294
🏁 Script executed:
python3 << 'EOF'
# Verify C++ sign extension behavior
# When casting unsigned to signed
# Case 1: 16-bit value with all bits set
unsigned_val = 0xFFFF
signed_val = int.from_bytes(unsigned_val.to_bytes(2, 'little', signed=False), 'little', signed=True)
print(f"Case 1 (16 bits, int16_t): 0xFFFF -> {signed_val}")
# Case 2: 12 bits extracted into 16-bit int without sign extension
unsigned_12bit = 0xFFF # 12 bits all set
# Direct cast (no sign extension)
print(f"Case 2 (12 bits into int16_t, no sign extension): 0xFFF -> {unsigned_12bit}")
# With sign extension
signed_12bit = unsigned_12bit if (unsigned_12bit & 0x800) == 0 else unsigned_12bit | ~0xFFF
print(f"Case 2 (12 bits into int16_t, WITH sign extension): 0xFFF -> {signed_12bit}")
# Case 3: High bit NOT set
unsigned_12bit_pos = 0x7FF # 12 bits, high bit = 0
print(f"Case 3 (12 bits positive, int16_t): 0x7FF -> {unsigned_12bit_pos}")
EOFRepository: Alliance-Algorithm/librmcs
Length of output: 287
🏁 Script executed:
rg "BitfieldMember" core/src --type cpp --type hpp | grep -E "(int8_t|int16_t|int32_t|int64_t|signed)" | wc -lRepository: Alliance-Algorithm/librmcs
Length of output: 116
🏁 Script executed:
find core/src -type f \( -name "*.cpp" -o -name "*.hpp" -o -name "*.h" \) -exec grep -l "BitfieldMember" {} \;Repository: Alliance-Algorithm/librmcs
Length of output: 130
🏁 Script executed:
cat core/src/protocol/protocol.hpp | grep -E "BitfieldMember.*int" -B5 -A5Repository: Alliance-Algorithm/librmcs
Length of output: 637
🏁 Script executed:
grep -r "signed\|negative\|sign.extend" core/src/utility/bitfield.hpp core/src/protocol/protocol.hppRepository: Alliance-Algorithm/librmcs
Length of output: 160
BitfieldMember 对有符号类型的处理存在潜在缺陷，需要澄清设计意图。
代码注释（第 61 行）声称"仅操作 std::byte 和无符号整数类型"，但 BitfieldMember 模板实际允许任意平凡类型，包括有符号整数。现有代码在 ImuAccelerometerPayload 和 ImuGyroscopePayload 中已使用 int16_t。
当前实现对全宽有符号类型（如 BitfieldMember<Index, 16, int16_t>）是安全的，因为 C++ 会正确处理整个值的符号转换。但当位宽小于类型大小时（如 BitfieldMember<0, 12, int16_t>），read_bits 中的 static_cast<typename Member::ValueType>(value) 不会执行符号扩展，导致负值被解释为正数。
建议：
- 更新文档准确反映当前行为（允许有符号类型但需满足特定条件），或
- 添加模板约束限制仅支持无符号类型，或
- 在
read_bits中实现符号扩展逻辑以完全支持有符号值
🤖 Prompt for AI Agents
In @core/src/utility/bitfield.hpp around lines 25 - 34, BitfieldMember currently
allows signed ValueType but read_bits does not perform sign extension, so
sub-width signed fields (e.g., BitfieldMember<0,12,int16_t>) decode incorrectly;
update the implementation used by read_bits to detect if Member::ValueType is
signed and Member::bit_width < (sizeof(Member::ValueType)*8) then perform proper
sign-extension after extracting the raw unsigned bitfield (e.g., if the sign bit
(1 << (bit_width-1)) is set, set the high bits of the ValueType to 1); reference
BitfieldMember, read_bits, and usages such as
ImuAccelerometerPayload/ImuGyroscopePayload when locating code, and also adjust
or add a clarifying comment/docstring about supporting signed sub-width fields
(or alternatively add a static_assert to forbid signed ValueType if you prefer
to restrict instead of adding sign extension).
core/src/utility/uncopyable.hpp
Outdated
| class Uncopyble { | ||
| public: | ||
| Uncopyble() = default; | ||
| Uncopyble(const Uncopyble&) = delete; | ||
| Uncopyble& operator=(const Uncopyble&) = delete; | ||
| Uncopyble(Uncopyble&&) = default; | ||
| Uncopyble& operator=(Uncopyble&&) = default; | ||
| }; |
There was a problem hiding this comment.
类名拼写错误：Uncopyble 应为 Uncopyable。
这是一个公开 API 的类名拼写错误，缺少字母 'a'。建议在合并前修复，以避免在整个代码库中产生不一致的命名。
🐛 建议修复
-class Uncopyble {
+class Uncopyable {
public:
- Uncopyble() = default;
- Uncopyble(const Uncopyble&) = delete;
- Uncopyble& operator=(const Uncopyble&) = delete;
- Uncopyble(Uncopyble&&) = default;
- Uncopyble& operator=(Uncopyble&&) = default;
+ Uncopyable() = default;
+ Uncopyable(const Uncopyable&) = delete;
+ Uncopyable& operator=(const Uncopyable&) = delete;
+ Uncopyable(Uncopyable&&) = default;
+ Uncopyable& operator=(Uncopyable&&) = default;
};🤖 Prompt for AI Agents
In @core/src/utility/uncopyable.hpp around lines 5 - 12, The class name is
misspelled: rename class Uncopyble to Uncopyable and update all internal
declarations (constructors, copy/move operations: Uncopyble(), Uncopyble(const
Uncopyble&), Uncopyble& operator=(const Uncopyble&), Uncopyble(Uncopyble&&),
Uncopyble& operator=(Uncopyble&&)) to use Uncopyable so the type and its member
signatures match; then update all usages, forward declarations and includes
across the codebase to reference Uncopyable to keep the public API consistent.
| void handle_downlink(const data::CanDataView& data) { | ||
| mcan_tx_frame_t frame{}; | ||
| if (data.is_extended_can_id) { | ||
| frame.use_ext_id = true; | ||
| frame.ext_id = data.can_id; | ||
| } else { | ||
| frame.use_ext_id = false; | ||
| frame.std_id = data.can_id; | ||
| } | ||
| frame.canfd_frame = false; | ||
| frame.rtr = data.is_remote_transmission; | ||
|
|
||
| core::utility::assert_debug(data.can_data.size() <= 8); | ||
| frame.dlc = data.can_data.size(); | ||
| if (!data.can_data.empty()) | ||
| std::memcpy(frame.data_8, data.can_data.data(), data.can_data.size()); | ||
|
|
||
| mcan_transmit_via_txfifo_nonblocking(can_base_, &frame, nullptr); | ||
| } |
There was a problem hiding this comment.
handle_downlink 未检查发送结果
mcan_transmit_via_txfifo_nonblocking() 的返回值未检查。如果 TX FIFO 已满，消息将被静默丢弃。
建议处理发送失败的情况
- mcan_transmit_via_txfifo_nonblocking(can_base_, &frame, nullptr);
+ auto status = mcan_transmit_via_txfifo_nonblocking(can_base_, &frame, nullptr);
+ if (status != status_success) {
+ // 考虑记录错误或返回失败状态
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void handle_downlink(const data::CanDataView& data) { | |
| mcan_tx_frame_t frame{}; | |
| if (data.is_extended_can_id) { | |
| frame.use_ext_id = true; | |
| frame.ext_id = data.can_id; | |
| } else { | |
| frame.use_ext_id = false; | |
| frame.std_id = data.can_id; | |
| } | |
| frame.canfd_frame = false; | |
| frame.rtr = data.is_remote_transmission; | |
| core::utility::assert_debug(data.can_data.size() <= 8); | |
| frame.dlc = data.can_data.size(); | |
| if (!data.can_data.empty()) | |
| std::memcpy(frame.data_8, data.can_data.data(), data.can_data.size()); | |
| mcan_transmit_via_txfifo_nonblocking(can_base_, &frame, nullptr); | |
| } | |
| void handle_downlink(const data::CanDataView& data) { | |
| mcan_tx_frame_t frame{}; | |
| if (data.is_extended_can_id) { | |
| frame.use_ext_id = true; | |
| frame.ext_id = data.can_id; | |
| } else { | |
| frame.use_ext_id = false; | |
| frame.std_id = data.can_id; | |
| } | |
| frame.canfd_frame = false; | |
| frame.rtr = data.is_remote_transmission; | |
| core::utility::assert_debug(data.can_data.size() <= 8); | |
| frame.dlc = data.can_data.size(); | |
| if (!data.can_data.empty()) | |
| std::memcpy(frame.data_8, data.can_data.data(), data.can_data.size()); | |
| auto status = mcan_transmit_via_txfifo_nonblocking(can_base_, &frame, nullptr); | |
| if (status != status_success) { | |
| // 考虑记录错误或返回失败状态 | |
| } | |
| } |
🤖 Prompt for AI Agents
In @firmware/src/can/can.hpp around lines 49 - 67, handle_downlink currently
ignores the return value of mcan_transmit_via_txfifo_nonblocking, so
TX-FIFO-full or other send failures are silently dropped; update handle_downlink
to check the function's return (e.g., the mcan return type/result code) and
handle failures explicitly: on success proceed as now, on TX-FIFO-full either
retry a bounded number of times, enqueue the frame to a software transmit queue,
or log/error-report the failure (use existing logging or an error counter/flag)
and avoid silent loss. Locate mcan_transmit_via_txfifo_nonblocking call in
handle_downlink and add return-value handling and a concrete failure path
(retry/enqueue/log) consistent with surrounding error handling policies.
There was a problem hiding this comment.
这是刻意的行为，目前阶段尚未实现任何遥测/丢包记录。
There was a problem hiding this comment.
@qzhhhi 感谢您的说明!
理解了,在当前开发阶段有意省略错误处理是合理的设计决策。既然目前尚未实现遥测和丢包记录功能,等到后续实现这些功能时再统一处理发送失败的情况会更加合适。
我已记录这个设计决策,以便在未来的审查中参考。
🧠 Learnings used
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 6
File: firmware/src/usb/interrupt_safe_buffer.hpp:25-47
Timestamp: 2025-12-26T09:45:56.870Z
Learning: 在 librmcs 固件的 `firmware/src/usb/interrupt_safe_buffer.hpp` 中，`InterruptSafeBuffer::allocate()` 在循环外加载 `out_` 值是有意为之的设计。该缓冲区采用裸机中断安全模型：`allocate()` 仅在 ISR 中调用（通过 CAN ISR 中的 serializer），`pop_batch()` 仅在主线程中调用（通过 `App::run()` 中的 `try_transmit()`）。由于项目不使用 RTOS，ISR 执行时主线程被暂停，因此 `out_` 在 `allocate()` 执行期间保持稳定。
- Define public data IDs and view types (CAN/UART/IMU) with callback interface - Introduce fixed little-endian bitfield utility and stack allocator/verify/assert helpers - Implement protocol layouts plus serializer and coroutine-based incremental deserializer - Add host-side transport abstraction and libusb bulk USB implementation - Add stream-based transmit buffer and protocol Handler to bridge transport <-> callbacks - Provide AllianceBoard convenience wrapper for common board usage
- Add missing includes in deserializer and handler files
… assertions - Replace `assert` with `assert_debug` to avoid macro collisions and make intent explicit - Extract assertion reporting/termination into `assert_func` with host implementation - Keep `assert_failed`/`assert_always` behavior consistent while reducing header dependencies
- Updated devcontainer configuration to include HOST_WORKSPACE_FOLDER. - Added .gitmodules to include the HPM SDK submodule. - Created .clangd for compiler flags specific to the RISC-V toolchain. - Introduced CMakeLists.txt for building the firmware with HPM SDK dependencies. - Added CMakePresets.json for build configurations (debug, release). - Created app.yaml for application configuration. - Initialized firmware/bsp/hpm_sdk as a submodule. - Implemented USB configuration in include/tusb_config.h. - Developed main application logic in src/app.cpp and src/app.hpp. - Implemented CAN communication in src/can/can.cpp and src/can/can.hpp. - Added interrupt-safe buffer management in src/usb/interrupt_safe_buffer.hpp. - Defined USB descriptors in src/usb/usb_descriptors.cpp and src/usb/usb_descriptors.hpp. - Created vendor communication handling in src/usb/vendor.cpp and src/usb/vendor.hpp. - Implemented assertion handling in src/utility/assert.cpp. - Added interrupt lock guard utility in src/utility/interrupt_lock_guard.hpp. - Introduced lazy initialization utility in src/utility/lazy.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)
…bytes - Encode UART length with byte count - Rename UART header length fields to reflect semantics - Serialize/deserialize UART payload length directly (no +1 / -1 mapping) BREAKING CHANGE: UART length fields are no longer "length - 1" (4-byte UART payloads now use the extended header), and Serializer::write_uart() adds a optional suffix_data parameter.
- Use core::protocol::kProtocolBufferSize for InterruptSafeBuffer storage/limits - Keep an in-flight batch and transmit it across multiple calls with an offset - Respect FS/HS max packet size and emit ZLP when needed before releasing the batch
- Enable DMA manager and initialize it at boot - Add UART driver with DMA RX fragments and DMA TX ring-buffered output - Route USB vendor UART downlink to UART3 and serialize uplink via usb::get_serializer() - Other UART events are reserved as TODO
Guard memcpy and pointer arithmetic when source spans may be empty (std::span::data() can be null): - Serializer UART write: skip copies for empty payload/suffix - Firmware UART TxBuffer enqueue: only copy when size > 0 - Firmware CAN TX: only copy CAN payload when non-empty - Deserializer feed: treat empty input spans as no-op Also make deserializer header self-contained by including <coroutine>.
- Align #include blocks with Google C++ style: - Sort standard headers alphabetically and remove stray blank lines - Keep associated headers first where applicable - Add missing standard/library includes for self-contained compilation
- Replace hardcoded /usr/include/libusb-1.0 and usb-1.0 linkage with pkg_check_modules(libusb-1.0) - Use pkg-config provided include dirs and libraries for better distro/custom install compatibility - Install pkg-config in the Docker build environment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 17
🤖 Fix all issues with AI agents
In @core/include/librmcs/data/datas.hpp:
- Line 71: 在文件中移除命名空间结束处的多余分号：将尾部的 "}; // namespace librmcs::data" 修正为 "}"
注释仍可保留为 "// namespace librmcs::data"；也就是定位到命名空间 librmcs::data 的结束标记并删除紧随其后的分号以符合
C++ 命名空间语法。
In @core/setup-clangd:
- Around line 46-50: The script creates symlinks to "../$mode/.clangd" and
"../$mode/build/compile_commands.json" without checking targets, causing
dangling links when host/.clangd or compile_commands.json are missing; either
add a real host/.clangd file mirroring firmware/.clangd, or modify the setup
code around the ln -s calls to test for existence of "../$mode/.clangd" and
"../$mode/build/compile_commands.json" (use [ -e ] or similar) and emit a clear
error message and non-zero exit if missing, and/or add a comment explaining that
compile_commands.json is produced by CMake and must be generated before running
this script so users know the prerequisite.
In @core/src/coroutine/lifo.hpp:
- Around line 111-112: The requires clause that currently reads
"std::is_same_v<T, void> || (std::is_move_assignable_v<T> &&
std::is_default_constructible_v<T>)" is too strict because result_ is never
default-initialized (it is assigned in return_value), so remove the
std::is_default_constructible_v<T> check so the constraint becomes
"std::is_same_v<T, void> || std::is_move_assignable_v<T>"; alternatively, if
result_ must be default-initialized elsewhere, change result_ to
std::optional<T> and keep the move-assignable constraint while eliminating the
default-constructible requirement; update the requires clause and any uses of
result_ accordingly in the coroutine's return_value and result_ member code.
In @core/src/protocol/deserializer.cpp:
- Around line 14-60: Add a short, explicit comment in
Deserializer::process_stream explaining that when an unknown FieldId hits the
default branch we must NOT call any consume_peeked*() before
enter_discard_mode() because peeked bytes must remain intact for the coroutine
suspend/resume and the discard/reset logic to work; likewise add a comment at
the EXTEND branch where consume_peeked_partial(sizeof(FieldHeader)) is used to
explain that we intentionally drop only the original FieldHeader byte and keep
the IdExtended byte as the start of the next header. Also add a small regression
note/test plan referencing Deserializer::process_stream, enter_discard_mode, and
the EXTEND handling to prevent future refactors from introducing a
consume_peeked() call for unknown FieldId or changing the partial consume
behavior.
In @core/src/utility/stack_allocator.hpp:
- Around line 56-82: The calls to utility::assert_always in stack_allocator.hpp
likely use the wrong namespace qualifier and will not compile; update all
assertions in allocate and deallocate (references: allocate, deallocate,
lifo_check_stack_, lifo_check_depth_, kMaxAllocs,
Impl::allocate/Impl::deallocate) to call the correct symbol (either
assert_always without the utility:: qualifier if it's in the current namespace,
or fully qualify it as ::librmcs::core::utility::assert_always), and also
normalize the size type usage to use std::size_t consistently (replace size_t in
deallocate signature with std::size_t if needed).
In @firmware/src/can/can.hpp:
- Around line 49-67: The handle_downlink implementation currently only asserts
can_data.size() <= 8; update it to enforce the same CAN constraints as
Serializer::required_can_size(): assert that if data.is_remote_transmission then
data.can_data.empty(); validate CAN ID ranges (if data.is_extended_can_id then
data.can_id <= 0x1FFFFFFF else data.can_id <= 0x7FF); and ensure frame.dlc is
set according to HPM MCAN DLC semantics (map payload byte count to the
controller's DLC encoding if MCAN expects coded DLC values rather than raw byte
count). Apply these same checks/adjustments to the other similar block (lines
~69-84) that constructs mcan_tx_frame_t and calls
mcan_transmit_via_txfifo_nonblocking so both downlink paths match Serializer
constraints.
In @firmware/src/spi/bmi088/gyro.hpp:
- Around line 152-157: The callback transmit_receive_completed_callback
currently reinterprets spi_.rx_buffer + 1 as a Data pointer which can cause
unaligned/aliasing crashes; instead, allocate a local Data variable, use
std::memcpy to copy sizeof(Data) bytes from spi_.rx_buffer + 1 into that local
variable (after keeping the existing assert that size == sizeof(Data) + 1), then
call handle_uplink(usb::vendor->serializer(), localData) and finally call
spi_.unlock(); this avoids unaligned/strict-aliasing issues while preserving
behavior.
In @firmware/src/usb/interrupt_safe_buffer.hpp:
- Around line 69-104: pop_batch() currently returns a Batch without clearing or
marking its written_size, while clear() is the only place that resets
written_size, which makes ownership unclear and can cause reuse bugs; fix by
making ownership explicit: either 1) mark the batch as taken inside pop_batch()
(e.g., set a taken flag or zero written_size via a new release path) and add a
matching release_batch(Batch&) API that the consumer must call to reset
written_size, or 2) document clearly in pop_batch()’s comment that the consumer
is responsible for calling written_size.store(0, ...) after processing and make
that requirement enforced (e.g., add an assertion or runtime check) so batches
aren’t reused prematurely — update pop_batch(), clear(), and the Batch structure
(written_size / taken flag) accordingly and mention Batch, pop_batch(), clear(),
written_size, and release_batch in the change.
In @host/include/librmcs/agent/alliance_board.hpp:
- Around line 15-64: The transmit helpers
(PacketBuilder::can0_transmit/can1_transmit/... and
dbus_transmit/uart0_transmit/...) ignore the bool return value of
builder_.write_can / builder_.write_uart so failures are silently dropped;
update PacketBuilder to propagate failures by adding a private bool ok_
initialized true, set ok_ = ok_ && builder_.write_can(...) or
builder_.write_uart(...) in each *_transmit, add a public bool ok() const
accessor (or alternatively make each *_transmit return bool), and ensure callers
check PacketBuilder::ok() after start_transmit(); use the existing PacketBuilder
and start_transmit symbols to locate and apply the change.
In @host/src/protocol/handler.cpp:
- Around line 98-110: process_result can fall through without returning when
SerializeResult has an unexpected value and assert_failed_debug is a no-op in
NDEBUG builds; update process_result (the static bool function handling
core::protocol::Serializer::SerializeResult) to ensure a definite return on all
code paths by adding a final return (e.g., return false) after the
assert_failed_debug() call (or replace the assert with a call that always
aborts) so the function always returns a bool even for unknown
Serializer::SerializeResult values.
In @host/src/protocol/stream_buffer.hpp:
- Around line 90-93: The destructor ~StreamBuffer() currently calls
finalize_buffer() whenever buffer_ is non-null, but finalize_buffer() asserts
payload_size > 0 and has no handling for 0-byte cases; change the destructor
logic in StreamBuffer so that before calling finalize_buffer() it checks the
current payload size (or a helper like payload_size) and if payload_size == 0
calls release() or simply drops the buffer instead of finalize_buffer(),
otherwise proceed to finalize_buffer(); apply the same guard to the similar
cleanup path referenced around the 209-219 region to avoid asserting or
submitting 0-byte transfers.
- Around line 90-93: The noexcept methods allocate(), ~StreamBuffer() and
finalize_buffer() call transport_.transmit() which may throw (see usb.cpp),
causing std::terminate; modify these methods to catch all exceptions from
transport_.transmit() (and any other throws inside them), log the error via the
existing logger, ensure buffer resources are released safely (free buffer_ in
the catch), and return/leave the object in a safe state
(allocate()/finalize_buffer() should return an empty span or no-op on failure
and ~StreamBuffer() must not allow exceptions to propagate). Reference the
symbols allocate, finalize_buffer, ~StreamBuffer, and transport_.transmit when
applying the changes.
In @host/src/transport/usb.cpp:
- Around line 435-437: Replace the abrupt std::terminate() in
usb_receive_complete_callback with the same error-flag pattern used elsewhere:
set stop_handling_events_ = true, record an appropriate error status (e.g., set
a last_error_ or usb_error_ member with a descriptive code/message), and ensure
the function returns cleanly so upper layers can detect the flag and handle the
device-disconnect/re-submit-failure; do not terminate the process or swallow the
error—propagate the status via the existing event loop/condition variable or
status-check path already used around stop_handling_events_.
- Around line 81-107: transmit() 和 release_transmit_buffer() 没有对传入的
std::unique_ptr<ITransportBuffer> buffer 做显式非空校验，可能在
static_cast<TransferWrapper*>(buffer.get()/buffer.release())
上导致未定义行为；在这两个方法开头增加对 buffer 的空检查（例如 if (!buffer) throw
std::invalid_argument("buffer must not be null");），在确认非空后再执行 static_cast/访问
transfer_ 或 release，并保持与类中 receive() 的 fail-fast 风格一致，确保错误类型与接口文档预期一致且不会在后续调用中对
nullptr 进行操作。
- Around line 46-66: The receive-path check of stop_handling_events_ in
usb_receive_complete_callback lacks synchronization (uses relaxed loads) while
the destructor (~Usb) stores relaxed and usb_transmit_complete_callback uses a
mutex; fix by making the memory ordering consistent: change the destructor's
store to stop_handling_events_.store(true, std::memory_order::release) and
change the check in usb_receive_complete_callback to use
stop_handling_events_.load(std::memory_order::acquire), or alternatively protect
the receive-path check with the same transmit mutex
(transmit_transfer_push_mutex_) so both callbacks use the same lock as
usb_transmit_complete_callback.
🧹 Nitpick comments (33)
firmware/.clangd (1)
1-11: 建议改善工具链路径的可移植性当前配置将 RISC-V 工具链路径硬编码为
/opt/riscv32-none-elf，这会影响跨环境协作：
- 不同开发者可能将工具链安装在不同位置
- CI/CD 环境可能使用不同的路径约定
- GCC 版本 15.1.0 同样被硬编码在 include 路径中
建议考虑以下改进方案之一：
- 在项目文档中明确说明工具链的安装路径要求
- 使用环境变量（如
$RISCV_TOOLCHAIN_PATH）替代硬编码路径- 提供
.clangd.example模板文件，将实际的.clangd加入.gitignorehost/src/utility/assert.cpp (1)
1-6: LGTM! 实现正确且清晰。断言失败处理函数实现合理：使用
std::println输出诊断信息后调用std::terminate()。包含顺序符合项目的 Google C++ Include Style 规范。可选优化：
<iostream>头文件可能是多余的，因为std::println(std::cerr, ...)的功能由<print>提供。建议验证是否可以移除。host/src/logging/logging.hpp (2)
3-10: 包含顺序符合规范，但<cstdio>似乎未被使用。当前使用
std::println和std::print(来自<print>) 以及std::cerr(来自<iostream>)。<cstdio>头文件可能是多余的。♻️ 建议移除未使用的头文件
#pragma once -#include <cstdio> #include <format> #include <iostream> #include <print> #include <string_view> #include <utility>
139-157:print_prefix实现正确处理了非法的Level::OFF情况。根据项目设计规范，
Level::OFF仅用于配置logging_level，传递给日志方法是非法的。此处使用assert_failed_debug()捕获此类编程错误是正确的。一个小建议：使用
switch语句可能比if-else if链更清晰，并能让编译器在未来添加新级别时发出警告。♻️ 可选：使用 switch 语句替代 if-else 链
static void print_prefix(Level level) { - std::string_view level_text = [level]() constexpr -> std::string_view { - if (level == Level::TRACE) - return "trace"; - else if (level == Level::DEBUG) - return "debug"; - else if (level == Level::INFO) - return "info"; - else if (level == Level::WARN) - return "warn"; - else if (level == Level::ERR) - return "error"; - else if (level == Level::CRITICAL) - return "critical"; - else - core::utility::assert_failed_debug(); - }(); + std::string_view level_text; + switch (level) { + case Level::TRACE: level_text = "trace"; break; + case Level::DEBUG: level_text = "debug"; break; + case Level::INFO: level_text = "info"; break; + case Level::WARN: level_text = "warn"; break; + case Level::ERR: level_text = "error"; break; + case Level::CRITICAL: level_text = "critical"; break; + default: core::utility::assert_failed_debug(); + } std::print(std::cerr, "[librmcs] [{}] ", level_text); }host/src/utility/final_action.hpp (1)
26-26: 建议为 disable() 添加 noexcept 说明符该方法仅执行简单的布尔赋值，无法抛出异常，添加 noexcept 可提升编译器优化潜力并提高接口清晰度。
♻️ 建议的改进
- void disable() { enabled_ = false; }; + void disable() noexcept { enabled_ = false; }firmware/src/utility/lazy.hpp (1)
22-22: 析构“什么都不做”在固件里可以接受，但建议把“仅允许静态存活期使用/不允许被销毁”的约束写清楚（Line 22），否则作为局部变量/成员被析构时会跳过T的析构。
另外，当前memory_order::relaxed在InterruptLockGuard语义下 OK，但如果operator bool()/get()未来可能跨并发上下文使用，建议把 Line 38/60 改成 release/acquire 以加固。Also applies to: 24-61, 74-77
firmware/src/can/can.cpp (1)
13-67: 考虑使用模板或宏减少 ISR 代码重复。四个 CAN ISR 结构几乎相同，仅
base、canX和DataId::CANx不同。可以考虑使用模板函数或辅助宏来减少重复代码，提高可维护性。当前实现功能正确：先检查标志、处理 RXFIFO0 消息、最后清除标志。
[[likely]]和[[unlikely]]属性使用恰当。♻️ 可选重构：使用模板减少重复
namespace { template <MCAN_Type* Base, auto& CanInstance, data::DataId Id> void can_isr_impl() { uint32_t flags = mcan_get_interrupt_flags(Base); if (!flags) [[unlikely]] return; if (flags & MCAN_INT_RXFIFO0_NEW_MSG) [[likely]] CanInstance->handle_uplink(Id, usb::vendor->serializer()); mcan_clear_interrupt_flags(Base, flags); } } // namespace SDK_DECLARE_EXT_ISR_M(IRQn_MCAN0, can0_isr) void can0_isr() { can_isr_impl<HPM_MCAN0, can0, data::DataId::CAN0>(); } // ... 其他 ISR 类似core/src/protocol/protocol.hpp (1)
93-103:ImuAccelerometerPayload和ImuGyroscopePayload定义相同，可考虑合并。两个 payload 结构完全相同（都是 6 字节，包含 X/Y/Z 三个 int16_t 字段）。如果语义上允许，可以定义一个通用的
ImuAxisPayload类型并为两者创建别名。当前实现功能正确，这只是一个可选的代码简化建议。
♻️ 可选：提取通用 payload 类型
+struct ImuAxisPayload : utility::Bitfield<6> { + using X = utility::BitfieldMember<0, 16, int16_t>; + using Y = utility::BitfieldMember<16, 16, int16_t>; + using Z = utility::BitfieldMember<32, 16, int16_t>; +}; + +using ImuAccelerometerPayload = ImuAxisPayload; +using ImuGyroscopePayload = ImuAxisPayload; -struct ImuAccelerometerPayload : utility::Bitfield<6> { - using X = utility::BitfieldMember<0, 16, int16_t>; - using Y = utility::BitfieldMember<16, 16, int16_t>; - using Z = utility::BitfieldMember<32, 16, int16_t>; -}; - -struct ImuGyroscopePayload : utility::Bitfield<6> { - using X = utility::BitfieldMember<0, 16, int16_t>; - using Y = utility::BitfieldMember<16, 16, int16_t>; - using Z = utility::BitfieldMember<32, 16, int16_t>; -};firmware/src/usb/usb_descriptors.hpp (3)
76-78: TODO 注释：idProduct需要动态生成。注释提到产品 ID 应动态生成。这可能是后续工作项。
需要我创建一个 issue 来跟踪此任务吗？
93-96: 端点常量命名使用CDC前缀，但实际是 Vendor 类。
EPNUM_CDC_0_DATA_OUT和EPNUM_CDC_0_DATA_IN命名暗示 CDC 类，但描述符实际配置的是 Vendor 类接口。考虑重命名为EPNUM_VENDOR_DATA_OUT/IN以提高代码可读性。♻️ 建议重命名
- static constexpr uint8_t EPNUM_CDC_0_DATA_OUT = 0x01; - static constexpr uint8_t EPNUM_CDC_0_DATA_IN = 0x81; + static constexpr uint8_t EPNUM_VENDOR_DATA_OUT = 0x01; + static constexpr uint8_t EPNUM_VENDOR_DATA_IN = 0x81;
118-125: 序列号硬编码为 "123456"，注释提示应使用芯片 ID。建议后续实现从芯片唯一 ID 寄存器读取序列号。
需要我帮助实现基于芯片 ID 的序列号生成吗？
firmware/src/usb/vendor.hpp (1)
57-57: TODO 注释：处理其他 UART 事件。代码中保留了处理其他 UART 事件的 TODO 注释。请确认是否需要在当前版本中实现，或者可以延后处理。
您希望我生成处理其他 UART ID 的实现代码，或者创建一个 issue 来跟踪此任务吗？
firmware/src/uart/uart.hpp (1)
64-75: 考虑将 UART 参数配置化。当前波特率（115200）和空闲检测阈值（16, 10）是硬编码的。如果未来需要支持不同的 UART 配置，可以考虑将这些参数添加到构造函数或配置结构中。
不过，如果当前的固定配置已满足所有使用场景，则可以保持现状。
core/src/protocol/deserializer.cpp (2)
114-151: UART 长度校验建议覆盖非扩展长度路径（防止协议/字段变更后溢出）。
Line 132-133 仅在扩展长度时检查uart_data_length > sizeof(pending_bytes_buffer_)；如果普通UartHeader::DataLength()的取值范围未来扩大，也可能触发 peek 缓存拷贝溢出风险。建议把同样的上限校验放到两条路径都执行（或抽成统一校验）。
153-195: IMU 解析逻辑清晰；建议把默认分支的错误原因区分记录（可选）。
现在default: co_return false;会统一进入丢弃模式；如果后续需要定位“payload type 非法 vs 数据截断”，可考虑在enter_discard_mode()前记录更具体的错误码/统计。host/include/librmcs/agent/alliance_board.hpp (1)
65-104: 回调覆写请补override，并按需统一 accel/gyro 的路由风格。
- Line 66 与 Line 81：这是对
data::IDataCallback的覆写，建议显式override final（基于 learnings）。- Line 98-103：当前 accel/gyro 直接吞掉数据；如果期望与 CAN/UART 一样提供“可覆写的 channel hook”，建议按同样模式做一层
final override→ 转发到virtualhook（或至少注明“默认不处理”是预期）。建议改法（override/final）
- bool can_receive_callback(data::DataId id, const data::CanDataView& data) final { + bool can_receive_callback(data::DataId id, const data::CanDataView& data) override final { @@ - bool uart_receive_callback(data::DataId id, const data::UartDataView& data) final { + bool uart_receive_callback(data::DataId id, const data::UartDataView& data) override final {firmware/src/spi/bmi088/gyro.hpp (2)
165-169: 给read_size加上 tx_buffer 容量断言（防止未来调用改动导致溢出）。
prepare_tx_buffer_read()对tx_buffer[1..]memsetread_size字节；建议在这里或read()入口加assert_debug(read_size + 1 <= sizeof(spi_.tx_buffer))一类的保护，避免后续复用该函数时引入越界。
171-176: 仅断言“非 InvalidArgument”可能不足以暴露上行拥塞（可选）。
如果Serializer::write_imu_gyroscope()还有“缓冲区满/不可用”等失败码，这里会被静默吞掉；按需求考虑统计丢包或在 debug 下断言更强条件。firmware/CMakeLists.txt (3)
18-22: 不要在 CMake 里写 ENV 变量（可选）；优先用普通 CMake 变量传递。
Line 19set(ENV{HPM_SDK_BASE} ...)会修改当前配置进程环境，可能让“同一个 build dir 的二次配置”表现不直观；可考虑改成set(HPM_SDK_BASE "...")并传给find_package(... HINTS ...)。
80-87:GLOB_RECURSE收集源码建议后续收敛为显式列表（工程可维护性）。
Line 80-85 的 glob 对快速集成很好，但对“新增/删除文件触发构建系统刷新”“可预期的编译单元边界”不够友好；如果后续开始拆分模块/做增量构建优化，建议逐步切到显式 source list。
92-97: 把 SDK includes 标记为 SYSTEM 的思路很好；建议再加一层 target 存在性保护。
如果${HPM_SDK_LIB_ITF}在某些 SDK 版本里不是有效 target，get_target_property()会报错；建议用if(TARGET ${HPM_SDK_LIB_ITF})包一下再取属性。host/src/transport/transport.hpp (2)
13-27: 接口文档把“外部析构 buffer = UB”写死，但实现（Usb::TransferWrapper::~TransferWrapper）会尝试自救并记录错误，建议统一语义。
当前文档强调“外部析构导致未定义行为”，但实现侧实际会destroy()并 log；建议把文档改成“编程错误，可能导致数据丢失/资源异常；实现会尽力检测并回收”，避免 API 使用者困惑。Also applies to: 44-61
66-128: 建议在 ITransport 上补充 transmit/acquire/release 的线程安全约束（目前只写了 receive 的回调并发语义）。
usb.cpp 里实际有多 mutex/线程，API 使用者很容易默认“可多线程随便调”；建议明确：是否允许并发 acquire/transmit/release、析构与并发调用是否定义等。host/src/protocol/stream_buffer.hpp (1)
121-124:current_ <= end_在buffer_ == nullptr且指针均为 nullptr 时用于断言，建议改成条件化断言避免不必要的指针关系比较。
更稳妥的写法是：(!buffer_) || (current_ && end_ && current_ <= end_)。Also applies to: 173-177
firmware/src/utility/ring_buffer.hpp (1)
118-145: 可选：emplace_back_n/push_back_n的 functor 入参可改为F&&并std::forward，减少一次拷贝。
对 ISR/高频路径可能有一点收益（不过不做也完全可用）。Also applies to: 174-181
firmware/src/uart/rx_buffer.hpp (1)
102-141: 审查try_dequeue中的内存序与异常条件处理几个需要关注的点：
内存序一致性（104、136-138 行）：
out_的加载使用acquire，存储使用release，与in_的更新（在update_in()中）形成正确的同步对。这与firmware/src/utility/ring_buffer.hpp中的readable()模式一致。异常条件处理（110-118 行）：当
readable > kProtocolMaxPayloadSize时，调试版本会assert_failed_always()，发布版本会静默丢弃数据。TODO 注释提醒添加丢弃计数器，这是个好建议。缓存失效调用（124、128-129 行）：在读取 DMA 缓冲区前正确地使缓存行失效，确保数据一致性。
建议：考虑实现 TODO 中提到的丢弃计数器/遥测钩子，以便在生产环境中监控数据丢失情况。需要我帮助生成实现方案吗？
firmware/src/uart/tx_buffer.hpp (1)
42-98:try_enqueue的空闲边界优化逻辑较复杂该函数实现了复杂的边界去重优化：
共享边界去重（59-68 行）：当新数据包的起始位置与前一个数据包的结束位置重合时，重用现有的边界标记。这是一个聪明的优化，可以减少
idle_buffer_的使用。零长度包 (ZLP) 处理（63-68、79-83 行）：对于
size == 0的情况，只需单个检查点即可强制 IDLE 等待。原子性保证（72-76 行）：非空且无共享边界的情况下，使用
push_back_n原子地推送[begin, end]两个标记。逻辑正确但复杂度较高。建议在代码中添加更详细的注释说明各种情况。
可选改进：考虑为不同的 idle 处理路径（共享边界 ZLP、共享边界非空、无共享边界 ZLP、无共享边界非空）添加内联注释示例，提高可维护性。
firmware/src/spi/spi.hpp (2)
82-127: SPI 锁定机制与传输流程锁定设计分析：
try_lock（84 行）：使用test_and_set获取锁，简单有效。transmit_receive（86-105 行）：
- 88-89 行：断言锁已持有且 SPI 未激活
- 对于 FIFO 内传输直接写入，超过 FIFO 大小会触发
assert_failed_always()transmit_receive_blocked（107-117 行）：
- 禁用 IRQ（108 行）确保同步执行
- 轮询等待传输完成（111-112 行）
- 恢复 IRQ（116 行）
unlock（124-127 行）：断言持有锁后清除问题：
transmit_receive函数在调用时断言持有锁（88 行），但没有文档说明调用者必须先调用try_lock()。从 BMI088 驱动的使用模式来看，初始化时使用assert_debug(spi_.try_lock())，这符合学习到的模式。建议：在
transmit_receive和unlock的函数注释中明确说明锁的所有权要求，例如：// Precondition: caller must hold the lock (via try_lock()) void transmit_receive(ISpiModule& module, std::size_t size);
103-103: DMA 传输的 TODO 项当传输大小超过硬件 FIFO（8 字节）时，代码会触发断言失败。根据
kMaxTransferSize = HPM_L1C_CACHELINE_SIZE（通常 64 字节），这限制了实际可用的传输大小。这是一个已知的限制（TODO 注释）。如果需要支持大于 8 字节的 SPI 传输，需要实现 DMA 路径。需要我帮助：
- 生成 DMA 传输的实现方案？
- 或者创建一个 issue 来跟踪这个增强需求？
host/src/protocol/handler.cpp (2)
1-18: 头文件包含顺序需要调整根据学习到的 Google C++ Include Style，源文件的包含顺序应为：
- 对应的头文件（
librmcs/protocol/handler.hpp）- 空行
- C 系统头文件
- 空行
- C++ 标准库头文件（
<cstddef>,<cstdint>, 等）- 空行
- 其他库头文件
- 空行
- 项目头文件（
core/...,host/...,librmcs/...）当前代码缺少组之间的空行分隔。
♻️ 建议的头文件顺序
#include "librmcs/protocol/handler.hpp" #include <cstddef> #include <cstdint> #include <cstring> #include <memory> #include <new> #include <span> #include <utility> #include "core/src/protocol/deserializer.hpp" #include "core/src/protocol/protocol.hpp" #include "core/src/protocol/serializer.hpp" #include "core/src/utility/assert.hpp" #include "host/src/logging/logging.hpp" #include "host/src/protocol/stream_buffer.hpp" #include "host/src/transport/transport.hpp" #include "librmcs/data/datas.hpp"基于学习，这是 Google C++ Include Style 的要求。
130-148: PacketBuilder 的转发方法每个
write_*方法都使用std::launder获取实际对象指针并转发调用（132、137、142-143、147 行）。实现正确且一致。
std::launder的重复使用可能略显冗余，但保证了类型安全。可选改进：考虑添加一个私有辅助方法来减少重复：
PacketBuilderImpl* impl() noexcept { return std::launder(reinterpret_cast<PacketBuilderImpl*>(storage_)); }然后各方法可简化为
return impl()->write_can(field_id, view);core/src/coroutine/lifo.hpp (2)
59-69: 考虑添加 const 重载的 context() 访问器
context()方法目前只提供非 const 版本。为了支持 const InlineLifoContext 对象访问底层上下文，建议添加 const 重载。♻️ 建议的改进
constexpr LifoContext& context() { return static_cast<LifoContext&>(*this); } + +constexpr const LifoContext& context() const { return static_cast<const LifoContext&>(*this); }
200-270: void 特化缺少identifier()方法主模板在第 179 行提供了
identifier()方法返回协程句柄地址，但void特化中缺少此方法。这导致两个特化之间的 API 不一致，可能给泛型代码带来困难。建议在
void特化中添加相同的identifier()方法以保持 API 一致性。♻️ 建议的补充
在
done()方法之后添加：[[nodiscard]] constexpr bool done() const noexcept { return handle_.done(); } +[[nodiscard]] constexpr void* identifier() const noexcept { return handle_.address(); } + constexpr void result() const noexcept { utility::assert_debug(handle_.done()); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
.clangd.devcontainer/devcontainer.json.gitmodulesDockerfilecore/.gitignorecore/include/librmcs/data/datas.hppcore/setup-clangdcore/src/coroutine/lifo.hppcore/src/protocol/constant.hppcore/src/protocol/deserializer.cppcore/src/protocol/deserializer.hppcore/src/protocol/protocol.hppcore/src/protocol/serializer.hppcore/src/utility/assert.hppcore/src/utility/bitfield.hppcore/src/utility/immovable.hppcore/src/utility/stack_allocator.hppcore/src/utility/uncopyable.hppcore/src/utility/verify.hppfirmware/.clangdfirmware/CMakeLists.txtfirmware/CMakePresets.jsonfirmware/app.yamlfirmware/bsp/hpm_sdkfirmware/include/tusb_config.hfirmware/src/app.cppfirmware/src/app.hppfirmware/src/can/can.cppfirmware/src/can/can.hppfirmware/src/gpio/gpio.cppfirmware/src/gpio/gpio.hppfirmware/src/spi/bmi088/accel.hppfirmware/src/spi/bmi088/gyro.hppfirmware/src/spi/spi.cppfirmware/src/spi/spi.hppfirmware/src/uart/rx_buffer.hppfirmware/src/uart/tx_buffer.hppfirmware/src/uart/uart.cppfirmware/src/uart/uart.hppfirmware/src/usb/helper.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/usb/usb_descriptors.cppfirmware/src/usb/usb_descriptors.hppfirmware/src/usb/vendor.cppfirmware/src/usb/vendor.hppfirmware/src/utility/assert.cppfirmware/src/utility/interrupt_lock_guard.hppfirmware/src/utility/lazy.hppfirmware/src/utility/ring_buffer.hpphost/CMakeLists.txthost/CMakePresets.jsonhost/include/librmcs/agent/alliance_board.hpphost/include/librmcs/protocol/handler.hpphost/src/logging/logging.hpphost/src/protocol/handler.cpphost/src/protocol/stream_buffer.hpphost/src/transport/transport.hpphost/src/transport/usb.cpphost/src/utility/assert.cpphost/src/utility/cross_os.hpphost/src/utility/final_action.hpphost/src/utility/ring_buffer.hpp
✅ Files skipped from review due to trivial changes (2)
- firmware/CMakePresets.json
- .gitmodules
🚧 Files skipped from review as they are similar to previous changes (8)
- firmware/src/gpio/gpio.hpp
- .clangd
- core/.gitignore
- firmware/bsp/hpm_sdk
- core/src/utility/verify.hpp
- host/CMakeLists.txt
- host/CMakePresets.json
- core/src/utility/immovable.hpp
🧰 Additional context used
🧠 Learnings (18)
📚 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/usb/helper.hpphost/src/utility/final_action.hppfirmware/src/utility/interrupt_lock_guard.hppcore/src/protocol/deserializer.hppcore/src/utility/assert.hppfirmware/src/can/can.hppcore/src/utility/stack_allocator.hppcore/src/protocol/protocol.hpphost/src/protocol/stream_buffer.hpphost/src/utility/cross_os.hpphost/src/transport/transport.hppfirmware/src/spi/bmi088/gyro.hppcore/src/utility/bitfield.hppfirmware/src/usb/usb_descriptors.hppfirmware/src/uart/uart.hppcore/src/utility/uncopyable.hppfirmware/src/uart/tx_buffer.hppfirmware/src/usb/vendor.hppfirmware/src/utility/ring_buffer.hpphost/include/librmcs/agent/alliance_board.hppfirmware/src/utility/lazy.hppcore/include/librmcs/data/datas.hpphost/src/logging/logging.hppfirmware/src/spi/bmi088/accel.hppcore/src/coroutine/lifo.hppcore/src/protocol/constant.hppcore/src/protocol/serializer.hppfirmware/src/app.hppfirmware/src/spi/spi.hppfirmware/src/uart/rx_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hpphost/include/librmcs/protocol/handler.hpphost/src/utility/ring_buffer.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/usb/helper.hppfirmware/src/utility/interrupt_lock_guard.hppfirmware/src/can/can.hppfirmware/src/spi/bmi088/gyro.hppfirmware/src/usb/usb_descriptors.hppfirmware/src/uart/uart.hppfirmware/src/uart/tx_buffer.hppfirmware/src/usb/vendor.hppfirmware/src/utility/ring_buffer.hppfirmware/src/utility/lazy.hppfirmware/src/spi/bmi088/accel.hppfirmware/src/app.hppfirmware/src/spi/spi.hppfirmware/src/uart/rx_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hpp
📚 Learning: 2025-12-26T09:45:56.870Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 6
File: firmware/src/usb/interrupt_safe_buffer.hpp:25-47
Timestamp: 2025-12-26T09:45:56.870Z
Learning: 在 librmcs 固件的 `firmware/src/usb/interrupt_safe_buffer.hpp` 中，`InterruptSafeBuffer::allocate()` 在循环外加载 `out_` 值是有意为之的设计。该缓冲区采用裸机中断安全模型：`allocate()` 仅在 ISR 中调用（通过 CAN ISR 中的 serializer），`pop_batch()` 仅在主线程中调用（通过 `App::run()` 中的 `try_transmit()`）。由于项目不使用 RTOS，ISR 执行时主线程被暂停，因此 `out_` 在 `allocate()` 执行期间保持稳定。
Applied to files:
firmware/src/utility/interrupt_lock_guard.hppfirmware/src/usb/usb_descriptors.cppfirmware/src/can/can.hppfirmware/src/can/can.cpphost/src/protocol/handler.cppfirmware/src/uart/tx_buffer.hppfirmware/src/utility/ring_buffer.hppfirmware/src/uart/rx_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hpphost/src/utility/ring_buffer.hpp
📚 Learning: 2026-01-03T15:20:51.624Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 9
File: firmware/src/spi/bmi088/accel.hpp:41-41
Timestamp: 2026-01-03T15:20:51.624Z
Learning: In firmware/src/spi/bmi088/accel.hpp and firmware/src/spi/bmi088/gyro.hpp, the use of assert_debug(spi_.try_lock()) during BMI088 driver initialization is intentional and correct. During the initialization phase, interrupts are disabled and all SPI calls are synchronous, so try_lock() should never fail in correct code. If it returns false, it indicates a logic error that should be caught during development.
Applied to files:
firmware/src/utility/interrupt_lock_guard.hppfirmware/src/spi/spi.cppfirmware/src/gpio/gpio.cppfirmware/src/spi/spi.hpp
📚 Learning: 2026-01-11T11:53:29.752Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 10
File: firmware/src/usb/vendor.hpp:87-93
Timestamp: 2026-01-11T11:53:29.752Z
Learning: 在 librmcs 固件项目中，TinyUSB 被配置为无缓冲模式（CFG_TUD_VENDOR_RX_BUFSIZE = 0, CFG_TUD_VENDOR_TX_BUFSIZE = 0），数据直接发送至 USB 端点。在 `firmware/src/usb/vendor.hpp` 的 `try_transmit()` 中，由于 `target_size` 总是 ≤ `max_packet_size` 且 `device_ready()` 预先检查端点可用性，`tud_vendor_n_write()` 在理想状况下应返回完整的 `target_size`，因此使用 `assert_debug(sent == target_size)` 作为调试断言是合理的。
Applied to files:
host/src/transport/usb.cppfirmware/src/usb/usb_descriptors.cppfirmware/src/can/can.hpphost/src/protocol/handler.cppfirmware/src/uart/tx_buffer.hppfirmware/src/usb/vendor.hppfirmware/include/tusb_config.hfirmware/src/uart/rx_buffer.hppfirmware/src/usb/interrupt_safe_buffer.hppfirmware/src/usb/vendor.cpp
📚 Learning: 2025-12-20T06:17:08.271Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/src/transport/usb.cpp:42-62
Timestamp: 2025-12-20T06:17:08.271Z
Learning: 在 librmcs 项目中,下位机使用 USB vendor 类,因此不需要也不应该调用 libusb_attach_kernel_driver 或 libusb_detach_kernel_driver。
Applied to files:
host/src/transport/usb.cppfirmware/src/usb/vendor.hppfirmware/src/usb/vendor.cpp
📚 Learning: 2026-01-11T13:13:34.439Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 11
File: firmware/src/usb/usb_descriptors.cpp:1-4
Timestamp: 2026-01-11T13:13:34.439Z
Learning: In the librmcs project, enforce Google C++ Include Style in all C++ source files (.cpp/.cc). The include order should be: 1) the corresponding header (e.g., firmware/src/usb/usb_descriptors.hpp), 2) a blank line, 3) C system headers (e.g., <unistd.h>), 4) a blank line, 5) C++ standard library headers (e.g., <cstdint>), 6) a blank line, 7) headers from other libraries, 8) a blank line, and 9) project headers. Between non-empty groups, use a single blank line. This structure must be preserved in each non-empty group across all relevant source files.
Applied to files:
host/src/transport/usb.cppfirmware/src/usb/usb_descriptors.cppfirmware/src/can/can.cppfirmware/src/uart/uart.cppfirmware/src/spi/spi.cppfirmware/src/app.cpphost/src/protocol/handler.cppcore/src/protocol/deserializer.cpphost/src/utility/assert.cppfirmware/src/gpio/gpio.cppfirmware/src/usb/vendor.cppfirmware/src/utility/assert.cpp
📚 Learning: 2025-12-20T04:28:56.876Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 4
File: core/src/protocol/deserializer.cpp:38-58
Timestamp: 2025-12-20T04:28:56.876Z
Learning: In core/src/protocol/deserializer.cpp, when an unknown FieldId is encountered in the default switch branch, the code does not explicitly consume peeked bytes before calling deserializing_error(). This is intentional: deserializing_error() resets input_cursor_ to input_end_ and pending_bytes_ to 0, causing the next peek_bytes() to suspend the coroutine until finish_transfer() resets the error state. The assertion at line 22 will not fail because the coroutine suspends before re-entering that path.
Applied to files:
core/src/protocol/deserializer.hpphost/src/protocol/handler.cpp
📚 Learning: 2026-01-11T13:22:14.180Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 11
File: host/CMakeLists.txt:5-7
Timestamp: 2026-01-11T13:22:14.180Z
Learning: 在 librmcs 项目中，C++23 特性支持的编译器版本要求为：GCC 14.2 (host) 和 GCC 15.1 (firmware)。MSVC 目前不支持该项目，相关问题在 issue #12 中跟踪。
Applied to files:
core/src/utility/assert.hppfirmware/CMakeLists.txtfirmware/src/utility/lazy.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.hppfirmware/src/utility/assert.cpp
📚 Learning: 2025-12-20T05:28:56.619Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: core/src/utility/assert.hpp:13-15
Timestamp: 2025-12-20T05:28:56.619Z
Learning: In the librmcs project, include order convention guarantees that `#include "..."` (project files) always come after `#include <...>` (system/library headers), which ensures that modifications to standard macros (like `#undef assert` in core/src/utility/assert.hpp) won't affect other libraries.
Applied to files:
firmware/src/usb/usb_descriptors.cpphost/src/utility/assert.cppfirmware/src/gpio/gpio.cppfirmware/src/utility/assert.cpp
📚 Learning: 2026-01-03T15:20:44.048Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 9
File: firmware/src/spi/bmi088/accel.hpp:41-41
Timestamp: 2026-01-03T15:20:44.048Z
Learning: For BMI088 SPI driver initialization (accel.hpp and gyro.hpp), use assert_debug(spi_.try_lock()) since interrupts are disabled and SPI calls are synchronous during init; a false result indicates a logic error. This pattern should be applicable to all files in firmware/src/spi/bmi088 with the same shall-be-initialization behavior. Reviewers should verify that try_lock() cannot fail in proper init and that such failures are strictly treated as development-time issues.
Applied to files:
firmware/src/spi/bmi088/gyro.hppfirmware/src/spi/bmi088/accel.hpp
📚 Learning: 2025-12-20T04:28:53.374Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 4
File: core/src/protocol/deserializer.cpp:38-58
Timestamp: 2025-12-20T04:28:53.374Z
Learning: In core/src/protocol/deserializer.cpp, for the default switch case handling an unknown FieldId, keep the current behavior: do not eagerly consume any peeked bytes before invoking deserializing_error(). This path intentionally relies on deserializing_error() resetting input_cursor_ to input_end_ and pending_bytes_ to 0, so that the next peek_bytes() suspends the coroutine until finish_transfer() clears the error state. Ensure this rationale is documented with a comment near the default branch and add a targeted test or regression note to prevent future reintroduction of a byte-consumption in this path. If future changes modify this behavior, verify that the coroutine suspension and error-reset semantics remain consistent and that the existing assertion at the relevant line remains valid.
Applied to files:
core/src/protocol/deserializer.cpp
📚 Learning: 2026-01-11T13:13:34.439Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 11
File: firmware/src/usb/usb_descriptors.cpp:1-4
Timestamp: 2026-01-11T13:13:34.439Z
Learning: 在 librmcs 项目中，C++ 实现文件（.cpp/.cc）的头文件包含顺序遵循 Google C++ 风格指南：首先是对应的头文件（如 dir2/foo2.h），然后是空行，接着是 C 系统头文件（如 <unistd.h>），空行，C++ 标准库头文件（如 <cstddef>），空行，其他库的头文件，空行，最后是项目的其他头文件。每个非空分组之间用空行隔开。例如，firmware/src/usb/usb_descriptors.cpp 中先包含 "firmware/src/usb/usb_descriptors.hpp"，然后是 <cstdint>，这是符合规范的。
Applied to files:
firmware/CMakeLists.txt
📚 Learning: 2025-12-20T05:26:36.937Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/include/librmcs/agent/alliance_board.hpp:98-103
Timestamp: 2025-12-20T05:26:36.937Z
Learning: In the librmcs codebase, when overriding virtual functions from a base class in C++, declare overrides with the override specifier and do not repeat virtual on the overriding function. The virtual keyword should only be used on the topmost declaration of a virtual function. This aligns with clang-tidy's modernize-use-override check. Apply this pattern to header files under host/include/librmcs (e.g., host/include/librmcs/**/*.hpp) and ensure all overrides use 'override' without 'virtual'.
Applied to files:
host/include/librmcs/agent/alliance_board.hpphost/include/librmcs/protocol/handler.hpp
📚 Learning: 2025-12-20T05:52:44.423Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/src/logging/logging.hpp:27-157
Timestamp: 2025-12-20T05:52:44.423Z
Learning: In the librmcs project, the Logger class in host/src/logging/logging.hpp intentionally does not include thread synchronization (std::mutex). The team has decided that interleaved output is acceptable to avoid blocking overhead. A full async logging solution with message queues would be needed for proper thread-safe logging, but this complexity is not desired at the current stage.
Applied to files:
host/src/logging/logging.hpp
📚 Learning: 2025-12-20T06:19:33.594Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: host/src/logging/logging.hpp:15-23
Timestamp: 2025-12-20T06:19:33.594Z
Learning: In the librmcs project Logger class (host/src/logging/logging.hpp), Level::OFF is designed exclusively as a configuration value for setting logging_level to disable all logging. Passing Level::OFF to logging methods (e.g., log(Level::OFF, "msg")) is illegal by design and intentionally triggers an assertion to catch programming errors. This is not a bug but an intentional safeguard.
Applied to files:
host/src/logging/logging.hpp
📚 Learning: 2025-12-20T06:17:50.471Z
Learnt from: qzhhhi
Repo: Alliance-Algorithm/librmcs PR: 5
File: core/src/coroutine/lifo.hpp:0-0
Timestamp: 2025-12-20T06:17:50.471Z
Learning: In coroutine result accessors (e.g., LifoTask::result()), prefer returning T& instead of T to express ownership transfer. Update the API so callers use std::move(task.result()) and adjust callers accordingly. Apply this pattern to similar accessors in core/src/coroutine; ensure const-correctness, avoid dangling references, and update any internal copies or move semantics to reflect the new ownership model.
Applied to files:
core/src/coroutine/lifo.hpp
🧬 Code graph analysis (21)
host/src/transport/usb.cpp (3)
host/src/logging/logging.hpp (2)
get_logger(160-160)get_logger(160-160)host/src/transport/transport.hpp (3)
buffer(90-90)buffer(104-104)callback(127-127)core/src/protocol/serializer.hpp (1)
size(20-20)
core/src/protocol/deserializer.hpp (5)
core/include/librmcs/data/datas.hpp (4)
id(62-62)id(64-64)data(66-66)data(68-68)core/src/coroutine/lifo.hpp (17)
assert_debug(183-186)assert_debug(188-191)assert_debug(257-257)assert_debug(263-263)static_cast(65-65)h(43-45)h(43-43)h(131-139)h(132-132)h(219-227)h(220-220)n(20-20)n(20-20)n(75-86)n(75-75)n(90-92)n(90-90)host/src/protocol/stream_buffer.hpp (4)
assert_debug(194-207)assert_debug(209-219)size(121-137)size(121-121)core/src/utility/assert.hpp (2)
assert_debug(36-44)assert_debug(36-37)core/src/protocol/serializer.hpp (15)
field_id(30-73)field_id(30-30)field_id(75-116)field_id(75-77)field_id(169-172)field_id(169-169)field_id(174-177)field_id(174-174)field_id(191-208)field_id(191-191)field_id(210-226)field_id(210-212)field_id(230-241)field_id(230-230)size(20-20)
core/src/utility/assert.hpp (1)
host/src/utility/assert.cpp (2)
noreturn(10-15)assert_func(10-10)
firmware/src/usb/usb_descriptors.cpp (1)
firmware/src/usb/usb_descriptors.hpp (4)
index(25-33)index(25-25)index(35-63)index(35-35)
firmware/src/can/can.hpp (2)
firmware/src/utility/lazy.hpp (3)
assert_debug(44-47)assert_debug(49-52)assert_debug(54-57)core/src/protocol/serializer.hpp (14)
field_id(30-73)field_id(30-30)field_id(75-116)field_id(75-77)field_id(169-172)field_id(169-169)field_id(174-177)field_id(174-174)field_id(191-208)field_id(191-191)field_id(210-226)field_id(210-212)field_id(230-241)field_id(230-230)
core/src/utility/stack_allocator.hpp (2)
core/src/coroutine/lifo.hpp (10)
n(20-20)n(20-20)n(75-86)n(75-75)n(90-92)n(90-90)p(22-24)p(22-22)p(95-107)p(95-95)core/src/utility/assert.hpp (2)
assert_always(30-34)assert_always(30-31)
host/src/transport/transport.hpp (3)
firmware/src/usb/vendor.hpp (2)
buffer(36-40)buffer(36-36)host/src/transport/usb.cpp (6)
buffer(81-98)buffer(81-81)buffer(100-106)buffer(100-100)callback(108-116)callback(108-108)core/src/protocol/deserializer.hpp (2)
buffer(43-70)buffer(43-43)
core/src/protocol/deserializer.cpp (2)
core/src/protocol/deserializer.hpp (7)
assert_debug(110-150)assert_debug(156-173)assert_debug(191-199)id(20-20)id(22-22)awaiting_field_first_byte_(72-85)callback_(215-227)core/include/librmcs/data/datas.hpp (2)
id(62-62)id(64-64)
host/src/utility/assert.cpp (2)
core/src/coroutine/lifo.hpp (4)
noreturn(118-120)noreturn(149-149)noreturn(206-208)noreturn(235-235)core/src/utility/assert.hpp (2)
noreturn(15-18)noreturn(20-28)
firmware/src/uart/tx_buffer.hpp (1)
firmware/src/utility/ring_buffer.hpp (4)
in(52-57)in(65-70)in(93-101)out(78-85)
firmware/src/utility/ring_buffer.hpp (1)
host/src/utility/ring_buffer.hpp (18)
RingBuffer(25-32)RingBuffer(25-25)RingBuffer(34-34)RingBuffer(36-36)RingBuffer(43-46)in(60-64)in(72-76)in(99-106)out(84-91)emplace_back_n(120-142)emplace_back_n(120-120)value(179-182)value(179-179)value(187-193)value(187-187)pop_front_n(205-231)pop_front_n(205-205)pop_front_n(248-250)
host/include/librmcs/agent/alliance_board.hpp (3)
host/src/protocol/handler.cpp (8)
data(46-48)data(46-46)data(50-52)data(50-50)id(34-38)id(34-35)id(40-44)id(40-41)core/src/protocol/deserializer.hpp (4)
data(24-24)data(26-26)id(20-20)id(22-22)core/include/librmcs/data/datas.hpp (4)
data(66-66)data(68-68)id(62-62)id(64-64)
core/include/librmcs/data/datas.hpp (3)
host/src/protocol/handler.cpp (8)
id(34-38)id(34-35)id(40-44)id(40-41)data(46-48)data(46-46)data(50-52)data(50-50)core/src/protocol/deserializer.hpp (4)
id(20-20)id(22-22)data(24-24)data(26-26)host/include/librmcs/agent/alliance_board.hpp (14)
id(66-74)id(66-66)id(81-90)id(81-81)data(19-22)data(19-19)data(23-26)data(23-23)data(27-30)data(27-27)data(31-34)data(31-31)data(36-39)data(36-36)
host/src/logging/logging.hpp (1)
core/src/utility/assert.hpp (1)
assert_failed_debug(21-21)
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(136-142)address(136-136)address(144-150)address(144-144)address(159-163)address(159-159)address(165-169)address(165-165)read(108-108)size(152-157)size(152-152)serializer(171-176)serializer(171-171)firmware/src/spi/spi.hpp (1)
size(32-32)
firmware/src/spi/spi.hpp (3)
firmware/src/spi/bmi088/accel.hpp (2)
size(158-163)size(158-158)firmware/src/spi/bmi088/gyro.hpp (2)
size(152-157)size(152-152)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/uart/rx_buffer.hpp (5)
firmware/src/uart/tx_buffer.hpp (6)
dmamux_src(146-170)dmamux_src(146-146)src(172-191)src(172-172)src(193-198)src(193-193)host/src/transport/usb.cpp (4)
reinterpret_cast(143-147)i(374-396)callback(108-116)callback(108-108)firmware/src/utility/ring_buffer.hpp (4)
in(52-57)in(65-70)in(93-101)out(78-85)host/src/utility/ring_buffer.hpp (4)
in(60-64)in(72-76)in(99-106)out(84-91)host/src/protocol/stream_buffer.hpp (4)
assert_debug(194-207)assert_debug(209-219)size(121-137)size(121-121)
firmware/src/usb/interrupt_safe_buffer.hpp (2)
firmware/src/utility/lazy.hpp (3)
assert_debug(44-47)assert_debug(49-52)assert_debug(54-57)host/src/utility/ring_buffer.hpp (1)
mask(52-52)
firmware/src/usb/vendor.cpp (1)
firmware/src/usb/vendor.hpp (2)
buffer(36-40)buffer(36-36)
host/include/librmcs/protocol/handler.hpp (2)
host/src/protocol/handler.cpp (7)
PacketBuilder(118-124)PacketBuilder(126-128)other(157-163)other(157-157)Handler(150-152)Handler(154-155)Handler(165-165)host/include/librmcs/agent/alliance_board.hpp (2)
PacketBuilder(58-59)PacketBuilder(58-58)
host/src/utility/ring_buffer.hpp (1)
firmware/src/utility/ring_buffer.hpp (13)
in(52-57)in(65-70)in(93-101)out(78-85)emplace_back_n(118-145)emplace_back_n(118-120)value(187-190)value(187-187)value(195-201)value(195-195)pop_front_n(213-240)pop_front_n(213-213)pop_front_n(257-259)
🪛 Clang (14.0.6)
firmware/src/usb/helper.hpp
[error] 3-3: 'core/src/protocol/serializer.hpp' file not found
(clang-diagnostic-error)
host/src/utility/final_action.hpp
[error] 3-3: 'utility' file not found
(clang-diagnostic-error)
firmware/src/utility/interrupt_lock_guard.hpp
[error] 3-3: 'cstdint' file not found
(clang-diagnostic-error)
host/src/transport/usb.cpp
[error] 1-1: 'atomic' file not found
(clang-diagnostic-error)
core/src/protocol/deserializer.hpp
[error] 3-3: 'coroutine' file not found
(clang-diagnostic-error)
core/src/utility/assert.hpp
[error] 3-3: 'source_location' file not found
(clang-diagnostic-error)
firmware/src/usb/usb_descriptors.cpp
[error] 1-1: 'firmware/src/usb/usb_descriptors.hpp' file not found
(clang-diagnostic-error)
firmware/src/can/can.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
core/src/utility/stack_allocator.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/can/can.cpp
[error] 1-1: 'firmware/src/can/can.hpp' file not found
(clang-diagnostic-error)
core/src/protocol/protocol.hpp
[error] 3-3: 'cstdint' file not found
(clang-diagnostic-error)
firmware/src/uart/uart.cpp
[error] 1-1: 'firmware/src/uart/uart.hpp' file not found
(clang-diagnostic-error)
firmware/src/spi/spi.cpp
[error] 1-1: 'firmware/src/spi/spi.hpp' file not found
(clang-diagnostic-error)
firmware/src/app.cpp
[error] 1-1: 'firmware/src/app.hpp' file not found
(clang-diagnostic-error)
host/src/protocol/stream_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
host/src/transport/transport.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/spi/bmi088/gyro.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
host/src/protocol/handler.cpp
[error] 1-1: 'librmcs/protocol/handler.hpp' file not found
(clang-diagnostic-error)
core/src/utility/bitfield.hpp
[error] 3-3: 'concepts' file not found
(clang-diagnostic-error)
firmware/src/usb/usb_descriptors.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
host/src/utility/assert.cpp
[error] 1-1: 'core/src/utility/assert.hpp' file not found
(clang-diagnostic-error)
firmware/src/uart/uart.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/uart/tx_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/src/usb/vendor.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/src/utility/ring_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/include/tusb_config.h
[error] 39-39: CFG_TUSB_MCU must be defined
(clang-diagnostic-error)
host/include/librmcs/agent/alliance_board.hpp
[error] 3-3: 'librmcs/data/datas.hpp' file not found
(clang-diagnostic-error)
firmware/src/utility/lazy.hpp
[error] 3-3: 'atomic' file not found
(clang-diagnostic-error)
core/include/librmcs/data/datas.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
host/src/logging/logging.hpp
[error] 3-3: 'cstdio' file not found
(clang-diagnostic-error)
firmware/src/spi/bmi088/accel.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
core/src/coroutine/lifo.hpp
[error] 3-3: 'coroutine' file not found
(clang-diagnostic-error)
core/src/protocol/constant.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
core/src/protocol/serializer.hpp
[error] 3-3: 'cstddef' file not found
(clang-diagnostic-error)
firmware/src/app.hpp
[error] 3-3: 'core/src/utility/immovable.hpp' file not found
(clang-diagnostic-error)
firmware/src/gpio/gpio.cpp
[error] 1-1: 'firmware/src/gpio/gpio.hpp' file not found
(clang-diagnostic-error)
firmware/src/spi/spi.hpp
[error] 3-3: 'atomic' file not found
(clang-diagnostic-error)
firmware/src/uart/rx_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/src/usb/interrupt_safe_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
firmware/src/usb/vendor.cpp
[error] 1-1: 'firmware/src/usb/vendor.hpp' file not found
(clang-diagnostic-error)
firmware/src/utility/assert.cpp
[error] 1-1: 'core/src/utility/assert.hpp' file not found
(clang-diagnostic-error)
host/include/librmcs/protocol/handler.hpp
[error] 3-3: 'cstdint' file not found
(clang-diagnostic-error)
host/src/utility/ring_buffer.hpp
[error] 3-3: 'algorithm' file not found
(clang-diagnostic-error)
🪛 Cppcheck (2.19.0)
firmware/src/can/can.cpp
[error] 13-13: 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/uart/uart.cpp
[error] 8-8: 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)
host/src/protocol/handler.cpp
[error] 108-108: Found an exit path from function with non-void return type that has missing return statement
(missingReturn)
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)
| virtual void gyroscope_receive_callback(const GyroscopeDataView& data) = 0; | ||
| }; | ||
|
|
||
| }; // namespace librmcs::data |
There was a problem hiding this comment.
修复命名空间结束处的多余分号。
命名空间结束时不应有分号。
🔧 建议的修复
-}; // namespace librmcs::data
+} // namespace librmcs::data📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }; // namespace librmcs::data | |
| } // namespace librmcs::data |
🤖 Prompt for AI Agents
In @core/include/librmcs/data/datas.hpp at line 71, 在文件中移除命名空间结束处的多余分号：将尾部的 "};
// namespace librmcs::data" 修正为 "}" 注释仍可保留为 "// namespace
librmcs::data"；也就是定位到命名空间 librmcs::data 的结束标记并删除紧随其后的分号以符合 C++ 命名空间语法。
| ( | ||
| cd "$core_dir" | ||
| ln -s "../$mode/.clangd" .clangd | ||
| ln -s "../$mode/build/compile_commands.json" compile_commands.json | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "setup-clangd" -o -name "*clangd*" | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 118
🏁 Script executed:
git ls-files | grep -E "(setup-clangd|clangd)" | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 112
🏁 Script executed:
# Check if core/setup-clangd exists and examine its content
if [ -f "core/setup-clangd" ]; then
echo "=== File found: core/setup-clangd ==="
wc -l core/setup-clangd
echo ""
echo "=== Full content ==="
cat -n core/setup-clangd
else
echo "File not found at core/setup-clangd"
# Search for it
find . -name "setup-clangd" 2>/dev/null
fiRepository: Alliance-Algorithm/librmcs
Length of output: 1481
🏁 Script executed:
# Check the directory structure for mode directories
ls -la | grep -E "^d" | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 520
🏁 Script executed:
# Check if target .clangd files exist
echo "=== Checking host directory ==="
ls -la host/ | head -20
echo ""
echo "=== Checking firmware directory ==="
ls -la firmware/ | head -20
echo ""
echo "=== Checking for .clangd files ==="
find . -name ".clangd" -type f 2>/dev/null
echo ""
echo "=== Checking for compile_commands.json files ==="
find . -name "compile_commands.json" -type f 2>/dev/nullRepository: Alliance-Algorithm/librmcs
Length of output: 1127
🏁 Script executed:
# Check if build directories exist
echo "=== host/build directory ==="
if [ -d "host/build" ]; then
ls -la host/build/ | head -20
else
echo "host/build does not exist"
fi
echo ""
echo "=== firmware/build directory ==="
if [ -d "firmware/build" ]; then
ls -la firmware/build/ | head -20
else
echo "firmware/build does not exist"
fiRepository: Alliance-Algorithm/librmcs
Length of output: 188
🏁 Script executed:
# Check README or build documentation for how these files are created
if [ -f "README.md" ]; then
echo "=== README.md content (first 100 lines) ==="
head -100 README.md
fiRepository: Alliance-Algorithm/librmcs
Length of output: 3192
脚本创建悬空符号链接 — host/.clangd 文件缺失
验证确认了原始评论的关切：脚本在 46-50 行创建符号链接时，并未验证目标文件是否存在。
具体问题：
host/.clangd不存在于仓库中，而firmware/.clangd存在。当模式为host时，脚本会创建指向不存在目标的符号链接。host/build/compile_commands.json和firmware/build/compile_commands.json在脚本运行时都不存在（应由 CMake 构建过程生成），但脚本未对此进行说明。
需要采取以下任一行动：
- 补充缺失的
host/.clangd配置文件（与firmware/.clangd对应） - 在脚本中添加目标存在性检验，失败时提供清晰的错误信息
- 在脚本注释或文档中说明先决条件（如需先运行构建以生成
compile_commands.json）
🤖 Prompt for AI Agents
In @core/setup-clangd around lines 46 - 50, The script creates symlinks to
"../$mode/.clangd" and "../$mode/build/compile_commands.json" without checking
targets, causing dangling links when host/.clangd or compile_commands.json are
missing; either add a real host/.clangd file mirroring firmware/.clangd, or
modify the setup code around the ln -s calls to test for existence of
"../$mode/.clangd" and "../$mode/build/compile_commands.json" (use [ -e ] or
similar) and emit a clear error message and non-zero exit if missing, and/or add
a comment explaining that compile_commands.json is produced by CMake and must be
generated before running this script so users know the prerequisite.
| requires( | ||
| std::is_same_v<T, void> || (std::is_move_assignable_v<T> && std::is_default_constructible_v<T>)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
移除不必要的 default_constructible 约束
第 112 行的 requires 子句要求 T 必须是默认可构造的，但 result_ 成员（第 152 行）仅在 return_value（第 145-147 行）中通过移动赋值初始化，从未被默认构造。这个约束过于严格，限制了可以用作返回类型的类型范围。
建议移除 std::is_default_constructible_v<T> 约束。
♻️ 建议的修复
-requires(
- std::is_same_v<T, void> || (std::is_move_assignable_v<T> && std::is_default_constructible_v<T>))
+requires(std::is_same_v<T, void> || std::is_move_assignable_v<T>)
class LifoTask {或者，如果 result_ 确实需要在某处默认初始化，可以使用 std::optional<T> 包装以消除此约束。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| requires( | |
| std::is_same_v<T, void> || (std::is_move_assignable_v<T> && std::is_default_constructible_v<T>)) | |
| requires(std::is_same_v<T, void> || std::is_move_assignable_v<T>) | |
| class LifoTask { |
🤖 Prompt for AI Agents
In @core/src/coroutine/lifo.hpp around lines 111 - 112, The requires clause that
currently reads "std::is_same_v<T, void> || (std::is_move_assignable_v<T> &&
std::is_default_constructible_v<T>)" is too strict because result_ is never
default-initialized (it is assigned in return_value), so remove the
std::is_default_constructible_v<T> check so the constraint becomes
"std::is_same_v<T, void> || std::is_move_assignable_v<T>"; alternatively, if
result_ must be default-initialized elsewhere, change result_ to
std::optional<T> and keep the move-assignable constraint while eliminating the
default-constructible requirement; update the requires clause and any uses of
result_ accordingly in the coroutine's return_value and result_ member code.
| coroutine::LifoTask<void> Deserializer::process_stream() { | ||
| while (true) { | ||
| utility::assert_debug(pending_bytes_ == 0 && requested_bytes_ == 0); | ||
|
|
||
| FieldId id; | ||
| { | ||
| awaiting_field_first_byte_ = true; | ||
| auto header_bytes = co_await peek_bytes(sizeof(FieldHeader)); | ||
| // Logically impossible; stack unwinding is invalid here. | ||
| utility::assert_debug(header_bytes); | ||
| auto header = FieldHeader::CRef{header_bytes}; | ||
| id = header.get<FieldHeader::Id>(); | ||
| awaiting_field_first_byte_ = false; | ||
| } | ||
| if (id == FieldId::EXTEND) { | ||
| auto header_bytes = co_await peek_bytes(sizeof(FieldHeaderExtended)); | ||
| if (!header_bytes) [[unlikely]] { | ||
| enter_discard_mode(); | ||
| continue; | ||
| } | ||
| auto header = FieldHeaderExtended::CRef{header_bytes}; | ||
| id = header.get<FieldHeaderExtended::IdExtended>(); | ||
| consume_peeked_partial(sizeof(FieldHeader)); | ||
| } | ||
|
|
||
| bool success = false; | ||
| switch (id) { | ||
| case FieldId::CAN0: | ||
| case FieldId::CAN1: | ||
| case FieldId::CAN2: | ||
| case FieldId::CAN3: | ||
| case FieldId::CAN4: | ||
| case FieldId::CAN5: | ||
| case FieldId::CAN6: | ||
| case FieldId::CAN7: success = co_await process_can_field(id); break; | ||
| case FieldId::UART_DBUS: | ||
| case FieldId::UART0: | ||
| case FieldId::UART1: | ||
| case FieldId::UART2: | ||
| case FieldId::UART3: success = co_await process_uart_field(id); break; | ||
| case FieldId::IMU: success = co_await process_imu_field(id); break; | ||
| default: break; | ||
| } | ||
| if (!success) | ||
| enter_discard_mode(); | ||
| } | ||
| } |
There was a problem hiding this comment.
为“未知 FieldId → 丢弃模式”补一条关键注释/回归说明（避免误消费 peeked 字节）。
Line 55-58 走 enter_discard_mode() 前目前没有 consume_peeked*()，这和你们既有约束一致：未知字段不应提前消费任何已 peek 的字节，依赖错误/丢弃态重置与协程挂起语义来恢复（否则下一次 peek_bytes() 的挂起/恢复可能被破坏）。基于 learnings，建议在 default: 附近加一条“不要 consume”的注释，并补一个针对未知 FieldId 的回归用例/说明，防止未来重构时引入 consume_peeked()。
另外，Line 28-37 的 EXTEND 分支 consume_peeked_partial(sizeof(FieldHeader)) 非常“协议技巧型”，也建议加注释解释：这里只丢弃 EXTEND 的那个字节，保留 IdExtended 作为后续各 *Header 的起始字节，避免未来误改成全量消费。
🤖 Prompt for AI Agents
In @core/src/protocol/deserializer.cpp around lines 14 - 60, Add a short,
explicit comment in Deserializer::process_stream explaining that when an unknown
FieldId hits the default branch we must NOT call any consume_peeked*() before
enter_discard_mode() because peeked bytes must remain intact for the coroutine
suspend/resume and the discard/reset logic to work; likewise add a comment at
the EXTEND branch where consume_peeked_partial(sizeof(FieldHeader)) is used to
explain that we intentionally drop only the original FieldHeader byte and keep
the IdExtended byte as the start of the next header. Also add a small regression
note/test plan referencing Deserializer::process_stream, enter_discard_mode, and
the EXTEND handling to prevent future refactors from introducing a
consume_peeked() call for unknown FieldId or changing the partial consume
behavior.
| static constexpr bool use_extended_field_header(FieldId field_id) { | ||
| utility::assert_debug(field_id != FieldId::EXTEND); | ||
| return static_cast<std::uint8_t>(field_id) > 0xF; | ||
| } | ||
|
|
||
| static constexpr std::size_t required_field_header_size(FieldId field_id) { | ||
| return use_extended_field_header(field_id) ? sizeof(FieldHeaderExtended) | ||
| : sizeof(FieldHeader); | ||
| } | ||
|
|
||
| static void write_field_header(std::byte*& cursor, FieldId field_id) noexcept { | ||
| if (use_extended_field_header(field_id)) { | ||
| auto header = FieldHeaderExtended::Ref(cursor); | ||
| cursor += 1; | ||
| header.set<FieldHeaderExtended::Id>(FieldId::EXTEND); | ||
| header.set<FieldHeaderExtended::IdExtended>(field_id); | ||
| } else { | ||
| auto header = FieldHeader::Ref(cursor); | ||
| header.set<FieldHeader::Id>(field_id); | ||
| } | ||
| } |
There was a problem hiding this comment.
为 write_field_header() 的 cursor 移动策略添加静态断言或注释
- 在
FieldHeader与FieldHeaderExtended定义处添加static_assert(sizeof(FieldHeaderExtended) == sizeof(FieldHeader) + 1);或在函数中注释说明布局不变量，以防结构体尺寸改动破坏cursor前进。 - 同样适用于行 191–208、210–226 的写头函数。
| static bool process_result(core::protocol::Serializer::SerializeResult result) { | ||
| using core::protocol::Serializer; | ||
| if (result == Serializer::SerializeResult::kSuccess) [[likely]] | ||
| return true; | ||
| if (result == Serializer::SerializeResult::kBadAlloc) { | ||
| logging::get_logger().error("Transmit buffer unavailable (acquire failed)"); | ||
| return true; | ||
| } | ||
| if (result == Serializer::SerializeResult::kInvalidArgument) { | ||
| return false; | ||
| } | ||
| core::utility::assert_failed_debug(); | ||
| } |
There was a problem hiding this comment.
process_result 缺少返回语句
静态分析工具 Cppcheck 报告 108 行存在缺失的返回语句。
分析代码流程：
- 100-101 行：
kSuccess返回true - 102-105 行：
kBadAlloc返回true - 106-108 行：
kInvalidArgument返回false - 109 行：
assert_failed_debug()
问题：如果 result 不是这三个已知值之一，且在 NDEBUG 模式下（assert_failed_debug() 变为 no-op），函数没有返回路径。
🐛 建议的修复
if (result == Serializer::SerializeResult::kInvalidArgument) {
return false;
}
core::utility::assert_failed_debug();
+ return false; // 或者使用 __builtin_unreachable() 等编译器提示
}虽然理论上 SerializeResult 只有这三个枚举值，但编译器无法保证这一点，尤其在发布构建中。
🧰 Tools
🪛 Cppcheck (2.19.0)
[error] 108-108: Found an exit path from function with non-void return type that has missing return statement
(missingReturn)
🤖 Prompt for AI Agents
In @host/src/protocol/handler.cpp around lines 98 - 110, process_result can fall
through without returning when SerializeResult has an unexpected value and
assert_failed_debug is a no-op in NDEBUG builds; update process_result (the
static bool function handling core::protocol::Serializer::SerializeResult) to
ensure a definite return on all code paths by adding a final return (e.g.,
return false) after the assert_failed_debug() call (or replace the assert with a
call that always aborts) so the function always returns a bool even for unknown
Serializer::SerializeResult values.
| ~StreamBuffer() override { | ||
| if (buffer_) | ||
| finalize_buffer(); | ||
| }; |
There was a problem hiding this comment.
析构/flush 逻辑会在“已获取 buffer 但未写入”时触发断言并可能提交 0 字节传输；建议改为 release 或跳过。
~StreamBuffer() 只要 buffer_ 非空就 finalize_buffer()，而 finalize_buffer() 断言 payload_size > 0 且无分支处理 0。
建议改动示例（0 字节走 release）
void finalize_buffer() noexcept {
core::utility::assert_debug(buffer_ && current_ && end_ && current_ <= end_);
std::byte* begin = buffer_->data().data();
std::size_t payload_size = current_ - begin;
- core::utility::assert_debug(payload_size > 0);
-
- transport_.transmit(std::move(buffer_), payload_size);
+ if (payload_size == 0) {
+ transport_.release_transmit_buffer(std::move(buffer_));
+ } else {
+ transport_.transmit(std::move(buffer_), payload_size);
+ }
buffer_.reset();
current_ = end_ = nullptr;
}Also applies to: 209-219
🤖 Prompt for AI Agents
In @host/src/protocol/stream_buffer.hpp around lines 90 - 93, The destructor
~StreamBuffer() currently calls finalize_buffer() whenever buffer_ is non-null,
but finalize_buffer() asserts payload_size > 0 and has no handling for 0-byte
cases; change the destructor logic in StreamBuffer so that before calling
finalize_buffer() it checks the current payload size (or a helper like
payload_size) and if payload_size == 0 calls release() or simply drops the
buffer instead of finalize_buffer(), otherwise proceed to finalize_buffer();
apply the same guard to the similar cleanup path referenced around the 209-219
region to avoid asserting or submitting 0-byte transfers.
allocate()/~StreamBuffer()/finalize_buffer() 标为 noexcept，但内部调用的 transport_.transmit() 可能抛异常（usb.cpp 会 throw），会直接 std::terminate。
这会让“USB 提交失败/设备断开”变成不可控崩溃；建议在 StreamBuffer 内部 catch 并降级为“释放 buffer + 返回空 span/记录错误”。
建议改动示例（catch 兜底，避免 terminate）
void finalize_buffer() noexcept {
...
- if (payload_size == 0) {
- transport_.release_transmit_buffer(std::move(buffer_));
- } else {
- transport_.transmit(std::move(buffer_), payload_size);
- }
+ try {
+ if (payload_size == 0) {
+ transport_.release_transmit_buffer(std::move(buffer_));
+ } else {
+ transport_.transmit(std::move(buffer_), payload_size);
+ }
+ } catch (...) {
+ // 尽量把 buffer 归还，避免泄漏；失败就只能吞掉（noexcept 语义）
+ try {
+ if (buffer_) transport_.release_transmit_buffer(std::move(buffer_));
+ } catch (...) {}
+ }
buffer_.reset();
current_ = end_ = nullptr;
}Also applies to: 121-137, 209-219
🤖 Prompt for AI Agents
In @host/src/protocol/stream_buffer.hpp around lines 90 - 93, The noexcept
methods allocate(), ~StreamBuffer() and finalize_buffer() call
transport_.transmit() which may throw (see usb.cpp), causing std::terminate;
modify these methods to catch all exceptions from transport_.transmit() (and any
other throws inside them), log the error via the existing logger, ensure buffer
resources are released safely (free buffer_ in the catch), and return/leave the
object in a safe state (allocate()/finalize_buffer() should return an empty span
or no-op on failure and ~StreamBuffer() must not allow exceptions to propagate).
Reference the symbols allocate, finalize_buffer, ~StreamBuffer, and
transport_.transmit when applying the changes.
| ~Usb() override { | ||
| { | ||
| std::lock_guard guard{transmit_transfer_push_mutex_}; | ||
| stop_handling_events_.store(true, std::memory_order::relaxed); | ||
| } | ||
| free_transmit_transfers_.pop_front_n([](TransferWrapper* wrapper) noexcept { | ||
| wrapper->destroy(); | ||
| delete wrapper; | ||
| }); | ||
|
|
||
| libusb_release_interface(libusb_device_handle_, target_interface_); | ||
|
|
||
| // libusb_close() reliably cancels all pending transfers and invokes their callbacks, | ||
| // avoiding race conditions present in other cancellation methods | ||
| libusb_close(libusb_device_handle_); | ||
|
|
||
| if (event_thread_.joinable()) | ||
| event_thread_.join(); | ||
|
|
||
| libusb_exit(libusb_context_); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the main USB transport file
find . -name "usb.cpp" -o -name "usb.hpp" | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 98
🏁 Script executed:
# Search for stop_handling_events_ to understand its usage pattern
rg "stop_handling_events_" -n -B 2 -A 2Repository: Alliance-Algorithm/librmcs
Length of output: 1531
🏁 Script executed:
# Look for the event_thread_ and callback setup to understand synchronization
rg "event_thread_" -n -B 2 -A 2Repository: Alliance-Algorithm/librmcs
Length of output: 954
🏁 Script executed:
# Search for usb_transmit_complete_callback to see the full callback implementation
rg "usb_transmit_complete_callback" -n -B 5 -A 10Repository: Alliance-Algorithm/librmcs
Length of output: 2424
🏁 Script executed:
# Get the full usb_receive_complete_callback implementation
rg "usb_receive_complete_callback" -A 15Repository: Alliance-Algorithm/librmcs
Length of output: 2241
🏁 Script executed:
# Check the handle_events implementation and the event loop
rg "handle_events" -A 30Repository: Alliance-Algorithm/librmcs
Length of output: 4112
🏁 Script executed:
# Check if there are any other accesses to stop_handling_events_
rg "stop_handling_events_" -B 3 -A 3Repository: Alliance-Algorithm/librmcs
Length of output: 1966
usb_receive_complete_callback 中的 stop_handling_events_ 检查缺乏同步保护。
该标志在 usb_receive_complete_callback 处以 relaxed 语义加载，且无任何互斥体保护，而 usb_transmit_complete_callback 有互斥体保护。析构时以 relaxed 语义设置标志后调用 libusb_close()，但 relaxed 存储不提供释放语义，无法保证回调线程中的 relaxed 加载看到新值。若标志对接收回调不可见，即便析构已开始，用户回调仍可被调用。
建议：
- 在接收回调的标志检查处使用
load(acquire) - 在析构的标志设置处使用
store(release) - 或为接收回调的标志检查添加互斥体保护（与发送回调一致）
🤖 Prompt for AI Agents
In @host/src/transport/usb.cpp around lines 46 - 66, The receive-path check of
stop_handling_events_ in usb_receive_complete_callback lacks synchronization
(uses relaxed loads) while the destructor (~Usb) stores relaxed and
usb_transmit_complete_callback uses a mutex; fix by making the memory ordering
consistent: change the destructor's store to stop_handling_events_.store(true,
std::memory_order::release) and change the check in
usb_receive_complete_callback to use
stop_handling_events_.load(std::memory_order::acquire), or alternatively protect
the receive-path check with the same transmit mutex
(transmit_transfer_push_mutex_) so both callbacks use the same lock as
usb_transmit_complete_callback.
🧩 Analysis chain
🏁 Script executed:
find . -name "usb.cpp" -path "*/transport/*" | head -5Repository: Alliance-Algorithm/librmcs
Length of output: 98
🏁 Script executed:
cat -n host/src/transport/usb.cpp | head -100Repository: Alliance-Algorithm/librmcs
Length of output: 4043
🏁 Script executed:
cat -n host/src/transport/usb.cpp | sed -n '330,350p'Repository: Alliance-Algorithm/librmcs
Length of output: 909
🏁 Script executed:
cat -n host/src/transport/usb.cpp | sed -n '400,450p'Repository: Alliance-Algorithm/librmcs
Length of output: 2309
🏁 Script executed:
cat -n host/src/transport/usb.cpp | grep -A 30 "usb_transmit_complete_callback"Repository: Alliance-Algorithm/librmcs
Length of output: 2971
🏁 Script executed:
wc -l host/src/transport/usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 100
🏁 Script executed:
cat -n host/src/transport/usb.cpp | sed -n '375,410p'Repository: Alliance-Algorithm/librmcs
Length of output: 1778
🏁 Script executed:
# Verify TransferWrapper structure to understand what gets recycled
grep -n "class TransferWrapper\|struct TransferWrapper" host/src/transport/usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 127
🏁 Script executed:
# Check libusb transfer status enum values and their meaning
grep -n "LIBUSB_TRANSFER" host/src/transport/usb.cpp | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 245
🏁 Script executed:
cat -n host/src/transport/usb.cpp | sed -n '119,160p'Repository: Alliance-Algorithm/librmcs
Length of output: 1777
🏁 Script executed:
# Check what the libusb_transfer status field contains and verify the issue
grep -n "status" host/src/transport/usb.cpp | head -15Repository: Alliance-Algorithm/librmcs
Length of output: 52
🏁 Script executed:
# Verify the complete transmit callback to see if TransferWrapper provides access to status
cat -n host/src/transport/usb.cpp | sed -n '342,372p'Repository: Alliance-Algorithm/librmcs
Length of output: 1563
receive/transmit 回调必须检查 libusb_transfer::status，否则失败/取消的传输会被误当成功处理。
目前 usb_transmit_complete_callback()（行 398-409）和 usb_receive_complete_callback()（行 411-438）都完全忽视 transfer->status 字段：
- Transmit 端：失败的 transfer 被直接回收到
free_transmit_transfers_池中（行 408），后续会被重用于新的发送，导致带有错误状态的 transfer 被重新提交。 - Receive 端：失败/取消的 transfer 仍然被无条件地重新提交（行 423），可能陷入失败重提交循环，或在重提交失败时触发
std::terminate()。 - 同步问题：析构函数使用
relaxed存储stop_handling_events_标志（行 49），但回调用relaxed加载（行 402, 412），不足以保证正确的同步顺序。
在 LIBUSB_TRANSFER_CANCELLED、LIBUSB_TRANSFER_STALL、LIBUSB_TRANSFER_NO_DEVICE 等失败状态下，应当区分处理：至少记录错误，停止重提交或回收，避免 wrapper 污染和拆卸期异常。
建议改动示例
void usb_receive_complete_callback(libusb_transfer* transfer) {
- if (stop_handling_events_.load(std::memory_order::relaxed)) [[unlikely]] {
+ if (stop_handling_events_.load(std::memory_order::acquire)) [[unlikely]] {
destroy_libusb_transfer(transfer);
return;
}
+
+ if (transfer->status != LIBUSB_TRANSFER_COMPLETED) [[unlikely]] {
+ logger_.error("Receive transfer failed: status={}", int(transfer->status));
+ destroy_libusb_transfer(transfer);
+ return;
+ }
if (transfer->actual_length > 0) {
const auto first = reinterpret_cast<std::byte*>(transfer->buffer);
const auto size = static_cast<std::size_t>(transfer->actual_length);
receive_callback_({first, size});
}
int ret = libusb_submit_transfer(transfer);（Transmit 回调同理，访问 wrapper->transfer_->status，非 COMPLETED 状态需销毁而非回收）
Also applies to: 336-340, 398-409
| void transmit(std::unique_ptr<ITransportBuffer> buffer, size_t size) override { | ||
| if (size > core::protocol::kProtocolBufferSize) | ||
| throw std::invalid_argument("Transmit size exceeds maximum transfer length"); | ||
|
|
||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto& transfer = static_cast<TransferWrapper*>(buffer.get())->transfer_; | ||
| transfer->length = static_cast<int>(size); | ||
|
|
||
| int ret = libusb_submit_transfer(transfer); | ||
| if (ret != 0) [[unlikely]] { | ||
| throw std::runtime_error( | ||
| std::format( | ||
| "Failed to submit transmit transfer: {} ({})", ret, libusb_errname(ret))); | ||
| } | ||
|
|
||
| // If success: Ownership is transferred to libusb | ||
| std::ignore = buffer.release(); | ||
| } | ||
|
|
||
| void release_transmit_buffer(std::unique_ptr<ITransportBuffer> buffer) override { | ||
| std::lock_guard guard{transmit_transfer_push_mutex_}; | ||
|
|
||
| // NOLINTNEXTLINE(cppcoreguidelines-pro-type-static-cast-downcast) | ||
| auto wrapper = static_cast<TransferWrapper*>(buffer.release()); | ||
| free_transmit_transfers_.emplace_back(wrapper); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 513
🏁 Script executed:
fd -type f -name "usb.cpp" | head -10Repository: Alliance-Algorithm/librmcs
Length of output: 240
🏁 Script executed:
fd -type f -path "*/host/*transport*" | head -20Repository: Alliance-Algorithm/librmcs
Length of output: 240
🏁 Script executed:
fd "usb.cpp" .Repository: Alliance-Algorithm/librmcs
Length of output: 98
🏁 Script executed:
fd -path "*host*usb.cpp" .Repository: Alliance-Algorithm/librmcs
Length of output: 238
🏁 Script executed:
find . -name "usb.cpp" 2>/dev/nullRepository: Alliance-Algorithm/librmcs
Length of output: 98
🏁 Script executed:
wc -l ./host/src/transport/usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 102
🏁 Script executed:
sed -n '70,120p' ./host/src/transport/usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 2052
🏁 Script executed:
rg "transmit\(" ./host/src/transport/usb.cpp -A 2 -B 2Repository: Alliance-Algorithm/librmcs
Length of output: 619
🏁 Script executed:
rg -t cpp "transmit\(" ./host/src --A 1 | head -40Repository: Alliance-Algorithm/librmcs
Length of output: 475
🏁 Script executed:
find ./host -name "*.hpp" -o -name "*.h" | xargs grep -l "ITransportBuffer\|transmit" | head -5Repository: Alliance-Algorithm/librmcs
Length of output: 234
🏁 Script executed:
cat -n ./host/src/transport/transport.hppRepository: Alliance-Algorithm/librmcs
Length of output: 6316
🏁 Script executed:
rg "\.transmit\(" ./host -B 2Repository: Alliance-Algorithm/librmcs
Length of output: 302
🏁 Script executed:
rg "if.*!.*throw std::invalid_argument" ./host/src/transport/usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 52
🏁 Script executed:
sed -n '1,150p' ./host/src/transport/usb.cpp | head -100Repository: Alliance-Algorithm/librmcs
Length of output: 3343
对 transmit() 和 release_transmit_buffer() 的 buffer 参数缺少显式非空校验，导致前置条件未被强制执行。
接口文档（transport.hpp 第 87、101 行）明确声明 buffer 必须非空，但实现中未校验。而同一类中的 receive() 方法已遵循 fail-fast 模式对 callback 参数进行了空检查（第 103-105 行）。若上层传入空 buffer，会直接对 nullptr 进行 static_cast，引发未定义行为。建议补充显式校验以保持代码一致性并增强防御性：
void transmit(std::unique_ptr<ITransportBuffer> buffer, size_t size) override {
+ if (!buffer)
+ throw std::invalid_argument("Transmit buffer cannot be null");
if (size > core::protocol::kProtocolBufferSize)
throw std::invalid_argument("Transmit size exceeds maximum transfer length");
void release_transmit_buffer(std::unique_ptr<ITransportBuffer> buffer) override {
+ if (!buffer)
+ throw std::invalid_argument("Release buffer cannot be null");
std::lock_guard guard{transmit_transfer_push_mutex_};🤖 Prompt for AI Agents
In @host/src/transport/usb.cpp around lines 81 - 107, transmit() 和
release_transmit_buffer() 没有对传入的 std::unique_ptr<ITransportBuffer> buffer
做显式非空校验，可能在 static_cast<TransferWrapper*>(buffer.get()/buffer.release())
上导致未定义行为；在这两个方法开头增加对 buffer 的空检查（例如 if (!buffer) throw
std::invalid_argument("buffer must not be null");），在确认非空后再执行 static_cast/访问
transfer_ 或 release，并保持与类中 receive() 的 fail-fast 风格一致，确保错误类型与接口文档预期一致且不会在后续调用中对
nullptr 进行操作。
| // TODO: Replace abrupt termination with a flag and exception-based error handling | ||
| std::terminate(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd host/src/transport && wc -l usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 81
🏁 Script executed:
cd host/src/transport && sed -n '420,450p' usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 1220
🏁 Script executed:
cd host/src/transport && grep -n "std::terminate\|exception\|try\|catch" usb.cpp | head -30Repository: Alliance-Algorithm/librmcs
Length of output: 271
🏁 Script executed:
cd host/src/transport && grep -n "std::terminate" usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 103
🏁 Script executed:
cd host/src/transport && sed -n '340,365p' usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 1152
🏁 Script executed:
cd host/src/transport && sed -n '400,440p' usb.cppRepository: Alliance-Algorithm/librmcs
Length of output: 1564
立即替换 std::terminate() 为错误标志机制。
usb_receive_complete_callback() 中对设备拔出或重提交失败的处理不应该导致整个 SDK 进程崩溃。设备断开连接是正常的外设操作事件，直接调用 std::terminate() 会严重影响 SDK 使用体验。
该文件已在其他地方使用 stop_handling_events_ 标志进行错误处理（第 413 行），应采用相同模式替代 std::terminate()，向上层报告错误状态而不是硬崩溃。
🤖 Prompt for AI Agents
In @host/src/transport/usb.cpp around lines 435 - 437, Replace the abrupt
std::terminate() in usb_receive_complete_callback with the same error-flag
pattern used elsewhere: set stop_handling_events_ = true, record an appropriate
error status (e.g., set a last_error_ or usb_error_ member with a descriptive
code/message), and ensure the function returns cleanly so upper layers can
detect the flag and handle the device-disconnect/re-submit-failure; do not
terminate the process or swallow the error—propagate the status via the existing
event loop/condition variable or status-check path already used around
stop_handling_events_.
LibRMCS v3 发布说明（中文）
概述
本次 v3 版本为一次大规模重构与功能迁移，包含核心协议与协程框架的新增、固件完整实现、主机端传输与工具库重建，同时移除了大量旧的示例与驱动实现。改动范围广，涉及公共 API 的新增与若干旧 API 的删除。
主要变更要点
1) 移除与精简（显著删除）
这些删除会显著改变以前基于这些类型/示例的向后兼容性。
2) 新增核心数据与协议层
3) 协程与内存管理工具
4) 固件（完整新增实现）
5) 主机端（Host）重建
6) 构建系统与配置
兼容性与影响
变更量纲要
关键设计亮点
建议的代码审查重点
总体而言，v3 是一次面向协议、固件与主机端重构与重写，新增了现代化的协程与传输抽象，同时移除了大量旧实现。合并前应重点评审协议的兼容性、错误路径与固件/主机端的资源/并发边界。