Conversation
Co-authored-by: zhzy-star <2807406212@qq.com> Co-authored-by: floatpigeon <floatpigeon@proton.me> Co-authored-by: fan <enbofan663@gmail.com>
Co-authored-by: zhzy-star <2807406212@qq.com> Co-authored-by: floatpigeon <floatpigeon@proton.me>
.Enabled shooting
fixed heat_controller.
Swapped commands about No.2 and No.3 steering
changed shot_frequency
|
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本 PR 升级了 Changes依赖与插件注册调整
爬虫步兵功能链（独立 DAG）
舵轮英雄与硬件重构（独立但相关）
通用控制器与行为调整
摩擦轮 PID 离线工具链
审计与文档
Sequence Diagram(s)sequenceDiagram
participant Remote as 遥控器/键盘
participant ClimbCtrl as 攀爬控制器
participant Motor as 攀爬电机
participant Sensor as 传感器
Remote->>ClimbCtrl: 启动/中止 请求（边沿检测）
ClimbCtrl->>ClimbCtrl: 状态机推进 (ALIGN→APPROACH→...)
ClimbCtrl->>Motor: 下发左右前后电机扭矩/前进速度覆盖
Motor-->>Sensor: 速度/扭矩 反馈
Sensor-->>ClimbCtrl: 俯仰/扭矩/速度 用于堵转与对齐判据
sequenceDiagram
participant Friction as 摩擦轮系统
participant Recorder as PID记录器
participant CSV as CSV 文件
participant Ident as identify_plant.py
participant Tune as tune_pid.py
Friction->>Recorder: 发送 setpoint/velocity/torque
Recorder->>CSV: 写入带 timestamp 的行 (enabled=1)
CSV->>Ident: 读取与分段
Ident->>Ident: 延迟扫描 + 最小二乘拟合 -> 输出 model/fit
Tune->>Ident: 加载 model/fit
Tune->>Tune: 仿真 + GA/BO 搜索 -> 输出 最佳 kp/kd
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp (1)
23-82:⚠️ Potential issue | 🟠 Major不要在 4 轮分支里继续读取已取消注册的温度输入。
fractional_temperature_已经被注释掉注册了，但friction_wheel_count_ == 4的日志仍然会解引用它。这样 4 轮模式会读取未绑定输入，日志要么是脏值，要么直接失效。要么保留注册并只在 2 轮分支省略字段，要么把 4 轮分支里的温度字段也一并移除。🛠️ 建议修复
- // register_input("/friction_wheels/temperature", fractional_temperature_); + if (friction_wheel_count_ == 4) + register_input("/friction_wheels/temperature", fractional_temperature_);🤖 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/shooting/shooting_recorder.cpp` around lines 23 - 82, ShootingRecorder is dereferencing fractional_temperature_ in the friction_wheel_count_ == 4 branch even though that input is not registered; remove the temperature field from the 4-wheel log format so it no longer reads *fractional_temperature_ (update the fmt::format format string and its argument list in the update() method where log_text is built for the 4-wheel branch), ensuring log_text matches the actual provided arguments and leaving the 2-wheel branch unchanged (alternatively re-register fractional_temperature_ in the constructor if you prefer to keep the field).
🧹 Nitpick comments (4)
rmcs_ws/src/rmcs_core/src/controller/shooting/friction_wheel_controller.cpp (1)
92-93: 建议移除已注释的调试日志。这里保留注释掉的
RCLCPP_INFO会增加噪音；若无短期启用计划，建议删除。🤖 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/shooting/friction_wheel_controller.cpp` around lines 92 - 93, 在 friction_wheel_controller.cpp 中删除那行已注释的调试日志（注释掉的 RCLCPP_INFO(logger_, "Bullet Fired: {%d}", *bullet_fired_);），因为它只是噪音；在处理与 bullet_fired_ 或 logger_ 的相关代码块（例如发射逻辑所在的函数）中直接移除该注释行即可，不需要替换或保留其它占位注释。rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp (1)
70-70: 如果这只是临时调试，建议删掉这行注释。保留一个被注释掉的模式赋值很容易让人误以为还有第二条分支；如果不打算回退，直接移除会更清晰。
🤖 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/hero_gimbal_controller.cpp` at line 70, 删除被注释的赋值行（// *gimbal_mode_ = gimbal_mode_keyboard_;），因为它会误导阅读者以为存在另一条分支；在函数中直接移除该注释即可；如果确实需要保留以备回退，请改为添加明确的 TODO or comment 说明原因并引用符号 gimbal_mode_ 和 gimbal_mode_keyboard_ 以便后续查找。rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_controller.cpp (1)
1-10: 建议显式包含已直接使用的标准头文件本文件直接使用了
std::clamp/std::isnan/std::numbers::pi/std::numeric_limits，建议补齐标准头，避免依赖传递包含。🔧 建议修改
+#include <algorithm> +#include <cmath> `#include` <eigen3/Eigen/Dense> +#include <limits> +#include <numbers> `#include` <rclcpp/node.hpp> `#include` <rmcs_description/tf_description.hpp> `#include` <rmcs_executor/component.hpp>🤖 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/climbable_infantry_chassis_controller.cpp` around lines 1 - 10, This file relies on standard utilities pulled in transitively (std::clamp, std::isnan, std::numbers::pi, std::numeric_limits); explicitly add the corresponding standard headers near the top of climbable_infantry_chassis_controller.cpp—include <algorithm> for std::clamp, <cmath> for std::isnan, <numbers> for std::numbers::pi and <limits> for std::numeric_limits—so the compilation doesn't depend on transitive includes from other headers.rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp (1)
183-185: 发射态每周期 INFO 日志会拖慢控制回路该分支在高频循环中持续打印，建议改为调试级别并节流。
🤖 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/shooting/putter_controller.cpp` around lines 183 - 185, 当前在高频控制循环中使用 RCLCPP_INFO 每周期打印（RCLCPP_INFO(get_logger(), "Shooting! Angle: %.2f, Velocity: %.2f", *bullet_feeder_angle_, *bullet_feeder_velocity_)），会拖慢回路；将其改为带节流的调试级别日志，例如使用 RCLCPP_DEBUG_THROTTLE 或 RCLCPP_INFO_THROTTLE，调用形式类似 RCLCPP_DEBUG_THROTTLE(get_logger(), period_seconds, "Shooting! Angle: %.2f, Velocity: %.2f", *bullet_feeder_angle_, *bullet_feeder_velocity_) 并根据需要设置合适的 period_seconds（例如 1.0 秒）以减少频繁输出。
🤖 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/steering-hero.yaml`:
- Around line 63-71: The YAML currently lists /gimbal/first_second_friction/...
which don't match the actual topic names and will cause ValueBroadcaster to miss
first-right data; update the two occurrences of first_second_friction to
first_right_friction so the topics become /gimbal/first_right_friction/velocity
and /gimbal/first_right_friction/control_torque to match the definitions used
elsewhere (referenced by ValueBroadcaster and the existing first_right_friction
entries).
In `@rmcs_ws/src/rmcs_core/plugins.xml`:
- Around line 83-88: 缺少原仓库历史条目 GimbalTfPublisher，导致旧配置在插件装载阶段失败；请在 plugins.xml
中恢复该 <class> 条目（使用历史的“无命名空间前缀”写法，即 type="GimbalTfPublisher" 并保持 base_class_type
与其它条目一致，如 rmcs_executor::Component），位置参考现有的 TfBroadcaster 和 ValueBroadcaster
条目，确保包含简短的 <description>，以与现有插件声明格式一致并能被加载器识别。
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpp`:
- Around line 108-113: The FORERAKED detection is dead because
detect_is_foreraked() lives inside update_auto_climb_control()’s IDLE branch but
update_auto_climb_control() is only called when auto_climb_state_ !=
AutoClimbState::IDLE (see the outer if around
stop_manual_support()/apply_auto_climb_control(update_auto_climb_control())).
Fix by invoking the FORERAKED check on the IDLE path: either call
detect_is_foreraked() (or call update_auto_climb_control()) from the outer else
branch before apply_auto_climb_control(update_manual_support_control(keyboard))
and apply_manual_support_zero_output(), or refactor update_auto_climb_control()
so its IDLE logic runs when auto_climb_state_ == IDLE; update the same pattern
at the other occurrences noted (lines ~163-167 and ~486-496). Ensure you
reference auto_climb_state_, AutoClimbState::IDLE, detect_is_foreraked(),
update_auto_climb_control(), apply_auto_climb_control(),
update_manual_support_control(), stop_manual_support(), and
apply_manual_support_zero_output() when making the change.
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_controller.cpp`:
- Around line 171-177: The AUTO-mode scaling currently clamps angular_velocity
gain by std::clamp(..., 0.0, 0.3) which contradicts the comment that it should
be "fully enabled at max speed"; change the upper clamp bound from 0.3 to 1.0 in
the AUTO block where measured_translational_speed is computed (see variables
angular_velocity, measured_translational_speed, translational_velocity_max in
climbable_infantry_chassis_controller.cpp) so the angular gain scales linearly
from 0 up to full (1.0) at max translational speed.
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/steering_wheel_status.cpp`:
- Around line 20-28: 输入主题名多写了后缀，导致 register_input 调用使用的主题（如
left_front_steering_angle_, left_back_steering_angle_,
right_back_steering_angle_, right_front_steering_angle_,
left_front_wheel_velocity_, left_back_wheel_velocity_,
right_back_wheel_velocity_, right_front_wheel_velocity_）与发布端在
steering-infantry.cpp 使用的基础路径不匹配；请在 steering_wheel_status.cpp 中把这 8 个
register_input 的主题名改回发布端使用的基础路径（移除 "/angle" 和 "/velocity" 后缀），确保变量名仍对应各个
register_input 调用以绑定到正确的发布者，从而让 calculate_chassis_velocity() 读到真实数据。
In `@rmcs_ws/src/rmcs_core/src/controller/pid/friction_wheel_pid_recorder.cpp`:
- Around line 22-39: The constructor currently calls declare_parameter(...) for
wheels, min_setpoint_abs, flush_every_n_samples, output_dir, and output_name
while automatically_declare_parameters_from_overrides(true) is enabled, causing
ParameterAlreadyDeclaredException when using YAML; replace those
declare_parameter calls with get_parameter_or(...) (or get_parameter for
required params) to read the already-declared values into the existing locals
(wheels, min_setpoint_abs_, flush_every_n_samples_, output_dir, output_name) and
keep the existing validation (throw if wheels.empty(), ensure
flush_every_n_samples > 0 and cast to flush_every_n_samples_) unchanged so the
logic around min_setpoint_abs_, flush_every_n_samples_, and
output_dir/output_name continues to work.
In
`@rmcs_ws/src/rmcs_core/src/controller/shooting/bullet_feeder_controller_42mm.cpp`:
- Around line 34-42: The new input registration
register_input("/gimbal/bullet_fired", bullet_fired_) is never read in update(),
so the 42mm feeder won't respond to firing events; either wire bullet_fired_
into the existing cooling/preload branches in update() (mirror the 17mm logic:
read bullet_fired_ and advance/reset feeder state in the cooling or preload
handling), or remove the register_input call to avoid exposing a dead interface;
locate register_input("/gimbal/bullet_fired", bullet_fired_) and update() in
bullet_feeder_controller_42mm.cpp and apply the appropriate change (do not
forget to keep PID fields like bullet_feeder_velocity_pid_ and
bullet_feeder_angle_pid_ unchanged unless intentionally tuning).
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/friction_wheel_controller.cpp`:
- Around line 155-160: The detect_friction_faulty() function always returns
false, disabling fault detection; update the logic so that when an actual
friction wheel velocity is significantly lower than the commanded control
velocity (i.e. abs(*friction_velocities_[i]) <
abs(*friction_control_velocities_[i] * 0.5)) the function returns true to
indicate a jam, and after the loop return false if no wheel triggered the
condition; modify detect_friction_faulty() accordingly so the branch that
currently returns false on threshold breach returns true instead (this will
allow friction_jammed_ handling to be reached).
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp`:
- Around line 177-178: The PID output for the bullet feeder is not being clipped
before assignment, so wrap the result of
bullet_feeder_velocity_pid_.update(velocity_err) with a clamp to respect
max_bullet_feeder_control_torque_ (and negative limit) before storing into
bullet_feeder_control_torque_; do the same wherever
bullet_feeder_control_torque_ is assigned (e.g., the other occurrences
referenced), using std::clamp or equivalent to ensure the value stays within
[-max_bullet_feeder_control_torque_, max_bullet_feeder_control_torque_].
- Around line 171-174: 当前用 angle_err = bullet_feeder_control_angle_ -
*bullet_feeder_angle_; 再用 if (angle_err < 0.1)
判断完成会把大负值（过冲）也认作完成；把判断改为基于绝对误差，例如使用 std::abs(angle_err) < 0.1，或明确判断误差在正负阈值内（例如
angle_err > -0.1 && angle_err < 0.1），在修复时修改产生 angle_err 的代码附近并确保包含 <cmath>（如果使用
std::abs），同时保留对 set_preloaded()、bullet_feeder_control_angle_ 和
bullet_feeder_angle_ 的引用以便定位。
- Around line 146-149: Restore the commented heat-limit check in the firing
decision so the controller does not enter the shoot flow when heat allowance is
exhausted: re-enable the condition using
control_bullet_allowance_limited_by_heat_ > 0 in the same if that checks
shoot_stage_ == ShootStage::PRELOADED and last_photoelectric_sensor_status_, or
explicitly allow bypass only when the system is in an OVERDRIVE mode (add an OR
clause that checks an overdrive flag) and ensure proper parenthesis/grouping so
the combined condition evaluates correctly.
In `@rmcs_ws/src/rmcs_core/src/hardware/climbable-infantry.cpp`:
- Around line 345-346: The asin argument for computing chassis_pitch_imu_ can
slightly exceed [-1,1] due to quaternion floating-point error; clamp the
computed scalar (2.0 * (imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1())) to the
range [-1.0, 1.0] before calling std::asin to avoid NaN. Locate the assignment
to chassis_pitch_imu_ (the line using imu_.q0(), imu_.q1(), imu_.q2(),
imu_.q3()) and wrap the intermediate value with a clamp (e.g., std::clamp or
manual min/max) then pass the clamped value into std::asin and negate as before.
- Around line 47-50: The subscription created by gimbal_calibrate_subscription_
uses create_subscription with rclcpp::QoS{0}, which triggers a runtime warning
because KEEP_LAST requires depth >= 1; change the QoS depth to at least 1 (e.g.,
rclcpp::QoS{1}) when constructing the subscription so the KEEP_LAST policy can
store messages; update the create_subscription call for "/gimbal/calibrate"
accordingly.
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp`:
- Around line 178-183:
摄像触发与光电状态目前只被注册/读取但未在回调中更新或自动复位：在负责读取光电传感器和触发相机的回调/更新逻辑中（即处理
photoelectric_sensor_status_ 和 camera_capturer_trigger_ 的函数），在检测到上升/下降沿时写入
camera_capturer_trigger_timestamp_（使用当前时间源）并仅在边沿时将 camera_capturer_trigger_ 置为
true，随后在同次循环结束或在下降沿时清零该触发位以实现自动复位；同时在检测到光电传感器释放时清零
photoelectric_sensor_status_（或在边沿检测中维护状态），确保变化检测（之前未触发的
camera_capturer_trigger_timestamp_ 读写比较）能生效并避免锁存值。确保修改涉及的标识符包括
camera_capturer_trigger_timestamp_, photoelectric_sensor_status_,
camera_capturer_trigger_ 以及相关回调/处理函数。
In `@tools/pid_tuning/tune_pid.py`:
- Around line 415-426: The code short-circuits to the single exact trace
whenever an exact run exists regardless of trace_scope; change the logic so the
exact trace is only returned immediately when trace_scope == "exact" (or
equivalent explicit exact mode). Specifically, in the block using trace_map and
identifiers trace_map, model.wheel_id, model.run_id, trace_scope, exact,
same_wheel, move or guard the early return of exact behind a condition that
checks trace_scope == "exact"; if trace_scope == "wheel" fall through to collect
same_wheel (including the exact run) and return the sorted list, preserving the
intended wheel-scope behavior. Ensure other scopes/None keep their existing
behavior.
---
Outside diff comments:
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp`:
- Around line 23-82: ShootingRecorder is dereferencing fractional_temperature_
in the friction_wheel_count_ == 4 branch even though that input is not
registered; remove the temperature field from the 4-wheel log format so it no
longer reads *fractional_temperature_ (update the fmt::format format string and
its argument list in the update() method where log_text is built for the 4-wheel
branch), ensuring log_text matches the actual provided arguments and leaving the
2-wheel branch unchanged (alternatively re-register fractional_temperature_ in
the constructor if you prefer to keep the field).
---
Nitpick comments:
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_controller.cpp`:
- Around line 1-10: This file relies on standard utilities pulled in
transitively (std::clamp, std::isnan, std::numbers::pi, std::numeric_limits);
explicitly add the corresponding standard headers near the top of
climbable_infantry_chassis_controller.cpp—include <algorithm> for std::clamp,
<cmath> for std::isnan, <numbers> for std::numbers::pi and <limits> for
std::numeric_limits—so the compilation doesn't depend on transitive includes
from other headers.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp`:
- Line 70: 删除被注释的赋值行（// *gimbal_mode_ =
gimbal_mode_keyboard_;），因为它会误导阅读者以为存在另一条分支；在函数中直接移除该注释即可；如果确实需要保留以备回退，请改为添加明确的
TODO or comment 说明原因并引用符号 gimbal_mode_ 和 gimbal_mode_keyboard_ 以便后续查找。
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/friction_wheel_controller.cpp`:
- Around line 92-93: 在 friction_wheel_controller.cpp 中删除那行已注释的调试日志（注释掉的
RCLCPP_INFO(logger_, "Bullet Fired: {%d}", *bullet_fired_);），因为它只是噪音；在处理与
bullet_fired_ 或 logger_ 的相关代码块（例如发射逻辑所在的函数）中直接移除该注释行即可，不需要替换或保留其它占位注释。
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp`:
- Around line 183-185: 当前在高频控制循环中使用 RCLCPP_INFO 每周期打印（RCLCPP_INFO(get_logger(),
"Shooting! Angle: %.2f, Velocity: %.2f", *bullet_feeder_angle_,
*bullet_feeder_velocity_)），会拖慢回路；将其改为带节流的调试级别日志，例如使用 RCLCPP_DEBUG_THROTTLE 或
RCLCPP_INFO_THROTTLE，调用形式类似 RCLCPP_DEBUG_THROTTLE(get_logger(), period_seconds,
"Shooting! Angle: %.2f, Velocity: %.2f", *bullet_feeder_angle_,
*bullet_feeder_velocity_) 并根据需要设置合适的 period_seconds（例如 1.0 秒）以减少频繁输出。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23099b2d-ed57-4e4a-8aa7-ef74d910b664
📒 Files selected for processing (27)
auth.jsondocs/zh-cn/steering_infantry_can_id_audit.mdlibrmcs-firmware-c_board-3.0.1-0.dev.4.gbaf538b.dfurmcs_ws/src/rmcs_bringup/config/climbable-infantry.yamlrmcs_ws/src/rmcs_bringup/config/steering-hero.yamlrmcs_ws/src/rmcs_core/CMakeLists.txtrmcs_ws/src/rmcs_core/librmcsrmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/steering_wheel_status.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/simple_gimbal_controller.cpprmcs_ws/src/rmcs_core/src/controller/pid/friction_wheel_pid_recorder.cpprmcs_ws/src/rmcs_core/src/controller/shooting/bullet_feeder_controller_42mm.cpprmcs_ws/src/rmcs_core/src/controller/shooting/friction_wheel_controller.cpprmcs_ws/src/rmcs_core/src/controller/shooting/heat_controller.cpprmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpprmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpprmcs_ws/src/rmcs_core/src/hardware/climbable-infantry.cpprmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpptools/pid_tuning/README.mdtools/pid_tuning/friction_wheel_pid_recorder.example.yamltools/pid_tuning/identify_plant.pytools/pid_tuning/tune_pid.py
Comment on lines
+63
to
+71
| - /gimbal/putter/velocity | ||
| - /gimbal/first_left_friction/velocity | ||
| - /gimbal/first_second_friction/velocity | ||
| - /gimbal/second_left_friction/velocity | ||
| - /gimbal/second_right_friction/velocity | ||
| - /gimbal/first_left_friction/control_torque | ||
| - /gimbal/first_second_friction/control_torque | ||
| - /gimbal/second_left_friction/control_torque | ||
| - /gimbal/second_right_friction/control_torque |
There was a problem hiding this comment.
把 first_second_friction 改回 first_right_friction。
同文件 Line 179-183 只定义了 first_right_friction，这里两个 first_second_friction 都不会命中真实话题。按现状，ValueBroadcaster 会漏掉 first-right 的速度/力矩数据，并额外订阅两个不存在的 topic。
🛠️ 建议修改
forward_list:
- /gimbal/putter/velocity
- /gimbal/first_left_friction/velocity
- - /gimbal/first_second_friction/velocity
+ - /gimbal/first_right_friction/velocity
- /gimbal/second_left_friction/velocity
- /gimbal/second_right_friction/velocity
- /gimbal/first_left_friction/control_torque
- - /gimbal/first_second_friction/control_torque
+ - /gimbal/first_right_friction/control_torque
- /gimbal/second_left_friction/control_torque
- /gimbal/second_right_friction/control_torque🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_bringup/config/steering-hero.yaml` around lines 63 - 71, The
YAML currently lists /gimbal/first_second_friction/... which don't match the
actual topic names and will cause ValueBroadcaster to miss first-right data;
update the two occurrences of first_second_friction to first_right_friction so
the topics become /gimbal/first_right_friction/velocity and
/gimbal/first_right_friction/control_torque to match the definitions used
elsewhere (referenced by ValueBroadcaster and the existing first_right_friction
entries).
Comment on lines
+83
to
+88
| <class type="rmcs_core::broadcaster::TfBroadcaster" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type="rmcs_core::broadcaster::ValueBroadcaster" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> |
There was a problem hiding this comment.
plugins.xml 缺少 GimbalTfPublisher 兼容注册项
建议恢复该条目（保持历史上“无命名空间前缀”的写法），避免旧配置在插件装载阶段失败。
🔧 建议修改
<class type="rmcs_core::broadcaster::ValueBroadcaster" base_class_type="rmcs_executor::Component">
<description>Test plugin.</description>
</class>
+ <class type="GimbalTfPublisher" base_class_type="rmcs_executor::Component">
+ <description>Test plugin.</description>
+ </class>Based on learnings: The GimbalTfPublisher entry in rmcs_ws/src/rmcs_core/plugins.xml is from the original repository and should be preserved as-is, even though it lacks the namespace prefix used by other entries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/plugins.xml` around lines 83 - 88, 缺少原仓库历史条目
GimbalTfPublisher，导致旧配置在插件装载阶段失败；请在 plugins.xml 中恢复该 <class>
条目（使用历史的“无命名空间前缀”写法，即 type="GimbalTfPublisher" 并保持 base_class_type 与其它条目一致，如
rmcs_executor::Component），位置参考现有的 TfBroadcaster 和 ValueBroadcaster 条目，确保包含简短的
<description>，以与现有插件声明格式一致并能被加载器识别。
Comment on lines
+108
to
+113
| if (auto_climb_state_ != AutoClimbState::IDLE) { | ||
| stop_manual_support(); | ||
| apply_auto_climb_control(update_auto_climb_control()); | ||
| } else { | ||
| apply_auto_climb_control(update_manual_support_control(keyboard)); | ||
| apply_manual_support_zero_output(); |
There was a problem hiding this comment.
FORERAKED 检测目前是死代码。
detect_is_foreraked() 只在 update_auto_climb_control() 的 IDLE 分支里调用，但 Line 108-113 只有 auto_climb_state_ != AutoClimbState::IDLE 时才会进入这个函数。因此空闲态永远不会自动切到 FORERAKED。
🛠️ 建议修改
- if (auto_climb_state_ != AutoClimbState::IDLE) {
+ detect_is_foreraked();
+ if (auto_climb_state_ != AutoClimbState::IDLE) {
stop_manual_support();
apply_auto_climb_control(update_auto_climb_control());
} else {Also applies to: 163-167, 486-496
🤖 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/climbable_infantry_chassis_climber_controller.cpp`
around lines 108 - 113, The FORERAKED detection is dead because
detect_is_foreraked() lives inside update_auto_climb_control()’s IDLE branch but
update_auto_climb_control() is only called when auto_climb_state_ !=
AutoClimbState::IDLE (see the outer if around
stop_manual_support()/apply_auto_climb_control(update_auto_climb_control())).
Fix by invoking the FORERAKED check on the IDLE path: either call
detect_is_foreraked() (or call update_auto_climb_control()) from the outer else
branch before apply_auto_climb_control(update_manual_support_control(keyboard))
and apply_manual_support_zero_output(), or refactor update_auto_climb_control()
so its IDLE logic runs when auto_climb_state_ == IDLE; update the same pattern
at the other occurrences noted (lines ~163-167 and ~486-496). Ensure you
reference auto_climb_state_, AutoClimbState::IDLE, detect_is_foreraked(),
update_auto_climb_control(), apply_auto_climb_control(),
update_manual_support_control(), stop_manual_support(), and
apply_manual_support_zero_output() when making the change.
Comment on lines
+171
to
+177
| // Keep AUTO rear-following gentle at low translation speed and fully enabled at max. | ||
| const double measured_translational_speed = | ||
| chassis_velocity_feedback_ready_ ? chassis_velocity_->vector.head<2>().norm() | ||
| : translational_velocity.norm(); | ||
| angular_velocity *= | ||
| std::clamp(measured_translational_speed / translational_velocity_max, 0.0, 0.3); | ||
| } break; |
There was a problem hiding this comment.
AUTO 模式角速度增益上限被固定为 0.3，控制会明显偏弱
当前实现与注释“满速时完全启用”不一致；如果目标是随速度线性放开，建议上限改为 1.0。
🔧 建议修改
angular_velocity *=
- std::clamp(measured_translational_speed / translational_velocity_max, 0.0, 0.3);
+ std::clamp(measured_translational_speed / translational_velocity_max, 0.0, 1.0);🤖 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/climbable_infantry_chassis_controller.cpp`
around lines 171 - 177, The AUTO-mode scaling currently clamps angular_velocity
gain by std::clamp(..., 0.0, 0.3) which contradicts the comment that it should
be "fully enabled at max speed"; change the upper clamp bound from 0.3 to 1.0 in
the AUTO block where measured_translational_speed is computed (see variables
angular_velocity, measured_translational_speed, translational_velocity_max in
climbable_infantry_chassis_controller.cpp) so the angular gain scales linearly
from 0 up to full (1.0) at max translational speed.
Comment on lines
+20
to
+28
| register_input("/chassis/left_front_steering/angle", left_front_steering_angle_); | ||
| register_input("/chassis/left_back_steering/angle", left_back_steering_angle_); | ||
| register_input("/chassis/right_back_steering/angle", right_back_steering_angle_); | ||
| register_input("/chassis/right_front_steering/angle", right_front_steering_angle_); | ||
|
|
||
| register_input("/chassis/left_front_wheel/velocity", left_front_wheel_velocity_); | ||
| register_input("/chassis/left_back_wheel/velocity", left_back_wheel_velocity_); | ||
| register_input("/chassis/right_back_wheel/velocity", right_back_wheel_velocity_); | ||
| register_input("/chassis/right_front_wheel/velocity", right_front_wheel_velocity_); |
There was a problem hiding this comment.
输入话题名和现有发布端不一致，组件会收不到真实数据。
rmcs_ws/src/rmcs_core/src/hardware/steering-infantry.cpp:260-271 里发布的是 /chassis/left_front_steering、/chassis/left_front_wheel 这类基础路径；这里多加了 /angle 和 /velocity 后缀后，这 8 个输入就不会绑定到现有发布端，calculate_chassis_velocity() 会一直读到默认值。
🛠️ 建议修正的话题名
- register_input("/chassis/left_front_steering/angle", left_front_steering_angle_);
- register_input("/chassis/left_back_steering/angle", left_back_steering_angle_);
- register_input("/chassis/right_back_steering/angle", right_back_steering_angle_);
- register_input("/chassis/right_front_steering/angle", right_front_steering_angle_);
+ register_input("/chassis/left_front_steering", left_front_steering_angle_);
+ register_input("/chassis/left_back_steering", left_back_steering_angle_);
+ register_input("/chassis/right_back_steering", right_back_steering_angle_);
+ register_input("/chassis/right_front_steering", right_front_steering_angle_);- register_input("/chassis/left_front_wheel/velocity", left_front_wheel_velocity_);
- register_input("/chassis/left_back_wheel/velocity", left_back_wheel_velocity_);
- register_input("/chassis/right_back_wheel/velocity", right_back_wheel_velocity_);
- register_input("/chassis/right_front_wheel/velocity", right_front_wheel_velocity_);
+ register_input("/chassis/left_front_wheel", left_front_wheel_velocity_);
+ register_input("/chassis/left_back_wheel", left_back_wheel_velocity_);
+ register_input("/chassis/right_back_wheel", right_back_wheel_velocity_);
+ register_input("/chassis/right_front_wheel", right_front_wheel_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
| register_input("/chassis/left_front_steering/angle", left_front_steering_angle_); | |
| register_input("/chassis/left_back_steering/angle", left_back_steering_angle_); | |
| register_input("/chassis/right_back_steering/angle", right_back_steering_angle_); | |
| register_input("/chassis/right_front_steering/angle", right_front_steering_angle_); | |
| register_input("/chassis/left_front_wheel/velocity", left_front_wheel_velocity_); | |
| register_input("/chassis/left_back_wheel/velocity", left_back_wheel_velocity_); | |
| register_input("/chassis/right_back_wheel/velocity", right_back_wheel_velocity_); | |
| register_input("/chassis/right_front_wheel/velocity", right_front_wheel_velocity_); | |
| register_input("/chassis/left_front_steering", left_front_steering_angle_); | |
| register_input("/chassis/left_back_steering", left_back_steering_angle_); | |
| register_input("/chassis/right_back_steering", right_back_steering_angle_); | |
| register_input("/chassis/right_front_steering", right_front_steering_angle_); | |
| register_input("/chassis/left_front_wheel", left_front_wheel_velocity_); | |
| register_input("/chassis/left_back_wheel", left_back_wheel_velocity_); | |
| register_input("/chassis/right_back_wheel", right_back_wheel_velocity_); | |
| register_input("/chassis/right_front_wheel", right_front_wheel_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/steering_wheel_status.cpp`
around lines 20 - 28, 输入主题名多写了后缀，导致 register_input 调用使用的主题（如
left_front_steering_angle_, left_back_steering_angle_,
right_back_steering_angle_, right_front_steering_angle_,
left_front_wheel_velocity_, left_back_wheel_velocity_,
right_back_wheel_velocity_, right_front_wheel_velocity_）与发布端在
steering-infantry.cpp 使用的基础路径不匹配；请在 steering_wheel_status.cpp 中把这 8 个
register_input 的主题名改回发布端使用的基础路径（移除 "/angle" 和 "/velocity" 后缀），确保变量名仍对应各个
register_input 调用以绑定到正确的发布者，从而让 calculate_chassis_velocity() 读到真实数据。
Comment on lines
+177
to
+178
| *bullet_feeder_control_torque_ = | ||
| bullet_feeder_velocity_pid_.update(velocity_err); |
There was a problem hiding this comment.
供弹扭矩输出未限幅，保护常量未生效
已经定义 max_bullet_feeder_control_torque_，但未用于输出裁剪，PID 峰值可能直接下发。
🔧 建议修改
- *bullet_feeder_control_torque_ =
- bullet_feeder_velocity_pid_.update(velocity_err);
+ *bullet_feeder_control_torque_ = std::clamp(
+ bullet_feeder_velocity_pid_.update(velocity_err),
+ -max_bullet_feeder_control_torque_,
+ max_bullet_feeder_control_torque_);
@@
- *bullet_feeder_control_torque_ =
- bullet_feeder_velocity_pid_.update(velocity_err);
+ *bullet_feeder_control_torque_ = std::clamp(
+ bullet_feeder_velocity_pid_.update(velocity_err),
+ -max_bullet_feeder_control_torque_,
+ max_bullet_feeder_control_torque_);Also applies to: 192-193, 452-453
🤖 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/shooting/putter_controller.cpp` around
lines 177 - 178, The PID output for the bullet feeder is not being clipped
before assignment, so wrap the result of
bullet_feeder_velocity_pid_.update(velocity_err) with a clamp to respect
max_bullet_feeder_control_torque_ (and negative limit) before storing into
bullet_feeder_control_torque_; do the same wherever
bullet_feeder_control_torque_ is assigned (e.g., the other occurrences
referenced), using std::clamp or equivalent to ensure the value stays within
[-max_bullet_feeder_control_torque_, max_bullet_feeder_control_torque_].
Comment on lines
+47
to
+50
| gimbal_calibrate_subscription_ = create_subscription<std_msgs::msg::Int32>( | ||
| "/gimbal/calibrate", rclcpp::QoS{0}, [this](std_msgs::msg::Int32::UniquePtr&& msg) { | ||
| gimbal_calibrate_subscription_callback(std::move(msg)); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "climbable-infantry.cpp" | head -20Repository: Alliance-Algorithm/RMCS
Length of output: 127
🏁 Script executed:
cat -n rmcs_ws/src/rmcs_core/src/hardware/climbable-infantry.cpp | sed -n '40,60p'Repository: Alliance-Algorithm/RMCS
Length of output: 1333
🌐 Web query:
ROS2 rclcpp QoS KeepLast depth 0 valid configuration
💡 Result:
No, using rclcpp QoS with HistoryPolicy::KEEP_LAST (KeepLast) and depth 0 is not a valid configuration. It triggers a warning: "A zero depth with KEEP_LAST doesn't make sense; no data could be stored. This will be interpreted as SYSTEM_DEFAULT". The constructor accepts depth=0 syntactically and sets the RMW QoS profile to KEEP_LAST with depth=0, but rclcpp issues the warning (unless print_depth_warning=false), and underlying RMW implementations typically treat it as system default (often KEEP_LAST depth=1). For KEEP_LAST, depth should be at least 1 to store samples. Default QoS uses KEEP_LAST depth=10.
Citations:
- 1: Warning about history depth of 0 even while using system default QoS ros2/rmw#341
- 2: SystemDefaultsQoS - force to have depth > 0 for Publishers ros2/rclcpp#2705
- 3: https://github.com/ros2/rclcpp/blob/rolling/rclcpp/include/rclcpp/qos.hpp
- 4: https://api.nav2.org/jazzy/html/qos_8cpp_source.html
/gimbal/calibrate 的 QoS 深度设为 0 会触发运行时警告
rclcpp::QoS{0} 与 KEEP_LAST 策略不兼容。ROS2 会发出警告："A zero depth with KEEP_LAST doesn't make sense; no data could be stored. This will be interpreted as SYSTEM_DEFAULT"，并将配置降级为系统默认值。对于 KEEP_LAST 策略，深度必须至少为 1 以存储消息样本。建议改为 rclcpp::QoS{1}。
🔧 建议修改
gimbal_calibrate_subscription_ = create_subscription<std_msgs::msg::Int32>(
- "/gimbal/calibrate", rclcpp::QoS{0}, [this](std_msgs::msg::Int32::UniquePtr&& msg) {
+ "/gimbal/calibrate", rclcpp::QoS{1}, [this](std_msgs::msg::Int32::UniquePtr&& msg) {
gimbal_calibrate_subscription_callback(std::move(msg));
});📝 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
| gimbal_calibrate_subscription_ = create_subscription<std_msgs::msg::Int32>( | |
| "/gimbal/calibrate", rclcpp::QoS{0}, [this](std_msgs::msg::Int32::UniquePtr&& msg) { | |
| gimbal_calibrate_subscription_callback(std::move(msg)); | |
| }); | |
| gimbal_calibrate_subscription_ = create_subscription<std_msgs::msg::Int32>( | |
| "/gimbal/calibrate", rclcpp::QoS{1}, [this](std_msgs::msg::Int32::UniquePtr&& msg) { | |
| gimbal_calibrate_subscription_callback(std::move(msg)); | |
| }); |
🤖 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/climbable-infantry.cpp` around lines 47 -
50, The subscription created by gimbal_calibrate_subscription_ uses
create_subscription with rclcpp::QoS{0}, which triggers a runtime warning
because KEEP_LAST requires depth >= 1; change the QoS depth to at least 1 (e.g.,
rclcpp::QoS{1}) when constructing the subscription so the KEEP_LAST policy can
store messages; update the create_subscription call for "/gimbal/calibrate"
accordingly.
Comment on lines
+345
to
+346
| *chassis_pitch_imu_ = -std::asin(2.0 * (imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1())); | ||
|
|
There was a problem hiding this comment.
IMU 俯仰角反解缺少夹紧，可能产出 NaN
四元数浮点误差会让 asin 入参轻微越界，进而污染姿态输出与下游控制。
🔧 建议修改
- *chassis_pitch_imu_ = -std::asin(2.0 * (imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1()));
+ const double sin_pitch = 2.0 * (imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1());
+ *chassis_pitch_imu_ = -std::asin(std::clamp(sin_pitch, -1.0, 1.0));📝 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
| *chassis_pitch_imu_ = -std::asin(2.0 * (imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1())); | |
| const double sin_pitch = 2.0 * (imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1()); | |
| *chassis_pitch_imu_ = -std::asin(std::clamp(sin_pitch, -1.0, 1.0)); |
🤖 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/climbable-infantry.cpp` around lines 345 -
346, The asin argument for computing chassis_pitch_imu_ can slightly exceed
[-1,1] due to quaternion floating-point error; clamp the computed scalar (2.0 *
(imu_.q0() * imu_.q2() - imu_.q3() * imu_.q1())) to the range [-1.0, 1.0] before
calling std::asin to avoid NaN. Locate the assignment to chassis_pitch_imu_ (the
line using imu_.q0(), imu_.q1(), imu_.q2(), imu_.q3()) and wrap the intermediate
value with a clamp (e.g., std::clamp or manual min/max) then pass the clamped
value into std::asin and negate as before.
Comment on lines
+178
to
+183
| steering_hero.register_output( | ||
| "/gimbal/photoelectric_sensor", photoelectric_sensor_status_, false); | ||
| steering_hero.register_output( | ||
| "/auto_aim/image_capturer/timestamp", camera_capturer_trigger_timestamp_, 0); | ||
| steering_hero.register_output( | ||
| "/auto_aim/image_capturer/trigger", camera_capturer_trigger_, 0); |
There was a problem hiding this comment.
GPIO 触发信号现在既不会产生时间戳，也不会自动复位。
camera_capturer_trigger_timestamp_ 这里只被注册和读取，从未在任何回调里赋值，所以 Line 226-228 的变更检测永远不会触发；同时 photoelectric_sensor_status_ 和 camera_capturer_trigger_ 在没有新边沿时也没有清零路径，第一次触发后就会一直保持 true。这样会让图像抓拍触发失效，并把光电信号变成锁存值。
Also applies to: 226-228, 311-317
🤖 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/steering-hero.cpp` around lines 178 - 183,
摄像触发与光电状态目前只被注册/读取但未在回调中更新或自动复位：在负责读取光电传感器和触发相机的回调/更新逻辑中（即处理
photoelectric_sensor_status_ 和 camera_capturer_trigger_ 的函数），在检测到上升/下降沿时写入
camera_capturer_trigger_timestamp_（使用当前时间源）并仅在边沿时将 camera_capturer_trigger_ 置为
true，随后在同次循环结束或在下降沿时清零该触发位以实现自动复位；同时在检测到光电传感器释放时清零
photoelectric_sensor_status_（或在边沿检测中维护状态），确保变化检测（之前未触发的
camera_capturer_trigger_timestamp_ 读写比较）能生效并避免锁存值。确保修改涉及的标识符包括
camera_capturer_trigger_timestamp_, photoelectric_sensor_status_,
camera_capturer_trigger_ 以及相关回调/处理函数。
Comment on lines
+415
to
+426
| if trace_map: | ||
| exact = trace_map.get((model.wheel_id, model.run_id)) | ||
| if exact is not None: | ||
| return [exact] | ||
| if trace_scope == "wheel": | ||
| same_wheel = [ | ||
| trace | ||
| for trace in trace_map.values() | ||
| if trace.wheel_id == model.wheel_id | ||
| ] | ||
| if same_wheel: | ||
| return sorted(same_wheel, key=lambda trace: trace.run_id) |
There was a problem hiding this comment.
--trace-scope wheel 被 exact-run 提前短路了。
现在只要 exact run 存在，就会在判断 trace_scope == "wheel" 之前直接返回单条 trace；这和 help/README 里“复用同 wheel 的所有 runs”不一致，也会让 wheel-scope 调优悄悄退化成 exact-scope。
🛠️ 建议修改
if trace_map:
- exact = trace_map.get((model.wheel_id, model.run_id))
- if exact is not None:
- return [exact]
if trace_scope == "wheel":
same_wheel = [
trace
for trace in trace_map.values()
if trace.wheel_id == model.wheel_id
]
if same_wheel:
return sorted(same_wheel, key=lambda trace: trace.run_id)
+ exact = trace_map.get((model.wheel_id, model.run_id))
+ if exact is not None:
+ return [exact]
raise ValueError(
f"no trace matched {model.wheel_id}/run{model.run_id}; "
"provide the corresponding recorder CSV or switch to synthetic step mode"📝 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
| if trace_map: | |
| exact = trace_map.get((model.wheel_id, model.run_id)) | |
| if exact is not None: | |
| return [exact] | |
| if trace_scope == "wheel": | |
| same_wheel = [ | |
| trace | |
| for trace in trace_map.values() | |
| if trace.wheel_id == model.wheel_id | |
| ] | |
| if same_wheel: | |
| return sorted(same_wheel, key=lambda trace: trace.run_id) | |
| if trace_map: | |
| if trace_scope == "wheel": | |
| same_wheel = [ | |
| trace | |
| for trace in trace_map.values() | |
| if trace.wheel_id == model.wheel_id | |
| ] | |
| if same_wheel: | |
| return sorted(same_wheel, key=lambda trace: trace.run_id) | |
| exact = trace_map.get((model.wheel_id, model.run_id)) | |
| if exact is not None: | |
| return [exact] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/pid_tuning/tune_pid.py` around lines 415 - 426, The code short-circuits
to the single exact trace whenever an exact run exists regardless of
trace_scope; change the logic so the exact trace is only returned immediately
when trace_scope == "exact" (or equivalent explicit exact mode). Specifically,
in the block using trace_map and identifiers trace_map, model.wheel_id,
model.run_id, trace_scope, exact, same_wheel, move or guard the early return of
exact behind a condition that checks trace_scope == "exact"; if trace_scope ==
"wheel" fall through to collect same_wheel (including the exact run) and return
the sorted list, preserving the intended wheel-scope behavior. Ensure other
scopes/None keep their existing behavior.
…th 1RMCS_board in hardware and optimize following mode parameters."
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/chassis/chassis_controller.cpp`:
- Line 22: The derivative gain kd for following_velocity_controller_ is set to
1000.0 which likely over-amplifies noise given PidCalculator computes derivative
as kd*(err - last_err_), so reduce kd to a conservative value or add derivative
filtering; update the initialization of following_velocity_controller_ (and/or
PidCalculator) to either lower kd to a tested value and/or implement a
derivative low-pass filter (or derivative-on-measurement with filtering) to
prevent noise amplification, then validate on hardware across climb modes to
confirm stability and that angular_velocity_max limits are not regularly hit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96a06c40-ae0d-44a4-b9ba-2dd5eb65f49d
📒 Files selected for processing (3)
rmcs_ws/src/rmcs_bringup/config/climbable-infantry.yamlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpprmcs_ws/src/rmcs_core/src/hardware/climbable-infantry.cpp
| get_component_name(), | ||
| rclcpp::NodeOptions{}.automatically_declare_parameters_from_overrides(true)) | ||
| , following_velocity_controller_(7.0, 0.0, 0.0) { | ||
| , following_velocity_controller_(9.0 , 0.0, 1000.0) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PID derivative gain tuning best practices for robotics chassis control
💡 Result:
Best practices for tuning the derivative gain (Kd) in PID controllers for robotics chassis control, typically applied to wheel velocity or steering rate in differential drive or similar chassis: 1. Role of Kd: Provides damping by predicting future error based on the rate of change of error. Reduces overshoot and oscillations after step inputs (e.g., velocity commands), stabilizes response in velocity control loops for chassis motors. 2. Tuning Process (Ziegler-Nichols inspired, manual iterative): - Start with Kd=0 (P-only or PI). - Increase proportional gain (Kp) until mild sustained oscillations occur. - Reduce Kp slightly (to ~0.6 * critical Kp). - Introduce small Kd (e.g., 0.01-0.1 initially) to dampen overshoot/oscillations without sluggishness. - Typical sequence: Tune P first for response, add Kd to eliminate overshoot, then small I if steady-state error persists. - Increase Kd gradually (5-20% increments) while testing step responses (e.g., cmd_vel twists in ROS). 3. Signs of Tuning: - Kd too low: Overshoot, ringing/oscillations in wheel velocity tracking. - Kd too high: Twitchy motors (noise amplification), overheating, sluggish response (overdamped). - Target: Critically damped - quick settling to setpoint with minimal/no overshoot. 4. Robotics-Specific Guidelines: - For wheel velocity PID (diff drive chassis): Kd often small (0.1-0.5 relative to Kp=0.5-8); e.g., DiffBot example Kp=0.6, Ki=0.3, Kd=0.5; RCR platform starts at 0.1. - Filter derivative term (low-pass) to mitigate encoder/gyro noise. - Test at varying speeds/loads; may need speed-dependent gains. - Often PD sufficient (low/no I) to avoid windup in velocity loops; use feedforward for steady-state. - Monitor logs: Plot commanded vs. actual velocity; aim for tight tracking without oscillation. 5. Tools/Setup: - Use ROS diff_drive_controller + low-level PID, or WPILib/REV PID. - Real-time tuning via serial/dynamic reconfigure; log encoder velocities. - Safety: Tune in place/low speed; check motor temps. Values are hardware/mass/speed dependent; no universal numbers - always empirical.
Citations:
- 1: https://docs.px4.io/v1.11/en/config_mc/pid_tuning_guide_multicopter.html
- 2: https://common.rosecityrobotics.com/operations/advanced/performance_tuning.html
- 3: https://docs.revrobotics.com/revlib/spark/closed-loop/getting-started-with-pid-tuning
- 4: https://ros-mobile-robots.com/packages/diffbot_base/pid/
- 5: PID Controller ros-mobile-robots/diffbot#35
- 6: https://docs.px4.io/v1.15/en/config_mc/pid_tuning_guide_multicopter.html
- 7: http://ardupilot.org/rover/docs/rover-tuning-steering-rate.html
🏁 Script executed:
# First, let's examine the chassis_controller.cpp file structure and the PID implementation
cd rmcs_ws && find . -name "*.cpp" -o -name "*.h" | grep -i "chassis\|pid" | head -20Repository: Alliance-Algorithm/RMCS
Length of output: 747
🏁 Script executed:
# Let's look at the actual file in question
cat -n src/rmcs_core/src/controller/chassis/chassis_controller.cpp | head -50Repository: Alliance-Algorithm/RMCS
Length of output: 159
🏁 Script executed:
# Search for PidCalculator implementation to understand the derivative term
fd -t f "\.cpp\|\.h" | xargs rg "class PidCalculator|struct PidCalculator" -A 20Repository: Alliance-Algorithm/RMCS
Length of output: 1734
🏁 Script executed:
# Look for any comments or documentation about the kd=1000 parameter
rg "kd|Kd|derivative" src/rmcs_core/src/controller/chassis/ -B 2 -A 2Repository: Alliance-Algorithm/RMCS
Length of output: 145
🏁 Script executed:
# Check if there's any derivative filtering applied
rg "low.pass|filter|smooth|derivative" src/rmcs_core/ -i -B 2 -A 2 | head -40Repository: Alliance-Algorithm/RMCS
Length of output: 122
🏁 Script executed:
# Read the full PidCalculator implementation
cat -n rmcs_ws/src/rmcs_core/src/controller/pid/pid_calculator.hppRepository: Alliance-Algorithm/RMCS
Length of output: 1963
🏁 Script executed:
# Read the chassis_controller.cpp file with correct path
cat -n rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_controller.cpp | head -150Repository: Alliance-Algorithm/RMCS
Length of output: 7008
🏁 Script executed:
# Check for any derivative filtering or scaling
rg "derivative|kd|low.pass|filter" rmcs_ws/src/rmcs_core/src/controller/ -i -B 2 -A 2Repository: Alliance-Algorithm/RMCS
Length of output: 16645
需验证 kd=1000 的 PID 导数增益调参。
导数增益从 0.0 大幅增加到 1000.0。根据 PidCalculator 实现，导数项为 kd * (err - last_err_)，误差变化率的微小变化（约 0.016 rad）即可驱动输出接近饱和值 ±16.0 rad/s。虽然输出被限制在 ±angular_velocity_max 范围内，但未发现任何导数滤波机制来抑制传感器噪声放大。
该参数用于爬坡模式的角速度控制。建议确认此调参已在实际硬件上充分测试，且系统在各爬升工况下表现稳定。
🤖 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` at line
22, The derivative gain kd for following_velocity_controller_ is set to 1000.0
which likely over-amplifies noise given PidCalculator computes derivative as
kd*(err - last_err_), so reduce kd to a conservative value or add derivative
filtering; update the initialization of following_velocity_controller_ (and/or
PidCalculator) to either lower kd to a tested value and/or implement a
derivative low-pass filter (or derivative-on-measurement with filtering) to
prevent noise amplification, then validate on hardware across climb modes to
confirm stability and that angular_velocity_max limits are not regularly hit.
…_controller,climbable-infantry)"Add code for climbing stairs with supercapacitor and adjust stair-climbing parameters."
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpp (1)
1-7:⚠️ Potential issue | 🟠 Major补上
<limits>头文件文件在第 144-145 行直接使用了
std::numeric_limits，但当前的包含列表中没有#include <limits>。虽然可能因为传递包含而编译成功，但这违反了"包含你使用的"原则，应在文件顶部明确添加#include <limits>。🤖 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_power_controller.cpp` around lines 1 - 7, This file uses std::numeric_limits (seen around lines 144-145) but does not directly include <limits>; add an explicit `#include` <limits> to the top of rmcs_core/src/controller/chassis/chassis_power_controller.cpp so the use of std::numeric_limits is correctly and directly covered (keep the include alongside the other C++/project headers).
♻️ Duplicate comments (1)
rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpp (1)
108-113:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
FORERAKED检测仍然不可达。
update()只有在auto_climb_state_ != IDLE时才调用update_auto_climb_control()，所以detect_is_foreraked()里的IDLE分支永远不会执行。当前这条自动切换到FORERAKED的路径还是死代码。🛠️ 建议修改
- if (auto_climb_state_ != AutoClimbState::IDLE) { - stop_manual_support(); - apply_auto_climb_control(update_auto_climb_control()); - } else { + if (auto_climb_state_ == AutoClimbState::IDLE) { + detect_is_foreraked(); + } + + if (auto_climb_state_ != AutoClimbState::IDLE) { + stop_manual_support(); + apply_auto_climb_control(update_auto_climb_control()); + } else { apply_auto_climb_control(update_manual_support_control(keyboard)); apply_manual_support_zero_output(); }Also applies to: 161-168, 486-496
🤖 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/climbable_infantry_chassis_climber_controller.cpp` around lines 108 - 113, The FORERAKED detection branch is unreachable because update_auto_climb_control() is only called when auto_climb_state_ != AutoClimbState::IDLE, so detect_is_foreraked()'s IDLE logic never runs; fix by ensuring the auto-climb detection logic is invoked in the manual branch as well (either call update_auto_climb_control() or extract the FORERAKED detection into a shared helper and call it from update() when auto_climb_state_ == IDLE), updating references to update_manual_support_control(keyboard) and apply_manual_support_zero_output() accordingly; apply the same change pattern to the other similar blocks (the ones around the other occurrences of update_auto_climb_control()/update_manual_support_control() mentioned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpp`:
- Around line 1-7: This file uses std::numeric_limits (seen around lines
144-145) but does not directly include <limits>; add an explicit `#include`
<limits> to the top of
rmcs_core/src/controller/chassis/chassis_power_controller.cpp so the use of
std::numeric_limits is correctly and directly covered (keep the include
alongside the other C++/project headers).
---
Duplicate comments:
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpp`:
- Around line 108-113: The FORERAKED detection branch is unreachable because
update_auto_climb_control() is only called when auto_climb_state_ !=
AutoClimbState::IDLE, so detect_is_foreraked()'s IDLE logic never runs; fix by
ensuring the auto-climb detection logic is invoked in the manual branch as well
(either call update_auto_climb_control() or extract the FORERAKED detection into
a shared helper and call it from update() when auto_climb_state_ == IDLE),
updating references to update_manual_support_control(keyboard) and
apply_manual_support_zero_output() accordingly; apply the same change pattern to
the other similar blocks (the ones around the other occurrences of
update_auto_climb_control()/update_manual_support_control() mentioned).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5210091e-e444-4404-92e5-a6501576b1ae
📒 Files selected for processing (3)
rmcs_ws/src/rmcs_bringup/config/climbable-infantry.yamlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpp
✅ Files skipped from review due to trivial changes (1)
- rmcs_ws/src/rmcs_bringup/config/climbable-infantry.yaml
added ui-climbable-infantry. [fix](chassis power controller):adjusted excess_power_limit.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpp (1)
113-136:⚠️ Potential issue | 🟠 Major避免
inf_参与后续功率计算，防止产生 NaN。第 116 行在 boost 激活且满足条件时将
power_limit设为inf_，但第 125-130 行的电压插值和第 136 行的虚拟缓冲缩放会继续对其进行算术运算。当插值系数为 0 或虚拟缓冲能量为 0 时，会产生inf * 0 = NaN，最终向/chassis/control_power_limit写入非法值。应在无限制功率的条件下单独分支处理，跳过后续插值和缓冲缩放步骤。🤖 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_power_controller.cpp` around lines 113 - 136, The code sets power_limit to inf_ when boost_mode_ and *supercap_enabled_ and certain conditions hold, but then continues with voltage interpolation and virtual_buffer_energy_ scaling which can produce inf*0 = NaN; modify the logic in the block around power_limit, chassis_power_limit_expected_ and the interpolation so that when power_limit == inf_ (or when the LAUNCH_RAMP/left motor condition triggers) you take a separate branch that assigns chassis_power_limit_expected_ and the final control power limit directly to an "unlimited" value (or returns early) and skip the subsequent voltage interpolation and multiplication by virtual_buffer_energy_/virtual_buffer_energy_limit_ (references: inf_, power_limit, boost_mode_, supercap_enabled_, chassis_power_limit_referee_, chassis_power_limit_expected_, supercap_voltage_, virtual_buffer_energy_, virtual_buffer_energy_limit_).
🤖 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/plugins.xml`:
- Around line 2-22: plugins.xml is missing registrations for MecanumHero and
OmniInfantry which causes pluginlib to fail at runtime; either restore their
<class> entries in plugins.xml for types rmcs_core::hardware::MecanumHero and
rmcs_core::hardware::OmniInfantry, or remove the corresponding
PLUGINLIB_EXPORT_CLASS lines from the sources to keep declarations consistent.
Locate the export macros
PLUGINLIB_EXPORT_CLASS(rmcs_core::hardware::MecanumHero, ...) in
mecanum_hero.cpp (around the existing export at line ~537) and
PLUGINLIB_EXPORT_CLASS(rmcs_core::hardware::OmniInfantry, ...) in
omni_infantry.cpp (around ~342) and then decide: add matching <class
type="rmcs_core::hardware::MecanumHero"...> and <class
type="rmcs_core::hardware::OmniInfantry"...> entries to plugins.xml (following
the pattern used for Status/Infantry/Hero etc.), or remove those two
PLUGINLIB_EXPORT_CLASS macros and any related build/install targets so the YAML
configs (mecanum-hero.yaml and omni-infantry.yaml) no longer reference
non-exported plugins.
In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp`:
- Around line 95-97: The lambda to_referee_angle uses std::numbers::pi but the
file is missing the <numbers> include; add `#include` <numbers> near other
includes in this translation unit so std::numbers::pi is available and portable
(locate the to_referee_angle lambda and ensure the header is added).
---
Outside diff comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpp`:
- Around line 113-136: The code sets power_limit to inf_ when boost_mode_ and
*supercap_enabled_ and certain conditions hold, but then continues with voltage
interpolation and virtual_buffer_energy_ scaling which can produce inf*0 = NaN;
modify the logic in the block around power_limit, chassis_power_limit_expected_
and the interpolation so that when power_limit == inf_ (or when the
LAUNCH_RAMP/left motor condition triggers) you take a separate branch that
assigns chassis_power_limit_expected_ and the final control power limit directly
to an "unlimited" value (or returns early) and skip the subsequent voltage
interpolation and multiplication by
virtual_buffer_energy_/virtual_buffer_energy_limit_ (references: inf_,
power_limit, boost_mode_, supercap_enabled_, chassis_power_limit_referee_,
chassis_power_limit_expected_, supercap_voltage_, virtual_buffer_energy_,
virtual_buffer_energy_limit_).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01b33f17-40ef-46a1-a54d-8cc5105c8e4d
📒 Files selected for processing (4)
rmcs_ws/src/rmcs_bringup/config/climbable-infantry.yamlrmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_power_controller.cpprmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp
✅ Files skipped from review due to trivial changes (1)
- rmcs_ws/src/rmcs_bringup/config/climbable-infantry.yaml
Comment on lines
+2
to
+22
| <class type="rmcs_core::hardware::cboard::Status" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type="rmcs_core::hardware::Infantry" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type="rmcs_core::hardware::Hero" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type="rmcs_core::hardware::SteeringHero" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type="rmcs_core::hardware::SteeringInfantry" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type="rmcs_core::hardware::ClimbableInfantry" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> | ||
| <class type = "rmcs_core::hardware::TunnelInfantry" base_class_type="rmcs_executor::Component"> | ||
| <description>Test plugin.</description> | ||
| </class> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "== plugin exports =="
rg -nP 'PLUGINLIB_EXPORT_CLASS\s*\(\s*rmcs_core::hardware::(MecanumHero|OmniInfantry)\b' rmcs_ws/src
echo
echo "== plugin references in configs/code =="
rg -nP 'rmcs_core::hardware::(MecanumHero|OmniInfantry)\b' rmcs_wsRepository: Alliance-Algorithm/RMCS
Length of output: 915
MecanumHero 和 OmniInfantry 插件注册缺失，会导致运行时装载失败。
原始源码仍导出这两个插件：
PLUGINLIB_EXPORT_CLASS(rmcs_core::hardware::MecanumHero, ...)在mecanum_hero.cpp:537PLUGINLIB_EXPORT_CLASS(rmcs_core::hardware::OmniInfantry, ...)在omni_infantry.cpp:342
同时，bringup 配置仍在引用它们：
mecanum-hero.yaml引用rmcs_core::hardware::MecanumHeroomni-infantry.yaml引用rmcs_core::hardware::OmniInfantry
移除 plugins.xml 中的注册但保留源码导出和配置引用，pluginlib 会在运行时装载失败。需要确保移除对应的源码导出或在 plugins.xml 中恢复注册。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/plugins.xml` around lines 2 - 22, plugins.xml is
missing registrations for MecanumHero and OmniInfantry which causes pluginlib to
fail at runtime; either restore their <class> entries in plugins.xml for types
rmcs_core::hardware::MecanumHero and rmcs_core::hardware::OmniInfantry, or
remove the corresponding PLUGINLIB_EXPORT_CLASS lines from the sources to keep
declarations consistent. Locate the export macros
PLUGINLIB_EXPORT_CLASS(rmcs_core::hardware::MecanumHero, ...) in
mecanum_hero.cpp (around the existing export at line ~537) and
PLUGINLIB_EXPORT_CLASS(rmcs_core::hardware::OmniInfantry, ...) in
omni_infantry.cpp (around ~342) and then decide: add matching <class
type="rmcs_core::hardware::MecanumHero"...> and <class
type="rmcs_core::hardware::OmniInfantry"...> entries to plugins.xml (following
the pattern used for Status/Infantry/Hero etc.), or remove those two
PLUGINLIB_EXPORT_CLASS macros and any related build/install targets so the YAML
configs (mecanum-hero.yaml and omni-infantry.yaml) no longer reference
non-exported plugins.
Comment on lines
+95
to
+97
| auto to_referee_angle = [](double angle) { | ||
| return static_cast<int>( | ||
| std::round((2 * std::numbers::pi - angle) / std::numbers::pi * 180)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ui-climbable-infantry.cpp" -type fRepository: Alliance-Algorithm/RMCS
Length of output: 136
🏁 Script executed:
cat -n ./rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp | head -110Repository: Alliance-Algorithm/RMCS
Length of output: 5848
补上 <numbers> 头文件。
第 97 行使用了 std::numbers::pi，但当前文件没有 #include <numbers>。这不是标准保证的传递包含，换工具链后可能直接编译失败。
🔧 建议修改
`#include` <algorithm>
`#include` <cmath>
`#include` <cstdint>
+#include <numbers>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp` around
lines 95 - 97, The lambda to_referee_angle uses std::numbers::pi but the file is
missing the <numbers> include; add `#include` <numbers> near other includes in
this translation unit so std::numbers::pi is available and portable (locate the
to_referee_angle lambda and ensure the header is added).
added track direction indicator and track start indicator adjusted some parameters
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp (1)
1-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
<numbers>头文件缺失，使用std::numbers::pi会在换工具链后编译失败。Line 105 使用了
std::numbers::pi，但文件中没有#include <numbers>（C++20 标准头）。当前编译可能借助传递包含偶然通过，但这不是标准保证的行为。🔧 建议修改
`#include` <algorithm> `#include` <cmath> `#include` <cstdint> +#include <numbers>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp` around lines 1 - 3, 该文件在第105行使用了符号 std::numbers::pi 但未包含 <numbers>，会导致在切换工具链或不同标准库实现时编译失败；在 rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp 顶部包含列表中添加对 <numbers> 的包含（即在现有 `#include` 行附近加入 `#include` <numbers>），以确保 std::numbers::pi 有效且移植性良好。rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpp (1)
108-113:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
FORERAKED检测仍然是死代码，安全保护完全失效。问题链路：
update()仅在auto_climb_state_ != IDLE时调用update_auto_climb_control()（Line 108）。update_auto_climb_control()内部的 IDLE 分支（Lines 163-168）因此结构上永远不会执行。detect_is_foreraked()本身（Lines 487-488）还额外加了!= IDLE的早返回——双重保险下此函数从未被触发。结果：机器人在空闲状态下发生前倾时，
FORERAKED状态永远无法被自动激活。修复方式：将
detect_is_foreraked()的调用移到update()的 IDLE 分支，并同时删除update_auto_climb_control()内部已成死代码的 IDLE 检测块。🐛 建议修改
if (auto_climb_state_ != AutoClimbState::IDLE) { stop_manual_support(); apply_auto_climb_control(update_auto_climb_control()); } else { + detect_is_foreraked(); apply_auto_climb_control(update_manual_support_control(keyboard)); apply_manual_support_zero_output(); }AutoClimbControl update_auto_climb_control() { - if (auto_climb_state_ == AutoClimbState::IDLE) { - detect_is_foreraked(); - if (auto_climb_state_ == AutoClimbState::IDLE) { - return {}; - } - } - auto_climb_timer_++; // ...Also applies to: 163-168, 486-497
🤖 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/climbable_infantry_chassis_climber_controller.cpp` around lines 108 - 113, The FORERAKED detection is dead because detect_is_foreraked() is never called when auto_climb_state_ == IDLE; fix by moving the detect_is_foreraked() invocation into update()'s IDLE branch (the branch that currently calls update_manual_support_control(keyboard), apply_auto_climb_control(...), apply_manual_support_zero_output(), and stop_manual_support()), so FORERAKED can be triggered while idle. Also remove the now-dead IDLE branch inside update_auto_climb_control() and delete the early-return check in detect_is_foreraked() that requires auto_climb_state_ != IDLE; keep the rest of apply_auto_climb_control(), update_manual_support_control(), and stop_manual_support() call ordering consistent with current idle behavior.
🤖 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/referee/app/ui/ui-climbable-infantry.cpp`:
- Around line 130-136: front_climber_motor_direction_indicator_[1] is passed
climber_motor_angle + half_intervel_angle before any wrap check, so if
climber_motor_angle is near 360 the value can exceed 360 and render incorrectly;
normalize the angle into [0,360) before calling set_angle for both
front_climber_motor_direction_indicator_[1] and
front_climber_motor_direction_indicator_[0] (use climber_motor_angle ±
half_intervel_angle, then if angle >= 360 subtract 360, if angle < 0 add 360, or
apply a modulo 360 normalization) so both calls receive a valid bounded angle.
- Around line 103-106: to_referee_angle currently invokes std::round on any
double (causing UB for NaN); update it to guard against NaN (and infinities)
before converting: inside the lambda to_referee_angle(double angle) check
std::isnan(angle) (or !std::isfinite(angle)) and return a safe sentinel (e.g. 0)
or an optional/invalid indicator instead of calling std::round on NaN;
alternatively, ensure all call sites that pass *chassis_angle_ (and
*chassis_control_angle_ usage) perform std::isnan checks before calling
to_referee_angle—reference the to_referee_angle lambda and the chassis_angle_ /
chassis_control_angle_ usages so you can add the guard either in the helper or
at each caller.
In `@rmcs_ws/src/rmcs_core/third_party/tinympc`:
- Line 1: The .gitmodules is missing an entry for the tinympc submodule (gitlink
6e710a2e5813ce261117299b84577294ebc02be0) so add a [submodule "tinympc"] section
with path = rmcs_core/third_party/tinympc and url = (correct repository URL for
tinympc) to .gitmodules, commit that change, then ensure the submodule is
initialized and populated by running git submodule init/update (or git submodule
update --init --recursive) so the tinympc directory is no longer empty and the
gitlink resolves correctly.
---
Duplicate comments:
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpp`:
- Around line 108-113: The FORERAKED detection is dead because
detect_is_foreraked() is never called when auto_climb_state_ == IDLE; fix by
moving the detect_is_foreraked() invocation into update()'s IDLE branch (the
branch that currently calls update_manual_support_control(keyboard),
apply_auto_climb_control(...), apply_manual_support_zero_output(), and
stop_manual_support()), so FORERAKED can be triggered while idle. Also remove
the now-dead IDLE branch inside update_auto_climb_control() and delete the
early-return check in detect_is_foreraked() that requires auto_climb_state_ !=
IDLE; keep the rest of apply_auto_climb_control(),
update_manual_support_control(), and stop_manual_support() call ordering
consistent with current idle behavior.
In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp`:
- Around line 1-3: 该文件在第105行使用了符号 std::numbers::pi 但未包含
<numbers>，会导致在切换工具链或不同标准库实现时编译失败；在
rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp 顶部包含列表中添加对 <numbers>
的包含（即在现有 `#include` 行附近加入 `#include` <numbers>），以确保 std::numbers::pi 有效且移植性良好。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41d3f468-28be-4819-9a8f-05d2a6b7c2d2
📒 Files selected for processing (3)
rmcs_ws/src/rmcs_core/src/controller/chassis/climbable_infantry_chassis_climber_controller.cpprmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpprmcs_ws/src/rmcs_core/third_party/tinympc
Comment on lines
+103
to
+106
| auto to_referee_angle = [](double angle) { | ||
| return static_cast<int>( | ||
| std::round((2 * std::numbers::pi - angle) / std::numbers::pi * 180)); | ||
| }; |
There was a problem hiding this comment.
to_referee_angle 在输入为 NaN 时存在未定义行为。
static_cast<int>(std::round(NaN)) 属于 C++ 未定义行为（浮点向整型转换时无法表示的值）。chassis_angle_ 在系统启动的早期帧中有可能为 NaN，而 chassis_control_angle_ 在 Line 139 有 !std::isnan() 保护，但同一函数中 Line 113 和 Line 129 调用 to_referee_angle(*chassis_angle_) 却没有类似保护。
🛡️ 建议修改
+ if (std::isnan(*chassis_angle_))
+ return;
+
int climber_motor_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/referee/app/ui/ui-climbable-infantry.cpp` around
lines 103 - 106, to_referee_angle currently invokes std::round on any double
(causing UB for NaN); update it to guard against NaN (and infinities) before
converting: inside the lambda to_referee_angle(double angle) check
std::isnan(angle) (or !std::isfinite(angle)) and return a safe sentinel (e.g. 0)
or an optional/invalid indicator instead of calling std::round on NaN;
alternatively, ensure all call sites that pass *chassis_angle_ (and
*chassis_control_angle_ usage) perform std::isnan checks before calling
to_referee_angle—reference the to_referee_angle lambda and the chassis_angle_ /
chassis_control_angle_ usages so you can add the guard either in the helper or
at each caller.
Comment on lines
+130
to
+136
| front_climber_motor_direction_indicator_[1].set_angle( | ||
| climber_motor_angle + half_intervel_angle, 5); | ||
| if (climber_motor_angle - half_intervel_angle < 0) { | ||
| climber_motor_angle += 360; | ||
| } | ||
| front_climber_motor_direction_indicator_[0].set_angle( | ||
| climber_motor_angle - half_intervel_angle, 5); |
There was a problem hiding this comment.
front_climber_motor_direction_indicator_[1] 缺少上界越界保护。
Line 130 在执行任何边界修正之前就已调用 set_angle(climber_motor_angle + 30, 5)。若 climber_motor_angle 接近 360（例如 chassis_angle_ ≈ 0），则传入值可能超过 360，导致圆弧渲染位置错误。相比之下，indicator[0] 在 Lines 132-134 有 < 0 的下界修正，但无对应上界处理。
🐛 建议修改
- front_climber_motor_direction_indicator_[1].set_angle(
- climber_motor_angle + half_intervel_angle, 5);
- if (climber_motor_angle - half_intervel_angle < 0) {
- climber_motor_angle += 360;
- }
- front_climber_motor_direction_indicator_[0].set_angle(
- climber_motor_angle - half_intervel_angle, 5);
+ int upper_angle = climber_motor_angle + half_intervel_angle;
+ if (upper_angle >= 360)
+ upper_angle -= 360;
+ front_climber_motor_direction_indicator_[1].set_angle(upper_angle, 5);
+ if (climber_motor_angle - half_intervel_angle < 0) {
+ climber_motor_angle += 360;
+ }
+ front_climber_motor_direction_indicator_[0].set_angle(
+ climber_motor_angle - half_intervel_angle, 5);📝 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
| front_climber_motor_direction_indicator_[1].set_angle( | |
| climber_motor_angle + half_intervel_angle, 5); | |
| if (climber_motor_angle - half_intervel_angle < 0) { | |
| climber_motor_angle += 360; | |
| } | |
| front_climber_motor_direction_indicator_[0].set_angle( | |
| climber_motor_angle - half_intervel_angle, 5); | |
| int upper_angle = climber_motor_angle + half_intervel_angle; | |
| if (upper_angle >= 360) | |
| upper_angle -= 360; | |
| front_climber_motor_direction_indicator_[1].set_angle(upper_angle, 5); | |
| if (climber_motor_angle - half_intervel_angle < 0) { | |
| climber_motor_angle += 360; | |
| } | |
| front_climber_motor_direction_indicator_[0].set_angle( | |
| climber_motor_angle - half_intervel_angle, 5); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/ui-climbable-infantry.cpp` around
lines 130 - 136, front_climber_motor_direction_indicator_[1] is passed
climber_motor_angle + half_intervel_angle before any wrap check, so if
climber_motor_angle is near 360 the value can exceed 360 and render incorrectly;
normalize the angle into [0,360) before calling set_angle for both
front_climber_motor_direction_indicator_[1] and
front_climber_motor_direction_indicator_[0] (use climber_motor_angle ±
half_intervel_angle, then if angle >= 360 subtract 360, if angle < 0 add 360, or
apply a modulo 360 normalization) so both calls receive a valid bounded angle.
| @@ -0,0 +1 @@ | |||
| Subproject commit 6e710a2e5813ce261117299b84577294ebc02be0 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 描述：检查 tinympc 子模块的提交历史和变更内容
# 进入子模块目录
cd rmcs_ws/src/rmcs_core/third_party/tinympc 2>/dev/null || {
echo "子模块目录不存在，可能需要初始化子模块"
exit 0
}
# 显示当前提交信息
echo "=== 当前子模块提交信息 ==="
git log -1 --format="%H%n%s%n%b" 6e710a2e5813ce261117299b84577294ebc02be0 2>/dev/null || echo "无法获取提交信息"
# 显示最近的提交历史
echo ""
echo "=== 最近 5 次提交 ==="
git log -5 --oneline 2>/dev/null || echo "无法获取提交历史"Repository: Alliance-Algorithm/RMCS
Length of output: 248
🏁 Script executed:
# 检查 .gitmodules 文件中的 tinympc 配置
cat .gitmodules | grep -A 3 "tinympc" || echo "未找到tinympc配置"
# 检查当前 tinympc 子模块的状态
cd rmcs_ws/src/rmcs_core/third_party/tinympc 2>/dev/null && {
echo "=== 子模块当前HEAD ==="
git rev-parse HEAD 2>/dev/null || echo "无法获取HEAD"
echo ""
echo "=== 子模块远程信息 ==="
git remote -v 2>/dev/null || echo "无法获取远程信息"
} || echo "tinympc目录不存在或无法进入"
# 检查git diff中关于tinympc的更改
echo ""
echo "=== 主仓库中tinympc的引用 ==="
cd - > /dev/null
git ls-files --stage | grep tinympc || echo "未找到submodule引用"Repository: Alliance-Algorithm/RMCS
Length of output: 407
🏁 Script executed:
# 检查 tinympc 目录在主仓库中的历史
git log --oneline -10 -- rmcs_ws/src/rmcs_core/third_party/tinympc | head -20
# 检查是否存在git subtree相关的提交
echo ""
echo "=== 搜索 subtree 相关提交 ==="
git log --grep="subtree\|tinympc" --oneline | head -10
# 检查当前工作目录和索引的差异
echo ""
echo "=== 检查tinympc目录状态 ==="
cd rmcs_ws/src/rmcs_core/third_party/tinympc 2>/dev/null && {
git status
echo ""
echo "=== 查看目录内容 ==="
ls -la | head -20
} || echo "无法进入目录"Repository: Alliance-Algorithm/RMCS
Length of output: 440
🏁 Script executed:
# 查看根目录中的 git 状态和改动
echo "=== 仓库根目录git状态 ==="
git status
echo ""
echo "=== 检查tinympc相关的git diff ==="
git diff HEAD rmcs_ws/src/rmcs_core/third_party/tinympc 2>/dev/null || echo "无差异"
echo ""
echo "=== 查看.gitmodules文件 ==="
cat .gitmodules 2>/dev/null || echo "不存在.gitmodules"
echo ""
echo "=== 检查git index中的tinympc记录 ==="
git ls-files -s rmcs_ws/src/rmcs_core/third_party/tinympcRepository: Alliance-Algorithm/RMCS
Length of output: 688
tinympc 子模块配置不完整，需要修复。
此更改更新 tinympc 子模块的 gitlink 引用至 6e710a2e5813ce261117299b84577294ebc02be0。但验证发现配置问题：
tinympc在 Git 索引中存在（mode 160000），但.gitmodules中无对应条目tinympc目录为空- 其他子模块（
fast_tf、hikcamera、serial）在.gitmodules中正确配置
需要：
- 在
.gitmodules中添加tinympc的配置条目（包括path和url） - 确保子模块能够正确初始化和更新
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rmcs_ws/src/rmcs_core/third_party/tinympc` at line 1, The .gitmodules is
missing an entry for the tinympc submodule (gitlink
6e710a2e5813ce261117299b84577294ebc02be0) so add a [submodule "tinympc"] section
with path = rmcs_core/third_party/tinympc and url = (correct repository URL for
tinympc) to .gitmodules, commit that change, then ensure the submodule is
initialized and populated by running git submodule init/update (or git submodule
update --init --recursive) so the tinympc directory is no longer empty and the
gitlink resolves correctly.
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
4 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.
chassis_power_controller
Climbable Infantry（可爬行步兵）功能扩展与若干控制器/工具改进
概述
本 PR 为“可爬行步兵”平台新增完整的硬件/控制/UI 支持、爬坡/爬楼梯相关控制器以及若干工具与配置，包含底盘功率控制的行为调整和多个子系统的改进与重构。
关键改动
新增与变更的控制器 / 组件
底盘功率控制调整
封装与插件注册
配置与文档
其他行为或微调
影响范围与审查指导
总结
本 PR 显著扩展了“可爬行步兵”平台的功能（硬件节点、爬楼控制器、UI、PID 记录与调参工具），并对底盘功率管理进行了关键策略调整。提交量大、跨模块修改多；建议按功能块分阶段 review 与在受控环境下开展硬件验证。