Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough新增并注册 rmcs_core 的 Sentry 硬件组件；为底盘与云台控制器接入导航速度输入并参数化若干 PID/限幅项；移除 hikcamera 子模块；新增远程/本地运维脚本与 zsh 补全；更新构建与若干硬件接口与配置文件。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as 用户/脚本
participant SSH as ssh-remote
participant ROS as ROS2
participant Sentry as Sentry Component
participant Top as TopBoard
participant Bottom as BottomBoard
participant HW as Motors/IMU/CAN/UART
User->>ROS: 发布导航命令 或 调用本地服务
alt 远端模式
User->>SSH: 通过 ssh-remote 执行远端命令
SSH->>ROS: 在远端环境调用 ROS2 服务
end
ROS->>Sentry: 下发控制命令 / 请求状态服务
Sentry->>Top: 读取云台传感器、发布 TF、发送云台命令
Sentry->>Bottom: 读取底盘传感器、发布 TF、发送底盘命令
Top->>HW: CAN/UART 交互（电机/IMU/伺服）
Bottom->>HW: CAN/UART/串口 交互（车轮/转向/裁判）
HW-->>Sentry: 传感器与反馈
Sentry-->>ROS: 发布状态输出 / 服务响应（含 yaw_angle/yaw_velocity）
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
rmcs_ws/src/rmcs_core/src/controller/gimbal/two_axis_gimbal_solver.hpp (1)
138-139: 潜在的零向量归一化风险。当
yaw_axis->z()和yaw_axis->x()同时为零时（即轴与 Y 方向对齐），pitch向量长度为零，调用normalize()会导致未定义行为（除以零）。虽然
yaw_axis_filtered_作为单位向量持续维护（第 128 行），在正常云台操作中此风险较低，但考虑添加防御性检查可提高鲁棒性。🛡️ 可选的防御性检查
pitch = {yaw_axis->z(), yaw_axis->x()}; + if (pitch.squaredNorm() < 1e-12) { + // Handle degenerate case where yaw axis is aligned with Y + pitch = {1.0, 0.0}; + } pitch.normalize();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/two_axis_gimbal_solver.hpp` around lines 138 - 139, The code constructs pitch = {yaw_axis->z(), yaw_axis->x()} then calls pitch.normalize() which can divide by zero if that vector is (near) zero; add a defensive check in the block that builds pitch (referencing pitch, yaw_axis and yaw_axis_filtered_) to compute the vector norm and only call normalize() when norm > epsilon (e.g. 1e-8); if the norm is <= epsilon, set pitch to a safe fallback (for example reuse a previous valid pitch, or compute a different orthogonal vector such as a unit vector along a canonical axis) and normalize that fallback, and optionally emit a debug/warning log so the rare case is visible..script/remote-status (1)
1-5: 脚本结构良好，但复杂的 sed 表达式可考虑添加注释脚本正确使用了
set -euo pipefail进行严格错误处理。第 5 行的嵌套引号和 sed 模式较复杂，建议添加简短注释说明其用途，以便后续维护。♻️ 可选：添加注释提升可读性
#!/bin/zsh set -euo pipefail +# 远程调用 /rmcs/service/status 服务，使用 sed 提取 message='...' 中的内容 ssh-remote "bash -lc 'source ~/env_setup.bash && msg=\$(ros2 service call /rmcs/service/status std_srvs/srv/Trigger \"{}\" | sed -n \"s/.*message='\\''\\(.*\\)'\\'')/\\1/p\") && printf \"%b\\n\" \"\$msg\"'"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.script/remote-status around lines 1 - 5, 在第 5 行包含复杂嵌套引号和 sed 表达式的 ssh-remote 命令（ssh-remote "bash -lc 'source ~/env_setup.bash && msg=$(ros2 service call /rmcs/service/status std_srvs/srv/Trigger \"{}\" | sed -n \"s/.*message='\\''\\(.*\\)'\\'')/\\1/p\") && printf \"%b\\n\" \"$msg\"'") 上方或行内添加一行简短注释，说明该 sed 表达式的目标（从 ros2 服务调用的输出中提取 message 字段并去除外层引号）以及为何需要双重/转义引号，便于后续维护与调试；保留现有命令不变，仅增注释以提高可读性。rmcs_ws/src/rmcs_bringup/config/sentry.yaml (1)
33-34: 建议将板卡序列号改为设备级覆盖配置Line 33-34 硬编码具体序列号会把该配置绑定到单台设备。建议基础配置保留占位值，并通过 launch 参数或机体专用 overlay 注入真实序列号。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_bringup/config/sentry.yaml` around lines 33 - 34, The current config hardcodes device-specific serials in the keys board_serial_top_board and board_serial_bottom_board; change these to placeholder values (e.g., empty strings or "UNKNOWN") in the base sentry.yaml and remove the hardcoded "d4-1d2b" and "d4-041d" values, then update the device-specific launch/overlay mechanism to inject the real serials via launch arguments or the machine-specific overlay so each device gets its own board_serial_top_board and board_serial_bottom_board at runtime.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp`:
- Around line 143-154: The code currently clamps only translational_velocity but
then adds nav_velocity (computed from navigation_command_velocity_ and
navigation_velocity_scale_) which can push the result over
translational_velocity_max; update the return path (after computing nav_velocity
and before returning translational_velocity + nav_velocity) to enforce the
translational magnitude limit by computing the combined vector
(translational_velocity + nav_velocity) and, if its norm exceeds
translational_velocity_max, scale it down to translational_velocity_max (e.g.,
combined *= translational_velocity_max / combined.norm()); use the existing
symbols translational_velocity, nav_velocity (and nav_velocity_odom),
navigation_command_velocity_, navigation_velocity_scale_, gimbal_yaw_angle_, and
translational_velocity_max to locate and implement the clamp.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 84-89: The current normalization of yaw_angle computed from
*top_yaw_angle_ + *bottom_yaw_angle_ only subtracts/adds 2π once and can still
be out of range for multi-turn inputs; update the normalization logic around the
local variable yaw_angle (computed from top_yaw_angle_ and bottom_yaw_angle_) so
it repeatedly wraps into [0, 2π) (or uses std::fmod with corrective
adds/subtracts) until yaw_angle is within bounds, then assign the normalized
value to *yaw_angle_; ensure you modify the block that reads top_yaw_angle_,
bottom_yaw_angle_ and writes yaw_angle_ so all multi-revolution inputs are
correctly folded into the target interval.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/simple_gimbal_controller.cpp`:
- Around line 74-80: The code multiplies navigation yaw_speed by a hardcoded 1
ms instead of the real controller timestep, causing incorrect yaw_shift scaling
when the executor update_rate differs; update the navigation integration to use
the actual control timestep variable (control_dt_) or accept dt from
rmcs_executor::Executor and replace the fixed 1e-3 multiplier where
navigation_command_velocity_, yaw_speed, and yaw_shift are updated (also update
the similar occurrence noted around the second usage) so yaw_shift += yaw_speed
* actual_dt.
In `@rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp`:
- Around line 82-90: 当前 ROS 回调与硬件更新在不同线程并发访问 TopBoard/BottomBoard
导致数据竞争（相关符号：Sentry::update、SentryCommand::update、command_update、TopBoard/BottomBoard，以及成员方法
last_raw_angle(), calibrate_zero_point(), update_status(),
generate_command()）；请将所有会读写 board 的 ROS callbacks/service handlers
改为“投递请求到更新线程”或在 Sentry 内用一个单一互斥锁保护对 top_board_ 和 bottom_board_ 的所有访问（包含
update(), command_update() 与回调处理路径），确保回调只 enqueue 请求结构并返回，由更新线程在
Sentry::update()/SentryCommand::update() 中串行消费并调用相应的 board 方法，从而消除数据竞态与校准穿插问题。
- Around line 1-10: The file is missing headers causing compile failures: add
`#include` <ostream> to provide the std::ostream overloads used by std::println in
the code that writes into std::ostringstream (the block around the
feedback_message usage) and add `#include` <array> to cover the std::array usage
at/near line 114; alternatively, replace the std::println(...) into the
ostringstream with a std::format(...) + ostringstream.write if you prefer not to
add headers—locate the code that constructs feedback_message and the std::array
instance to apply the fix.
---
Nitpick comments:
In @.script/remote-status:
- Around line 1-5: 在第 5 行包含复杂嵌套引号和 sed 表达式的 ssh-remote 命令（ssh-remote "bash -lc
'source ~/env_setup.bash && msg=$(ros2 service call /rmcs/service/status
std_srvs/srv/Trigger \"{}\" | sed -n \"s/.*message='\\''\\(.*\\)'\\'')/\\1/p\")
&& printf \"%b\\n\" \"$msg\"'") 上方或行内添加一行简短注释，说明该 sed 表达式的目标（从 ros2 服务调用的输出中提取
message 字段并去除外层引号）以及为何需要双重/转义引号，便于后续维护与调试；保留现有命令不变，仅增注释以提高可读性。
In `@rmcs_ws/src/rmcs_bringup/config/sentry.yaml`:
- Around line 33-34: The current config hardcodes device-specific serials in the
keys board_serial_top_board and board_serial_bottom_board; change these to
placeholder values (e.g., empty strings or "UNKNOWN") in the base sentry.yaml
and remove the hardcoded "d4-1d2b" and "d4-041d" values, then update the
device-specific launch/overlay mechanism to inject the real serials via launch
arguments or the machine-specific overlay so each device gets its own
board_serial_top_board and board_serial_bottom_board at runtime.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/two_axis_gimbal_solver.hpp`:
- Around line 138-139: The code constructs pitch = {yaw_axis->z(),
yaw_axis->x()} then calls pitch.normalize() which can divide by zero if that
vector is (near) zero; add a defensive check in the block that builds pitch
(referencing pitch, yaw_axis and yaw_axis_filtered_) to compute the vector norm
and only call normalize() when norm > epsilon (e.g. 1e-8); if the norm is <=
epsilon, set pitch to a safe fallback (for example reuse a previous valid pitch,
or compute a different orthogonal vector such as a unit vector along a canonical
axis) and normalize that fallback, and optionally emit a debug/warning log so
the rare case is visible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5ef8bfb0-d456-4f8e-88a9-4827652f1f19
📒 Files selected for processing (18)
.clang-format.gitmodules.script/complete/_save-map-once.script/remote-status.script/save-map-onceDockerfilermcs_ws/src/hikcamerarmcs_ws/src/rmcs_bringup/config/sentry.yamlrmcs_ws/src/rmcs_core/package.xmlrmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/steering_wheel_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/simple_gimbal_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/two_axis_gimbal_solver.hpprmcs_ws/src/rmcs_core/src/hardware/device/dji_motor.hpprmcs_ws/src/rmcs_core/src/hardware/device/lk_motor.hpprmcs_ws/src/rmcs_core/src/hardware/sentry.cpp
💤 Files with no reviewable changes (2)
- .gitmodules
- rmcs_ws/src/hikcamera
Comment on lines
+143
to
+154
| // Navigation Control | ||
| auto nav_velocity_odom = Eigen::Vector2d{Eigen::Vector2d::Zero()}; | ||
| if (navigation_command_velocity_.ready()) { | ||
| auto raw_command = *navigation_command_velocity_; | ||
| if (std::isfinite(raw_command.x()) && std::isfinite(raw_command.y())) { | ||
| nav_velocity_odom.x() = raw_command.x() * navigation_velocity_scale_; | ||
| nav_velocity_odom.y() = raw_command.y() * navigation_velocity_scale_; | ||
| } | ||
| } | ||
| auto nav_velocity = Eigen::Rotation2Dd{*gimbal_yaw_angle_} * nav_velocity_odom; | ||
|
|
||
| return translational_velocity + nav_velocity; |
There was a problem hiding this comment.
导航速度叠加后要重新限幅。
这里先把手动输入限到 translational_velocity_max，再直接叠加导航速度并返回。手动和导航同时生效时，最终输出可以明显超过 10.0，直接绕过这里定义的最大平移速度。
🔧 建议修改
- auto nav_velocity = Eigen::Rotation2Dd{*gimbal_yaw_angle_} * nav_velocity_odom;
-
- return translational_velocity + nav_velocity;
+ translational_velocity += Eigen::Rotation2Dd{*gimbal_yaw_angle_} * nav_velocity_odom;
+ if (const auto norm = translational_velocity.norm();
+ norm > translational_velocity_max) {
+ translational_velocity *= translational_velocity_max / norm;
+ }
+ return translational_velocity;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Navigation Control | |
| auto nav_velocity_odom = Eigen::Vector2d{Eigen::Vector2d::Zero()}; | |
| if (navigation_command_velocity_.ready()) { | |
| auto raw_command = *navigation_command_velocity_; | |
| if (std::isfinite(raw_command.x()) && std::isfinite(raw_command.y())) { | |
| nav_velocity_odom.x() = raw_command.x() * navigation_velocity_scale_; | |
| nav_velocity_odom.y() = raw_command.y() * navigation_velocity_scale_; | |
| } | |
| } | |
| auto nav_velocity = Eigen::Rotation2Dd{*gimbal_yaw_angle_} * nav_velocity_odom; | |
| return translational_velocity + nav_velocity; | |
| // Navigation Control | |
| auto nav_velocity_odom = Eigen::Vector2d{Eigen::Vector2d::Zero()}; | |
| if (navigation_command_velocity_.ready()) { | |
| auto raw_command = *navigation_command_velocity_; | |
| if (std::isfinite(raw_command.x()) && std::isfinite(raw_command.y())) { | |
| nav_velocity_odom.x() = raw_command.x() * navigation_velocity_scale_; | |
| nav_velocity_odom.y() = raw_command.y() * navigation_velocity_scale_; | |
| } | |
| } | |
| translational_velocity += Eigen::Rotation2Dd{*gimbal_yaw_angle_} * nav_velocity_odom; | |
| if (const auto norm = translational_velocity.norm(); | |
| norm > translational_velocity_max) { | |
| translational_velocity *= translational_velocity_max / norm; | |
| } | |
| return translational_velocity; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp` around
lines 143 - 154, The code currently clamps only translational_velocity but then
adds nav_velocity (computed from navigation_command_velocity_ and
navigation_velocity_scale_) which can push the result over
translational_velocity_max; update the return path (after computing nav_velocity
and before returning translational_velocity + nav_velocity) to enforce the
translational magnitude limit by computing the combined vector
(translational_velocity + nav_velocity) and, if its norm exceeds
translational_velocity_max, scale it down to translational_velocity_max (e.g.,
combined *= translational_velocity_max / combined.norm()); use the existing
symbols translational_velocity, nav_velocity (and nav_velocity_odom),
navigation_command_velocity_, navigation_velocity_scale_, gimbal_yaw_angle_, and
translational_velocity_max to locate and implement the clamp.
There was a problem hiding this comment.
@qzhhhi，了解，感谢说明！translational_velocity_max 的限幅确实只针对遥控手动输入部分，导航速度叠加在限幅之后是有意为之的设计。我的评论有误，抱歉打扰。
Comment on lines
+84
to
+89
| double yaw_angle = *top_yaw_angle_ + *bottom_yaw_angle_; | ||
| if (yaw_angle < 0) | ||
| yaw_angle += 2 * std::numbers::pi; | ||
| else if (yaw_angle > 2 * std::numbers::pi) | ||
| yaw_angle -= 2 * std::numbers::pi; | ||
| *yaw_angle_ = yaw_angle; |
There was a problem hiding this comment.
角度归一化仅处理一次环绕，存在越界边界情况
在 Line 84-88，当前只做一次 ±2π 修正；当输入和超过一圈（例如 > 4π 或 < -2π）时，yaw_angle 仍可能不在目标区间内，导致下游姿态使用异常。
建议修复（支持多圈输入）
- double yaw_angle = *top_yaw_angle_ + *bottom_yaw_angle_;
- if (yaw_angle < 0)
- yaw_angle += 2 * std::numbers::pi;
- else if (yaw_angle > 2 * std::numbers::pi)
- yaw_angle -= 2 * std::numbers::pi;
+ double yaw_angle = *top_yaw_angle_ + *bottom_yaw_angle_;
+ while (yaw_angle < 0)
+ yaw_angle += 2 * std::numbers::pi;
+ while (yaw_angle > 2 * std::numbers::pi)
+ yaw_angle -= 2 * std::numbers::pi;
*yaw_angle_ = yaw_angle;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around
lines 84 - 89, The current normalization of yaw_angle computed from
*top_yaw_angle_ + *bottom_yaw_angle_ only subtracts/adds 2π once and can still
be out of range for multi-turn inputs; update the normalization logic around the
local variable yaw_angle (computed from top_yaw_angle_ and bottom_yaw_angle_) so
it repeatedly wraps into [0, 2π) (or uses std::fmod with corrective
adds/subtracts) until yaw_angle is within bounds, then assign the normalized
value to *yaw_angle_; ensure you modify the block that reads top_yaw_angle_,
bottom_yaw_angle_ and writes yaw_angle_ so all multi-revolution inputs are
correctly folded into the target interval.
There was a problem hiding this comment.
@qzhhhi 明白，感谢说明！由于 top_yaw_angle_ 和 bottom_yaw_angle_ 的取值范围已被约束，两者之和不会超出单次 ±2π 修正能覆盖的范围，现有的 if/else if 逻辑完全正确。此审查意见有误，抱歉造成困扰。
✏️ Learnings added
Learnt from: qzhhhi
Repo: Alliance-Algorithm/RMCS PR: 56
File: rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp:103-108
Timestamp: 2026-03-21T17:31:27.009Z
Learning: In `rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`, the inputs `top_yaw_angle_` and `bottom_yaw_angle_` are bounded such that their sum never exceeds `4π` or goes below `-2π`. Therefore, the single `if/else if` correction (adding or subtracting `2π` once) to normalize `yaw_angle` into `[0, 2π]` is always sufficient, and using `while` loops is unnecessary. Do not flag this as a bug.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: qzhhhi
Repo: Alliance-Algorithm/RMCS PR: 54
File: rmcs_ws/src/rmcs_core/src/hardware/device/lk_motor.hpp:465-475
Timestamp: 2026-03-11T17:19:30.956Z
Learning: In `rmcs_ws/src/rmcs_core/src/hardware/device/lk_motor.hpp`, the `to_absolute_command_angle()` method intentionally uses `(raw_angle_modulus_ - encoder_zero_point_)` in the subtraction term. This is NOT a bug. It is a deliberate two-in-one correction: (1) subtract one full output-shaft turn to compensate for LK motor's internal multi-turn angle being off by one turn at power-on, and (2) add back the `encoder_zero_point_ / raw_angle_modulus_` zero-point offset. Replacing this with just `encoder_zero_point_` would remove the one-turn correction and flip the zero-point offset sign, causing incorrect absolute angle commands. There is a known limitation: the correction is hardcoded to 1 turn; if the motor powers on in a different region or is rotated by external force before the program starts, the actual offset may be N turns (marked TODO for future fix).
rmcs_ws/src/rmcs_core/src/controller/gimbal/simple_gimbal_controller.cpp
Show resolved
Hide resolved
Comment on lines
+1
to
+10
| #include <cstddef> | ||
| #include <format> | ||
| #include <memory> | ||
| #include <print> | ||
| #include <ranges> | ||
| #include <span> | ||
| #include <sstream> | ||
| #include <string_view> | ||
| #include <tuple> | ||
| #include <utility> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does C++23 <print> provide std::println overloads for std::ostringstream/std::basic_ostream, or only for stdout and FILE*?
💡 Result:
C++23 provides both kinds of std::println overloads, but they’re split across headers:
<print>declares thestd::printlnoverloads that write tostdout(no stream parameter) and to aFILE*(first parameter isFILE*). [1]- The overloads that write to a C++ iostream are declared in
<ostream>:std::println(std::ostream& os, /*format*/, args...)(and similarly for wide output). This works withstd::ostringstreamtoo, since it’s anstd::ostream. [2]
So: not only stdout/FILE*; there is an std::ostream& overload—but it’s in <ostream>, not <print>. [1][2]
🏁 Script executed:
# First, let's find and examine the sentry.cpp file
find . -name "sentry.cpp" -type fRepository: Alliance-Algorithm/RMCS
Length of output: 114
🏁 Script executed:
# Let's check the content of the file at lines 1-130 to see includes and the problematic code
cat -n rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp | head -130Repository: Alliance-Algorithm/RMCS
Length of output: 5674
缺少必要的头文件导致编译失败。
代码在第 99-117 行使用 std::println(feedback_message, ...) 将内容写入 std::ostringstream，但 <print> 头文件只提供写入 stdout 和 FILE* 的重载。对 std::ostream 的重载声明在 <ostream> 头文件中，需要添加 #include <ostream>。
此外，第 114 行使用 std::array 但文件未包含 <array> 头文件。
建议添加 #include <ostream> 和 #include <array>，或改用 std::format() 结合 ostringstream 的方式。
🧰 Tools
🪛 Clang (14.0.6)
[error] 1-1: 'cstddef' file not found
(clang-diagnostic-error)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp` around lines 1 - 10, The file
is missing headers causing compile failures: add `#include` <ostream> to provide
the std::ostream overloads used by std::println in the code that writes into
std::ostringstream (the block around the feedback_message usage) and add
`#include` <array> to cover the std::array usage at/near line 114; alternatively,
replace the std::println(...) into the ostringstream with a std::format(...) +
ostringstream.write if you prefer not to add headers—locate the code that
constructs feedback_message and the std::array instance to apply the fix.
Comment on lines
+82
to
+90
| void update() override { | ||
| top_board_->update(); | ||
| bottom_board_->update(); | ||
| } | ||
|
|
||
| void command_update() { | ||
| top_board_->command_update(); | ||
| bottom_board_->command_update(); | ||
| } |
There was a problem hiding this comment.
把 ROS 回调和硬件更新串行化。
这些 service/subscription 回调直接读写 TopBoard / BottomBoard 里的电机对象，而 rmcs_executor::Executor 会在独立线程里调用 Sentry::update()，SentryCommand::update() 也会继续触发 command_update()。现在 last_raw_angle()、calibrate_zero_point()、update_status()、generate_command() 都可能并发访问同一批状态，属于未同步的数据竞争；校准请求还可能正好插进一次控制帧中间。至少要把回调改成“投递请求，由更新线程处理”，或者用统一的互斥锁把 board 访问串行化。
Also applies to: 93-148, 150-158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp` around lines 82 - 90, 当前 ROS
回调与硬件更新在不同线程并发访问 TopBoard/BottomBoard
导致数据竞争（相关符号：Sentry::update、SentryCommand::update、command_update、TopBoard/BottomBoard，以及成员方法
last_raw_angle(), calibrate_zero_point(), update_status(),
generate_command()）；请将所有会读写 board 的 ROS callbacks/service handlers
改为“投递请求到更新线程”或在 Sentry 内用一个单一互斥锁保护对 top_board_ 和 bottom_board_ 的所有访问（包含
update(), command_update() 与回调处理路径），确保回调只 enqueue 请求结构并返回，由更新线程在
Sentry::update()/SentryCommand::update() 中串行消费并调用相应的 board 方法，从而消除数据竞态与校准穿插问题。
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 68-81: The top yaw loop uses
top_yaw_angle_pid_.update(-true_angle) directly as the velocity-PID input,
missing real velocity feedback; change the velocity input to (angle_setpoint -
actual_velocity) similar to bottom yaw by calling the IMU-derived velocity
(e.g., top_yaw_velocity_imu()) — compute auto angle_setpoint =
top_yaw_angle_pid_.update(-true_angle), then pass angle_setpoint -
top_yaw_velocity_imu() into top_yaw_velocity_pid_.update(...) and assign the
resulting torque to top_yaw_control_torque_.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0b39e10-1546-4132-b2f9-8212a885f19e
📒 Files selected for processing (2)
rmcs_ws/src/rmcs_bringup/config/sentry.yamlrmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- rmcs_ws/src/rmcs_bringup/config/sentry.yaml
Comment on lines
+68
to
+81
|
|
||
| /// @FIXME: | ||
| /// The implement of dual yaw controlling is not completed | ||
| /// Let it stable as a common gimbal | ||
| { | ||
| auto true_angle = *top_yaw_angle_; | ||
| if (true_angle > std::numbers::pi) | ||
| true_angle -= 2 * std::numbers::pi; | ||
|
|
||
| const auto velocity = top_yaw_angle_pid_.update(-true_angle); | ||
| const auto torque = top_yaw_velocity_pid_.update(velocity); | ||
|
|
||
| *top_yaw_control_torque_ = torque; | ||
| } |
There was a problem hiding this comment.
Top yaw 速度环缺少实际速度反馈
对比第 83-85 行的 bottom yaw 控制实现，它使用 pid_angle.update(error) - bottom_yaw_velocity_imu() 作为速度环输入，即速度设定值减去实际速度。
但 top yaw 在第 77-78 行直接将角度 PID 输出作为速度 PID 输入，缺少实际速度反馈。这会导致速度环实际上是开环工作的，无法有效抑制扰动。
建议修复：添加速度反馈
{
auto true_angle = *top_yaw_angle_;
if (true_angle > std::numbers::pi)
true_angle -= 2 * std::numbers::pi;
- const auto velocity = top_yaw_angle_pid_.update(-true_angle);
- const auto torque = top_yaw_velocity_pid_.update(velocity);
+ const auto velocity_setpoint = top_yaw_angle_pid_.update(-true_angle);
+ // 使用 IMU 或编码器速度反馈
+ const auto torque = top_yaw_velocity_pid_.update(
+ velocity_setpoint - *gimbal_yaw_velocity_imu_);
*top_yaw_control_torque_ = torque;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around
lines 68 - 81, The top yaw loop uses top_yaw_angle_pid_.update(-true_angle)
directly as the velocity-PID input, missing real velocity feedback; change the
velocity input to (angle_setpoint - actual_velocity) similar to bottom yaw by
calling the IMU-derived velocity (e.g., top_yaw_velocity_imu()) — compute auto
angle_setpoint = top_yaw_angle_pid_.update(-true_angle), then pass
angle_setpoint - top_yaw_velocity_imu() into top_yaw_velocity_pid_.update(...)
and assign the resulting torque to top_yaw_control_torque_.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp (2)
103-108:⚠️ Potential issue | 🟡 Minor
yaw_angle归一化仍只处理一次环绕Line 103-107 只做一次
±2π修正；多圈输入时可能仍越界。该问题在历史评论中已指出，当前实现仍未闭环。建议修复
double yaw_angle = *top_yaw_angle_ + *bottom_yaw_angle_; - if (yaw_angle < 0) - yaw_angle += 2 * std::numbers::pi; - else if (yaw_angle > 2 * std::numbers::pi) - yaw_angle -= 2 * std::numbers::pi; + while (yaw_angle < 0) + yaw_angle += 2 * std::numbers::pi; + while (yaw_angle >= 2 * std::numbers::pi) + yaw_angle -= 2 * std::numbers::pi; *yaw_angle_ = yaw_angle;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around lines 103 - 108, 当前对 yaw_angle 的归一化只做了一次 ±2π 修正，无法处理多圈输入；在 dual_yaw_controller.cpp 中计算完 double yaw_angle = *top_yaw_angle_ + *bottom_yaw_angle_; 后，改为将角度完整归一化到 [0, 2π)（例如使用 std::fmod 或循环：yaw_angle = std::fmod(yaw_angle, 2*std::numbers::pi); if (yaw_angle < 0) yaw_angle += 2*std::numbers::pi;），然后再写回 *yaw_angle_，确保针对任意倍数 2π 的输入都能正确映射。
82-87:⚠️ Potential issue | 🟠 MajorTop yaw 速度环仍缺少实际速度反馈
Line 84-85 仍是“角度环输出直接喂给速度环”，没有减去实际角速度，速度环等效开环，和 bottom yaw 控制结构不一致。这个问题在历史评论中已提过，当前提交仍存在。
建议修复
- const auto velocity = top_yaw_angle_pid_.update(-true_angle); - const auto torque = top_yaw_velocity_pid_.update(velocity); + const auto velocity_setpoint = top_yaw_angle_pid_.update(-true_angle); + const auto torque = top_yaw_velocity_pid_.update( + velocity_setpoint - *gimbal_yaw_velocity_imu_);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around lines 82 - 87, The angle loop currently feeds its output directly into the velocity PID (top_yaw_angle_pid_.update -> top_yaw_velocity_pid_.update) creating an open-loop velocity controller; change it to compute a velocity setpoint from top_yaw_angle_pid_.update, then subtract the measured top yaw angular velocity (use the actual sensor/state variable, e.g. *top_yaw_velocity_ or whatever the project uses for measured rate) to form a velocity error, pass that error into top_yaw_velocity_pid_.update, and then write the resulting torque into *top_yaw_control_torque_; this makes the top yaw control structure consistent with bottom yaw.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp`:
- Line 153: The variable nav_velocity_odom is misleading because the code
applies Eigen::Rotation2Dd{*gimbal_yaw_angle_} to it (the same rotation used for
joystick/local inputs), so confirm the coordinate frame emitted by
/rmcs_navigation/command_velocity and either (a) rename nav_velocity_odom to a
clearer name (e.g., nav_velocity_local or nav_velocity_cmd) that matches the
actual frame, or (b) add a concise comment near the use of nav_velocity_odom and
gimbal_yaw_angle_ stating the expected source frame and why the rotation is
applied; if the frame is actually odom but the rotation is wrong, adjust the
rotation logic accordingly to convert from the true source frame to the chassis
frame.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 63-68: The code currently binds chassis_output_status_ to true
when the referee input is missing, which bypasses the safety check that triggers
on !*chassis_output_status_; change the fallback to fail-safe by calling
chassis_output_status_.bind_directly(false) instead of true, and apply the same
change to any other similar fallback bindings in this file (the analogous block
at lines 72-75) so missing referee inputs default to false and allow the
protection branch to run.
---
Duplicate comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 103-108: 当前对 yaw_angle 的归一化只做了一次 ±2π 修正，无法处理多圈输入；在
dual_yaw_controller.cpp 中计算完 double yaw_angle = *top_yaw_angle_ +
*bottom_yaw_angle_; 后，改为将角度完整归一化到 [0, 2π)（例如使用 std::fmod 或循环：yaw_angle =
std::fmod(yaw_angle, 2*std::numbers::pi); if (yaw_angle < 0) yaw_angle +=
2*std::numbers::pi;），然后再写回 *yaw_angle_，确保针对任意倍数 2π 的输入都能正确映射。
- Around line 82-87: The angle loop currently feeds its output directly into the
velocity PID (top_yaw_angle_pid_.update -> top_yaw_velocity_pid_.update)
creating an open-loop velocity controller; change it to compute a velocity
setpoint from top_yaw_angle_pid_.update, then subtract the measured top yaw
angular velocity (use the actual sensor/state variable, e.g. *top_yaw_velocity_
or whatever the project uses for measured rate) to form a velocity error, pass
that error into top_yaw_velocity_pid_.update, and then write the resulting
torque into *top_yaw_control_torque_; this makes the top yaw control structure
consistent with bottom yaw.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 40e57a0a-69e4-4358-9b80-54fb8bb99c4f
📒 Files selected for processing (3)
rmcs_ws/src/rmcs_bringup/config/sentry.yamlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp
✅ Files skipped from review due to trivial changes (1)
- rmcs_ws/src/rmcs_bringup/config/sentry.yaml
Comment on lines
+63
to
+68
| if (!chassis_output_status_.ready()) { | ||
| RCLCPP_WARN( | ||
| get_logger(), | ||
| "Failed to fetch \"/referee/chassis/output_status\", fallback to enabled."); | ||
| chassis_output_status_.bind_directly(true); | ||
| } |
There was a problem hiding this comment.
缺失裁判系统状态时回退为 true 会绕过失效保护
Line 63-68 在输入缺失时绑定 true，但 Line 72 依赖 !*chassis_output_status_ 进入保护分支；这会导致输入丢失时仍继续输出控制（fail-open）。建议改为 fail-safe（回退 false）。
建议修复
if (!chassis_output_status_.ready()) {
RCLCPP_WARN(
get_logger(),
- "Failed to fetch \"/referee/chassis/output_status\", fallback to enabled.");
- chassis_output_status_.bind_directly(true);
+ "Failed to fetch \"/referee/chassis/output_status\", fallback to disabled.");
+ chassis_output_status_.bind_directly(false);
}Also applies to: 72-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around
lines 63 - 68, The code currently binds chassis_output_status_ to true when the
referee input is missing, which bypasses the safety check that triggers on
!*chassis_output_status_; change the fallback to fail-safe by calling
chassis_output_status_.bind_directly(false) instead of true, and apply the same
change to any other similar fallback bindings in this file (the analogous block
at lines 72-75) so missing referee inputs default to false and allow the
protection branch to run.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmcs_ws/src/rmcs_bringup/config/sentry.yaml`:
- Around line 46-47: The YAML keys upper_limit and lower_limit are semantically
inverted (upper_limit < lower_limit) and conflict with how they are used as
std::clamp(lower, upper) in precise_two_axis_gimbal_solver.hpp and
player_viewer.cpp; rename the parameters to clear names such as min_pitch_angle
and max_pitch_angle (and update their numeric values accordingly) or swap the
values to match the existing names, then update all call sites that read these
keys and any inline comments to reflect the new names/semantics (ensure the code
using std::clamp still supplies min then max).
- Around line 5-30: sentry.yaml maps rmcs::Brain -> rmcs_navigation but
rmcs::Brain is not exported in plugins.xml so pluginlib::ClassLoader will fail
to instantiate; fix by either updating the mapping in sentry.yaml to the actual
exported class name that provides rmcs_navigation (replace rmcs::Brain with the
exported symbol found in plugins.xml) or add an export entry for rmcs::Brain ->
rmcs_navigation into the appropriate plugins.xml (ensure the exported class name
exactly matches rmcs::Brain and the export tag matches the pluginlib format) so
rmcs_navigation can be loaded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e402f03-941d-43c2-bed9-d893d2beee29
📒 Files selected for processing (1)
rmcs_ws/src/rmcs_bringup/config/sentry.yaml
Comment on lines
+46
to
+47
| upper_limit: -0.39518 | ||
| lower_limit: 0.36 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 只读检查：定位 upper_limit/lower_limit 的读取与限幅实现
set -euo pipefail
fd -t f '.*gimbal.*controller.*\.(cpp|hpp)$' rmcs_ws/src/rmcs_core/src/controller
echo
rg -n -C3 'upper_limit|lower_limit|clamp\(|std::clamp' rmcs_ws/src/rmcs_core/src/controller/gimbalRepository: Alliance-Algorithm/RMCS
Length of output: 11900
俯仰角限制参数的命名与数值语义相反，已被开发者注意到但可能导致维护混淆。
Line 46 的 upper_limit 值 (-0.39518) 确实小于 Line 47 的 lower_limit 值 (0.36)。代码实现中，这两个参数被作为 std::clamp() 的下界和上界使用（见 precise_two_axis_gimbal_solver.hpp 第 88 行和 player_viewer.cpp 第 105 行），且开发者已在代码中注释说明此语义反转。限幅行为本身没有问题，但参数命名约定与实际数值范围相反，容易误导后续维护者。建议明确文档或重新命名参数以匹配角度的实际含义（如 min_pitch_angle / max_pitch_angle）。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_bringup/config/sentry.yaml` around lines 46 - 47, The YAML
keys upper_limit and lower_limit are semantically inverted (upper_limit <
lower_limit) and conflict with how they are used as std::clamp(lower, upper) in
precise_two_axis_gimbal_solver.hpp and player_viewer.cpp; rename the parameters
to clear names such as min_pitch_angle and max_pitch_angle (and update their
numeric values accordingly) or swap the values to match the existing names, then
update all call sites that read these keys and any inline comments to reflect
the new names/semantics (ensure the code using std::clamp still supplies min
then max).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp (2)
1-13:⚠️ Potential issue | 🟠 Major补齐
<array>头文件，避免依赖传递包含导致的编译脆弱性。Line 128 使用了
std::array，但当前 include 列表没有<array>。这在不同标准库实现下可能直接编译失败。建议显式添加头文件。🔧 建议修改
+#include <array> `#include` <cmath> `#include` <cstddef> `#include` <format>#!/bin/bash set -euo pipefail FILE="rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp" echo "== include 区域 ==" cat -n "$FILE" | sed -n '1,25p' echo echo "== std::array 使用位置 ==" rg -n "std::array" "$FILE" echo echo "== 是否显式包含 <array> ==" rg -n "^#include <array>$" "$FILE" || trueAlso applies to: 127-129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp` around lines 1 - 13, 源文件在 include 区域缺少 <array> 导致在 Line 128 使用 std::array 时可能编译失败；在头部包含列表中显式添加 <array> 就能修复此问题；定位到使用 std::array 的位置（例如在 sentry.cpp 中出现的 std::array 实例），在文件开头的其他 system includes（例如 `#include` <span> 之后或合适的排序位置）添加 `#include` <array>，确保不依赖传递包含并通过编译。
96-104:⚠️ Potential issue | 🔴 Critical请串行化对
TopBoard/BottomBoard的访问，当前存在数据竞争。Line 96-104 的更新路径与 Line 107-162 的 service/subscription 回调都在直接读写同一批电机/板卡对象；若回调线程与执行线程并发运行，会出现竞态（状态读取不一致、校准穿插控制帧）。建议统一为“回调只入队，
update/command_update线程消费”，或用同一把互斥锁覆盖所有 board 访问路径。Also applies to: 107-135, 137-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp` around lines 96 - 104, 当前对 TopBoard/BottomBoard 的并发访问会导致数据竞争；在 Sentry 的 update() 和 command_update()（调用 top_board_->update()/bottom_board_->update()/command_update()）与 service/subscription 回调中存在同时读写同一批电机/板卡对象的问题。修复方法：要么将所有回调改为仅把事件/请求入队列，由同一个线程在 update()/command_update() 中统一消费并操作 TopBoard/BottomBoard；要么在所有访问板卡的路径（包括所有 service/subscription 回调与 update()/command_update()）上使用同一把互斥锁（例如 board_mutex）来串行化对 top_board_ 和 bottom_board_ 的读写，确保在调用 top_board_->* 或 bottom_board_->* 前后都持有该锁并在消费队列时也持有锁以避免竞态。
🧹 Nitpick comments (1)
rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp (1)
146-151:ready()检查冗余，可简化。在
before_updating()中，如果navigation_command_velocity_未就绪，已经调用make_and_bind_directly()创建了 NaN 向量。因此在update()阶段，ready()始终返回true，真正的有效性判断是isfinite()检查。♻️ 建议简化
// Navigation Control auto nav_velocity_odom = Eigen::Vector2d{Eigen::Vector2d::Zero()}; - if (navigation_command_velocity_.ready()) { - auto raw_command = *navigation_command_velocity_; - if (std::isfinite(raw_command.x()) && std::isfinite(raw_command.y())) { - nav_velocity_odom.x() = raw_command.x() * navigation_velocity_scale_; - nav_velocity_odom.y() = raw_command.y() * navigation_velocity_scale_; - } + auto raw_command = *navigation_command_velocity_; + if (std::isfinite(raw_command.x()) && std::isfinite(raw_command.y())) { + nav_velocity_odom.x() = raw_command.x() * navigation_velocity_scale_; + nav_velocity_odom.y() = raw_command.y() * navigation_velocity_scale_; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp` around lines 146 - 151, Remove the redundant ready() check in update(): always dereference navigation_command_velocity_ (as done via auto raw_command = *navigation_command_velocity_) and rely solely on std::isfinite(raw_command.x()) / std::isfinite(raw_command.y()) to decide whether to update nav_velocity_odom; keep the scaling by navigation_velocity_scale_ when finite and leave (or explicitly set) nav_velocity_odom unchanged when not finite. This change uses the existing before_updating()/make_and_bind_directly() behavior and removes the unnecessary navigation_command_velocity_.ready() guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rmcs_ws/src/rmcs_core/CMakeLists.txt`:
- Around line 18-22: 当前 CMakeLists.txt 在 FetchContent_Declare 的 SOURCE_DIR
中硬编码了本地绝对路径（librmcs），导致在 CI/新环境中失败；请改为使用可配置的 CMake 变量（例如
LIBRMCS_SOURCE_DIR）作为优先本地路径并在不存在时回退到远程 URL（例如 LIBRMCS_URL 或官方发布 tarball/GIT
URL），在定义时根据变量存在与否为 FetchContent_Declare 的 SOURCE_DIR 或 URL/URL_HASH 设置值，保留目标名
librmcs 并继续调用 FetchContent_MakeAvailable(librmcs)，同时添加缓存变量说明（CACHE STRING）和一条
message() 输出以便在构建日志中可见所用的来源。
---
Duplicate comments:
In `@rmcs_ws/src/rmcs_core/src/hardware/sentry.cpp`:
- Around line 1-13: 源文件在 include 区域缺少 <array> 导致在 Line 128 使用 std::array
时可能编译失败；在头部包含列表中显式添加 <array> 就能修复此问题；定位到使用 std::array 的位置（例如在 sentry.cpp 中出现的
std::array 实例），在文件开头的其他 system includes（例如 `#include` <span> 之后或合适的排序位置）添加
`#include` <array>，确保不依赖传递包含并通过编译。
- Around line 96-104: 当前对 TopBoard/BottomBoard 的并发访问会导致数据竞争；在 Sentry 的 update()
和 command_update()（调用
top_board_->update()/bottom_board_->update()/command_update()）与
service/subscription 回调中存在同时读写同一批电机/板卡对象的问题。修复方法：要么将所有回调改为仅把事件/请求入队列，由同一个线程在
update()/command_update() 中统一消费并操作 TopBoard/BottomBoard；要么在所有访问板卡的路径（包括所有
service/subscription 回调与 update()/command_update()）上使用同一把互斥锁（例如
board_mutex）来串行化对 top_board_ 和 bottom_board_ 的读写，确保在调用 top_board_->* 或
bottom_board_->* 前后都持有该锁并在消费队列时也持有锁以避免竞态。
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp`:
- Around line 146-151: Remove the redundant ready() check in update(): always
dereference navigation_command_velocity_ (as done via auto raw_command =
*navigation_command_velocity_) and rely solely on std::isfinite(raw_command.x())
/ std::isfinite(raw_command.y()) to decide whether to update nav_velocity_odom;
keep the scaling by navigation_velocity_scale_ when finite and leave (or
explicitly set) nav_velocity_odom unchanged when not finite. This change uses
the existing before_updating()/make_and_bind_directly() behavior and removes the
unnecessary navigation_command_velocity_.ready() guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1680c38-0204-48a6-8caa-4d52dfbb9d70
📒 Files selected for processing (5)
rmcs_ws/src/rmcs_bringup/config/sentry.yamlrmcs_ws/src/rmcs_core/CMakeLists.txtrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpprmcs_ws/src/rmcs_core/src/hardware/sentry.cpp
✅ Files skipped from review due to trivial changes (2)
- rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpp
- rmcs_ws/src/rmcs_bringup/config/sentry.yaml
rmcs_ws/src/rmcs_core/CMakeLists.txt
Outdated
Comment on lines
18
to
22
| FetchContent_Declare( | ||
| librmcs | ||
| URL https://github.com/Alliance-Algorithm/librmcs/releases/download/v3.0.0/librmcs-sdk-src-3.0.0.zip | ||
| URL_HASH SHA256=b39f51c21baacdcbf3f0176119b8850137a108b88a67e12395d37d89e5ef53e8 | ||
| DOWNLOAD_EXTRACT_TIMESTAMP TRUE | ||
| SOURCE_DIR /workspaces/RMCS/develop_ws/librmcs-sdk-src-3.0.1-0.dev.4.gbaf538b | ||
| ) | ||
| FetchContent_MakeAvailable(librmcs) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CMake FetchContent block =="
cat -n rmcs_ws/src/rmcs_core/CMakeLists.txt | sed -n '16,26p'
echo
echo "== env/build scripts (检查是否有该 SOURCE_DIR 的准备逻辑) =="
cat -n .script/template/env_setup.bash | sed -n '1,20p'
cat -n .script/build-rmcs | sed -n '1,30p'
echo
echo "== 搜索硬编码路径出现位置 =="
rg -n "develop_ws/librmcs-sdk-src|librmcs-sdk-src-3.0.1-0.dev.4.gbaf538b" \
rmcs_ws/src/rmcs_core/CMakeLists.txt .script || trueRepository: Alliance-Algorithm/RMCS
Length of output: 2231
🏁 Script executed:
#!/bin/bash
echo "=== Search for all references to 'develop_ws' or 'librmcs' paths ==="
rg -n "develop_ws|librmcs-sdk-src" . --type cmake --type bash --type yaml --type toml 2>/dev/null || echo "(no results)"
echo
echo "=== Check for CI workflow files ==="
find .github/workflows -type f 2>/dev/null | head -5 || echo "(no .github/workflows found)"
echo
echo "=== Check CMakeLists.txt for any other FetchContent sources ==="
rg -n "FetchContent_Declare|SOURCE_DIR|URL" rmcs_ws/src/rmcs_core/CMakeLists.txt
echo
echo "=== Check if 3.0.1-0.dev.4 version is documented ==="
rg -n "3\.0\.1.*0\.dev\.4|librmcs.*version|librmcs.*3\." . --max-count 10 2>/dev/null || echo "(no version refs found)"Repository: Alliance-Algorithm/RMCS
Length of output: 745
🌐 Web query:
librmcs v3.0.1-0.dev.4 github alliance-algorithm release
💡 Result:
librmcs is a portable header-only C++ library implementing the core communication functionality of RMCS (RoboMaster Control System), part of the Alliance-Algorithm GitHub organization. The repository is at https://github.com/alliance-algorithm/librmcs. LibRMCS v3 is currently in development and testing phase, with no good documentation/tutorials available yet. The v2 branch has the latest stable version. There are 3 releases total, with the latest being v3.0.0b2 released on 2026-02-24. No evidence found of a v3.0.1-0.dev.4 release or tag. The last push to the repository was on 2026-02-27. This specific version may be a development build, commit tag, or internal designation not published as a formal GitHub release.
Citations:
- 1: https://github.com/alliance-algorithm/librmcs
- 2: https://www.dyrector.io
- 3: https://www.nahar.dev
- 4: https://gitlab.com/gtd-gmbh/libmcs/-/tags
移除硬编码的本地开发路径，改用可配置的本地路径加远程 URL 回退方案。
当前 line 20 的 SOURCE_DIR /workspaces/RMCS/develop_ws/librmcs-sdk-src-3.0.1-0.dev.4.gbaf538b 是绝对路径，在 CI 和新环境中该目录不存在会直接导致配置失败。搜索结果显示该路径只出现在此处，且构建脚本中没有创建它的逻辑。
建议采用可配置本地路径加远程 URL 回退的方案，允许开发者指定本地 librmcs 源码目录，同时为 CI/新环境提供从官方发行版本下载的机制。
🔧 建议修改
include(FetchContent)
set(BUILD_STATIC_LIBRMCS ON CACHE BOOL "Build static librmcs SDK" FORCE)
+
+set(LIBRMCS_SOURCE_DIR "" CACHE PATH "Local path to librmcs source tree")
+if(LIBRMCS_SOURCE_DIR AND EXISTS "${LIBRMCS_SOURCE_DIR}/CMakeLists.txt")
+ FetchContent_Declare(
+ librmcs
+ SOURCE_DIR "${LIBRMCS_SOURCE_DIR}"
+ )
+else()
+ FetchContent_Declare(
+ librmcs
+ URL https://github.com/Alliance-Algorithm/librmcs/releases/download/v3.0.0/librmcs-sdk-src-3.0.0.zip
+ )
+endif()
-FetchContent_Declare(
- librmcs
- SOURCE_DIR /workspaces/RMCS/develop_ws/librmcs-sdk-src-3.0.1-0.dev.4.gbaf538b
-)
FetchContent_MakeAvailable(librmcs)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/CMakeLists.txt` around lines 18 - 22, 当前 CMakeLists.txt
在 FetchContent_Declare 的 SOURCE_DIR 中硬编码了本地绝对路径（librmcs），导致在 CI/新环境中失败；请改为使用可配置的
CMake 变量（例如 LIBRMCS_SOURCE_DIR）作为优先本地路径并在不存在时回退到远程 URL（例如 LIBRMCS_URL 或官方发布
tarball/GIT URL），在定义时根据变量存在与否为 FetchContent_Declare 的 SOURCE_DIR 或 URL/URL_HASH
设置值，保留目标名 librmcs 并继续调用 FetchContent_MakeAvailable(librmcs)，同时添加缓存变量说明（CACHE
STRING）和一条 message() 输出以便在构建日志中可见所用的来源。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
哨兵机器人迁移（dev/migrate-sentry）
核心功能
控制器与导航集成
数值稳定性与实现改进
硬件接口与 API 变更
配置、依赖与构建
脚本与补全
子模块与样式
变更量与审查建议