Feature/full tracker implementation - #16
Conversation
Enabled real-time 3D visualization of predicted armor plates in the world coordinate system, allowing for intuitive debugging via Foxglove Studio.
Fixed an issue where the visualization module incorrectly displayed data from legacy namespaces, causing ghosting effects or misaligned 3D markers in Foxglove.
…gle target Developed a predictive motion model to estimate the future trajectory of a single armor plate. Multi-target logic is integrated but remains pending validation.
Cleaned up the codebase by deleting dead code, unused imports, and obsolete functions to improve maintainability and readability.
Walkthrough移除基于共享内存的 ControlSystem,引入模板化 Feishu IPC 并将 Runtime 切换为 Feishu 驱动;新增 Tracker(ArmorFilter/Decider/RobotState/Snapshot/State/优先级),引入通用 EKF 与多项时间/角度/坐标工具;PoseEstimator、新的可视化接口及若干 API/类型重命名同步更新。 Changes
Sequence Diagram(s)sequenceDiagram
participant AutoAim as AutoAim Process
participant Feishu as Feishu IPC (shm)
participant Runtime as Runtime Loop
participant Tracker as Tracker
participant PoseEst as PoseEstimator
participant Visual as Visualization
Runtime->>Feishu: if feishu.updated() then fetch() -> ControlState
Feishu-->>Runtime: ControlState (includes camera_to_odom_transform, invincible_devices)
Runtime->>PoseEst: set_camera2world_transform(control.camera_to_odom_transform)
Runtime->>Tracker: filter_armors(armors_2d)
Tracker-->>Runtime: filtered_armors_2d
Runtime->>PoseEst: solved_pnp(filtered_armors_2d) -> armors_3d
Runtime->>Tracker: decide(armors_3d, t)
Tracker-->>Runtime: Output{state, target_id, optional snapshot}
alt state == Tracking && snapshot present
Runtime->>Visual: predicted_armors(snapshot)
Visual-->>Runtime: render predicted armors
end
Runtime->>Visual: solved_pnp_armors(armors_3d)
Runtime->>Feishu: commit(ControlState{invincible_devices, bullet_speed})
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (23)
src/module/tracker/state.cpp (2)
61-63: 多余的分号。
if语句块后的分号是多余的。🔎 建议修复
if (temp_lost_count > max_switch_count) { set_state(State::Lost); - }; + }if (temp_lost_count > max_allow) { set_state(State::Lost); - }; + }Also applies to: 77-79
94-98: 阈值常量可以考虑使用static constexpr。这些阈值是编译期常量,可以声明为
static constexpr以避免每个实例都存储副本。🔎 建议修复
- const int min_detect_count { 5 }; - const int outpost_max_temp_lost_count { 75 }; - const int normal_max_temp_lost_count { 15 }; - const int max_switch_count { 200 }; - const std::chrono::milliseconds timeout { std::chrono::milliseconds(100) }; + static constexpr int min_detect_count { 5 }; + static constexpr int outpost_max_temp_lost_count { 75 }; + static constexpr int normal_max_temp_lost_count { 15 }; + static constexpr int max_switch_count { 200 }; + static constexpr std::chrono::milliseconds timeout { std::chrono::milliseconds(100) };src/utility/time.hpp (1)
5-8: 时间差计算工具函数实现正确
delta_time函数实现简洁清晰,返回两个时间点之间的秒级时间差。参数命名late和early明确表达了预期的时间顺序。可选建议:考虑添加文档注释说明参数顺序(late 应晚于 early),或在调试模式下添加断言验证
late >= early,避免潜在的负值时间差。🔎 可选的参数验证增强
+#include <cassert> + namespace rmcs::util { constexpr auto delta_time(std::chrono::steady_clock::time_point const& late, std::chrono::steady_clock::time_point const& early) -> auto { + assert(late >= early && "late time_point must be >= early time_point"); return std::chrono::duration<double>(late - early); }或添加文档注释:
namespace rmcs::util { +/// 计算两个时间点之间的时间差(秒) +/// @param late 较晚的时间点 +/// @param early 较早的时间点 +/// @return 时间差(秒),以 double 表示 +/// @note 调用者需确保 late >= early constexpr auto delta_time(std::chrono::steady_clock::time_point const& late, std::chrono::steady_clock::time_point const& early) -> auto { return std::chrono::duration<double>(late - early); }src/utility/robot/color.hpp (1)
12-22: CampColor 字符串转换函数实现正确
to_string函数为CampColor枚举提供了字符串表示,实现清晰且涵盖所有枚举值。可选建议:第 21 行的 fallback
return "UNKNOWN"在完整的 switch 语句下实际不可达,但作为防御性编程是合理的。如果想避免某些编译器的"unreachable code"警告,可以移除此行,或使用[[fallthrough]]特性(需要添加 default 分支)。另一个现代化选项:考虑返回
std::string_view而非const char*,以提供更安全的字符串视图类型。🔎 可选的现代化改进
选项 1:使用 std::string_view(推荐)
#pragma once #include <cstdint> +#include <string_view> namespace rmcs { enum class CampColor : uint8_t { UNKNOWN, RED, BLUE, }; -inline auto to_string(CampColor color) noexcept -> const char* { +inline auto to_string(CampColor color) noexcept -> std::string_view { switch (color) { case CampColor::UNKNOWN: return "UNKNOWN"; case CampColor::RED: return "RED"; case CampColor::BLUE: return "BLUE"; } return "UNKNOWN"; }选项 2:移除不可达的 fallback return
inline auto to_string(CampColor color) noexcept -> const char* { switch (color) { case CampColor::UNKNOWN: return "UNKNOWN"; case CampColor::RED: return "RED"; case CampColor::BLUE: return "BLUE"; } - return "UNKNOWN"; + __builtin_unreachable(); // 或使用 [[unreachable]] (C++23) }src/kernel/tracker.hpp (1)
13-23: Tracker API 设计清晰,PIMPL 模式使用恰当。整体 API 设计良好:
- 使用 PIMPL 模式隐藏实现细节,提供良好的编译隔离
initialize方法使用std::expected进行错误处理,避免异常且符合现代 C++ 实践- 方法职责清晰:配置初始化、状态设置、装甲板过滤、目标决策
Line 18 的
set_invincible_armors方法按值传递DeviceIds。如果DeviceIds类型较大,建议改为const DeviceIds&以避免不必要的拷贝:🔎 可选优化建议
- auto set_invincible_armors(DeviceIds devices) -> void; + auto set_invincible_armors(const DeviceIds& devices) -> void;如果
DeviceIds是轻量级类型(如位集或小型结构),则当前设计已足够高效。src/utility/math/angle.hpp (1)
5-7: 建议增强代码可读性:使用显式返回类型和添加文档。使用
atan2(sin(angle), cos(angle))实现角度归一化到[-π, π]范围是标准做法,逻辑正确。函数在生产代码中被广泛使用。建议 1:使用显式返回类型
将返回类型从
auto改为显式的double,提高代码可读性:建议的改进
-inline auto normalize_angle(double angle) -> auto { +inline auto normalize_angle(double angle) -> double { return std::atan2(std::sin(angle), std::cos(angle)); }建议 2:添加文档注释
建议添加注释说明归一化范围为
[-π, π]:建议的文档
+/// 将角度归一化到 [-π, π] 范围 +/// @param angle 输入角度(弧度) +/// @return 归一化后的角度(弧度) inline auto normalize_angle(double angle) -> double { return std::atan2(std::sin(angle), std::cos(angle)); }代码库中存在
normalize_angle_90函数(test/solve_pnp.cpp:141-159),它归一化到[-π/2, π/2]范围。两个函数的使用场景已明确区分(生产代码 vs 测试代码),混淆风险低。src/kernel/visualization.cpp (1)
138-161: 将魔法数字提取为命名常量Line 156 中的硬编码值
15. / 180 * CV_PI(15度的俯仰角)应该提取为命名常量,以提高代码可读性和可维护性。🔎 建议的重构
在文件顶部或类定义中添加常量:
namespace { constexpr double kArmorPitchAngleDeg = 15.0; constexpr double kArmorPitchAngleRad = kArmorPitchAngleDeg * CV_PI / 180.0; }然后在 Line 156 处使用:
- armor.orientation = util::euler_to_quaternion(angle, 15. / 180 * CV_PI, 0); + armor.orientation = util::euler_to_quaternion(angle, kArmorPitchAngleRad, 0);src/module/tracker/state.hpp (1)
10-16: 建议为枚举值添加简要注释。当前每个枚举值后只有
//占位符。建议添加简要说明,例如:
Lost: 未检测到目标Detecting: 正在检测目标Tracking: 正在跟踪目标TemporaryLost: 临时丢失目标Switching: 正在切换目标这将提高代码可读性,帮助理解状态机的行为。
src/utility/rclcpp/visual/transform.hpp (2)
9-15: Config 中的引用成员需注意生命周期
Config::rclcpp是引用类型,调用者需确保RclcppNode的生命周期长于Transform实例。当前在component.cpp中的使用是安全的(rclcpp是成员变量),但建议在文档或注释中说明此约束。
17-21: 移动语义缺失类已删除拷贝构造和赋值,但未显式声明移动构造和移动赋值。对于持有
unique_ptr的类型,编译器会自动生成移动操作,但显式声明= default可提高代码可读性:🔎 建议添加移动语义声明
Transform(const Transform&) = delete; Transform& operator=(const Transform&) = delete; + Transform(Transform&&) = default; + Transform& operator=(Transform&&) = default;src/utility/rclcpp/visual/transform.cpp (1)
32-40: 建议使用显式void返回类型以提高可读性
move和update方法使用auto返回类型但没有返回值,建议使用显式的-> void以保持与公共 API 的一致性。建议修复
- auto move(const Translation& t, const Orientation& q) noexcept { + auto move(const Translation& t, const Orientation& q) noexcept -> void { t.copy_to(msg.transform.translation); q.copy_to(msg.transform.rotation); } - auto update() noexcept { + auto update() noexcept -> void { msg.header.stamp = rclcpp_clock.now(); rclcpp_pub->publish(msg); }src/module/tracker/armor_filter.cpp (1)
29-33:void函数使用return语句风格不一致
set_enemy_color和set_invincible_armors返回void,但使用了return pimpl->...语法。虽然语法上正确,但风格上不常见,建议与其他代码保持一致。建议修复
-auto ArmorFilter::set_enemy_color(CampColor color) -> void { return pimpl->set_enemy_color(color); } +auto ArmorFilter::set_enemy_color(CampColor color) -> void { pimpl->set_enemy_color(color); } auto ArmorFilter::set_invincible_armors(DeviceIds devices) -> void { - return pimpl->set_invincible_armors(devices); + pimpl->set_invincible_armors(devices); }src/module/tracker/decider.cpp (1)
107-129: 未使用的常量mode1和mode2
mode1和mode2已定义但从未使用。如果这些是预留的配置模式,建议添加注释说明用途,否则应移除以避免混淆。src/kernel/tracker.cpp (1)
14-16:state_machine成员未使用
state_machine已声明但在任何方法中都未使用。如果这是 WIP 代码,请考虑添加注释说明计划用途。src/kernel/feishu.hpp (2)
30-45:fetch()缺少requires约束
commit()使用requires(util::ShmRoleSelector<Side, StateType>::is_sender)约束,但fetch()没有对应的约束。虽然static_assert会在编译时捕获错误,但添加requires约束可以提供更好的错误信息和 IDE 支持。建议修复
template <typename StateType> auto fetch() noexcept -> std::optional<StateType> + requires(!util::ShmRoleSelector<Side, StateType>::is_sender) {
51-58:get_client仅支持两种状态类型如果传入的
DataType既不是AutoAimState也不是ControlState,函数会默认返回control_client。建议添加static_assert确保类型安全。建议修复
template <typename DataType> auto get_client() noexcept -> auto& { if constexpr (std::same_as<DataType, AutoAimState>) { return auto_aim_client; - } else { + } else if constexpr (std::same_as<DataType, ControlState>) { return control_client; + } else { + static_assert(sizeof(DataType) == 0, "Unsupported state type"); } }src/module/predictor/robot_state.cpp (1)
145-162: 误差计算逻辑重复
get_errorlambda(Lines 147-151)和min_errorlambda(Lines 158-161)实现了相同的逻辑。建议复用以减少代码重复和维护负担。建议修复
+ auto get_error = [&](auto const& pred) { + auto ypd_pred = util::xyz2ypd(pred.template head<3>()); + return std::abs(util::normalize_angle(ypr_in_world[0] - pred[3])) + + std::abs(util::normalize_angle(ypd_in_world[0] - ypd_pred[0])); + }; + auto it = - std::ranges::min_element(armors_xyza, [&](auto const& a_xyza, auto const& b_xyza) { - auto get_error = [&](auto const& pred) { - auto ypd_pred = util::xyz2ypd(pred.template head<3>()); - return std::abs(util::normalize_angle(ypr_in_world[0] - pred[3])) - + std::abs(util::normalize_angle(ypd_in_world[0] - ypd_pred[0])); - }; - - return get_error(a_xyza) < get_error(b_xyza); - }); + std::ranges::min_element(armors_xyza, {}, + [&](auto const& xyza) { return get_error(xyza); }); int best_id = static_cast<int>(std::distance(armors_xyza.begin(), it)); - auto min_error = [&](const auto& pred) { - auto ypd_pred = util::xyz2ypd(pred.template head<3>()); - return std::abs(util::normalize_angle(ypr_in_world[0] - pred[3])) - + std::abs(util::normalize_angle(ypd_in_world[0] - ypd_pred[0])); - }(*it); + auto min_error_val = get_error(*it); - return { best_id, min_error, (min_error < angle_error_threshold) }; + return { best_id, min_error_val, (min_error_val < angle_error_threshold) };src/utility/math/conversion.hpp (2)
32-42: xyz2ypd 存在潜在的除零和数值稳定性问题当
x和y同时接近零时,std::atan2(y, x)虽然可以处理,但std::sqrt(x * x + y * y)在第 38 行作为atan2的第二个参数时,如果x = y = 0,会导致atan2(z, 0)的结果可能不稳定。此外,此函数也标记为
constexpr,但使用了std::atan2和std::sqrt,这些在 C++20 之前不是constexpr。🔎 建议添加边界检查
inline constexpr auto xyz2ypd(Eigen::Vector3d const& xyz) -> Eigen::Vector3d { const auto x = xyz[0]; const auto y = xyz[1]; const auto z = xyz[2]; + const auto xy_norm = std::sqrt(x * x + y * y); const auto yaw = std::atan2(y, x); - const auto pitch = std::atan2(z, std::sqrt(x * x + y * y)); + const auto pitch = std::atan2(z, xy_norm); const auto distance = std::sqrt(x * x + y * y + z * z); const auto result = Eigen::Vector3d { yaw, pitch, distance }; return result; }
73-133: eulers 函数实现复杂,建议添加文档和单元测试这是一个将四元数转换为欧拉角的复杂函数,支持不同的轴序和内/外旋转。以下几点需要注意:
- 第 103-119 行的万向锁处理逻辑较为复杂,需要确保正确性
- 第 118 行
eulers[0] = 2 * half_diff;与第 114 行eulers[2] = -2 * half_diff;符号不一致,请确认这是预期行为- 函数末尾有多余的分号(第 133 行)
建议为此函数添加详细的文档说明轴序约定,并编写单元测试覆盖各种边界情况。
🔎 移除多余分号
-}; +}src/module/predictor/ekf_parameter.hpp (1)
186-206:R矩阵中的观测噪声参数可能需要调优观测噪声协方差的计算使用了硬编码的常数(如
4e-3、1/200、9e-2)。这些值可能需要根据实际传感器特性进行调优。建议将这些魔法数字提取为命名常量或配置参数,便于后续调整。
src/utility/math/kalman_filter/ekf.hpp (3)
6-8:DefaultAdd和DefaultSubtract应声明为constexpr这些 lambda 表达式是无状态的,可以声明为
inline constexpr以允许在编译时使用。🔎 建议修改
-inline auto DefaultAdd = [](auto const& a, auto const& b) { return a + b; }; +inline constexpr auto DefaultAdd = [](auto const& a, auto const& b) { return a + b; }; -inline auto DefaultSubtract = [](auto const& a, auto const& b) { return a - b; }; +inline constexpr auto DefaultSubtract = [](auto const& a, auto const& b) { return a - b; };
100-115: 缺少对z_sub_op的static_assert验证函数对
x_add_op进行了签名验证(第 114-115 行),但没有对z_sub_op进行类似验证。为了一致性和更好的错误消息,建议添加。🔎 建议添加验证
static_assert(std::is_invocable_r_v<XVec, AddOp, XVec, XVec>, "x_add_op 必须接受两个 XVec 并返回 XVec"); + static_assert(std::is_invocable_r_v<ZVec, SubOp, ZVec, ZVec>, + "z_sub_op 必须接受两个 ZVec 并返回 ZVec");
127-130: 卡尔曼增益计算可以简化当前实现:
auto K = P_ * H.transpose() * S.ldlt().solve(RMat::Identity());这等价于先求
S^-1,然后乘以P_ * H^T。更直接的做法是利用solve直接求解S * K^T = (P * H^T)^T:auto K = S.ldlt().solve(H * P_).transpose();不过当前实现也是正确的,这只是一个可选的性能优化。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (43)
config/config.yamlsrc/component.cppsrc/kernel/control_system.cppsrc/kernel/control_system.hppsrc/kernel/feishu.hppsrc/kernel/pose_estimator.cppsrc/kernel/pose_estimator.hppsrc/kernel/predictor.cppsrc/kernel/predictor.hppsrc/kernel/tracker.cppsrc/kernel/tracker.hppsrc/kernel/visualization.cppsrc/kernel/visualization.hppsrc/module/debug/visualization/armor_visualizer.cppsrc/module/debug/visualization/armor_visualizer.hppsrc/module/predictor/ekf_parameter.hppsrc/module/predictor/robot_state.cppsrc/module/predictor/robot_state.hppsrc/module/predictor/snapshot.hppsrc/module/tracker/armor_filter.cppsrc/module/tracker/armor_filter.hppsrc/module/tracker/decider.cppsrc/module/tracker/decider.hppsrc/module/tracker/state.cppsrc/module/tracker/state.hppsrc/runtime.cppsrc/utility/math/angle.hppsrc/utility/math/conversion.hppsrc/utility/math/kalman_filter/ekf.hppsrc/utility/rclcpp/visual/armor.cppsrc/utility/rclcpp/visual/transform.cppsrc/utility/rclcpp/visual/transform.hppsrc/utility/robot/armor.hppsrc/utility/robot/color.hppsrc/utility/robot/id.hppsrc/utility/robot/priority.hppsrc/utility/robot/robot.hppsrc/utility/shared/client.hppsrc/utility/shared/context.hppsrc/utility/time.hpptest/CMakeLists.txttest/asset.ymltest/feishu_test.cpp
💤 Files with no reviewable changes (4)
- src/kernel/predictor.cpp
- src/kernel/control_system.cpp
- src/kernel/control_system.hpp
- src/kernel/predictor.hpp
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/module/tracker/decider.cppsrc/module/tracker/state.cppsrc/utility/rclcpp/visual/armor.cppsrc/module/tracker/armor_filter.cppsrc/utility/rclcpp/visual/transform.cpptest/feishu_test.cppsrc/kernel/tracker.cppsrc/module/predictor/robot_state.cppsrc/module/debug/visualization/armor_visualizer.cppsrc/component.cppsrc/kernel/pose_estimator.cppsrc/kernel/visualization.cppsrc/runtime.cpp
📚 Learning: 2025-12-15T21:13:59.238Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
Applied to files:
test/asset.yml
📚 Learning: 2025-12-15T21:13:54.155Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:54.155Z
Learning: In C++ tests under the rmcs_auto_aim_v2 project, prefer local asset testing by reading resources from TEST_ASSETS_ROOT (if set) or fallback to /tmp/auto_aim. Use simple asset filenames (e.g., 'blue-0.5m.jpg') and ensure assets are pre-downloaded by running download_assets.sh against test/asset.yml. Remove any network-download logic from tests to rely on deterministic local assets. This guideline should apply to all test files in the test directory (not just test/solve_pnp.cpp) to improve test reliability and speed.
Applied to files:
test/feishu_test.cpp
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/kernel/pose_estimator.hppsrc/component.cppsrc/module/debug/visualization/armor_visualizer.hppsrc/kernel/visualization.cpp
🧬 Code graph analysis (24)
src/utility/robot/id.hpp (2)
src/utility/tf/static_tf.hpp (2)
contains(148-151)contains(148-148)test/device_id.cpp (1)
generate_devices(7-15)
src/module/predictor/robot_state.hpp (3)
src/module/predictor/robot_state.cpp (8)
t(39-51)t(39-39)armor(22-30)armor(22-22)armor(53-81)armor(53-53)armor(133-165)armor(133-133)src/module/predictor/snapshot.hpp (2)
t(15-19)t(15-15)src/module/predictor/ekf_parameter.hpp (2)
armor(16-31)armor(16-16)
src/module/predictor/snapshot.hpp (4)
src/module/predictor/robot_state.cpp (3)
t(39-51)t(39-39)ekf(32-32)src/module/predictor/robot_state.hpp (1)
t(20-20)src/module/predictor/ekf_parameter.hpp (4)
dt(85-102)dt(85-85)dt(177-184)dt(177-177)src/utility/time.hpp (2)
delta_time(5-8)delta_time(5-6)
src/module/tracker/decider.cpp (7)
src/kernel/visualization.cpp (2)
armors(133-136)armors(133-133)src/kernel/visualization.hpp (1)
armors(30-30)src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)src/kernel/tracker.hpp (2)
armors(19-19)armors(21-22)src/module/predictor/robot_state.hpp (3)
t(20-20)armor(22-22)armor(23-23)src/utility/time.hpp (2)
delta_time(5-8)delta_time(5-6)src/module/identifier/armor_detection.hpp (1)
namespace rmcs::identifier {(12-22)
src/module/tracker/state.cpp (2)
src/module/tracker/state.hpp (2)
found(18-18)state(20-20)src/utility/robot/id.hpp (1)
OUTPOST(164-169)
src/module/tracker/armor_filter.hpp (1)
src/module/tracker/armor_filter.cpp (4)
color(7-7)color(7-7)devices(9-9)devices(9-9)
src/utility/math/angle.hpp (1)
test/solve_pnp.cpp (1)
normalize_angle_90(141-160)
src/utility/time.hpp (4)
src/utility/details.hpp (1)
use_memory_header(4-9)src/utility/coroutine/context.hpp (1)
namespace rmcs {(3-7)src/utility/monostate.hpp (1)
namespace rmcs {(3-5)src/utility/singleton/running.cpp (1)
running(4-17)
src/utility/rclcpp/visual/armor.cpp (2)
src/utility/rclcpp/visual/armor.hpp (4)
Armor(7-38)Armor(26-26)Armor(9-36)struct Impl(34-34)src/utility/rclcpp/visual/posture.cpp (1)
RCL_SYSTEM_TIME(11-37)
src/kernel/feishu.hpp (2)
src/kernel/control_system.hpp (2)
namespace rmcs::kernel {(5-21)class ControlSystem {(7-19)src/kernel/control_system.cpp (3)
F(7-41)F(15-21)update_command(16-21)
src/module/tracker/armor_filter.cpp (4)
src/module/tracker/armor_filter.hpp (2)
color(10-10)devices(12-12)src/kernel/tracker.cpp (10)
devices(48-50)devices(48-48)armors(52-56)armors(52-52)armors(58-61)armors(58-58)armors(65-72)armors(65-65)set_invincible_armors(83-85)set_invincible_armors(83-83)src/kernel/tracker.hpp (3)
devices(18-18)armors(19-19)armors(21-22)src/module/identifier/armor_detection.cpp (1)
ArmorDetection(52-53)
src/utility/rclcpp/visual/transform.cpp (1)
src/utility/rclcpp/visual/transform.hpp (3)
Transform(17-17)Transform(18-18)Transform(20-20)
src/kernel/tracker.cpp (4)
src/module/tracker/armor_filter.hpp (1)
devices(12-12)src/module/tracker/decider.hpp (1)
armors(23-23)src/module/predictor/robot_state.hpp (3)
t(20-20)armor(22-22)armor(23-23)src/module/tracker/state.hpp (1)
found(18-18)
src/module/tracker/state.hpp (1)
src/module/tracker/state.cpp (2)
state(105-119)state(105-105)
src/utility/math/kalman_filter/ekf.hpp (2)
src/module/predictor/ekf_parameter.hpp (10)
a(71-75)a(71-71)a(77-83)a(77-77)x(141-165)x(141-142)x(167-175)x(167-167)x(208-259)x(208-208)src/module/predictor/robot_state.cpp (3)
x(34-37)x(120-131)x(120-120)
src/module/debug/visualization/armor_visualizer.cpp (3)
src/kernel/visualization.cpp (2)
armors(133-136)armors(133-133)src/kernel/visualization.hpp (1)
armors(30-30)src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)
src/kernel/pose_estimator.hpp (1)
src/kernel/pose_estimator.cpp (6)
transform(103-105)transform(103-103)armors(61-101)armors(61-61)armors(107-132)armors(107-107)
src/kernel/pose_estimator.cpp (1)
src/kernel/pose_estimator.hpp (2)
transform(26-26)armors(28-28)
src/module/debug/visualization/armor_visualizer.hpp (2)
src/kernel/visualization.cpp (2)
armors(133-136)armors(133-133)src/kernel/visualization.hpp (1)
armors(30-30)
src/utility/math/conversion.hpp (1)
src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
src/utility/rclcpp/visual/transform.hpp (1)
src/utility/rclcpp/visual/transform.cpp (2)
Transform(49-50)Transform(52-52)
src/runtime.cpp (6)
src/utility/logging/printer.cpp (2)
log(33-33)log(33-33)src/utility/rclcpp/configuration.hpp (2)
configuration(9-15)configuration(9-9)src/utility/image/armor.cpp (2)
draw(7-54)draw(7-7)src/utility/image/armor.hpp (1)
draw(7-7)src/module/tracker/decider.cpp (2)
now(63-75)now(63-63)src/module/tracker/state.cpp (2)
now(24-84)now(24-24)
src/kernel/visualization.hpp (2)
src/kernel/visualization.cpp (4)
armors(133-136)armors(133-133)snapshot(138-161)snapshot(138-138)src/module/predictor/snapshot.hpp (2)
t(15-19)t(15-15)
src/module/predictor/ekf_parameter.hpp (3)
src/utility/math/conversion.hpp (4)
xyz2ypd(32-42)xyz2ypd(32-32)xyz2ypd_jacobian(44-71)xyz2ypd_jacobian(44-44)src/utility/robot/id.hpp (3)
OUTPOST(164-169)INFANTRY_3(136-144)result(120-127)src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (47)
src/module/tracker/state.cpp (4)
1-9: 类型别名定义清晰。使用
steady_clock进行时间管理是正确的选择,适合测量时间间隔。
12-22: 离线检测逻辑正确。超时检测和状态重置逻辑清晰,使用
steady_clock进行时间差计算是可靠的方式。
105-119:constexpr与std::string返回类型的兼容性。
std::string作为返回类型时,constexpr的实际效果有限(C++20 允许在 constexpr 上下文中使用std::string,但结果无法在编译期保留)。如果希望实现真正的编译期字符串转换,可以考虑返回std::string_view。当前实现在运行时使用没有问题,保留
constexpr也无害。
122-130: Pimpl 模式实现正确。公共 API 正确地委托给内部实现,
noexcept修饰符使用恰当。在update中使用Clock::now()获取当前时间,使内部实现可测试(可接受外部时间戳)。config/config.yaml (1)
8-8: 配置更新与 PR 目标一致配置变更符合完整追踪器实现的需求:
- 图像源切换到
hikcamera(实时相机采集)- 新增
tracker配置段,指定敌方颜色为红色- 可视化监控地址更新为局域网 IP
192.168.2.154这些变更与 PR 描述的单车追踪、预测及可视化功能集成相符。
Also applies to: 49-51, 73-73
src/utility/robot/id.hpp (1)
120-127: elements() 方法的实现正确新增的
elements()方法为枚举DeviceIds集合中的设备 ID 提供了便捷接口,逻辑正确且实现清晰。项目使用 C++23 标准,完全支持返回std::vector的constexpr方法,因此constexpr修饰符的使用完全适当且有效。test/asset.yml (1)
3-12: 确保运行 download_assets.sh 验证资源可访问性test/asset.yml 中的所有测试资源 URL 已配置正确,指向
work.heyeuuu19.com。资源可访问性会在执行cd test && ./download_assets.sh时自动验证——脚本使用 curl 下载每个资源,若任何 URL 不可访问则会报错并退出。开发者在运行测试前应确保此脚本执行成功。src/utility/robot/armor.hpp (1)
17-21: 颜色映射函数实现正确。新增的
armor_color2camp_color函数逻辑清晰,将ArmorColor映射到CampColor。与已有的camp_color2armor_color函数(lines 23-27)形成对称设计,支持 ArmorFilter 模块的颜色过滤功能。使用constexpr允许编译期求值,设计良好。test/CMakeLists.txt (1)
118-122:test_feishu目标配置正确,无需添加额外依赖项。该测试仅包含标准 C++ 库头文件和项目内部头文件 (
kernel/feishu.hpp),不依赖 yaml-cpp、rclcpp 或其他外部库。配置与其他无外部依赖的测试目标(如test_pipeline、test_duck_type、test_transform_communication)保持一致。src/utility/shared/context.hpp (2)
41-41: 新增无敌设备字段,设计良好。新增的
invincible_devices字段用于标识无敌状态的设备,与装甲板过滤逻辑配合使用。初始化为DeviceIds::None()是恰当的默认值。移除的imu_state和targets字段在现有代码中无被引用,不会产生依赖问题。结构体保持平凡可复制特性。
31-31: 新增目标设备字段,设计合理。新增的
target字段用于标识自瞄目标设备,初始化为DeviceId::UNKNOWN是合理的默认值。该字段与 PR 的 Tracker 决策流程和 Feishu IPC 模块对齐。已确认代码库中不存在对
AutoAimState::angular_speed的任何引用,说明该字段的移除已完整处理。src/utility/robot/robot.hpp (1)
1-7: LGTM!机器人半径常量定义清晰,使用
constexpr确保编译时求值。实现简洁且正确。test/feishu_test.cpp (1)
23-73: 测试结构合理使用 fork() 进行进程间 IPC 测试的方法恰当,静态断言(lines 20-21)提供了良好的编译时检查,子进程使用
std::_Exit(0)也是正确的做法。src/kernel/pose_estimator.hpp (2)
3-4: 头文件包含合理添加
<expected>和<yaml-cpp/yaml.h>以支持新的 API 和配置需求是恰当的。
26-28: 相机到世界坐标系转换 API 设计良好新增的
set_camera2world_transform和camera2world方法提供了清晰的接口,用于设置和应用相机到世界坐标系的变换。从相关代码片段可以看出,实现使用了 Eigen 进行正确的 3D 变换(旋转和平移)。src/kernel/visualization.cpp (1)
133-136: 改进了 const 正确性将参数类型从
std::span<Armor3D>更改为std::span<Armor3D const>是一个好的改进,表明该函数不会修改输入的装甲板数据。src/module/debug/visualization/armor_visualizer.hpp (1)
16-17: API 更新合理,提升了灵活性和 const 正确性。将 span 元素类型从
Armor3D更新为Armor3D const提升了 const 正确性,新增的name和link_name参数允许更灵活地为不同的可视化场景指定标签和链接名称。这与相关文件(visualization.cpp、visualization.hpp)中的使用方式一致。src/module/tracker/armor_filter.hpp (3)
10-10: 参数传递方式合理。
CampColor作为枚举类型,按值传递是合适的。
14-14: 过滤方法 API 设计合理。接受
const span并返回std::vector的设计适合过滤场景,方法标记为const表明不会修改过滤器状态。
12-12: 该参数传递方式无需改进。
DeviceIds实际上是一个仅包含uint16_t成员的轻量级 struct,而非std::vector或std::set等容器类型。对于这样的小型值类型,按值传递是高效且符合 C++ 惯例的做法,无需改为按 const 引用传递。Likely an incorrect or invalid review comment.
src/kernel/pose_estimator.cpp (1)
103-105: Setter 实现简洁正确。设置器直接存储传入的变换,实现合理。
src/module/predictor/snapshot.hpp (1)
15-19: 预测方法实现正确且高效。
predict_at方法正确使用util::delta_time计算时间差,并应用EKFParameters::f(dt)进行状态外推。方法标记为const表明无副作用,设计合理。注释清晰地说明了方法的意图。src/runtime.cpp (4)
100-102: 控制状态获取失败时直接 continue 可能导致追踪器状态停滞当
feishu.fetch<ControlState>()返回空时,直接 continue 跳过本帧处理。这意味着 tracker 的状态机不会更新,可能导致追踪状态过时或不一致。考虑在 continue 之前更新 tracker 状态(例如标记为未检测到目标),以保持状态机时序一致性。
139-141: 可视化使用的坐标系需确认一致性
visualize_armors使用armors_3d_opt(相机坐标系),而predicted_armors使用snapshot(世界坐标系)。根据相关代码片段,visualize_armors使用"camera_link"帧,predicted_armors使用"odom_imu_link"帧,这是合理的设计。
131-135: 结构化绑定与多重 continue 逻辑清晰使用结构化绑定解构 tracker 决策结果,并分别检查状态和快照有效性,逻辑清晰。
46-52: Tracker 初始化集成正确Feishu、Tracker 和日志组件的初始化与现有模式一致,遵循了统一的配置和错误处理流程。
src/module/tracker/decider.hpp (1)
11-24: 接口设计简洁合理
Decider使用 PIMPL 模式封装实现,Output结构体清晰地封装了状态机状态、目标设备 ID 和可选的预测快照。公共 API 简洁明了,符合单一职责原则。src/kernel/visualization.hpp (1)
30-33: 新增预测装甲板可视化接口
predicted_armors方法的签名设计合理,接受Snapshot和时间点用于前向预测可视化。与实现代码(visualization.cpp 第 137-160 行)一致。src/module/debug/visualization/armor_visualizer.cpp (2)
9-17: ArmorShadow 增加命名空间字段设计合理新增
ns字段用于追踪装甲板的命名空间,使得同一个可视化器可以区分不同命名的装甲板集合(如"solved_pnp_armors"和"predicted_armors"),避免在切换可视化目标时出现残留或错误复用。
68-72: 重建逻辑完整覆盖所有变更条件
needs_rebuild函数正确检查了所有需要重建的条件:genre、color、id和ns,确保当任一属性变化时触发重建。src/module/predictor/robot_state.hpp (1)
9-30: RobotState 接口设计清晰PIMPL 模式封装 EKF 实现细节,公共 API 提供了完整的状态估计生命周期方法:初始化、预测、匹配、更新和快照获取。
MatchResult结构体提供了清晰的匹配结果封装。src/component.cpp (3)
61-63: auto_aim_state 获取后未使用
auto_aim_state被获取并赋值,但在当前实现中未被使用。如果这是后续功能的占位符,建议添加注释说明预期用途;否则可考虑移除以减少不必要的 IPC 开销。
53-59: TODO 注释标记待完成功能
invincible_devices和bullet_speed的 TODO 注释清晰标记了待从裁判系统获取的数据。当前使用硬编码值作为占位符是合理的临时方案。提交失败时仅打印日志,未采取进一步恢复措施,在当前场景下是可接受的。
27-34: Visual Transform 配置正确
visual::Transform的配置结构清晰,正确设置了 topic 名称和坐标系帧(odom_imu_link→camera_link)。src/module/tracker/decider.cpp (1)
79-96: 评分函数逻辑可能需要调整当
priority_mode不包含设备时,该设备的基础优先级评分为 0,这可能导致未知设备比低优先级设备获得更低的分数。建议添加默认处理或日志警告。src/utility/shared/client.hpp (1)
7-11: 良好的类型安全 IPC 设计使用
concept和模板特化来实现编译时角色选择是一个干净且类型安全的设计。src/module/predictor/robot_state.cpp (1)
1-188: 整体实现质量良好EKF 状态估计、装甲板匹配和预测逻辑的整体实现结构清晰,使用 PIMPL 模式有效隔离了实现细节。建议在修复上述问题后进行充分测试。
src/utility/math/conversion.hpp (2)
135-141: LGTM!
ypd2xyz函数逻辑正确,实现了从 yaw-pitch-distance 到笛卡尔坐标的转换。与xyz2ypd形成互逆操作。
143-151: euler_to_quaternion 缺少constexpr一致性此函数仅标记为
inline,而文件中其他类似函数使用了inline constexpr。虽然constexpr在 Eigen 中存在问题(如前所述),但为了风格一致性,建议统一处理方式。另外,函数使用 ZYX(yaw-pitch-roll)顺序进行四元数组合,但参数命名为
yaw_rad, pitch_rad, roll_rad与函数体中yawAngle * pitchAngle * rollAngle的顺序一致,实现正确。src/module/predictor/ekf_parameter.hpp (6)
58-69:armor_num中的is_balance判断可能不完整
is_balancelambda 仅检查INFANTRY_3、INFANTRY_4、INFANTRY_5,但根据src/utility/robot/id.hpp中的kSmallArmorDevices(),SENTRY也是小装甲板设备。平衡步兵的判断逻辑是否正确取决于业务需求,建议确认。此外,
is_balance可以声明为constexpr以与函数签名保持一致。
77-83:z_subtract仅规范化部分角度分量函数规范化了索引 0、1、3 的角度,但索引 2(distance)未处理。请确认这是预期行为——距离差不需要规范化,但需确保不会因漏掉角度分量而产生问题。
根据观测向量定义
[yaw, pitch, distance, armor_yaw],这里的逻辑是正确的。
104-138: 过程噪声矩阵Q的后三维为零状态向量的最后三个分量(r, l, h)对应的过程噪声为零,意味着这些参数在预测步骤中被视为常量。这在某些场景下是合理的(如装甲板半径不变),但如果这些参数需要在线估计,零过程噪声会导致估计器无法修正它们。
请确认这是预期的设计决策。
140-165:h_armor_xyz中的角度计算逻辑清晰函数正确实现了从状态向量到装甲板三维位置的映射,考虑了多装甲板配置(通过
id参数)和长短轴差异(通过use_l_h标志)。注释清晰地解释了状态向量各分量的含义。
208-259: 观测雅可比矩阵H实现正确函数正确组合了两个雅可比矩阵:
H_armor_xyza:状态向量到装甲板 (x, y, z, yaw) 的导数H_armor_ypda:装甲板 (x, y, z) 到观测 (yaw, pitch, distance) 的导数链式法则应用正确:
H = H_armor_ypda * H_armor_xyza。
46-56: 常量已正确定义并包含这些常量已在
src/utility/robot/robot.hpp中定义(第 4-6 行),并且该头文件已在ekf_parameter.hpp的第 9 行正确包含。无需任何修改。Likely an incorrect or invalid review comment.
src/utility/math/kalman_filter/ekf.hpp (2)
139-144: Joseph 形式协方差更新实现正确使用 Joseph 形式
P = (I - KH) * P * (I - KH)^T + K * R * K^T提供了更好的数值稳定性,特别是在增益接近 1 时。对称化处理P_ = 0.5 * (P_next + P_next.transpose())也是正确的做法。
28-39: 类型别名定义清晰完整定义了所有必要的类型别名,使 EKF 参数化更易用。
PDig和RDig用于对角矩阵初始化是个好设计。
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/module/tracker/decider.cpp (1)
37-46: 过期目标清理逻辑已修复之前的审查评论指出
last_seen_time条目未清理的问题,当前代码在 Line 42 已正确清理last_seen_time.erase(item.first),问题已解决。清理逻辑现在能够正确处理过期追踪器,避免内存泄漏。src/module/predictor/robot_state.cpp (1)
39-44: 未初始化 EKF 状态下设置initialized = true存在问题当
!initialized时,predict()设置initialized = true但未初始化 EKF 的状态向量和协方差矩阵。后续调用将使用未初始化的 EKF 数据进行预测,可能导致错误的状态估计或未定义行为。建议修复
auto predict(Stamp const& t) -> void { if (!initialized) { time_stamp = t; - initialized = true; + // 保持 initialized = false,等待 initialize() 被调用后再进行预测 return; }
🧹 Nitpick comments (4)
src/module/tracker/decider.cpp (1)
107-129: 建议将优先级模式声明为静态常量
mode1和mode2是编译时已知的常量映射,建议将它们声明为static constexpr或static const,以避免每个Decider::Impl实例都存储一份副本,节省内存。🔎 建议的重构方案
- const PriorityMode mode1 = { + static inline const PriorityMode mode1 = { { DeviceId::HERO, RobotPriority::SECOND }, // ... }; - const PriorityMode mode2 = { + static inline const PriorityMode mode2 = { { DeviceId::HERO, RobotPriority::FIRST }, // ... };或者如果支持 C++17,使用:
- const PriorityMode mode1 = { + static constexpr std::array<std::pair<DeviceId, RobotPriority>, 9> mode1_data = {{ // ... - }; + }};src/kernel/tracker.cpp (2)
45-47:void函数不需要return语句
set_invincible_armors返回类型为void,使用return filter.set_invincible_armors(devices)是多余的写法。建议修复
auto set_invincible_armors(DeviceIds devices) -> void { - return filter.set_invincible_armors(devices); + filter.set_invincible_armors(devices); }
49-52: 简化返回逻辑可以直接返回
filter.filter(armors)而不需要中间变量。建议修复
auto filter_armors(std::span<Armor2D> const& armors) const -> std::vector<Armor2D> { - auto result = filter.filter(armors); - return result; + return filter.filter(armors); }src/module/predictor/robot_state.cpp (1)
109-117:constexpr标记可能不适用于这些方法
correct()、calculate_armors()和match()被标记为constexpr,但这些方法使用了std::vector、std::ranges::min_element等在编译期可能不完全支持的操作。虽然 C++20 放宽了constexpr的限制,但这些方法实际上不太可能在编译期被调用。建议移除
constexpr标记,除非确实需要编译期求值能力,以避免潜在的编译器兼容性问题。Also applies to: 119-130, 132-164
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/component.cppsrc/kernel/pose_estimator.cppsrc/kernel/tracker.cppsrc/kernel/tracker.hppsrc/module/predictor/robot_state.cppsrc/module/predictor/robot_state.hppsrc/module/tracker/armor_filter.cppsrc/module/tracker/decider.cppsrc/module/tracker/decider.hppsrc/module/tracker/state.hppsrc/runtime.cppsrc/utility/rclcpp/visual/posture.cppsrc/utility/robot/priority.hppsrc/utility/shared/context.hpptest/feishu_test.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- test/feishu_test.cpp
- src/module/tracker/armor_filter.cpp
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/kernel/tracker.cppsrc/runtime.cppsrc/component.cppsrc/module/predictor/robot_state.cppsrc/module/tracker/decider.cppsrc/utility/rclcpp/visual/posture.cppsrc/kernel/pose_estimator.cpp
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/runtime.cppsrc/component.cpp
🧬 Code graph analysis (10)
src/utility/robot/priority.hpp (1)
src/utility/robot/id.hpp (3)
id_underlyings(7-162)UNKNOWN(25-38)id_underlyings(9-24)
src/kernel/tracker.cpp (4)
src/kernel/tracker.hpp (4)
yaml(16-16)devices(18-18)armors(19-19)armors(21-22)src/module/tracker/armor_filter.cpp (8)
filter(35-37)filter(35-35)devices(9-9)devices(9-9)armors(11-19)armors(11-11)set_invincible_armors(31-33)set_invincible_armors(31-31)src/module/tracker/armor_filter.hpp (1)
devices(12-12)src/module/tracker/decider.hpp (1)
armors(23-23)
src/module/tracker/decider.hpp (2)
src/module/tracker/decider.cpp (4)
mode(12-12)mode(12-12)armors(14-61)armors(14-14)src/kernel/tracker.cpp (4)
armors(49-52)armors(49-49)armors(54-57)armors(54-54)
src/component.cpp (4)
src/utility/rclcpp/visual/armor.cpp (2)
config(26-30)config(26-27)src/kernel/control_system.cpp (3)
F(7-41)F(15-21)update_state(43-45)src/kernel/control_system.hpp (2)
class ControlSystem {(7-19)namespace rmcs::kernel {(5-21)src/utility/shared/client.hpp (1)
struct AutoAimClient {(10-13)
src/utility/shared/context.hpp (3)
src/kernel/control_system.hpp (2)
class ControlSystem {(7-19)namespace rmcs::kernel {(5-21)src/utility/rclcpp/visual/posture.hpp (2)
Posture(6-30)Posture(8-28)src/kernel/transform_tree.hpp (1)
util(6-58)
src/module/predictor/robot_state.cpp (5)
src/module/predictor/robot_state.hpp (3)
armor(22-22)armor(23-23)t(20-20)src/module/predictor/ekf_parameter.hpp (22)
armor(16-31)armor(16-16)x(141-165)x(141-142)x(167-175)x(167-167)x(208-259)x(208-208)device(33-44)device(33-33)device(46-56)device(46-46)device(58-69)device(58-58)device(106-138)device(106-106)dt(85-102)dt(85-85)dt(177-184)dt(177-177)xyz(186-206)xyz(186-187)src/module/predictor/snapshot.hpp (2)
t(15-19)t(15-15)src/utility/time.hpp (2)
delta_time(5-8)delta_time(5-6)src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
src/module/tracker/decider.cpp (8)
src/module/tracker/decider.hpp (2)
mode(21-21)armors(23-23)src/kernel/tracker.hpp (2)
armors(19-19)armors(21-22)src/kernel/visualization.cpp (2)
armors(133-136)armors(133-133)src/module/predictor/robot_state.cpp (11)
t(39-51)t(39-39)armor(22-30)armor(22-22)armor(53-81)armor(53-53)armor(132-164)armor(132-132)device(109-117)update(181-181)update(181-181)src/module/predictor/robot_state.hpp (3)
t(20-20)armor(22-22)armor(23-23)src/module/predictor/snapshot.hpp (2)
t(15-19)t(15-15)src/utility/time.hpp (2)
delta_time(5-8)delta_time(5-6)src/utility/robot/id.hpp (1)
OUTPOST(164-169)
src/kernel/tracker.hpp (3)
src/kernel/tracker.cpp (8)
yaml(28-43)yaml(28-28)devices(45-47)devices(45-45)armors(49-52)armors(49-49)armors(54-57)armors(54-54)src/module/tracker/armor_filter.hpp (1)
devices(12-12)src/module/tracker/decider.hpp (1)
armors(23-23)
src/module/predictor/robot_state.hpp (3)
src/module/predictor/robot_state.cpp (8)
t(39-51)t(39-39)armor(22-30)armor(22-22)armor(53-81)armor(53-53)armor(132-164)armor(132-132)src/module/predictor/snapshot.hpp (2)
t(15-19)t(15-15)src/module/predictor/ekf_parameter.hpp (2)
armor(16-31)armor(16-16)
src/kernel/pose_estimator.cpp (1)
src/kernel/pose_estimator.hpp (2)
transform(26-26)armors(28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (33)
src/utility/rclcpp/visual/posture.cpp (1)
12-12: LGTM!使用稳定时钟提升时间戳可靠性。将时钟源从
RCL_SYSTEM_TIME切换到RCL_STEADY_TIME是一个很好的改进。单调稳定时钟不受系统时间调整(如 NTP 同步)的影响,更适合用于消息时间戳和 tf 变换,能够避免时间跳变导致的问题。src/utility/robot/priority.hpp (1)
7-17: LGTM!优先级定义清晰合理。枚举定义和类型别名都很清晰:
RobotPriority枚举值与序数命名一致PriorityMode别名提供了从设备 ID 到优先级的清晰映射- 代码简洁,符合最佳实践
src/kernel/pose_estimator.cpp (3)
32-32: LGTM!相机到世界坐标系变换存储。新增的
camera2world_transform成员为后续坐标变换提供了必要的存储。
103-132: LGTM!坐标变换实现正确。
camera2world方法的实现逻辑正确:
- 从变换矩阵中提取平移向量和旋转四元数
- 对每个装甲板应用刚体变换:位置变换使用
q * p + t,姿态变换使用四元数乘法q1 * q2- 字段命名
.position和.orientation语义清晰变换数学逻辑准确,符合三维刚体变换的标准实现。
145-151: LGTM!公开接口正确委托给实现。公开方法正确地将调用委托给 pimpl 实现,保持了良好的封装性。
src/module/predictor/robot_state.hpp (1)
9-30: LGTM!机器人状态跟踪接口设计清晰。
RobotState类的设计很好:
- PIMPL 模式正确应用,隐藏实现细节
MatchResult结构提供了清晰的匹配结果输出- 公开方法命名准确(
is_converged拼写正确)- 接口完整涵盖初始化、预测、匹配、更新和查询等核心功能
- 与实现文件中的 EKF 跟踪逻辑良好对接
代码结构清晰,符合最佳实践。
src/module/tracker/state.hpp (1)
14-28: 此代码与项目 C++ 标准兼容。函数
to_string声明为constexpr并返回std::string。项目配置已确认使用 C++23 标准(在所有 CMakeLists.txt 中设置CMAKE_CXX_STANDARD 23),C++23 完全支持 constexpr std::string。不需要修改为std::string_view或const char*。src/utility/shared/context.hpp (3)
19-22: 字段重命名提升了语义清晰度将
posture重命名为position更准确地表达了Translation类型的语义,提升了代码可读性。
24-32: 新增目标设备字段支持追踪决策添加
target字段(默认值为DeviceId::UNKNOWN)使AutoAimState能够记录当前瞄准的目标设备,与追踪器的决策输出保持一致。
35-48: 添加无敌设备字段支持装甲板过滤新增的
invincible_devices字段(默认值为DeviceIds::None())使ControlState能够指定当前无敌的设备,追踪器将利用该信息过滤掉无敌装甲板。src/runtime.cpp (7)
76-81: 追踪器初始化逻辑正确追踪器的初始化流程与其他组件保持一致,包括配置读取和错误处理。
100-102: 通过 Feishu 获取控制状态的实现正确使用
feishu.fetch<ControlState>()获取控制状态,并在状态不可用时安全地跳过当前循环,逻辑清晰且安全。
109-113: 装甲板过滤逻辑实现正确先设置无敌设备列表,再过滤 2D 装甲板,最后在结果为空时提前退出。这样可以避免对无效目标进行不必要的位姿估计计算。
124-129: 位姿估计和坐标变换流程正确先对过滤后的装甲板执行 PnP 求解,然后使用控制状态中的相机到世界坐标系变换将结果转换到世界坐标系。处理顺序符合预期。
139-142: 该代码注释不适当,代码实现是正确的第 140 行使用
*armors_3d_opt(相机坐标系)是正确的选择。可视化模块的visualize_armors()方法明确使用"camera_link"作为坐标系参考,这表明它期望接收相机坐标系中的装甲板数据。使用相机坐标系的装甲数据与可视化模块的设计完全一致,无需更改。Likely an incorrect or invalid review comment.
46-46: AutoAimSide 模板参数已正确定义且兼容
AutoAimSide在src/utility/shared/client.hpp中定义为空结构体,并满足Feishu模板所需的IPCSide概念约束。测试文件test/feishu_test.cpp也采用相同方式使用Feishu<AutoAimSide>,代码无任何问题。
22-22: 命名空间路径验证无误
rmcs::tracker::State是正确的命名空间路径。在src/module/tracker/state.hpp中,State枚举直接定义在namespace rmcs::tracker下,无嵌套的StateMachine作用域。第 22 行的类型别名using TrackerState = rmcs::tracker::State;使用正确。src/module/tracker/decider.hpp (2)
11-19: 决策器接口设计清晰使用 PIMPL 模式隐藏实现细节,
Output结构体清晰地封装了决策结果(状态、目标设备 ID 和可选的快照)。
21-23: 公共方法接口设计合理
set_priority_mode允许运行时配置优先级策略,update方法作为主要决策入口,接收装甲板数据和时间戳,返回决策输出。src/kernel/tracker.hpp (2)
13-16: 追踪器接口使用现代 C++ 错误处理使用 PIMPL 模式隐藏实现,
initialize方法返回std::expected进行错误处理,符合现代 C++ 最佳实践。
18-22: 追踪器方法签名设计合理方法参数使用
std::span作为输入(非拥有型视图),返回std::vector或结构化输出,符合现代 C++ 的最佳实践。src/module/tracker/decider.cpp (4)
14-19: 预测阶段实现正确在处理新观测之前,先将所有现有追踪器的状态推进到当前时间点,符合卡尔曼滤波的标准流程。
21-35: 观测分发阶段实现正确对每个装甲板观测,按设备 ID 分发到对应的追踪器。首次观测时创建新追踪器并初始化,后续观测更新现有追踪器并记录最后观测时间。
63-96: 战术仲裁和评分函数实现合理仲裁逻辑筛选活跃候选者(100ms 内观测到的),评分函数综合考虑优先级、距离和粘滞性。TODO 注释表明评分函数需要进一步调优,这是合理的标记。
132-142: PIMPL 转发方法实现正确构造函数、析构函数和公共方法正确地转发到
pimpl实现,遵循标准的 PIMPL 模式。src/component.cpp (2)
53-59: TODO 占位值需要后续完善
invincible_devices和bullet_speed目前使用硬编码的占位值。已有 TODO 注释说明,确认这些是已知的待办事项。当前实现可接受,但请确保在后续迭代中从裁判系统获取正确的数据。
27-33: Transform 可视化配置清晰
visual::Transform::Config的初始化使用了指定初始化器,结构清晰,可读性好。src/kernel/tracker.cpp (2)
62-68: 公共接口实现清晰构造函数、析构函数和
initialize方法的 Pimpl 委托实现正确。
18-24:metas元组结构正确代码遵循序列化框架的标准模式。
metas元组中每个成员需要一个成对的元素:成员指针和对应的字符串名称。当前结构(&Config::enemy_color和"enemy_color")正确地为单个成员提供了必需的一对元素,大小为2满足框架要求的偶数条件。其他类似的配置结构在整个代码库中使用相同模式。src/module/predictor/robot_state.cpp (4)
83-94:is_converged逻辑已正确修复OUTPOST 特殊处理现在使用统一的
min_updates变量,逻辑清晰且避免了之前的冗余条件问题。
53-81:update()方法实现完整EKF 更新流程包含匹配验证、测量向量构建、滤波器更新和前哨站特殊修正,逻辑合理。
10-17: 带初始化的构造函数实现正确
Impl(Armor3D const& armor, Stamp const& t)正确初始化所有成员并设置 EKF 初始状态。
167-187: 公共接口委托实现正确所有公共方法正确委托到 Pimpl 实现,接口清晰。
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime.cpp (1)
41-42:framerate未被使用
FramerateCounter已初始化并设置了间隔,但在主循环中从未被调用(如tick()或获取帧率)。这是死代码,建议移除或完成帧率监控的实现。
♻️ Duplicate comments (3)
src/utility/rclcpp/visual/transform.cpp (1)
49-50: 构造函数noexcept但可能抛出异常此问题已在之前的评审中指出:
std::make_unique<Impl>(config)可能抛出std::bad_alloc,且Impl构造函数内部调用了可能抛出异常的操作。请参考之前的评审建议进行修复,同时需要更新头文件中的声明以保持一致。src/utility/math/conversion.hpp (2)
15-30: 移除这些函数的constexpr关键字这些函数仍然被标记为
constexpr,但如先前评审所指出的,Eigen 的矩阵/向量运算在 C++17/C++20 中不支持constexpr。此问题在之前的评审中已经标记但尚未修复。建议移除constexpr以避免误导。🔎 建议的修复
-inline constexpr auto opencv2ros_position(const Eigen::Vector3d& position) -> Eigen::Vector3d { +inline auto opencv2ros_position(const Eigen::Vector3d& position) -> Eigen::Vector3d { auto result = Eigen::Vector3d { position.z(), -position.x(), -position.y() }; return result; } -inline constexpr auto opencv2ros_rotation(const Eigen::Matrix3d& rotation_matrix) +inline auto opencv2ros_rotation(const Eigen::Matrix3d& rotation_matrix) -> Eigen::Matrix3d { return kCoordTransformMatrix * rotation_matrix * kCoordTransformMatrix.transpose(); } -inline constexpr auto ros2opencv_position(const Eigen::Vector3d& position) -> Eigen::Vector3d { +inline auto ros2opencv_position(const Eigen::Vector3d& position) -> Eigen::Vector3d { auto result = Eigen::Vector3d { -position.y(), -position.z(), position.x() }; return result; } -inline constexpr auto ros2opencv_rotation(const Eigen::Matrix3d& rotation_matrix) +inline auto ros2opencv_rotation(const Eigen::Matrix3d& rotation_matrix) -> Eigen::Matrix3d { return kCoordTransformMatrix.transpose() * rotation_matrix * kCoordTransformMatrix; }
32-42: 移除xyz2ypd的constexpr关键字该函数使用了
std::atan2和std::sqrt,这些函数在 C++17/C++20 中并非constexpr(仅在 C++23 中可用)。此外,Eigen 向量操作也不支持constexpr。constexpr关键字实际上不会生效,建议移除。🔎 建议的修复
-inline constexpr auto xyz2ypd(Eigen::Vector3d const& xyz) -> Eigen::Vector3d { +inline auto xyz2ypd(Eigen::Vector3d const& xyz) -> Eigen::Vector3d { const auto x = xyz[0]; const auto y = xyz[1]; const auto z = xyz[2];
🧹 Nitpick comments (2)
src/kernel/tracker.cpp (1)
47-50: 参数类型风格不一致(可选优化)
Impl::filter_armors使用std::span<Armor2D> const&而公共接口使用std::span<Armor2D>。std::span本身很轻量(仅指针+大小),按值传递是惯用写法。建议统一为按值传递以保持一致性。🔎 建议修改
- auto filter_armors(std::span<Armor2D> const& armors) const -> std::vector<Armor2D> { + auto filter_armors(std::span<Armor2D> armors) const -> std::vector<Armor2D> {src/kernel/feishu.hpp (1)
49-56: 建议为 get_client 添加类型安全检查当前实现在添加新
StateType时会默默返回control_client。建议添加static_assert确保类型安全。🔎 建议修改
template <typename DataType> auto get_client() noexcept -> auto& { if constexpr (std::same_as<DataType, AutoAimState>) { return auto_aim_client; - } else { + } else if constexpr (std::same_as<DataType, ControlState>) { return control_client; + } else { + static_assert(sizeof(DataType) == 0, "Unsupported StateType"); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/kernel/feishu.hppsrc/kernel/tracker.cppsrc/runtime.cppsrc/utility/math/conversion.hppsrc/utility/rclcpp/visual/transform.cpp
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/kernel/tracker.cppsrc/utility/rclcpp/visual/transform.cppsrc/runtime.cpp
📚 Learning: 2025-12-16T22:21:32.866Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 12
File: package.xml:15-15
Timestamp: 2025-12-16T22:21:32.866Z
Learning: In the rmcs_auto_aim_v2 repository, do not flag Chinese comment style issues such as "唯二" for standardization. The maintainer prefers to keep the current informal/colloquial Chinese phrasing in comments.
Applied to files:
src/runtime.cpp
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/runtime.cpp
🧬 Code graph analysis (4)
src/kernel/tracker.cpp (3)
src/kernel/tracker.hpp (4)
yaml(16-16)devices(18-18)armors(19-19)armors(21-22)src/module/tracker/armor_filter.hpp (1)
devices(12-12)src/module/tracker/decider.hpp (1)
armors(23-23)
src/utility/rclcpp/visual/transform.cpp (2)
src/utility/rclcpp/visual/armor.cpp (1)
rclcpp_pub(85-100)src/utility/rclcpp/visual/transform.hpp (3)
Transform(17-17)Transform(18-18)Transform(20-20)
src/utility/math/conversion.hpp (1)
src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
src/runtime.cpp (6)
src/utility/logging/printer.cpp (2)
log(33-33)log(33-33)src/utility/rclcpp/configuration.hpp (2)
configuration(9-15)configuration(9-9)src/module/identifier/armor_detection.cpp (2)
image(13-39)image(13-13)src/utility/image/armor.cpp (2)
draw(7-54)draw(7-7)src/utility/image/armor.hpp (1)
draw(7-7)src/module/tracker/decider.cpp (2)
now(63-75)now(63-63)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (18)
src/utility/rclcpp/visual/transform.cpp (2)
1-9: LGTM!包含文件和类型别名设置正确,
TransformMsg别名提高了代码可读性。
43-47: LGTM!公共方法正确委托给 pimpl 实现,符合 pimpl 模式的标准用法。
src/utility/math/conversion.hpp (3)
2-2: LGTM!新增的头文件引入正确支持了新增的工具函数:
angle.hpp提供了normalize_angle,cvdef.h提供了CV_PI常量。Also applies to: 4-4
44-83: LGTM!良好的数值保护该函数正确实现了先前评审中建议的除零保护(第 49-56 行)。通过检查
xy_norm2和norm2是否小于 epsilon,避免了在坐标接近零点时的数值不稳定性。实现安全且正确。
155-163: LGTM!该函数正确实现了欧拉角到四元数的转换,使用标准的 Eigen 旋转表示。未标记为
constexpr是正确的选择。src/runtime.cpp (5)
20-23: 类型别名修正确认类型别名已正确更新为
rmcs::tracker::State,解决了之前重复命名空间嵌套的问题。
46-52: 组件初始化模式一致新增的
Feishu、Tracker和Printer组件遵循了与其他运行时组件相同的初始化模式。
76-81: Tracker 初始化遵循标准模式Tracker 的初始化和错误处理与其他组件保持一致。
100-113: 控制流逻辑清晰主循环采用管道式处理,通过多个早期
continue进行逐步筛选:
- 获取控制状态
- 识别装甲板
- 过滤装甲板
这种模式清晰且易于理解。
135-144: 追踪器决策和快照处理决策逻辑正确:仅在
Tracking状态且存在快照时继续处理。target_device未使用是预期行为(根据维护者说明,将在下一个模块处理)。src/kernel/tracker.cpp (5)
1-11: PIMPL 模式实现正确头文件包含和命名空间使用正确,类型别名清晰。
12-24: Impl 结构和配置设计合理
Config使用Serializable模式进行 YAML 序列化,结构清晰。ArmorFilter和Decider作为组合成员是合适的设计。
26-41: 初始化方法实现正确配置序列化和敌方颜色验证逻辑清晰,错误处理使用
std::expected符合项目风格。
52-58: 决策方法和 TODO 标记
decide方法正确委托给Decider。TODO 注释表明优先级选择是后续功能,符合项目的标记习惯。
60-78: 公共接口实现正确PIMPL 委托模式实现正确,生命周期管理(构造/析构)符合预期。
src/kernel/feishu.hpp (3)
1-11: Feishu 模板类设计清晰使用
IPCSide模板参数和requires约束实现编译期角色检查是良好的设计,确保发送方和接收方在编译时就能正确区分。
12-28: commit 方法实现正确
requires约束确保编译时角色检查,constexpr if提供额外的防御性编程。[[unlikely]]标记适当用于异常路径。
58-62: ensure_open 辅助方法实现正确短路求值逻辑正确:仅在未打开时尝试打开。
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/utility/rclcpp/visual/transform.cpp (1)
47-48: 构造函数标记noexcept但可能抛出异常
std::make_unique<Impl>(config)可能抛出std::bad_alloc,且Impl构造函数调用了util::panic和 ROS2 的create_publisher,这些都可能抛出异常。根据之前的审查记录,此问题仍未修复。🔎 建议的修复方案
-Transform::Transform(const Config& config) noexcept +Transform::Transform(const Config& config) : pimpl { std::make_unique<Impl>(config) } { }同时需要更新
src/utility/rclcpp/visual/transform.hpp中的声明:- explicit Transform(const Config&) noexcept; + explicit Transform(const Config&);src/utility/math/conversion.hpp (1)
15-30: 仍需移除这些函数的constexpr关键字这些函数(
opencv2ros_position、opencv2ros_rotation、ros2opencv_position、ros2opencv_rotation)仍被标记为constexpr,但使用了 Eigen 的矩阵/向量运算(矩阵乘法、转置、向量构造)。根据 Eigen 在 C++17/C++20 的实现限制,这些操作无法在编译期求值。建议将inline constexpr改为inline。
🧹 Nitpick comments (3)
src/runtime.cpp (1)
41-42: 未使用的framerate计数器
framerate变量在第 41-42 行初始化并设置了间隔,但在主循环中从未被调用来记录帧率或输出日志。这可能是遗留代码或尚未完成的功能。🔎 建议的修复方案
如果不需要帧率统计,建议移除:
- auto framerate = FramerateCounter {}; - framerate.set_interval(5s);如果需要帧率统计,应在主循环中调用:
// 在主循环的适当位置添加 framerate.tick();src/kernel/feishu.hpp (2)
18-27:if constexpr与requires约束存在冗余
commit方法已通过requires(util::ShmRoleSelector<Side, StateType>::is_sender)约束确保只有 sender 侧可以调用。第 18 行的if constexpr检查with_write方法是否存在是额外的防御性检查,而第 26 行的static_assert在requires约束生效时永远不会被触发。🔎 简化建议
如果
requires约束已经保证了调用侧的正确性,可以简化实现:template <typename StateType> auto commit(StateType const& state) noexcept -> bool requires(util::ShmRoleSelector<Side, StateType>::is_sender) { auto& client = this->template get_client<StateType>(); - if constexpr (requires { client.with_write([](StateType&) { }); }) { - if (!ensure_open(client, util::shm_name<StateType>)) [[unlikely]] { - return false; - } - - client.with_write([&](StateType& data) { data = state; }); - return true; - } else { - static_assert(sizeof(StateType) == 0, "Error: This side can only READ this state."); + if (!ensure_open(client, util::shm_name<StateType>)) [[unlikely]] { + return false; } + + client.with_write([&](StateType& data) { data = state; }); + return true; }当然,保留
if constexpr作为额外的编译期防护也是合理的选择。
57-64:get_client类型分发仅覆盖两种状态类型
get_client<DataType>()使用if constexpr分发到auto_aim_client或control_client。对于非AutoAimState的类型,默认返回control_client,这可能在未来扩展时导致错误。🔎 建议添加编译期检查
template <typename DataType> auto get_client() noexcept -> auto& { if constexpr (std::same_as<DataType, AutoAimState>) { return auto_aim_client; - } else { + } else if constexpr (std::same_as<DataType, ControlState>) { return control_client; + } else { + static_assert(sizeof(DataType) == 0, "Unsupported state type"); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/kernel/feishu.hppsrc/runtime.cppsrc/utility/math/conversion.hppsrc/utility/rclcpp/visual/transform.cppsrc/utility/rclcpp/visual/transform.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utility/rclcpp/visual/transform.hpp
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/runtime.cppsrc/utility/rclcpp/visual/transform.cpp
📚 Learning: 2025-12-16T22:21:32.866Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 12
File: package.xml:15-15
Timestamp: 2025-12-16T22:21:32.866Z
Learning: In the rmcs_auto_aim_v2 repository, do not flag Chinese comment style issues such as "唯二" for standardization. The maintainer prefers to keep the current informal/colloquial Chinese phrasing in comments.
Applied to files:
src/runtime.cpp
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/runtime.cpp
🧬 Code graph analysis (3)
src/runtime.cpp (8)
src/utility/logging/printer.cpp (2)
log(33-33)log(33-33)src/utility/rclcpp/configuration.hpp (2)
configuration(9-15)configuration(9-9)src/kernel/visualization.hpp (2)
image(18-20)image(18-18)src/kernel/visualization.cpp (2)
image(91-131)image(91-91)src/module/identifier/model.cpp (6)
image(162-214)image(162-163)image(262-272)image(262-262)image(274-309)image(274-274)src/module/identifier/armor_detection.cpp (2)
image(13-39)image(13-13)src/utility/image/armor.hpp (1)
draw(7-7)src/module/tracker/decider.cpp (2)
now(63-75)now(63-63)
src/utility/rclcpp/visual/transform.cpp (2)
src/utility/rclcpp/visual/armor.cpp (1)
rclcpp_pub(85-100)src/utility/rclcpp/visual/transform.hpp (3)
Transform(17-17)Transform(18-18)Transform(20-20)
src/utility/math/conversion.hpp (1)
src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (11)
src/runtime.cpp (3)
100-103: 控制状态获取逻辑清晰先检查
updated<ControlState>()再调用fetch<ControlState>(),避免了不必要的共享内存读取。逻辑正确且高效。
125-134: 位姿估计流程正确先调用
solve_pnp获取相机坐标系下的装甲板位置,然后设置camera2world_transform,最后转换到世界坐标系。流程顺序正确。
136-144: Tracker 决策与可视化流程完整追踪器决策返回状态、目标设备和快照,正确地在
Tracking状态且存在快照时才继续处理。target_device的使用已根据之前的讨论推迟到下一个模块实现。src/utility/rclcpp/visual/transform.cpp (2)
32-35:move与impl_move的noexcept一致性已改善之前的审查指出
impl_move标记为noexcept而move未标记,存在不一致。当前版本中impl_move(第 45 行)已移除noexcept,与Impl::move(第 32-35 行)保持一致。Also applies to: 45-45
11-15:Impl结构体设计合理使用 pimpl 模式隐藏实现细节,静态
rclcpp_clock用于时间戳生成,结构清晰。src/kernel/feishu.hpp (2)
30-43:fetch()方法注释已更新之前的审查指出注释与实现不一致。当前版本第 36 行的注释已更新为"直接读取当前共享内存中的数据;如需检测是否有新数据,请先调用 updated()",准确反映了代码行为。
66-69:ensure_open懒加载逻辑简洁
ensure_open方法在客户端未打开时尝试打开,已打开时直接返回true。逻辑清晰,支持懒加载模式。src/utility/math/conversion.hpp (4)
44-83: 雅可比函数的数值保护实现得当该函数正确实现了除零保护(Lines 49-56),当坐标接近原点时返回零矩阵。这解决了之前评审中提出的除零风险问题。实现逻辑清晰且数值稳定。
85-145: 已正确移除constexpr该函数现在使用
inline auto而非inline constexpr auto,正确反映了函数内部调用std::acos、std::atan2、std::abs和normalize_angle(使用std::sin/std::cos)等在 C++20 中非 constexpr 的操作。实现逻辑包含了适当的奇异点处理(Lines 115-132)。
147-153: 已正确移除constexpr该函数现在使用
inline auto而非inline constexpr auto,正确反映了函数调用std::cos和std::sin等在 C++20 中非 constexpr 的操作。从 yaw-pitch-distance 到 xyz 的转换逻辑正确。
155-163: 欧拉角到四元数转换实现正确该函数使用标准的
Eigen::AngleAxisd构建旋转,并按照 ZYX(yaw-pitch-roll)顺序组合。实现清晰且符合常见的欧拉角转换约定。
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @src/component.cpp:
- Around line 55-56: Replace the hardcoded assignment to
control_state.bullet_speed with a value loaded from the YAML config key
fire_control.v_initial (config/config.yaml); in component.cpp, instead of
control_state.bullet_speed = 25, read the configured v_initial via the project’s
existing configuration API (selecting the correct mode subsection such as BATTLE
or OUTPOST when applicable), convert/validate the numeric value (m/s) and
provide a sensible fallback or error if missing, and ensure tests (e.g.,
test/feishu_test.cpp) can override this config for different scenarios.
In @src/module/predictor/ekf_parameter.hpp:
- Around line 1-9: 当前头文件 ekf_parameter.hpp 使用了
std::cos、std::sin、std::atan2、std::log 等数学函数但未显式包含 <cmath>,会导致潜在编译错误;在文件顶部添加
#include <cmath> 以保证这些函数的声明可用,并保持对 std:: 前缀的使用(例如在使用 std::cos/std::sin 的位置如 EKF
参数计算代码中无需改动,只需新增头文件包含)。
- Around line 182-202: In the static method R(Eigen::Vector3d const& xyz,
Eigen::Vector3d const& ypr, Eigen::Vector3d const& ypd) you call log(...)
without the std:: qualifier; update the two occurrences that compute R_dig (the
entries using log(std::abs(delta_yaw) + 1) and log(std::abs(distance) + 1) / 200
+ 9e-2) to use std::log so they consistently use the std namespace (keep
references to delta_yaw, distance, R_dig and the returned EKF::RMat unchanged).
In @src/utility/clock.hpp:
- Around line 1-7: There are duplicate local Clock type aliases scattered in the
codebase; replace each local alias (e.g., any "using Clock =
std::chrono::steady_clock;" or equivalent) with a reference to the centralized
alias: inside namespace scopes use "using Clock = util::Clock;" and in global
scopes use "using Clock = rmcs::util::Clock;"; update each file that currently
defines its own Clock (the ones called out in the review) to remove the
std::chrono alias and import the centralized util::Clock instead so all code
consistently uses rmcs::util::Clock.
In @src/utility/math/angle.hpp:
- Around line 9-10: deg2rad and rad2deg currently use the non-standard macro
M_PI which can be missing on some platforms; update these functions to use a
portable pi constant: if the project is built with C++20, replace M_PI with
std::numbers::pi in deg2rad and rad2deg (ensure <numbers> is included),
otherwise add a project-defined constexpr double kPi (or similar) and use that
constant instead of M_PI; keep function names deg2rad and rad2deg and ensure
headers include the new constant or <numbers> so the code compiles across
platforms.
🧹 Nitpick comments (15)
src/utility/time.hpp (1)
6-9: 实现正确且简洁函数逻辑正确,参数传值方式也恰当(
time_point是廉价复制类型)。可选建议:将返回类型从
auto改为显式的std::chrono::duration<double>,以提高代码可读性:♻️ 可选的显式返回类型
-constexpr auto delta_time(std::chrono::steady_clock::time_point late, - std::chrono::steady_clock::time_point early) -> auto { +constexpr auto delta_time(std::chrono::steady_clock::time_point late, + std::chrono::steady_clock::time_point early) -> std::chrono::duration<double> { return std::chrono::duration<double>(late - early); }src/utility/robot/constant.hpp (1)
4-6: 建议添加文档注释并明确常量含义常量定义正确,但缺少文档说明:
- 单位未标明(推测为米)
kOtherRadius命名不够明确,"Other" 指代的具体对象不清楚📝 建议的改进
namespace rmcs { +// 机器人半径常量(单位:米) +constexpr double kBaseRadius = 0.3205; // 基地半径 +constexpr double kOutpostRadius = 0.2765; // 前哨站半径 +constexpr double kOtherRadius = 0.2; // 其他机器人半径(如步兵、英雄等) -constexpr double kBaseRadius = 0.3205; -constexpr double kOutpostRadius = 0.2765; -constexpr double kOtherRadius = 0.2; }或者考虑将
kOtherRadius重命名为更具体的名称,如kStandardRobotRadius或kDefaultRobotRadius。src/utility/shared/context.hpp (1)
27-28: 统一初始化语法风格。
should_control使用{ false },而should_shoot使用= { false }。虽然两者功能相同,但建议统一使用一种风格以保持代码一致性。♻️ 建议的统一写法
- bool should_control { false }; - bool should_shoot = { false }; + bool should_control { false }; + bool should_shoot { false };src/component.cpp (1)
1-14: 添加空行以符合编码风格。根据代码审查记录,头文件区和 namespace 声明之间应该有空行。建议在第 10 行后添加空行。
基于审查历史记录。
♻️ 建议的格式调整
#include <rmcs_executor/component.hpp> + namespace rmcs {src/kernel/feishu.hpp (2)
1-5: 添加空行以符合编码风格。根据代码审查记录,头文件区和 namespace 声明之间应该有空行。建议在第 4 行后添加空行。
基于审查历史记录。
♻️ 建议的格式调整
#include "utility/shared/interprocess.hpp" + namespace rmcs::kernel {
42-46: 注意fetch()返回引用的生命周期问题。
fetch()返回recv_buffer成员的 const 引用。这意味着:
- 返回的引用仅在下次调用
fetch()之前有效- 调用者不应存储此引用供长期使用
建议在注释中明确说明这一点,或考虑返回值而非引用以避免潜在的悬空引用问题。
📝 建议的注释改进
auto fetch() noexcept -> const RecvData& { - // Note:直接读取当前共享内存中的数据;如需检测是否有新数据,请先调用 updated() + // Note:直接读取当前共享内存中的数据;如需检测是否有新数据,请先调用 updated() + // 警告:返回的引用仅在下次调用 fetch() 之前有效,不应长期持有 recv_client.with_read([&](RecvData const& shared) { recv_buffer = shared; }); return recv_buffer; }src/module/debug/visualization/armor_visualizer.cpp (1)
68-72: 建议使用const std::string&以保持代码库一致性。
needs_rebuild函数的name参数使用了std::string_view。虽然在此处仅用于比较是安全的,但根据之前的审查反馈(creeper5820 的评论),代码库倾向于使用const std::string&以避免生命周期问题。♻️ 建议的修改
static auto needs_rebuild( - ArmorShadow const& shadow, Armor3D const& input, std::string_view name) -> bool { + ArmorShadow const& shadow, Armor3D const& input, std::string const& name) -> bool { return shadow.genre != input.genre || shadow.color != input.color || shadow.id != input.id || shadow.ns != name; }基于过往审查反馈。
src/kernel/visualization.cpp (2)
14-15: 未使用的类型别名
Clock和Stamp类型别名已定义但在此文件中从未使用。建议删除这些死代码以保持代码整洁。♻️ 建议修复
using namespace rmcs::kernel; using namespace rmcs::util; -using Clock = std::chrono::steady_clock; -using Stamp = Clock::time_point;
136-140: 返回值处理不一致
solved_pnp_armors正确返回visualize的结果,但predicted_armors忽略了返回值并始终返回true。建议保持一致的错误处理方式。♻️ 建议修复
auto predicted_armors(std::span<Armor3D const> armors) const -> bool { if (!is_initialized) return false; - armor_visualizer->visualize(armors, "predicted_armors", "odom_imu_link"); - return true; + return armor_visualizer->visualize(armors, "predicted_armors", "odom_imu_link"); }src/kernel/tracker.cpp (1)
45-48: 参数类型不一致
Impl::filter_armors接受std::span<Armor2D> const&,而公共方法Tracker::filter_armors接受std::span<Armor2D>。虽然这可以工作,但签名不一致可能会造成混淆。建议统一使用std::span<Armor2D const>作为参数类型,因为std::span本身是轻量级的,不需要通过引用传递。♻️ 建议修复
- auto filter_armors(std::span<Armor2D> const& armors) const -> std::vector<Armor2D> { + auto filter_armors(std::span<Armor2D const> armors) const -> std::vector<Armor2D> { auto result = filter.filter(armors); return result; }Also applies to: 70-72
src/runtime.cpp (1)
132-140: 时间戳可能不一致
tracker.decide在第 133 行使用Clock::now(),而predicted_armors在第 140 行再次调用Clock::now()。这两次调用之间可能存在时间差,导致预测结果与可视化之间的不一致。建议在循环开始时捕获一次时间戳并复用。♻️ 建议修复
if (auto image = capturer.fetch_image()) { + auto const now = Clock::now(); if (!feishu.updated()) continue; auto control_state = feishu.fetch(); // ... 中间代码 ... auto [tracker_state, target_device, snapshot_opt] = - tracker.decide(armors_3d, Clock::now()); + tracker.decide(armors_3d, now); if (tracker_state != TrackerState::Tracking) continue; if (!snapshot_opt) continue; auto const& snapshot = *snapshot_opt; if (visualization.initialized()) { - visualization.predicted_armors(snapshot.predicted_armors(Clock::now())); + visualization.predicted_armors(snapshot.predicted_armors(now)); }src/module/tracker/decider.cpp (1)
106-128: 未使用的优先级模式映射
mode1和mode2常量已定义但从未使用。如果这些是预设的优先级配置,应该提供一种方式让外部代码选择使用它们,或者在初始化时设置默认值。否则应删除这些死代码。♻️ 建议修复方案
选项 1:删除未使用的代码
- const PriorityMode mode1 = { - { DeviceId::HERO, 2 }, - // ... 其他条目 - }; - - const PriorityMode mode2 = { - { DeviceId::HERO, 1 }, - // ... 其他条目 - };选项 2:在初始化时设置默认优先级模式
+ Impl() { + priority_mode = mode1; // 使用默认优先级模式 + } + auto set_priority_mode(PriorityMode const& mode) -> void { priority_mode = mode; }src/module/predictor/snapshot.cpp (1)
49-49: 魔数:硬编码的俯仰角俯仰角
15. / 180 * std::numbers::pi(15度)是一个硬编码的魔数。建议将其提取为命名常量以提高可读性和可维护性。♻️ 建议修复
+namespace { +constexpr double kArmorPitchRad = 15.0 / 180.0 * std::numbers::pi; +} // namespace + // 在 predicted_armors 方法中: - armor.orientation = util::euler_to_quaternion(angle, 15. / 180 * std::numbers::pi, 0); + armor.orientation = util::euler_to_quaternion(angle, kArmorPitchRad, 0);src/module/predictor/robot_state.cpp (1)
152-169: 重复的误差计算逻辑
match方法中误差计算逻辑重复了两次:一次在min_element的比较器中(第 154-158 行),另一次在计算min_error时(第 165-169 行)。建议提取为局部 lambda 并复用。♻️ 建议修复
+ auto get_error = [&](auto const& pred) { + auto ypd_pred = util::xyz2ypd(pred.template head<3>()); + return std::abs(util::normalize_angle(ypr_in_world[0] - pred[3])) + + std::abs(util::normalize_angle(ypd_in_world[0] - ypd_pred[0])); + }; + auto it = - std::ranges::min_element(armors_xyza, [&](auto const& a_xyza, auto const& b_xyza) { - auto get_error = [&](auto const& pred) { - auto ypd_pred = util::xyz2ypd(pred.template head<3>()); - return std::abs(util::normalize_angle(ypr_in_world[0] - pred[3])) - + std::abs(util::normalize_angle(ypd_in_world[0] - ypd_pred[0])); - }; - - return get_error(a_xyza) < get_error(b_xyza); - }); + std::ranges::min_element(armors_xyza, [&](auto const& a_xyza, auto const& b_xyza) { + return get_error(a_xyza) < get_error(b_xyza); + }); int best_id = static_cast<int>(std::distance(armors_xyza.begin(), it)); - auto min_error = [&](const auto& pred) { - auto ypd_pred = util::xyz2ypd(pred.template head<3>()); - return std::abs(util::normalize_angle(ypr_in_world[0] - pred[3])) - + std::abs(util::normalize_angle(ypd_in_world[0] - ypd_pred[0])); - }(*it); + auto min_error = get_error(*it);src/kernel/pose_estimator.cpp (1)
109-143: 代码重复:单个和批量 camera2world 方法单个
Armor3D的变换逻辑(第 109-121 行)与批量处理版本(第 123-143 行)中的循环体重复。批量版本应该直接调用单个版本以消除重复。♻️ 建议修复
auto camera2world(std::span<Armor3D const> armors) const -> std::vector<Armor3D> { auto result = std::vector<Armor3D> {}; result.reserve(armors.size()); for (const auto& armor : armors) { - auto transformed = armor; - - auto position = Eigen::Vector3d {}; - transformed.translation.copy_to(position); - transformed.translation = - camera2world_orientation * position + camera2world_translation; - - auto quat = Eigen::Quaterniond {}; - transformed.orientation.copy_to(quat); - transformed.orientation = camera2world_orientation * quat; - - result.emplace_back(transformed); + result.emplace_back(camera2world(armor)); } return result; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
config/config.yamlsrc/component.cppsrc/kernel/feishu.hppsrc/kernel/pose_estimator.cppsrc/kernel/pose_estimator.hppsrc/kernel/tracker.cppsrc/kernel/tracker.hppsrc/kernel/visualization.cppsrc/kernel/visualization.hppsrc/module/debug/visualization/armor_visualizer.cppsrc/module/debug/visualization/armor_visualizer.hppsrc/module/identifier/armor_detection.cppsrc/module/predictor/ekf_parameter.hppsrc/module/predictor/robot_state.cppsrc/module/predictor/robot_state.hppsrc/module/predictor/snapshot.cppsrc/module/predictor/snapshot.hppsrc/module/tracker/armor_filter.cppsrc/module/tracker/decider.cppsrc/module/tracker/decider.hppsrc/module/tracker/state.hppsrc/runtime.cppsrc/utility/clock.hppsrc/utility/math/angle.hppsrc/utility/math/conversion.hppsrc/utility/rclcpp/visual/armor.cppsrc/utility/robot/color.hppsrc/utility/robot/constant.hppsrc/utility/robot/id.hppsrc/utility/robot/priority.hppsrc/utility/shared/client.hppsrc/utility/shared/context.hppsrc/utility/time.hpp
💤 Files with no reviewable changes (1)
- src/utility/shared/client.hpp
🚧 Files skipped from review as they are similar to previous changes (7)
- src/module/tracker/armor_filter.cpp
- src/module/tracker/state.hpp
- src/utility/rclcpp/visual/armor.cpp
- src/module/predictor/robot_state.hpp
- config/config.yaml
- src/utility/robot/id.hpp
- src/module/debug/visualization/armor_visualizer.hpp
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-16T22:21:32.866Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 12
File: package.xml:15-15
Timestamp: 2025-12-16T22:21:32.866Z
Learning: In the rmcs_auto_aim_v2 repository, do not flag Chinese comment style issues such as "唯二" for standardization. The maintainer prefers to keep the current informal/colloquial Chinese phrasing in comments.
Applied to files:
src/utility/robot/priority.hppsrc/runtime.cppsrc/component.cpp
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/kernel/pose_estimator.hppsrc/kernel/visualization.cppsrc/component.cpp
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/kernel/pose_estimator.cppsrc/module/predictor/snapshot.cppsrc/module/predictor/robot_state.cppsrc/module/identifier/armor_detection.cppsrc/runtime.cppsrc/kernel/visualization.cppsrc/component.cppsrc/module/debug/visualization/armor_visualizer.cppsrc/kernel/tracker.cppsrc/module/tracker/decider.cpp
📚 Learning: 2025-12-15T21:13:59.238Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
Applied to files:
src/component.cpp
🧬 Code graph analysis (18)
src/utility/robot/constant.hpp (7)
src/utility/robot/armor.hpp (2)
DARK(9-101)struct Armor { }(68-68)src/module/identifier/classifier.hpp (1)
namespace rmcs {(3-7)src/module/identifier/armor.hpp (1)
namespace rmcs {(5-17)src/utility/monostate.hpp (1)
namespace rmcs {(3-5)src/utility/model/armor_detection.hpp (1)
lt_x(5-135)src/utility/acsii_art.hpp (1)
ascii_banner(4-17)src/kernel/control_system.hpp (1)
namespace rmcs::kernel {(5-21)
src/kernel/tracker.hpp (4)
src/kernel/tracker.cpp (8)
yaml(24-39)yaml(24-24)devices(41-43)devices(41-41)armors(45-48)armors(45-45)armors(50-53)armors(50-50)src/module/tracker/armor_filter.cpp (4)
devices(9-9)devices(9-9)armors(11-17)armors(11-11)src/module/tracker/armor_filter.hpp (1)
devices(12-12)src/module/tracker/decider.cpp (2)
armors(13-60)armors(13-13)
src/kernel/pose_estimator.hpp (2)
src/kernel/pose_estimator.cpp (8)
transform(104-107)transform(104-104)armors(62-102)armors(62-62)armors(123-143)armors(123-123)armor(109-121)armor(109-109)src/module/predictor/robot_state.cpp (6)
armor(19-27)armor(19-19)armor(57-88)armor(57-57)armor(140-172)armor(140-140)
src/kernel/pose_estimator.cpp (1)
src/kernel/pose_estimator.hpp (3)
transform(26-26)armor(29-29)armors(28-28)
src/module/tracker/decider.hpp (4)
src/kernel/tracker.cpp (4)
armors(45-48)armors(45-45)armors(50-53)armors(50-50)src/kernel/tracker.hpp (2)
armors(22-22)armors(24-24)src/kernel/visualization.hpp (2)
armors(29-29)armors(30-30)src/module/predictor/snapshot.hpp (2)
t(30-30)t(31-31)
src/module/predictor/snapshot.cpp (5)
src/module/predictor/ekf_parameter.hpp (14)
device(32-43)device(32-32)device(45-55)device(45-45)device(57-65)device(57-57)device(102-134)device(102-102)dt(81-98)dt(81-81)dt(173-180)dt(173-173)armor(15-30)armor(15-15)src/module/predictor/snapshot.hpp (6)
t(30-30)t(31-31)Snapshot(18-19)Snapshot(20-20)Snapshot(21-21)Snapshot(24-24)src/utility/time.hpp (2)
delta_time(6-9)delta_time(6-7)src/utility/robot/armor.hpp (2)
camp_color2armor_color(23-27)camp_color2armor_color(23-23)src/utility/math/conversion.hpp (2)
euler_to_quaternion(152-160)euler_to_quaternion(152-153)
src/kernel/visualization.hpp (2)
src/kernel/visualization.cpp (6)
image(89-129)image(89-89)armors(131-134)armors(131-131)armors(136-140)armors(136-136)src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)
src/module/predictor/robot_state.cpp (5)
src/module/predictor/ekf_parameter.hpp (22)
armor(15-30)armor(15-15)device(32-43)device(32-32)device(45-55)device(45-45)device(57-65)device(57-57)device(102-134)device(102-102)x(137-161)x(137-138)x(163-171)x(163-163)x(204-255)x(204-204)dt(81-98)dt(81-81)dt(173-180)dt(173-173)xyz(182-202)xyz(182-183)src/utility/time.hpp (2)
delta_time(6-9)delta_time(6-7)src/utility/robot/id.hpp (6)
id(110-112)id(110-110)id(117-117)id(117-117)id(118-118)id(118-118)src/utility/math/conversion.hpp (2)
xyz2ypd(29-39)xyz2ypd(29-29)src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
src/module/identifier/armor_detection.cpp (1)
test/device_id.cpp (1)
TEST(53-64)
src/runtime.cpp (5)
src/utility/rclcpp/configuration.hpp (2)
configuration(9-15)configuration(9-9)src/kernel/visualization.cpp (2)
image(89-129)image(89-89)src/kernel/visualization.hpp (3)
image(17-19)image(17-17)image(27-27)src/module/identifier/model.cpp (6)
image(162-214)image(162-163)image(262-272)image(262-262)image(274-309)image(274-274)src/module/tracker/decider.cpp (2)
now(62-74)now(62-62)
src/utility/clock.hpp (2)
src/utility/image/image.hpp (2)
namespace rmcs {(5-22)class Image {(7-20)src/module/debug/framerate.hpp (1)
ms(5-44)
src/utility/time.hpp (2)
src/utility/image/image.hpp (2)
namespace rmcs {(5-22)class Image {(7-20)src/utility/singleton/running.hpp (1)
get_running(3-9)
src/utility/robot/color.hpp (1)
src/module/tracker/armor_filter.cpp (2)
color(7-7)color(7-7)
src/module/predictor/snapshot.hpp (4)
src/module/predictor/snapshot.cpp (11)
ekf_x(73-73)ekf_x(73-73)stamp(31-31)Snapshot(56-58)Snapshot(60-61)Snapshot(63-63)Snapshot(71-71)t(25-28)t(25-25)t(33-53)t(33-33)src/module/predictor/robot_state.cpp (3)
device(117-125)t(38-55)t(38-38)src/module/predictor/ekf_parameter.hpp (8)
device(32-43)device(32-32)device(45-55)device(45-45)device(57-65)device(57-57)device(102-134)device(102-102)src/module/predictor/robot_state.hpp (1)
t(23-23)
src/kernel/visualization.cpp (2)
src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)src/kernel/visualization.hpp (2)
armors(29-29)armors(30-30)
src/module/debug/visualization/armor_visualizer.cpp (4)
src/utility/robot/armor.hpp (2)
armor_color2camp_color(17-21)armor_color2camp_color(17-17)src/kernel/visualization.cpp (4)
armors(131-134)armors(131-131)armors(136-140)armors(136-136)src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)src/kernel/visualization.hpp (2)
armors(29-29)armors(30-30)
src/module/tracker/decider.cpp (3)
src/module/predictor/robot_state.hpp (3)
t(23-23)armor(25-25)armor(26-26)src/module/predictor/snapshot.hpp (2)
t(30-30)t(31-31)src/utility/time.hpp (2)
delta_time(6-9)delta_time(6-7)
src/utility/math/conversion.hpp (1)
src/utility/math/angle.hpp (2)
normalize_angle(5-7)normalize_angle(5-5)
🔇 Additional comments (36)
src/utility/robot/color.hpp (2)
2-4: 包含声明正确。为新增的
to_string函数正确添加了<string_view>头文件。
14-24: 实现符合最佳实践。函数正确使用了
constexpr和std::string_view,这符合之前评审建议的风格。实现简洁高效:
constexpr允许编译期求值noexcept保证不抛异常std::string_view避免不必要的内存分配- 所有枚举值均已显式处理
- 第 23 行的回退返回值作为防御性编程是合理的(防止无效的枚举强制转换)
src/kernel/pose_estimator.hpp (2)
26-29: 方法签名设计良好新增的
camera2world转换方法设计合理:
set_camera2world_transform正确使用 const 引用传递Transform- 两个
camera2world重载分别处理单个和批量装甲板转换std::span参数传值正确(span 是轻量级视图类型)- 方法标记为 const 正确(仅读取存储的变换参数)
3-4: C++23 标准已被项目完全支持项目在所有构建配置中均明确设置
CMAKE_CXX_STANDARD 23并将CMAKE_CXX_STANDARD_REQUIRED设为ON。新增的<expected>头文件是 C++23 的标准特性,完全适用于本项目。同时,yaml-cpp已在 CMakeLists.txt 中正确配置为必需依赖。这些包含文件的使用是恰当的。src/utility/math/angle.hpp (1)
5-7:normalize_angle实现正确使用
std::atan2(std::sin(angle), std::cos(angle))将角度归一化到[-π, π]区间是标准且正确的做法。src/utility/robot/priority.hpp (1)
7-7: DeviceId 的可哈希性无需担忧
DeviceId是enum class DeviceId : uint16_t,具有整数基础类型。在 C++ 中,所有具有整数基础类型的枚举都会自动获得std::hash的支持,无需显式特化。该类型别名定义正确,代码已在项目中多处成功使用(如decider.cpp中的初始化和成员变量声明),证明可哈希性完全满足要求。src/utility/shared/context.hpp (2)
37-51: LGTM!状态结构的重构合理:
- 移除
imu_state和targets字段,简化了控制状态- 新增
invincible_devices字段用于裁判系统集成- 注释清楚说明了坐标系对应关系
19-22: 字段重命名已完成,无遗留引用。对
Transform::posture到position的重命名进行了验证,代码库中所有对该字段的引用都已正确更新为新的字段名。包括src/kernel/pose_estimator.cpp和src/component.cpp在内的所有使用 Transform 结构的文件都已采用新的.position字段,无遗留的旧字段名引用。src/component.cpp (2)
18-33: LGTM!构造函数的初始化逻辑正确:
- TF 输入注册合理
- 可视化变换配置清晰,注释完善
68-81: LGTM!私有成员声明合理:
- 使用
unique_ptr管理可视化变换对象- Feishu IPC 集成正确
- 成员布局清晰
src/kernel/feishu.hpp (1)
50-55: LGTM!私有成员设计合理:
- 发送和接收客户端分离
- 使用
recv_buffer缓存读取的数据- 默认初始化正确
src/module/identifier/armor_detection.cpp (1)
33-34: 方法重命名已完整同步。
DeviceIds::kLargeArmorDevices()重命名为kLargeArmor()已在整个代码库中完成。所有测试文件(包括test/device_id.cpp:54)已使用新的 API,不存在对旧方法名的引用。src/utility/math/conversion.hpp (6)
2-2: LGTM!新增的
utility/math/angle.hpp头文件是必要的,eulers函数需要调用其中的normalize_angle。
29-39: LGTM!
xyz2ypd函数的实现正确,使用inline而非constexpr符合 C++20 的限制(std::atan2和std::sqrt在 C++20 中不是 constexpr)。数学逻辑清晰准确。
41-80: LGTM!除零保护实现得很好。
xyz2ypd_jacobian函数的实现正确:
- 在 lines 46-53 正确添加了除零保护,当
xy_norm2或norm2小于eps时返回零矩阵- 数学推导准确,雅可比矩阵的计算逻辑清晰
- 已解决之前审查中提出的除零风险问题
82-142: LGTM!
eulers函数实现正确:
- 正确处理了万向锁情况(lines 112-129)
- 使用
inline而非constexpr符合标准限制- 调用
normalize_angle规范化角度(lines 131-132)- 逻辑复杂但数学推导准确
144-150: LGTM!
ypd2xyz函数是xyz2ypd的逆变换,实现简洁正确。
152-160: LGTM!
euler_to_quaternion函数使用 Eigen 库的标准接口正确实现了欧拉角到四元数的转换,遵循 ZYX 旋转顺序。src/module/tracker/decider.hpp (1)
1-28: LGTM!设计良好的接口。
Decider类的设计优秀:
- 使用 PIMPL 模式实现良好的封装
Output结构体清晰地表达了决策输出(状态、目标ID、可选的快照)- 公共接口简洁明确:
set_priority_mode配置优先级,update执行决策- 类型别名
Clock提供了良好的抽象src/module/debug/visualization/armor_visualizer.cpp (2)
9-17: LGTM!
ArmorShadow新增ns字段用于跟踪命名空间变化,与operator==默认比较正确集成。
24-66: LGTM!
visualize方法的更新正确:
- 新增的
name和link_name参数使用const std::string&类型合理- 参数正确传递到配置和 shadow 状态(lines 49-50, 58)
- Line 58 正确地将
name存储为std::string副本- 重建检测逻辑集成良好(line 41)
src/kernel/visualization.hpp (1)
27-30: LGTM!清晰的 API 设计。新增的可视化方法设计良好:
solved_pnp_armors和predicted_armors命名清晰,表达了不同的可视化目的- 使用
std::span<Armor3D const>提供高效的视图而无需拷贝- 返回
bool表示操作状态是合理的选择src/kernel/tracker.hpp (1)
1-26: LGTM!优秀的接口设计。
Tracker类的设计非常好:
- 使用 PIMPL 模式提供良好的封装和编译隔离
initialize方法使用std::expected提供类型安全的错误处理- 公共接口职责清晰:
set_invincible_armors:配置无敌装甲板filter_armors:装甲板过滤decide:核心决策逻辑Clock::time_point类型别名清晰易懂src/kernel/tracker.cpp (1)
1-76: LGTM!Tracker 的 PIMPL 实现结构清晰,初始化逻辑正确处理了敌方颜色配置验证。代码组织良好。
src/runtime.cpp (1)
93-145: LGTM!运行时主循环的重构逻辑清晰,正确集成了 Tracker 状态管理和 Feishu 门控机制。组件初始化和错误处理都很完善。
src/module/tracker/decider.cpp (1)
10-60: LGTM!Decider 的实现结构良好,清晰地分为预测、分发、清理、仲裁和输出组装五个阶段。内存泄漏问题(
last_seen_time清理)已正确处理。评分函数综合考虑了优先级、距离和粘滞性,设计合理。src/module/predictor/snapshot.cpp (1)
56-81: LGTM!Snapshot 的 PIMPL 实现正确,拷贝和移动语义处理得当,包含自赋值检查。EKF 状态预测和装甲板生成逻辑清晰。
src/module/predictor/robot_state.cpp (1)
8-101: LGTM!RobotState 的 EKF 集成实现完善。初始化逻辑正确,
predict方法处理了超时重置,is_converged使用设备特定的阈值。之前的审查问题(未初始化状态下的 predict 行为和 OUTPOST 收敛逻辑)已正确修复。src/kernel/pose_estimator.cpp (1)
32-33: LGTM!相机到世界坐标系变换的存储和应用逻辑正确。使用 Eigen 类型直接存储避免了之前讨论的重复拷贝问题。变换应用顺序(先旋转后平移)符合标准刚体变换。
Also applies to: 104-107
src/module/predictor/snapshot.hpp (1)
13-36: PIMPL 模式实现正确类的设计遵循了 PIMPL 惯用法,正确声明了所有特殊成员函数(拷贝/移动构造函数和赋值运算符)。接口清晰,封装合理。
src/module/predictor/ekf_parameter.hpp (6)
15-30: 初始化逻辑正确从装甲板位姿计算旋转中心位置和状态向量的逻辑正确,考虑了半径偏移和方向角。
32-65: 设备相关配置实现合理针对不同设备类型(前哨站、基地、步兵、其他)的配置参数设置合理,初始协方差、半径和装甲板数量的逻辑正确。
67-98: 状态转移和角度归一化实现正确状态加法、测量差分中的角度归一化处理正确,状态转移矩阵 F 实现了标准的匀速运动模型。
102-134: 过程噪声模型实现正确分段白噪声模型的离散化公式正确(a=dt⁴/4, b=dt³/2, c=dt²),针对不同设备类型使用不同的加速度方差设置合理。
137-180: 测量模型实现正确装甲板中心位置计算考虑了4装甲板情况下的长短轴差异(l 和 h),测量函数将三维位置正确转换为 yaw-pitch-distance 表示。
204-255: 雅可比矩阵计算实现正确测量函数的雅可比矩阵通过链式法则正确计算,考虑了4装甲板情况下 l 和 h 参数的偏导数,数学推导合理。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/feishu_test.cpp:
- Around line 42-43: The child test uses std::_Exit(0) which bypasses
destructors and atexit handlers causing Feishu/AutoAimState shared memory not to
be flushed; replace std::_Exit(0) with exit(0) (or return 0 from the child
function) so destructors for feishu_child and related objects run and the shared
AutoAimState is properly synchronized after feishu_child.commit(auto_state).
🧹 Nitpick comments (1)
test/feishu_test.cpp (1)
24-24: 建议添加更明确的同步机制两个进程都通过
sleep_for(50ms)来"确保共享内存就绪",这是一种较弱的同步机制,依赖于假设共享内存在 50ms 内完成初始化。虽然在当前场景下可能工作,但更健壮的做法是使用显式的同步原语(如信号量或条件变量)来确保双方都已就绪后再进行通信。不过,修复第 43 行的
std::_Exit问题应该优先处理,因为这更可能是导致管道失败的根本原因。Also applies to: 47-47
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/device_id.cpptest/feishu_test.cpp
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
test/feishu_test.cpptest/device_id.cpp
📚 Learning: 2025-12-15T21:13:54.155Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:54.155Z
Learning: In C++ tests under the rmcs_auto_aim_v2 project, prefer local asset testing by reading resources from TEST_ASSETS_ROOT (if set) or fallback to /tmp/auto_aim. Use simple asset filenames (e.g., 'blue-0.5m.jpg') and ensure assets are pre-downloaded by running download_assets.sh against test/asset.yml. Remove any network-download logic from tests to rely on deterministic local assets. This guideline should apply to all test files in the test directory (not just test/solve_pnp.cpp) to improve test reliability and speed.
Applied to files:
test/feishu_test.cpptest/device_id.cpp
🧬 Code graph analysis (2)
test/feishu_test.cpp (2)
src/kernel/feishu.hpp (1)
Feishu(30-33)src/module/tracker/decider.cpp (2)
now(62-74)now(62-62)
test/device_id.cpp (1)
src/utility/robot/id.hpp (5)
HERO(129-135)HERO(145-154)HERO(155-163)INFANTRY_3(136-144)INFANTRY_3(170-176)
🪛 GitHub Actions: Run Test
test/feishu_test.cpp
[error] 65-65: CTest failed: FeishuIntegration.BidirectionalCommunication. Value of: auto_state.has_value() Actual: false Expected: true. Test exited with code 1. Command: '/__w/rmcs_auto_aim_v2/rmcs_auto_aim_v2/ws/src/rmcs_auto_aim_v2/test/build/test_feishu --gtest_output=xml:/__w/rmcs_auto_aim_v2/rmcs_auto_aim_v2/ws/src/rmcs_auto_aim_v2/test/build/test_results/rmcs_auto_aim_v2_tests/test_feishu.gtest.xml'.
🔇 Additional comments (1)
test/device_id.cpp (1)
54-54: LGTM!API 重命名正确。测试正确更新为新的 API 命名:
kLargeArmor()和kSmallArmor(),与src/utility/robot/id.hpp中的新方法签名一致。Also applies to: 60-60
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @test/feishu_test.cpp:
- Around line 41-42: The test currently calls std::_Exit(0) after
ASSERT_TRUE(feishu_child.commit(auto_state)), which bypasses destructors
(including Feishu) and can prevent shared memory from being flushed; replace
std::_Exit(0) with a call that allows destructors to run (e.g., exit(0) or
return from the test function) so that Feishu's destructor executes and shared
memory is properly synchronized after feishu_child.commit(auto_state).
🧹 Nitpick comments (1)
src/kernel/feishu.hpp (1)
41-46:fetch()返回引用需确保调用者理解生命周期
fetch()返回const RecvData&,该引用指向内部recv_buffer。调用者应注意:
- 返回的引用在下次
fetch()调用后可能失效- 如需持久化数据,应拷贝到局部变量
当前设计性能友好,但建议在文档中明确说明。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/kernel/feishu.hpptest/feishu_test.cpp
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
test/feishu_test.cpp
📚 Learning: 2025-12-15T21:13:54.155Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:54.155Z
Learning: In C++ tests under the rmcs_auto_aim_v2 project, prefer local asset testing by reading resources from TEST_ASSETS_ROOT (if set) or fallback to /tmp/auto_aim. Use simple asset filenames (e.g., 'blue-0.5m.jpg') and ensure assets are pre-downloaded by running download_assets.sh against test/asset.yml. Remove any network-download logic from tests to rely on deterministic local assets. This guideline should apply to all test files in the test directory (not just test/solve_pnp.cpp) to improve test reliability and speed.
Applied to files:
test/feishu_test.cpp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
test/feishu_test.cpp (1)
17-72: 测试整体逻辑清晰,覆盖了双向 IPC 通信场景测试正确验证了
Feishu<AutoAim>和Feishu<Control>之间的双向通信,包括:
- 使用
updated()+fetch()的正确模式- 合理的超时机制(500ms)
- 子进程退出状态验证
除
std::_Exit问题外,整体实现良好。src/kernel/feishu.hpp (4)
7-14: shm_name 模板特化设计合理使用主模板返回
nullptr并为具体类型提供特化,是一种类型安全的设计。ensure_open方法正确处理了nullptr的情况。
24-28: 类型别名设计清晰,利用编译期条件确保类型安全
SendData/RecvData根据角色自动推导,SendClient/RecvClient类型正确匹配 Send/Recv 语义。
30-33: 构造函数中忽略ensure_open返回值是故意设计这种惰性初始化模式允许在对端进程尚未就绪时构造对象,后续调用
commit/fetch/updated时会重试打开。考虑在构造失败时添加日志输出以便调试,但当前设计在 IPC 场景下是合理的。
58-61:ensure_open辅助函数实现简洁高效短路求值逻辑清晰:已打开则直接返回
true,否则尝试打开。name空指针检查避免了潜在的未定义行为。
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime.cpp (1)
40-41:framerate计数器未被使用
FramerateCounter实例被创建并设置了间隔,但在整个循环中从未调用tick()或fps()方法。相比之下,其他文件如tool/streaming.cpp和src/kernel/capturer.cpp中的帧率计数器正确地通过tick()定期更新。应在主循环中调用framerate.tick()以启用该功能,或删除未使用的代码。
🤖 Fix all issues with AI agents
In @src/kernel/visualization.cpp:
- Around line 134-138: The predicted_armors method currently ignores the return
value of armor_visualizer->visualize and always returns true; make it consistent
with solved_pnp_armors by returning the boolean result of
armor_visualizer->visualize(armors, "predicted_armors", "odom_imu_link") (while
still short-circuiting with false if !is_initialized) so callers get accurate
success/failure feedback from visualize.
🧹 Nitpick comments (3)
src/utility/math/angle.hpp (1)
6-8: 考虑性能优化和返回类型一致性。当前实现使用
std::atan2(std::sin(angle), std::cos(angle))模式归一化角度,虽然数值稳定性好,但需要调用三个超越函数,计算开销较大。如果此函数在性能敏感路径中频繁调用,建议使用std::remainder替代,可达到相同效果且性能更优。此外,为保持与
deg2rad和rad2deg的一致性,建议将返回类型auto改为显式的double。♻️ 建议的性能优化方案
-inline auto normalize_angle(double angle) -> auto { - return std::atan2(std::sin(angle), std::cos(angle)); +inline auto normalize_angle(double angle) -> double { + return std::remainder(angle, 2.0 * std::numbers::pi); }src/utility/shared/context.hpp (1)
25-26: 初始化语法不一致
should_control使用{ false }而should_shoot使用= { false },建议统一风格。♻️ 建议的修复
bool should_control { false }; - bool should_shoot = { false }; + bool should_shoot { false };src/runtime.cpp (1)
123-126: 考虑可视化失败时的日志级别当
solved_pnp_armors失败时使用info级别记录。如果可视化失败是意外情况,考虑使用warn级别以便更容易识别问题。♻️ 建议的修复
if (visualization.initialized()) { auto success = visualization.solved_pnp_armors(*armors_3d_opt); - if (!success) rclcpp_node.info("可视化PNP结算后的装甲板失败"); + if (!success) rclcpp_node.warn("可视化PNP结算后的装甲板失败"); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
config/config.yamlsrc/kernel/visualization.cppsrc/module/debug/framerate.hppsrc/module/predictor/ekf_parameter.hppsrc/runtime.cppsrc/utility/image/image.cppsrc/utility/image/image.hppsrc/utility/math/angle.hppsrc/utility/shared/context.hpptest/feishu_test.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- test/feishu_test.cpp
- src/module/predictor/ekf_parameter.hpp
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/kernel/visualization.cpp
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/kernel/visualization.cppsrc/runtime.cppsrc/utility/image/image.cpp
📚 Learning: 2025-12-16T22:21:32.866Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 12
File: package.xml:15-15
Timestamp: 2025-12-16T22:21:32.866Z
Learning: In the rmcs_auto_aim_v2 repository, do not flag Chinese comment style issues such as "唯二" for standardization. The maintainer prefers to keep the current informal/colloquial Chinese phrasing in comments.
Applied to files:
src/runtime.cpp
🧬 Code graph analysis (2)
src/kernel/visualization.cpp (7)
src/kernel/tracker.cpp (4)
armors(45-48)armors(45-45)armors(50-53)armors(50-50)src/module/tracker/decider.cpp (2)
armors(13-60)armors(13-13)src/kernel/pose_estimator.cpp (4)
armors(62-102)armors(62-62)armors(123-143)armors(123-123)src/kernel/pose_estimator.hpp (1)
armors(28-28)src/kernel/visualization.hpp (2)
armors(29-29)armors(30-30)src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)src/module/predictor/snapshot.cpp (2)
predicted_armors(79-81)predicted_armors(79-79)
src/runtime.cpp (1)
src/module/tracker/decider.cpp (2)
now(62-74)now(62-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (13)
src/utility/math/angle.hpp (1)
10-11: 已正确使用std::numbers::pi解决跨平台兼容性问题。这两个函数现在使用
std::numbers::pi替代了之前的M_PI,成功解决了先前评审中提出的跨平台兼容性问题。实现正确,函数被正确标记为constexpr,可在编译期求值。src/module/debug/framerate.hpp (2)
2-11: LGTM!重构方向与 PR 目标一致。将
std::chrono::steady_clock替换为集中式的util::Clock符合 PR 描述中提到的时间管理统一化目标,这有助于在整个项目中实现一致的时间处理。
3-3: 删除#include <chrono>不会导致编译错误。
utility/clock.hpp已在第 3 行包含<chrono>,framerate.hpp通过包含"utility/clock.hpp"传递性地获得了<chrono>头文件。因此,std::chrono::milliseconds、std::chrono::seconds和std::chrono_literals等所有必要的符号都可用。代码可以正常编译。Likely an incorrect or invalid review comment.
src/utility/image/image.cpp (1)
6-6: 时间戳类型迁移正确。实现文件中的类型更改与头文件声明一致,从
Stamp迁移到Clock::time_point的改动符合 PR 的集中化时间管理目标。Also applies to: 13-13, 17-17
src/utility/image/image.hpp (3)
2-2: 集中化时钟依赖的良好实践。通过
utility/clock.hpp统一时钟依赖,减少了对标准库<chrono>的直接依赖,符合 PR 中统一时间管理的重构目标。
11-11: 通过 util::Clock 统一时钟类型。将 Clock 别名从直接使用
std::chrono::steady_clock改为util::Clock,提供了更好的抽象层,便于统一管理整个代码库的时钟实现。
17-18: 不存在遗留的Image::Stamp引用。验证确认
Stamp类型别名已完全移除,代码库中无任何对Image::Stamp的引用。直接使用Clock::time_point的迁移已彻底完成。此 API 变更的实现是一致且完整的。config/config.yaml (1)
50-52: Tracker 配置看起来合理新增的 tracker 配置块与 PR 中引入的 Tracker 模块对齐。
enemy_color的位置选择符合当前只有装甲板过滤器使用该配置的设计。src/utility/shared/context.hpp (1)
35-48: ControlState 结构变更合理移除
imu_state和targets,新增invincible_devices字段,与新的 Tracker/Feishu 驱动流程一致。static_assert确保结构仍满足 trivially copyable 约束。src/kernel/visualization.cpp (1)
152-157: 新的可视化 API 封装正确
solved_pnp_armors和predicted_armors的公共接口正确转发到 Impl 实现,与头文件声明一致。src/runtime.cpp (3)
73-78: Tracker 初始化遵循一致的模式初始化代码块与其他组件(capturer、identifier、pose_estimator)保持一致的风格和错误处理模式。
97-109: Feishu 驱动的控制流合理使用
feishu.updated()进行门控可避免在没有新状态时进行不必要的处理。invincible_devices正确传递给 tracker 进行装甲板过滤。
131-140: 追踪器决策和预测流程结构清晰结构化绑定获取追踪状态、目标设备和快照,通过状态检查实现早期返回。
target_device未使用是预期行为(将在后续模块中处理)。预测装甲板可视化正确使用了当前时间点。
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @src/kernel/feishu.hpp:
- Around line 38-43: fetch() returns a reference to the mutable member
recv_buffer which can cause data races when called concurrently; either document
that the Feishu instance is not thread-safe and callers must synchronize access,
or make fetch() thread-safe by protecting accesses to recv_buffer with an
internal mutex (e.g., add a std::mutex member and lock it in fetch() and any
other methods that read/write recv_buffer such as updated()/any setter). Ensure
the chosen approach is applied consistently and update class-level comments to
state the thread-safety guarantee or add the mutex and lock/unlock around
recv_client.with_read and the return path.
In @src/module/predictor/ekf_parameter.hpp:
- Around line 138-163: dz_dh should be initialized as a double literal and r
needs non-negative protection: change any dz_dh initialization to use 0.0 (not
0) to avoid int→double ambiguities (affecting matrix init), and inside
h_armor_xyz() (and the H() Jacobian function) clamp computed radius r = r_min +
l using something like r = std::max(r, eps) (with a small eps > 0) to prevent r
from becoming negative during divergence; update references to r_min and l
accordingly and pick a consistent eps constant used by both h_armor_xyz and H.
- Around line 59-67: Rename the misleading local variable is_balance to
is_infantry inside the static method armor_num in ekf_parameter.hpp: update the
declaration and all uses (where it checks
DeviceIds::kInfantry().contains(device)) so the logic remains the same but the
name accurately reflects that the check tests membership in the infantry set;
ensure any related comments or nearby references are updated to match the new
identifier.
- Around line 17-82: 这些成员函数不应是 constexpr,因为它们返回或操作 Eigen 矩阵/向量(非字面类型);请从类内定义中移除
x(), P_initial_dig(), x_add(), z_subtract(), F(), Q(), h_armor_xyz(), h(), R(),
H() 的 constexpr 关键字(保留 static/内联定义),仅对返回标量的 radius() 与 armor_num() 保持
constexpr;其它函数保持为普通静态成员函数以避免常量表达式约束并确保符号如
x、P_initial_dig、x_add、z_subtract、F、Q、h_armor_xyz、h、R、H 在实现处去掉 constexpr。
In @src/module/tracker/decider.cpp:
- Around line 104-126: mode1 and mode2 are defined but never used; either
document them as intentional presets or remove them to avoid dead code. If they
are intended presets for switching priority modes, expose them via a clear
accessor (e.g., get_preset_mode or similar) or integrate them into
set_priority_mode so callers can select a preset; otherwise delete the unused
PriorityMode constants or add a comment above mode1/mode2 explicitly stating
they are reserved presets.
In @src/runtime.cpp:
- Around line 132-139: The log call inside the visualization block can spam when
solved_pnp_armors fails; add throttling by tracking last_log_time and only
calling rclcpp_node.info when enough time has passed (e.g., >1s). Modify the
block around visualization.solved_pnp_armors to use a static timestamp (or
existing FramerateCounter helper) and util::delta_time/Clock::now() to compare
intervals, update last_log_time after logging, and leave success handling
otherwise unchanged.
In @src/utility/math/angle.hpp:
- Around line 8-10: The function normalize_angle is marked constexpr but calls
std::atan2/std::sin/std::cos which are not constexpr under C++23, causing
compile failures; change the declaration from constexpr to inline (or remove
constexpr) for normalize_angle(double) so it remains a header-inline utility
without requiring C++26, keeping the implementation identical and ensuring
callers receive the same behavior.
🧹 Nitpick comments (13)
src/module/identifier/lightbar.cpp (1)
45-47: 提醒:待实现的 TODO 标记TODO 注释表明此处的实现尚未完成。当前函数不会将任何灯条添加到结果向量中,导致始终返回空向量。请确保在后续提交中完成灯条对象的创建和添加逻辑。
是否需要我协助生成灯条对象的创建代码,或者为此任务开启一个新的 issue 进行跟踪?
src/utility/robot/constant.hpp (1)
5-7: 建议添加文档注释并明确常量含义。这些常量缺少必要的文档说明,建议改进:
- 缺少注释:应说明每个常量代表什么(
kBaseRadius对应基地/哨兵机器人,kOutpostRadius对应前哨站,kOtherRadius作为其他机器人类型的默认值)- 缺少单位说明:虽然从数值推测是米(meters),但应明确标注
- 命名过于宽泛:
kOtherRadius含义不明确,建议使用更具描述性的名称(如kStandardRobotRadius或kInfantryRobotRadius)- 缺少来源说明:这些数值来自官方规则手册还是实测?建议注释中说明
📝 建议的改进方案
+// Robot radius constants (in meters) +// Values from RoboMaster official competition rules + -constexpr double kBaseRadius = 0.3205; -constexpr double kOutpostRadius = 0.2765; -constexpr double kOtherRadius = 0.2; +constexpr double kBaseRadius = 0.3205; // Base/Sentry robot radius +constexpr double kOutpostRadius = 0.2765; // Outpost robot radius +constexpr double kStandardRobotRadius = 0.2; // Standard infantry/hero robot radiussrc/kernel/tracker.cpp (2)
42-42: 参数类型不一致
Impl::filter_armors接受std::span<Armor2D> const&,而公共方法Tracker::filter_armors接受std::span<Armor2D>。建议统一签名以保持一致性。♻️ 建议修改
-auto Tracker::filter_armors(std::span<Armor2D> armors) const -> std::vector<Armor2D> { +auto Tracker::filter_armors(std::span<Armor2D> const& armors) const -> std::vector<Armor2D> { return pimpl->filter_armors(armors); }同时需要更新头文件中的声明。
Also applies to: 67-67
43-45: 可简化返回语句这些方法中的临时变量可以省略,直接返回结果。
♻️ 建议简化
auto filter_armors(std::span<Armor2D> const& armors) const -> std::vector<Armor2D> { - auto result = filter.filter(armors); - return result; + return filter.filter(armors); } auto decide(std::span<Armor3D const> armors, Clock::time_point t) -> Decider::Output { - auto decider_output = decider.update(armors, t); - return decider_output; + return decider.update(armors, t); }Also applies to: 48-50
src/module/tracker/decider.cpp (1)
34-42: 清理逻辑正确但可读性可优化在
std::erase_if的 lambda 中同时修改last_seen_time和primary_target_id是安全的,但副作用较多。考虑添加注释说明或拆分为独立步骤以提高可读性。src/runtime.cpp (1)
100-104: FIXME 注释合理关于离线调试模式的需求是合理的。当前的
feishu.updated()强依赖会阻止独立测试。是否需要我帮助实现一个调试模式,在
Feishu不可用时使用默认的ControlState?src/kernel/feishu.hpp (2)
8-15: 建议为模板变量添加文档说明。
shm_name模板变量的默认值为nullptr,这对于未特化的类型会导致ensure_open返回false。建议添加注释说明哪些类型支持共享内存通信,以及如何为新类型添加特化。
19-59: 建议为公共 API 添加文档注释。
Feishu类缺少文档说明,建议为以下内容添加注释:
- 类的整体用途和通信模型(双向 IPC)
commit方法:返回false时的含义和应对方式fetch方法:失败时返回缓冲区数据的行为(当前仅在第 39 行有简短说明)updated方法:与fetch的配合使用方式- 线程安全性:明确说明单个实例不应被多线程并发访问
这将帮助使用者正确理解和使用该 IPC 机制。
src/utility/math/angle.hpp (1)
12-13: 同样的constexpr兼容性问题
std::numbers::pi在 C++20 中是constexpr,但如果保持与normalize_angle的一致性,建议统一使用inline。这两个函数本身的数学运算可以是constexpr,但为了代码风格一致性可考虑统一。src/utility/math/conversion.hpp (3)
50-53: 边界情况处理的注意事项当坐标接近零点时返回零矩阵是合理的数值稳定性处理,但调用方(如 EKF)应意识到这种行为。建议在注释中补充说明调用方应如何处理这种情况,或在函数文档中明确说明返回零矩阵的含义。
82-142: 复杂算法建议添加来源注释此 Euler 角提取算法较为复杂,包含 gimbal lock 处理和多种轴序支持。建议在函数头部添加注释说明算法来源(如 scipy 的
Rotation.as_euler实现),便于后续维护和验证。另外,第 142 行函数结束后的分号是多余的,建议移除。
🔧 移除多余分号
return eulers; -}; +}
152-160: 参数传递方式和旋转顺序文档
- 对于
double类型,按值传递比const&更高效(避免间接寻址)。- 旋转顺序
yaw * pitch * roll(Z-Y-X) 是常见约定,建议在注释中明确说明,便于调用方理解。🔧 建议修改
+// 旋转顺序: Z(yaw) → Y(pitch) → X(roll),即 ZYX 内旋 inline auto euler_to_quaternion( - const double& yaw_rad, const double& pitch_rad, const double& roll_rad) -> Eigen::Quaterniond { + double yaw_rad, double pitch_rad, double roll_rad) -> Eigen::Quaterniond { Eigen::AngleAxisd rollAngle(roll_rad, Eigen::Vector3d::UnitX()); Eigen::AngleAxisd pitchAngle(pitch_rad, Eigen::Vector3d::UnitY()); Eigen::AngleAxisd yawAngle(yaw_rad, Eigen::Vector3d::UnitZ()); Eigen::Quaterniond q = yawAngle * pitchAngle * rollAngle; return q; }src/module/predictor/ekf_parameter.hpp (1)
104-136: 噪声模型里把 r/l/h 的过程噪声固定为 0:确认这是“刻意锁死”还是遗漏
Q()的最后三维(r/l/h,对应 Line 131-133)全为 0,会让这些状态仅靠初始协方差/量测更新演化;同时OUTPOST/BASE的P_initial_dig把 l/h 直接设为 0(Line 37-40),等价于完全不估计它们。若你的“单车预测/装甲切换半径差(l)/高度差(h)”确实只针对 4 装甲板目标,这没问题;否则建议把策略写成显式注释或按armor_num/device 分支设置。Also applies to: 184-204
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
src/component.cppsrc/kernel/feishu.hppsrc/kernel/tracker.cppsrc/kernel/visualization.cppsrc/module/debug/framerate.hppsrc/module/debug/visualization/armor_visualizer.cppsrc/module/identifier/lightbar.cppsrc/module/predictor/ekf_parameter.hppsrc/module/tracker/decider.cppsrc/runtime.cppsrc/utility/clock.hppsrc/utility/math/angle.hppsrc/utility/math/conversion.hppsrc/utility/robot/constant.hppsrc/utility/robot/priority.hppsrc/utility/time.hpp
🚧 Files skipped from review as they are similar to previous changes (3)
- src/utility/time.hpp
- src/utility/clock.hpp
- src/module/debug/framerate.hpp
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
📚 Learning: 2025-12-15T20:25:43.104Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: src/component.cpp:36-36
Timestamp: 2025-12-15T20:25:43.104Z
Learning: In the rmcs_auto_aim_v2 C++ repository, //... placeholder comments are intentional markers and should not be flagged for removal or replacement with TODOs during code reviews. Do not treat them as issues; if clarification is needed, note their purpose in a review comment or replace with a more explicit marker only when appropriate.
Applied to files:
src/module/tracker/decider.cppsrc/runtime.cppsrc/kernel/tracker.cppsrc/module/debug/visualization/armor_visualizer.cppsrc/module/identifier/lightbar.cppsrc/kernel/visualization.cppsrc/component.cpp
📚 Learning: 2025-12-16T22:21:32.866Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 12
File: package.xml:15-15
Timestamp: 2025-12-16T22:21:32.866Z
Learning: In the rmcs_auto_aim_v2 repository, do not flag Chinese comment style issues such as "唯二" for standardization. The maintainer prefers to keep the current informal/colloquial Chinese phrasing in comments.
Applied to files:
src/runtime.cppsrc/utility/robot/priority.hppsrc/component.cpp
📚 Learning: 2025-12-15T09:35:52.883Z
Learnt from: creeper5820
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 10
File: tool/CMakeLists.txt:37-42
Timestamp: 2025-12-15T09:35:52.883Z
Learning: In the rmcs_auto_aim_v2 project, the visualization executable in tool/CMakeLists.txt is specifically for rclcpp visualization and only uses ROS libraries (rclcpp, visualization_msgs, geometry_msgs). It does not require yaml-cpp, OpenVINO, or OpenCV dependencies.
Applied to files:
src/component.cpp
📚 Learning: 2025-12-15T21:13:59.238Z
Learnt from: heyeuu
Repo: Alliance-Algorithm/rmcs_auto_aim_v2 PR: 11
File: test/solve_pnp.cpp:0-0
Timestamp: 2025-12-15T21:13:59.238Z
Learning: 在 rmcs_auto_aim_v2 项目中,test/solve_pnp.cpp 已重构为使用本地文件测试:通过 TEST_ASSETS_ROOT 环境变量或默认路径 /tmp/auto_aim 读取资源,测试用例使用简单文件名(如 "blue-0.5m.jpg"),资源由 download_assets.sh 脚本从 test/asset.yml 预下载,移除了测试代码中的网络下载逻辑。
Applied to files:
src/component.cpp
🧬 Code graph analysis (6)
src/runtime.cpp (4)
src/utility/rclcpp/configuration.hpp (2)
configuration(9-15)configuration(9-9)src/module/tracker/decider.cpp (2)
now(61-72)now(61-61)src/kernel/control_system.hpp (2)
namespace rmcs::kernel {(5-21)class ControlSystem {(7-19)src/kernel/control_system.cpp (1)
F(7-41)
src/utility/robot/constant.hpp (3)
src/utility/robot/armor.hpp (2)
DARK(9-101)struct Armor { }(68-68)src/utility/robot/color.hpp (1)
UNKNOWN(4-12)src/utility/monostate.hpp (1)
namespace rmcs {(3-5)
src/module/debug/visualization/armor_visualizer.cpp (2)
src/utility/robot/armor.hpp (2)
armor_color2camp_color(17-21)armor_color2camp_color(17-17)src/module/debug/visualization/armor_visualizer.hpp (1)
armors(16-17)
src/module/identifier/lightbar.cpp (1)
src/module/identifier/lightbar.hpp (4)
struct Details(9-9)RED(4-26)RED(6-13)struct LightbarFinder {(15-24)
src/utility/math/conversion.hpp (1)
src/utility/math/angle.hpp (2)
normalize_angle(8-10)normalize_angle(8-8)
src/module/predictor/ekf_parameter.hpp (4)
src/module/predictor/robot_state.cpp (13)
armor(19-27)armor(19-19)armor(57-88)armor(57-57)armor(140-172)armor(140-140)r(90-101)x(33-36)x(127-138)x(127-127)device(117-125)distance(192-192)distance(192-192)src/utility/math/conversion.hpp (6)
eulers(82-142)eulers(82-83)xyz2ypd(29-39)xyz2ypd(29-29)xyz2ypd_jacobian(41-80)xyz2ypd_jacobian(41-41)src/utility/robot/id.hpp (8)
OUTPOST(164-169)result(120-127)id(110-112)id(110-110)id(117-117)id(117-117)id(118-118)id(118-118)src/utility/math/angle.hpp (2)
normalize_angle(8-10)normalize_angle(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (16)
src/module/identifier/lightbar.cpp (1)
41-43: 请验证角度检查逻辑的正确性当前的角度检查仅判断
angle < max_angle_error,但std::atan2返回的角度范围为 [-π, π],可能包含负值。通常的角度误差检查会使用绝对值(如std::abs(angle) > max_angle_error)或检查角度是否在某个范围内。请确认当前逻辑是否符合预期的过滤行为,特别是对于负角度的处理。
src/utility/robot/constant.hpp (1)
1-1: 头文件保护符合规范。使用
#pragma once是现代 C++ 的标准做法,简洁且被广泛支持。src/module/debug/visualization/armor_visualizer.cpp (2)
9-17: 结构体扩展合理新增的
ns字段与needs_rebuild检查逻辑配合良好,确保当可视化命名空间变化时能正确重建视觉对象。默认的operator==也会自动包含新字段的比较。
24-25: API 扩展清晰
visualize方法新增name和link_name参数,使得同一个ArmorVisualizer可以为不同的可视化场景(如solved_pnp_armors和predicted_armors)提供不同的命名空间和坐标系配置。实现与头文件声明一致。Also applies to: 49-50
src/component.cpp (1)
25-31: 可视化变换配置正确
visual_camera2odom的配置清晰,父子坐标系设置与visualization.cpp中predicted_armors使用的odom_imu_link保持一致。src/kernel/visualization.cpp (2)
128-136: 可视化方法拆分合理将
visualize_armors拆分为solved_pnp_armors(相机坐标系)和predicted_armors(世界坐标系)是合理的设计,清晰地区分了 PnP 解算结果和预测结果的可视化。
150-155: 公共接口实现一致公共方法正确地委托给内部实现,保持了 PIMPL 模式的一致性。
src/module/tracker/decider.cpp (1)
79-83:priority_mode初始为空可能导致评分不完整
priority_mode默认为空映射,这意味着在调用set_priority_mode之前,calculate_score中的优先级评分分支不会生效。所有目标仅依赖距离和粘滞性评分。请确认这是预期行为,或者考虑使用
mode1作为默认值:- PriorityMode priority_mode; + PriorityMode priority_mode = mode1;Also applies to: 99-99
src/runtime.cpp (2)
144-154: 跟踪流程实现清晰从
tracker.decide获取状态和快照,然后基于状态进行分支处理的逻辑是合理的。使用结构化绑定使代码更加清晰。
147-149: 确认状态与快照对应关系根据
Decider::update()的实现(decider.cpp 第 46-58 行),返回State::Tracking的分支中总是通过target_tracker->get_snapshot()赋值snapshot。这意味着第 149 行的if (!snapshot_opt) continue;检查可能确实是冗余的,前提是RobotState::get_snapshot()保证返回有效值。建议确认
RobotState::get_snapshot()是否在任何情况下都不返回std::nullopt;如果是,该检查可被安全移除或至少添加说明注释。src/utility/robot/priority.hpp (1)
1-10: 类型别名定义正确
PriorityMode作为DeviceId到优先级整数的映射设计合理。DeviceId作为enum class类型自动获得std::hash支持,可直接用于std::unordered_map而无需自定义哈希特化。src/kernel/feishu.hpp (2)
45-47: 实现正确!
updated方法正确使用了短路求值,确保只在通道打开时才检查更新状态。逻辑清晰且高效。
31-36: 不存在 noexcept 规范问题。
AutoAimState和ControlState都有static_assert(std::is_trivially_copyable_v<...>)断言,这意味着它们的拷贝赋值运算符是编译器生成的平凡拷贝操作,隐式为noexcept。因此commit方法中的shared = data拷贝操作不会抛出异常,noexcept规范是正确的。src/utility/math/conversion.hpp (2)
29-39: LGTM!
xyz2ypd函数实现正确,使用标准球坐标转换公式。std::atan2能正确处理零向量边界情况。
144-150: LGTM!
ypd2xyz正确实现了xyz2ypd的逆变换,公式正确。src/module/predictor/ekf_parameter.hpp (1)
1-11: 已支持的功能,无需额外修改文件正常编译且在实际代码中正被使用。
std::numbers::pi通过utility/math/angle.hpp的传递包含已可用(angle.hpp 明确包含<numbers>),Eigen::Quaterniond和其他 Eigen 类型通过utility/math/conversion.hpp(包含<eigen3/Eigen/Geometry>)和utility/math/kalman_filter/ekf.hpp(包含<eigen3/Eigen/Dense>)的传递包含已可用。在 C++23 中,static constexpr函数返回非字面类型时仍可用于运行时调用,而本文件的这些函数仅在 robot_state.cpp 中的运行时被调用,不存在编译问题。
| static constexpr auto armor_num(DeviceId const& device) -> int { | ||
| auto is_balance = DeviceIds::kInfantry().contains(device); | ||
|
|
||
| auto num = int {}; | ||
| if (device == DeviceId::OUTPOST || device == DeviceId::BASE) num = 3; | ||
| else if (is_balance) num = 2; | ||
| else num = 4; | ||
| return num; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.hpp" -o -name "*.h" -o -name "*.cpp" | head -20Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 687
🏁 Script executed:
# First, let's find where DeviceIds is defined
rg -l "class DeviceIds|struct DeviceIds" --type cpp --type hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 106
🏁 Script executed:
# Look for the DeviceId enum/class definition and DeviceIds methods
rg -B 2 -A 10 "kInfantry\(|kBalance\(|kSentry\(" -t cpp -t hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 106
🏁 Script executed:
# Check armor_num() usage
rg -B 2 -A 2 "armor_num\(" -t cpp -t hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 106
🏁 Script executed:
# Look for "balance" or "Balance" references related to devices
rg "is_balance|kBalance|Balance.*device|device.*[Bb]alance" -t cpp -t hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 106
🏁 Script executed:
# Find DeviceIds definition without type restrictions
rg "kInfantry|kBalance|kSentry|class DeviceIds|struct DeviceIds" -nRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 303
🏁 Script executed:
# Search for armor_num usage
rg "armor_num" -n -B 2 -A 2Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 9036
🏁 Script executed:
# Look at the ekf_parameter.hpp file directly
cat src/module/predictor/ekf_parameter.hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 9842
🏁 Script executed:
# Search for balance-related code
rg "is_balance|Balance" -nRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 267
🏁 Script executed:
# View the DeviceIds definition and kInfantry method
cat src/utility/robot/id.hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 6027
🏁 Script executed:
# Also search for any comments or documentation about device categories
rg "kInfantry|Infantry|balance|Balance" src/utility/robot/id.hpp -n -B 2 -A 2Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 216
is_balance 变量命名应改为 is_infantry
DeviceIds::kInfantry() 包含 INFANTRY_3、INFANTRY_4、INFANTRY_5 三种步兵,而非仅"平衡步兵"(通常指 INFANTRY_4)。该变量在第 60 行检查设备是否属于标准步兵集合以分配 2 个装甲板,但命名为 is_balance 造成语义误导,易使人误认为仅针对特定步兵类型。建议改为 is_infantry 或类似更准确的名称。
🤖 Prompt for AI Agents
In @src/module/predictor/ekf_parameter.hpp around lines 59 - 67, Rename the
misleading local variable is_balance to is_infantry inside the static method
armor_num in ekf_parameter.hpp: update the declaration and all uses (where it
checks DeviceIds::kInfantry().contains(device)) so the logic remains the same
but the name accurately reflects that the check tests membership in the infantry
set; ensure any related comments or nearby references are updated to match the
new identifier.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/module/predictor/ekf_parameter.hpp (3)
47-67: 考虑统一代码风格。
radius()使用switch语句,而armor_num()使用if-else。为了代码风格一致性,建议两者都使用switch或都使用if-else。♻️ 建议的重构(将 armor_num 改为 switch)
static auto armor_num(DeviceId const& device) -> int { - auto is_balance = DeviceIds::kInfantry().contains(device); - - auto num = int {}; - if (device == DeviceId::OUTPOST || device == DeviceId::BASE) num = 3; - else if (is_balance) num = 2; - else num = 4; - return num; + switch (device) { + case DeviceId::OUTPOST: + case DeviceId::BASE: + return 3; + default: + if (DeviceIds::kInfantry().contains(device)) { + return 2; + } + return 4; + } }
183-203: 建议添加注释解释自适应噪声公式。Line 196 中的观测噪声协方差公式比较复杂:
R_dig << 4e-3, 4e-3, std::log(std::abs(delta_yaw) + 1) + 1, std::log(std::abs(distance) + 1) / 200 + 9e-2;虽然自适应噪声是良好的实践(基于 delta_yaw 和 distance 调整不确定性),但具体的系数(200、9e-2)和对数公式的物理意义不够明确。建议添加注释说明:
- delta_yaw 如何影响测量不确定性
- distance 的对数关系的依据
- 系数 200 和 9e-2 的来源(经验值或理论推导)
这将提高代码的可维护性,便于后续参数调优。
138-147: 考虑注释语言的一致性。代码中包含中文注释(Lines 138, 140-147, 206-213),而代码标识符和其他部分使用英文。如果团队有国际协作或开源计划,建议统一使用英文注释。如果团队主要为中文使用者,当前的中文注释也可以接受。
这不是关键问题,但统一注释语言可以提高代码库的一致性和可维护性。
Also applies to: 206-213
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/module/predictor/ekf_parameter.hppsrc/utility/math/angle.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utility/math/angle.hpp
🧰 Additional context used
🧬 Code graph analysis (1)
src/module/predictor/ekf_parameter.hpp (2)
src/utility/math/conversion.hpp (4)
xyz2ypd(29-39)xyz2ypd(29-29)xyz2ypd_jacobian(41-80)xyz2ypd_jacobian(41-41)src/utility/math/angle.hpp (2)
normalize_angle(8-10)normalize_angle(8-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (10)
src/module/predictor/ekf_parameter.hpp (10)
14-15: LGTM!EKF 类型别名定义清晰,11 维状态向量和 4 维观测向量与后续实现一致。
34-45: LGTM!基于设备类型的初始协方差对角线设置合理,针对 OUTPOST、BASE 和其他设备的不同特性进行了参数调优。
75-81: 验证对 pitch 角度的归一化处理。Line 78 对观测向量的索引 1(pitch)进行了角度归一化。
normalize_angle()将角度映射到 [-π, π],但 pitch 角在物理意义上通常应限制在 [-π/2, π/2] 范围内。请确认:
- 在实际场景中,pitch 是否可能超出 ±π/2 范围?
- 如果 pitch 始终在 [-π/2, π/2] 内,此归一化是否必要?
- 如果 pitch 可能接近 ±π/2 边界,使用
normalize_angle()是否会导致错误的角度折叠?
83-100: LGTM!状态转移矩阵 F 正确实现了恒速模型,每个位置-速度对使用标准的 [1, dt; 0, 1] 块,最后三个状态变量(r, l, h)被假设为常量。
104-136: 验证 l 和 h 状态变量的过程噪声设置。Lines 131-133 将状态变量 r, l, h 的过程噪声设置为零,这意味着它们被假设为常量。然而,对于 4 装甲板配置,l(半径差)和 h(高度差)在装甲板切换时应该会变化。如果过程噪声为零,滤波器将无法适应这些参数的变化,可能导致装甲板切换时的跟踪性能下降。
建议:
- 验证 l 和 h 在实际场景中是否确实会变化
- 如果会变化,考虑为 l 和 h 添加小的非零过程噪声
138-162: 验证装甲板位置计算的符号约定。Lines 156-157 中装甲板位置的计算使用了负号:
const auto pos_x = center_x - r * std::cos(angle); const auto pos_y = center_y - r * std::sin(angle);然而,在
x()方法(Lines 26-27)中,旋转中心的计算使用了正号:const auto center_x = trans_x + r * std::cos(yaw); const auto center_y = trans_y + r * std::sin(yaw);这种符号不一致可能表明:
- 坐标系定义中,装甲板位置相对于旋转中心的方向是相反的(装甲板在中心后方)
- 或者这是一个逻辑错误
请验证这种符号约定是否符合实际的坐标系定义和物理模型。
164-172: LGTM!观测模型
h()正确地将状态向量转换为观测向量(yaw, pitch, distance, armor_yaw),逻辑清晰。
174-181: LGTM!状态预测函数
f(dt)正确实现,使用 F(dt) 进行线性传播并归一化 yaw 角度。
205-256: LGTM!测量雅可比矩阵 H 的计算数学上正确,偏导数推导与观测模型
h_armor_xyz()一致。链式法则的应用(H = H_armor_ypda * H_armor_xyza)准确,对 4 装甲板配置中 l 和 h 的条件性偏导数处理得当。注意:偏导数计算中的符号与
h_armor_xyz()中的负号约定(Lines 156-157)保持一致,确保了数学上的自洽性。
17-32: 无需修改。Armor3D::genre的类型为ArmorGenre,而ArmorGenre在 src/utility/robot/armor.hpp 中明确定义为DeviceId的类型别名(第36行)。因此对radius(armor.genre)的调用是类型安全的,与函数签名完全兼容。
实现单车预测
单车预测器完整实现(更新)
核心变化概述
主要新增/改动模块
IPC 与运行时
Tracker 与决策链
单车跟踪与预测(predictor 模块)
PoseEstimator 与坐标变换
可视化与 armor_visualizer
数学、EKF 与工具库
公共数据结构与 API 变更
测试与构建
配置文件变更
向后兼容性与审查要点
技术亮点