feat: Integrate steering-hero mainline support#80
Open
qzhhhi wants to merge 3 commits into
Open
Conversation
Integrate hardware, chassis, gimbal, shooting, and UI support for steering-hero and steering-hero-little. Also correct LK motor torque constants and retune the omni-infantry / sentry compensating parameters to keep mainline vehicle control output unchanged. Remove the unmaintained mecanum-hero configuration. Known issues (not blocking this merge): - The Hero UI shooter condition / state_word path is still leftover debug wiring and is not connected to the current active runtime path - HeroFrictionWheelController still preserves the historical friction-wheel index convention; for now this is only documented with a TODO and should later be replaced by an explicit first-stage mapping - steering-hero-little has a mismatch between player viewer limit parameters and control logic, which still needs follow-up calibration - hero_chassis_power_controller inherits the risk of reading uninitialized members from the existing chassis_power_controller - steering-hero-little uses different vehicle_radius parameters in steering_wheel_status and steering_wheel_controller
Walkthrough新增并重构舵轮机器人配置、硬件板卡、底盘/舵轮/云台/射击控制器和相关 UI 与记录器，且升级了 librmcs 依赖与插件注册。 Changes整体变更汇总
Estimated code review effort:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (17)
rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp-128-140 (1)
128-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick win故障计数需要在恢复正常后清零。
friction_faulty_count_现在只增不减：健康周期不会清零，reset_all_controls()也不会清零。这样故障会跨正常运行和重新使能持续累积，最终把瞬时掉速误判成永久 jam。💡 建议修正
void reset_all_controls() { friction_enabled_ = false; + friction_faulty_count_ = 0; last_primary_friction_velocity_ = nan_; primary_friction_velocity_decrease_integral_ = 0; @@ if (detect_friction_faulty()) { if (friction_faulty_count_ == 200) { friction_enabled_ = false; *friction_jammed_ = true; } else { friction_faulty_count_++; *friction_ready_ = true; } return; } + friction_faulty_count_ = 0; *friction_ready_ = true; *bullet_fired_ = detect_bullet_fire();Also applies to: 168-180
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp` around lines 128 - 140, The fault counter friction_faulty_count_ is never cleared and should be reset when controls are reinitialized; update reset_all_controls() to set friction_faulty_count_ = 0 and also ensure any other control-resetting routine (the similar block around the code at lines 168-180) resets friction_faulty_count_ as well so faults do not persist across a healthy recovery or re-enable cycle; locate the symbols reset_all_controls() and friction_faulty_count_ and add the counter reset in those reset paths.rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp-255-259 (1)
255-259:⚠️ Potential issue | 🟠 Major | ⚡ Quick win在正常切换阶段时重置
putter_timeout_count_。这个计数器只在“真正超时”时清零；如果某次射击正常结束但累计到了较大的值，下一次
SHOOTING会继承旧计数并提前触发超时。至少在进入SHOOTING和回到PRELOADING/PRELOADED时要复位。💡 建议修正
- void set_preloading() { shoot_stage_ = ShootStage::PRELOADING; } + void set_preloading() { + putter_timeout_count_ = 0; + shoot_stage_ = ShootStage::PRELOADING; + } - void set_preloaded() { shoot_stage_ = ShootStage::PRELOADED; } + void set_preloaded() { + putter_timeout_count_ = 0; + shoot_stage_ = ShootStage::PRELOADED; + } - void set_shooting() { shoot_stage_ = ShootStage::SHOOTING; } + void set_shooting() { + putter_timeout_count_ = 0; + shoot_stage_ = ShootStage::SHOOTING; + }Also applies to: 307-320
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp` around lines 255 - 259, When changing shoot_stage_ you must reset putter_timeout_count_ to avoid inheriting stale timeout counts; modify set_preloading(), set_preloaded(), and set_shooting() (and the analogous stage-switching methods in the block around the other occurrence at the 307-320 region) to set putter_timeout_count_ = 0 (or call a single reset helper) whenever the stage changes to PRELOADING, PRELOADED, or SHOOTING so each new cycle starts with a cleared timeout counter.rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp-60-65 (1)
60-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick win不要原地排序
velocities。
velocities现在既当“时间顺序历史窗口”用，又在分析函数里被原地sort。一旦样本数超过 1000，Line 62 的erase(begin())删除的就不再是最旧样本，而是最小样本，统计窗口会被悄悄污染。💡 建议修正
- sort(velocities.begin(), velocities.end()); - velocity_max = velocities.back(); - velocity_min = velocities.front(); + auto sorted = velocities; + std::sort(sorted.begin(), sorted.end()); + velocity_max = sorted.back(); + velocity_min = sorted.front(); - range_ = velocities.back() - velocities.front(); - range2_ = velocities[int(velocities.size() - 2)] - velocities[1]; + range_ = sorted.back() - sorted.front(); + range2_ = sorted[sorted.size() - 2] - sorted[1];Also applies to: 145-177, 179-214, 216-243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp` around lines 60 - 65, velocities is being used as a time-ordered circular window (push_back + erase(begin())) but is also sorted in-place inside analysis3(), so once the window exceeds max_velocities_size the erased element is the smallest value not the oldest; fix by never sorting velocities in-place: either make a local copy (e.g., std::vector<double> sorted = velocities) and sort that inside analysis3() (or sort the copy before analysis), or move sorting logic into analysis3() so it works on a copied container; preserve the push_back(*initial_speed_) and erase(velocities.begin()) behavior to remove the oldest sample. Apply the same change to the other analysis calls that currently sort the velocities container in-place so all statistical analyses operate on a sorted copy rather than mutating velocities.rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp-271-279 (1)
271-279:⚠️ Potential issue | 🟠 Major | ⚡ Quick win检测到已预装时不要再进入 jam 保护。
photoelectric_sensor_status_为真时你已经在 Line 273 切到了PRELOADED，但后面仍然无条件执行enter_jam_protection()。这会让正常预装路径也走 800 周期反转/冷却，和注释描述相反。💡 建议修正
if (locked_detect_count_ > 300) { if (*photoelectric_sensor_status_) { set_preloaded(); + locked_detect_count_ = 0; + return; } // If the photoelectric sensor was not triggered, treat it as a simple jam, // reverse briefly, then continue until stall. locked_detect_count_ = 0; enter_jam_protection(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp` around lines 271 - 279, The branch handling locked_detect_count_ > 300 must not call enter_jam_protection() when photoelectric_sensor_status_ is true and set_preloaded() was invoked; modify the logic in the block around locked_detect_count_ to make enter_jam_protection() (and the locked_detect_count_ = 0 reset that precedes it) conditional on !(*photoelectric_sensor_status_)—i.e., if sensor true call set_preloaded() and skip jam protection, otherwise treat as a jam: reset locked_detect_count_ and call enter_jam_protection().rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp-196-213 (1)
196-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick win在访问
friction_velocities_[2]之前先约束配置规模。这里无条件读取第 3 个摩擦轮，但构造函数只校验了数组非空，没有保证
friction_count_ >= 3。一旦接到 2 轮配置，Line 201/212 会直接越界。💡 两种可接受的修正方向
- } else if (friction_wheels.size() == 0) + } else if (friction_wheels.size() == 0) throw std::runtime_error( "Empty array error: 'friction_wheels' and 'friction_velocities' cannot be empty!"); + else if (friction_wheels.size() <= 2) + throw std::runtime_error( + "'friction_wheels' must contain at least 3 entries for bullet fire detection");或把“用于开火检测的轮子索引”改成显式参数，而不是硬编码
2。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp` around lines 196 - 213, The code unconditionally dereferences friction_velocities_[2], which can out-of-bounds if friction_count_ < 3; either add a guard checking friction_count_ (or friction_velocities_.size()) >= 3 before any access/use of friction_velocities_[2] and skip the whole block if not, or replace the hard-coded index with an explicit configured "first-stage" index (e.g., a member like primary_stage_index_) and validate that index is within range before using it; update uses of last_primary_friction_velocity_, primary_friction_velocity_decrease_integral_, target_friction_velocity(), and fired to reference the validated index instead of the literal 2.rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp-70-78 (1)
70-78:⚠️ Potential issue | 🟠 Major | ⚡ Quick win保留 2 轮配置时不要写空日志。
log_text现在只在friction_wheel_count_ == 4时赋值，但成员默认值仍是2。这意味着默认/2轮配置会持续向日志文件写空行，表面上“正常运行”，实际没有任何有效统计输出。💡 两种修正方向
- if (friction_wheel_count_ == 4) { + if (friction_wheel_count_ == 4) { log_text = fmt::format( "{},{},{:.3f},{:.3f},{:.3f},{:.3f},{:.3f},{:.3f},{:.3f}", *initial_speed_, (int)velocities.size(), // velocity_, excellence_rate_, pass_rate_, range_, range2_, velocity_max, velocity_min); + } else { + throw std::runtime_error("ShootingRecorder currently only supports 4 friction wheels"); }或者补回 2 轮配置的日志格式分支。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp` around lines 70 - 78, The current code only assigns log_text when friction_wheel_count_ == 4 but always writes log_stream_ << log_text << std::endl, causing empty log lines for the default 2-wheel case; fix by either adding the missing 2-wheel logging branch that constructs log_text (mirroring the 4-wheel format) or guard the write with the same condition (only write to log_stream_ when friction_wheel_count_ == 4 or when log_text is non-empty); update the logic around friction_wheel_count_, log_text, and the final log_stream_ write so no empty lines are emitted.rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp-71-95 (1)
71-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick win修复
Ctrl+F被切换两次的问题。当
ctrl和f在同一帧一起从false -> true时，Line 71-72 和 Line 85-86 都会执行active_profile_ ^= 1，结果相互抵消，Ctrl+F实际上不会切换 profile。这里需要把F与Ctrl+F的边沿判定改成互斥分支。💡 一个直接的修正方式
- if (!last_keyboard_.f && keyboard.f) { + if (!profile_switch_last && profile_switch_now) { active_profile_ ^= 1; - - if (!std::isnan(friction_soft_start_stop_percentage_)) { - double sum = 0.0; - for (size_t i = 0; i < friction_count_; i++) - sum += *friction_velocities_[i] / target_friction_velocity(i); - friction_soft_start_stop_percentage_ = - std::clamp(sum / static_cast<double>(friction_count_), 0.0, 1.0); - } - } - const bool profile_switch_now = keyboard.ctrl && keyboard.f; - const bool profile_switch_last = last_keyboard_.ctrl && last_keyboard_.f; - - if (!profile_switch_last && profile_switch_now) { + // ... + } else if (!last_keyboard_.f && keyboard.f) { active_profile_ ^= 1; - - if (!std::isnan(friction_soft_start_stop_percentage_)) { - double sum = 0.0; - for (size_t i = 0; i < friction_count_; i++) - sum += *friction_velocities_[i] / target_friction_velocity(i); - friction_soft_start_stop_percentage_ = - std::clamp(sum / static_cast<double>(friction_count_), 0.0, 1.0); - } + // ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp` around lines 71 - 95, The profile toggle for plain F and Ctrl+F is currently executed in two separate if blocks causing double-toggle when both keys transition in the same frame; make the edge checks mutually exclusive so only one branch can flip active_profile_. Update the keyboard edge logic in hero_friction_wheel_controller.cpp (references: last_keyboard_, keyboard, active_profile_, friction_soft_start_stop_percentage_, friction_count_, friction_velocities_, target_friction_velocity()) so that the Ctrl+F branch and the plain F branch are exclusive (e.g., check the Ctrl+F condition first and use else/else-if for the plain F case, or require plain F to have !keyboard.ctrl), preserving the existing friction_soft_start_stop_percentage_ computation in the taken branch.rmcs_ws/src/rmcs_bringup/config/steering-hero-little.yaml-285-295 (1)
285-295:⚠️ Potential issue | 🟠 Major | ⚡ Quick win统一
vehicle_radius。这里
steering_wheel_status.vehicle_radius配的是0.286378，但steering_wheel_controller.vehicle_radius配的是0.318198。状态反演和控制解算会长期基于两套不同的底盘几何，速度估计、功率估算和轮组力矩分配都会互相打架。请先统一成同一半径，再做一次实车标定。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_bringup/config/steering-hero-little.yaml` around lines 285 - 295, 配置中的两个参数实体 steering_wheel_status.ros__parameters.vehicle_radius 和 steering_wheel_controller.ros__parameters.vehicle_radius 值不一致（0.286378 vs 0.318198），请将它们统一为同一半径值（选择正确的实测/标定值），在文件中同时更新 steering_wheel_status 和 steering_wheel_controller 下的 vehicle_radius 为该统一值，然后在实车上重新做一次标定验证所有基于车体半径的估算和控制器参数（速度估计、功率估算、轮组力矩分配等）。rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp-334-336 (1)
334-336:⚠️ Potential issue | 🟠 Major | ⚡ Quick win触发位也会在首次变化后卡成常高。
这段逻辑只会把
/auto_aim/image_capturer/trigger置成true，不会在未触发周期回落到false。第一次收到新时间戳后，下游看到的就是永久触发。建议改成单周期脉冲
- if (last_camera_capturer_trigger_timestamp_ != *camera_capturer_trigger_timestamp_) - *camera_capturer_trigger_ = true; + *camera_capturer_trigger_ = + last_camera_capturer_trigger_timestamp_ != *camera_capturer_trigger_timestamp_; last_camera_capturer_trigger_timestamp_ = *camera_capturer_trigger_timestamp_;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp` around lines 334 - 336, The current logic only sets *camera_capturer_trigger_ = true when last_camera_capturer_trigger_timestamp_ differs, but never clears it, causing a latched high trigger; change the update so that after comparing last_camera_capturer_trigger_timestamp_ with *camera_capturer_trigger_timestamp_ you set *camera_capturer_trigger_ to true for that single cycle and explicitly set it to false otherwise, and then update last_camera_capturer_trigger_timestamp_; ensure you reference and modify last_camera_capturer_trigger_timestamp_, camera_capturer_trigger_timestamp_, and camera_capturer_trigger_ so the downstream receives a one-cycle pulse instead of a permanent high.rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp-345-347 (1)
345-347:⚠️ Potential issue | 🟠 Major | ⚡ Quick win相机触发信号会在第一次边沿后永久置高。
这里只在时间戳变化时写入
true，其余路径从不回写false，所以/auto_aim/image_capturer/trigger一旦触发过就会锁死为高电平。下游如果按脉冲语义消费，这里会变成持续触发。建议改成单周期脉冲
- if (last_camera_capturer_trigger_timestamp_ != *camera_capturer_trigger_timestamp_) - *camera_capturer_trigger_ = true; + *camera_capturer_trigger_ = + last_camera_capturer_trigger_timestamp_ != *camera_capturer_trigger_timestamp_; last_camera_capturer_trigger_timestamp_ = *camera_capturer_trigger_timestamp_;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp` around lines 345 - 347, 当前实现只在时间戳变化时把 *camera_capturer_trigger_ 置为 true 且从不回写 false，导致触发信号永久为高；修改在检测到时间戳变化时写 true，否则明确写 false，保证每次边沿只输出单周期脉冲；在代码使用的标识符包括 last_camera_capturer_trigger_timestamp_, camera_capturer_trigger_timestamp_, camera_capturer_trigger_，请在相同代码块中将分支改为 if (last_camera_capturer_trigger_timestamp_ != *camera_capturer_trigger_timestamp_) { *camera_capturer_trigger_ = true; } else { *camera_capturer_trigger_ = false; } 并随后更新 last_camera_capturer_trigger_timestamp_ = *camera_capturer_trigger_timestamp_;，同时保持对空指针的检查（如果有）一致。rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp-39-43 (1)
39-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick win原始角度这里仍在走
Float编码，界面会被渲染成三位小数。
shape.hpp里的Float::set_value(double)会按value * 1000编码；这里把raw_angle转成double后喂给Float，最终会显示成123.000这类格式。下面的count_digits()又只按整数位数估宽，新增的摩擦档位框位置也会随之错位。这里应改成Integer控件，或者至少不要走Float::set_value(double)这条路径。Also applies to: 163-179, 507-509
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp` around lines 39 - 43, 这些角度控件（yaw_angle_number_, pitch_angle_number_, bottom_yaw_angle_number_ 以及文件中另两处相同模式的实例）当前使用了 Float 路径导致通过 Float::set_value(double) 按 value*1000 编码并显示三位小数；请改为使用整数控件或整数接口：在创建这些控件时用对应的 Integer 类型（或 Shape::Integer 构造器）替代 Float/Float-based 构造，或者在赋值时调用 Integer::set_value(int) 并把 raw_angle 取整后传入，以确保显示为整数位且宽度计算（count_digits()）一致，修正由小数显示引起的布局错位。rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp-287-313 (1)
287-313:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Ctrl+E跟踪开关现在是电平触发，长按会在每帧反复翻转。
bottom_yaw_tracking_enabled_只被写入却没有参与可见性分支，tracking_pink_line_还被无条件设成可见。结果是：长按Ctrl+E会不停改写锚点并来回切换，松手后的最终状态取决于刷新次数；即使“关闭追踪”，粉线也仍然显示。这里需要改成上升沿触发，并在未启用时直接隐藏后返回。Also applies to: 521-522
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp` around lines 287 - 313, The Ctrl+E toggle in update_bottom_yaw_tracking_lines is level-triggered causing rapid flips and the pink line remains visible when tracking is off; change it to rising-edge detection by tracking the previous key state (e.g. add a prev_ctrl_e_pressed_ bool) and only flip bottom_yaw_tracking_enabled_ when ctrl_e_pressed is true and prev_ctrl_e_pressed_ is false, and when enabling set bottom_yaw_anchor_angle_rad_ = *bottom_yaw_angle_; when disabling immediately hide the line (tracking_pink_line_.set_visible(false)) and return early so the rest of the function doesn't update/force visibility; apply the same edge-trigger fix to the other occurrence noted (lines ~521-522) where the same variables keyboard_, bottom_yaw_tracking_enabled_, bottom_yaw_anchor_angle_rad_, bottom_yaw_angle_, and tracking_pink_line_ are used.rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp-75-86 (1)
75-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick win进入 ENCODER 时先锁定、但没有先采样锁定角。
这里用
top_yaw_angle_只做了可用性判断，却继续发布旧的top_yaw_encoder_angle_。如果上一拍缓存还是NaN（例如角度刚恢复、同一拍切到 ENCODER），上云台会被标记为已锁定，但top_yaw_control_angle_会持续输出NaN，直到再次切换模式。修复建议
if (entering_encoder) { if (std::isfinite(*top_yaw_angle_)) { + top_yaw_encoder_angle_ = *top_yaw_angle_; top_yaw_encoder_locked_ = true; } else { top_yaw_encoder_locked_ = false; + top_yaw_encoder_angle_ = nan_; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around lines 75 - 86, When switching into the encoder mode the code only checks std::isfinite(*top_yaw_angle_) and sets top_yaw_encoder_locked_ but never samples/stores the current encoder reading, so top_yaw_control_angle_ may remain NaN; update the branch that runs when entering_encoder to, when std::isfinite(*top_yaw_angle_) is true, assign top_yaw_encoder_angle_ = *top_yaw_angle_ (and set top_yaw_encoder_locked_ = true), otherwise set top_yaw_encoder_locked_ = false and clear top_yaw_encoder_angle_ appropriately, ensuring subsequent writes to *top_yaw_control_angle_ use the freshly sampled top_yaw_encoder_angle_ (and fallback to nan_ when locked is false).rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp-136-152 (1)
136-152:⚠️ Potential issue | 🟠 Major | ⚡ Quick winENCODER 模式下把自动瞄准静默关掉了。
update_encoder_control()完全没有处理auto_aim_control_direction_，所以一旦按E切到 ENCODER，右键/拨杆触发的自动瞄准就会退化成纯手动增量控制。当前 PR 已经给PreciseTwoAxisGimbalSolver加了SetControlDirection，但这里没有接上。修复建议
PreciseTwoAxisGimbalSolver::ControlAngle update_encoder_control() { + if (auto_aim_control_direction_.ready() + && (mouse_->right || *switch_right_ == rmcs_msgs::Switch::UP) + && !auto_aim_control_direction_->isZero()) { + return encoder_gimbal_solver.update( + PreciseTwoAxisGimbalSolver::SetControlDirection{ + OdomImu::DirectionVector{*auto_aim_control_direction_}}); + } + if (!encoder_gimbal_solver.enabled()) { return encoder_gimbal_solver.update( PreciseTwoAxisGimbalSolver::SetControlPitch{encoder_init_pitch_}); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp` around lines 136 - 152, update_encoder_control() ignores auto_aim_control_direction_ so ENCODER mode loses auto-aim; fix by applying the stored auto_aim_control_direction_ to the solver when enabled: if auto_aim_control_direction_ is set, call encoder_gimbal_solver.update(PreciseTwoAxisGimbalSolver::SetControlDirection{auto_aim_control_direction_}) (before or alongside the existing PreciseTwoAxisGimbalSolver::SetControlShift update) so both direction and manual shift are sent to the solver; use the existing symbols update_encoder_control, auto_aim_control_direction_, encoder_gimbal_solver, and PreciseTwoAxisGimbalSolver::SetControlDirection to locate and implement the change.rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_climber_controller.cpp-136-137 (1)
136-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
stop_manual_support()会把自动攀爬的回撤计时器一直重置。自动攀爬分支里每个周期都会先调用这里，而这里把
back_climber_recover_count重置成1500。结果是apply_climb_control()里的倒计时永远降不下去，SUPPORT_RETRACT的快/慢回撤切换和 8.0→0.5 的扭矩限幅都不会按预期生效。可直接收敛到根因的修正
void stop_manual_support() { - back_climber_recover_count = 1500; manual_support_retracting_ = false; manual_support_retract_block_count_ = 0; }Also applies to: 505-531, 555-559
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_climber_controller.cpp` around lines 136 - 137, 当前问题是 stop_manual_support() 在自动攀爬路径每周期被调用时把 back_climber_recover_count 重置为 1500，导致 update_auto_climb_control()/apply_climb_control() 的倒计时永远无法递减，从而 SUPPORT_RETRACT 的快/慢回撤切换与 8.0→0.5 扭矩限幅不生效；修复方法是取消或条件化该重置：修改 stop_manual_support() 或调用方（涉及 stop_manual_support(), update_auto_climb_control(), apply_climb_control(), 以及变量 back_climber_recover_count 和 SUPPORT_RETRACT）使得只有在真正进入/退出手动支撑模式时才将 back_climber_recover_count 置为初始值，或在自动攀爬分支前移除对 stop_manual_support() 的调用，从而允许倒计时在自动攀爬分支中正常递减。rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpp-42-42 (1)
42-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick win可选的
climbing_forward_velocity_没有做兜底绑定。这里把它注册成了可选输入，但
before_updating()没有像其他可选输入那样补默认值；下面两处又直接*climbing_forward_velocity_。如果爬坡控制器未加载，或者启动顺序比这里晚，这里会直接解引用未就绪输入。沿用本文件现有的可选输入处理方式即可
void before_updating() override { if (!gimbal_yaw_angle_.ready()) { gimbal_yaw_angle_.make_and_bind_directly(0.0); RCLCPP_WARN(get_logger(), "Failed to fetch \"/gimbal/yaw/angle\". Set to 0.0."); } if (!gimbal_yaw_angle_error_.ready()) { gimbal_yaw_angle_error_.make_and_bind_directly(0.0); RCLCPP_WARN( get_logger(), "Failed to fetch \"/gimbal/yaw/control_angle_error\". Set to 0.0."); } + if (!climbing_forward_velocity_.ready()) { + climbing_forward_velocity_.make_and_bind_directly(nan); + } chassis_velocity_feedback_ready_ = chassis_velocity_.ready(); if (!chassis_velocity_feedback_ready_) { chassis_velocity_.make_and_bind_directly(0.0, 0.0, 0.0); } }Also applies to: 51-65, 143-146, 166-178
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpp` at line 42, The optional input climbing_forward_velocity_ is registered but not given a fallback, so before_updating() must provide a safe default or binding and avoid unguarded dereferences; update before_updating() to check whether climbing_forward_velocity_ is bound and if not bind/set a sensible default (e.g., zero) or otherwise guard uses of *climbing_forward_velocity_, following the same pattern used for other optional inputs in this file (the register_input(...) optional registrations and their before_updating() fallbacks); apply the identical fix to the other optionally-registered inputs that are later directly dereferenced to prevent null/unready access.rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp-134-151 (1)
134-151:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLAUNCH_RAMP 在
supercap_voltage_ <= 12.0时会让chassis_control_power_limit_变成 NaN（至少驱动上限）。在update_control_power_limit()中，LAUNCH_RAMP分支把total_power_limit置为inf_，随后又做referee + (total_power_limit - referee) * clamp(scale)插值；当scale == 0（supercap_voltage_ <= 12.0）时会出现inf_ * 0，结果变为 NaN，进一步经split_control_power_limit()影响drive_limit = total_power_limit - front_limit，从而*chassis_control_power_limit_变为 NaN（front_limit通常仍为有限值）。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp` around lines 134 - 151, The LAUNCH_RAMP path can set total_power_limit to inf_ and the later interpolation with scale==0 produces inf * 0 => NaN; in update_control_power_limit() (variables total_power_limit, chassis_power_limit_expected_, supercap_voltage_) guard that case: compute the scale first, and if std::isinf(total_power_limit) and scale <= 0.0 set total_power_limit = *chassis_power_limit_referee_ (and chassis_power_limit_expected_ accordingly); otherwise proceed with the existing interpolation so that inf only propagates when scale>0 and you never perform inf*0.
🟡 Minor comments (1)
rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp-62-66 (1)
62-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCtrl+E 和 E 现在实际上没有区别。
kEInitViewerAngle和kCtrlInitViewerAngle都是0.38905，而这里的注释又说明当前限位区间是[0.68, 1.17]。结果就是按哪个快捷键都会在 Line 109 被clamp到同一个边界角，和“两个初始化角”这个新行为不一致。Also applies to: 99-100, 132-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp` around lines 62 - 66, The two init-angle constants kEInitViewerAngle and kCtrlInitViewerAngle are both set to 0.38905 (outside the current clamp range [0.68,1.17]), so Ctrl+E and E end up clamped to the same boundary and lose distinct behavior; update the constants (or the clamp range) so the two initialization angles are different and lie inside the allowed range, then verify uses in the keyboard handling code (the block using last_keyboard_, keyboard, viewer_init_angle_, viewer_reset_, scope_viewer_reset_) and the other mentioned spots (around lines referenced 99-100 and 132-139) to ensure each branch sets a distinct in-range value and the clamp no longer collapses them to the same boundary.
🧹 Nitpick comments (2)
rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp (1)
152-152: ⚡ Quick win建议把
sort(...)显式改为std::sort(...)（152/185/237）
velocities是std::vector<double>，其迭代器可通过 ADL 找到std::sort，因此未限定调用不一定会编译失败；但显式写std::sort更清晰，也避免潜在的同名/宏冲突。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp` at line 152, The call to sort(velocities.begin(), velocities.end()) in shooting_recorder.cpp should be made explicit to avoid ADL/macro conflicts: replace the unqualified sort calls (found in the shooting recorder code where velocities is a std::vector<double>) with std::sort(velocities.begin(), velocities.end()); do this for all occurrences noted (around the spots corresponding to the original lines 152, 185, 237) so the RMCSShootingRecorder (or relevant function/method) uses the fully-qualified std::sort.rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpp (1)
185-190: ⚡ Quick win把这段缩放逻辑参数化，或者把注释改成真实行为。
注释写的是“满速时 fully enabled”，但
std::clamp(..., 0.0, 0.3)实际把 AUTO 模式的跟随角速度永久封顶在 30%。如果这是刻意降响应，建议把0.3提成参数并修正文案；如果不是，这里上限应当是1.0。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpp` around lines 185 - 190, The scaling logic that caps AUTO rear-follow angular velocity is misleading: update the implementation so the clamp upper bound is parameterized (e.g., rear_follow_angular_scale_max) and used in the computation that modifies angular_velocity (referencing measured_translational_speed, translational_velocity_max, chassis_velocity_feedback_ready_, chassis_velocity_), or if the 0.3 cap was intentional change the comment to accurately state that AUTO is limited to 30% angular responsiveness; add the new parameter to the controller config/defaults and use it in the std::clamp call (or replace 0.3 with 1.0 if you intend no cap) and update the comment accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp`:
- Around line 69-90: On first update() the code may read uninitialized
virtual_buffer_energy_ and chassis_power_limit_expected_ (they are set in
update_control_power_limit()), and LAUNCH_RAMP logic can produce inf_ * 0 ->
NaN; fix by ensuring power state is initialized before any computation: call
reset_power_control() or initialize virtual_buffer_energy_ and
chassis_power_limit_expected_ before calling update_virtual_buffer_energy()
(e.g., move update_control_power_limit() earlier or perform an explicit
first-run init in update()), and guard the LAUNCH_RAMP branch in
update_control_power_limit() so operations with total_power_limit == inf_ never
multiply/divide by zero (e.g., detect isinf(total_power_limit) or clamp == 0 and
short-circuit the (inf - ref) * clamp computation to a safe value or fallback to
referee value). Ensure changes reference update(),
update_virtual_buffer_energy(), update_control_power_limit(),
virtual_buffer_energy_, chassis_power_limit_expected_, LAUNCH_RAMP, boost_mode_,
supercap_enabled_, total_power_limit, inf_, chassis_control_power_limit_, and
front_climber_control_power_limit_.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/hero_heat_controller.cpp`:
- Around line 18-19: The constructor initializes heat_per_shot and reserved_heat
but does not validate heat_per_shot, which is later used as a divisor in
limited_by_heat; add a parameter check in the constructor (after reading
heat_per_shot) to ensure heat_per_shot > 0 and throw a clear exception (e.g.,
std::invalid_argument or an rclcpp-appropriate exception) with a descriptive
message if it is not, so the object fails fast and avoids division-by-zero or
negative-heat logic in limited_by_heat calculation.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp`:
- Around line 216-243: analysis3() assumes velocities has >=3 elements and
unconditionally accesses velocities[1] and velocities[size-2], causing OOB when
update() runs on 1–2 samples; fix by early-guarding on velocities.size(): if
velocities.empty() set velocity_, excellence_rate_, pass_rate_,
velocity_max/min, range_, range2_ to sensible defaults (e.g., 0) and return; if
velocities.size() == 1 set min=max=that value and range_/range2_ to 0; if
velocities.size() == 2 compute min/max and range_ normally but set range2_ to 0
(only compute velocities[int(velocities.size()-2)] - velocities[1] when
velocities.size() >= 3); also ensure all divisions by velocities.size() are
guarded to avoid divide-by-zero when computing velocity_, excellence_rate_, and
pass_rate_.
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp`:
- Around line 128-132: top_board_ and bottom_board_ are declared as
std::shared_ptr but constructed with std::make_unique, causing a type mismatch;
change the construction to std::make_shared when assigning to top_board_ and
bottom_board_ (or alternatively change their declarations to std::unique_ptr) so
the pointer types match (look for top_board_, bottom_board_ initialization).
Separately, camera_capturer_trigger_ is set true only on timestamp change (the
assignment near 334-336) and never reset, causing a permanent high level; add
logic to clear camera_capturer_trigger_ after producing the output pulse (e.g.,
reset it after one cycle/after sending the trigger or on an explicit ack) or
implement edge-detect state so the trigger is true for only one iteration
(modify the code path that sets camera_capturer_trigger_ and the loop or
function that emits the hardware signal).
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp`:
- Around line 122-129: The member pointers top_board_, bottom_board_one_, and
bottom_board_two_ are std::shared_ptr but are being initialized with
std::make_unique, causing a type mismatch; replace std::make_unique with
std::make_shared for each initialization (creating shared_ptrs to TopBoard,
BottomBoard_one, and BottomBoard_two respectively) while keeping the same
constructor arguments (*this, *command_component_,
get_parameter("board_serial_...").as_string()) so the types match and
compilation succeeds.
---
Major comments:
In `@rmcs_ws/src/rmcs_bringup/config/steering-hero-little.yaml`:
- Around line 285-295: 配置中的两个参数实体
steering_wheel_status.ros__parameters.vehicle_radius 和
steering_wheel_controller.ros__parameters.vehicle_radius 值不一致（0.286378 vs
0.318198），请将它们统一为同一半径值（选择正确的实测/标定值），在文件中同时更新 steering_wheel_status 和
steering_wheel_controller 下的 vehicle_radius
为该统一值，然后在实车上重新做一次标定验证所有基于车体半径的估算和控制器参数（速度估计、功率估算、轮组力矩分配等）。
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/chassis_climber_controller.cpp`:
- Around line 136-137: 当前问题是 stop_manual_support() 在自动攀爬路径每周期被调用时把
back_climber_recover_count 重置为 1500，导致
update_auto_climb_control()/apply_climb_control() 的倒计时永远无法递减，从而 SUPPORT_RETRACT
的快/慢回撤切换与 8.0→0.5 扭矩限幅不生效；修复方法是取消或条件化该重置：修改 stop_manual_support() 或调用方（涉及
stop_manual_support(), update_auto_climb_control(), apply_climb_control(), 以及变量
back_climber_recover_count 和 SUPPORT_RETRACT）使得只有在真正进入/退出手动支撑模式时才将
back_climber_recover_count 置为初始值，或在自动攀爬分支前移除对 stop_manual_support()
的调用，从而允许倒计时在自动攀爬分支中正常递减。
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpp`:
- Line 42: The optional input climbing_forward_velocity_ is registered but not
given a fallback, so before_updating() must provide a safe default or binding
and avoid unguarded dereferences; update before_updating() to check whether
climbing_forward_velocity_ is bound and if not bind/set a sensible default
(e.g., zero) or otherwise guard uses of *climbing_forward_velocity_, following
the same pattern used for other optional inputs in this file (the
register_input(...) optional registrations and their before_updating()
fallbacks); apply the identical fix to the other optionally-registered inputs
that are later directly dereferenced to prevent null/unready access.
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp`:
- Around line 134-151: The LAUNCH_RAMP path can set total_power_limit to inf_
and the later interpolation with scale==0 produces inf * 0 => NaN; in
update_control_power_limit() (variables total_power_limit,
chassis_power_limit_expected_, supercap_voltage_) guard that case: compute the
scale first, and if std::isinf(total_power_limit) and scale <= 0.0 set
total_power_limit = *chassis_power_limit_referee_ (and
chassis_power_limit_expected_ accordingly); otherwise proceed with the existing
interpolation so that inf only propagates when scale>0 and you never perform
inf*0.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 75-86: When switching into the encoder mode the code only checks
std::isfinite(*top_yaw_angle_) and sets top_yaw_encoder_locked_ but never
samples/stores the current encoder reading, so top_yaw_control_angle_ may remain
NaN; update the branch that runs when entering_encoder to, when
std::isfinite(*top_yaw_angle_) is true, assign top_yaw_encoder_angle_ =
*top_yaw_angle_ (and set top_yaw_encoder_locked_ = true), otherwise set
top_yaw_encoder_locked_ = false and clear top_yaw_encoder_angle_ appropriately,
ensuring subsequent writes to *top_yaw_control_angle_ use the freshly sampled
top_yaw_encoder_angle_ (and fallback to nan_ when locked is false).
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp`:
- Around line 136-152: update_encoder_control() ignores
auto_aim_control_direction_ so ENCODER mode loses auto-aim; fix by applying the
stored auto_aim_control_direction_ to the solver when enabled: if
auto_aim_control_direction_ is set, call
encoder_gimbal_solver.update(PreciseTwoAxisGimbalSolver::SetControlDirection{auto_aim_control_direction_})
(before or alongside the existing PreciseTwoAxisGimbalSolver::SetControlShift
update) so both direction and manual shift are sent to the solver; use the
existing symbols update_encoder_control, auto_aim_control_direction_,
encoder_gimbal_solver, and PreciseTwoAxisGimbalSolver::SetControlDirection to
locate and implement the change.
In
`@rmcs_ws/src/rmcs_core/src/controller/shooting/hero_friction_wheel_controller.cpp`:
- Around line 128-140: The fault counter friction_faulty_count_ is never cleared
and should be reset when controls are reinitialized; update reset_all_controls()
to set friction_faulty_count_ = 0 and also ensure any other control-resetting
routine (the similar block around the code at lines 168-180) resets
friction_faulty_count_ as well so faults do not persist across a healthy
recovery or re-enable cycle; locate the symbols reset_all_controls() and
friction_faulty_count_ and add the counter reset in those reset paths.
- Around line 196-213: The code unconditionally dereferences
friction_velocities_[2], which can out-of-bounds if friction_count_ < 3; either
add a guard checking friction_count_ (or friction_velocities_.size()) >= 3
before any access/use of friction_velocities_[2] and skip the whole block if
not, or replace the hard-coded index with an explicit configured "first-stage"
index (e.g., a member like primary_stage_index_) and validate that index is
within range before using it; update uses of last_primary_friction_velocity_,
primary_friction_velocity_decrease_integral_, target_friction_velocity(), and
fired to reference the validated index instead of the literal 2.
- Around line 71-95: The profile toggle for plain F and Ctrl+F is currently
executed in two separate if blocks causing double-toggle when both keys
transition in the same frame; make the edge checks mutually exclusive so only
one branch can flip active_profile_. Update the keyboard edge logic in
hero_friction_wheel_controller.cpp (references: last_keyboard_, keyboard,
active_profile_, friction_soft_start_stop_percentage_, friction_count_,
friction_velocities_, target_friction_velocity()) so that the Ctrl+F branch and
the plain F branch are exclusive (e.g., check the Ctrl+F condition first and use
else/else-if for the plain F case, or require plain F to have !keyboard.ctrl),
preserving the existing friction_soft_start_stop_percentage_ computation in the
taken branch.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp`:
- Around line 255-259: When changing shoot_stage_ you must reset
putter_timeout_count_ to avoid inheriting stale timeout counts; modify
set_preloading(), set_preloaded(), and set_shooting() (and the analogous
stage-switching methods in the block around the other occurrence at the 307-320
region) to set putter_timeout_count_ = 0 (or call a single reset helper)
whenever the stage changes to PRELOADING, PRELOADED, or SHOOTING so each new
cycle starts with a cleared timeout counter.
- Around line 271-279: The branch handling locked_detect_count_ > 300 must not
call enter_jam_protection() when photoelectric_sensor_status_ is true and
set_preloaded() was invoked; modify the logic in the block around
locked_detect_count_ to make enter_jam_protection() (and the
locked_detect_count_ = 0 reset that precedes it) conditional on
!(*photoelectric_sensor_status_)—i.e., if sensor true call set_preloaded() and
skip jam protection, otherwise treat as a jam: reset locked_detect_count_ and
call enter_jam_protection().
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp`:
- Around line 60-65: velocities is being used as a time-ordered circular window
(push_back + erase(begin())) but is also sorted in-place inside analysis3(), so
once the window exceeds max_velocities_size the erased element is the smallest
value not the oldest; fix by never sorting velocities in-place: either make a
local copy (e.g., std::vector<double> sorted = velocities) and sort that inside
analysis3() (or sort the copy before analysis), or move sorting logic into
analysis3() so it works on a copied container; preserve the
push_back(*initial_speed_) and erase(velocities.begin()) behavior to remove the
oldest sample. Apply the same change to the other analysis calls that currently
sort the velocities container in-place so all statistical analyses operate on a
sorted copy rather than mutating velocities.
- Around line 70-78: The current code only assigns log_text when
friction_wheel_count_ == 4 but always writes log_stream_ << log_text <<
std::endl, causing empty log lines for the default 2-wheel case; fix by either
adding the missing 2-wheel logging branch that constructs log_text (mirroring
the 4-wheel format) or guard the write with the same condition (only write to
log_stream_ when friction_wheel_count_ == 4 or when log_text is non-empty);
update the logic around friction_wheel_count_, log_text, and the final
log_stream_ write so no empty lines are emitted.
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp`:
- Around line 334-336: The current logic only sets *camera_capturer_trigger_ =
true when last_camera_capturer_trigger_timestamp_ differs, but never clears it,
causing a latched high trigger; change the update so that after comparing
last_camera_capturer_trigger_timestamp_ with *camera_capturer_trigger_timestamp_
you set *camera_capturer_trigger_ to true for that single cycle and explicitly
set it to false otherwise, and then update
last_camera_capturer_trigger_timestamp_; ensure you reference and modify
last_camera_capturer_trigger_timestamp_, camera_capturer_trigger_timestamp_, and
camera_capturer_trigger_ so the downstream receives a one-cycle pulse instead of
a permanent high.
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp`:
- Around line 345-347: 当前实现只在时间戳变化时把 *camera_capturer_trigger_ 置为 true 且从不回写
false，导致触发信号永久为高；修改在检测到时间戳变化时写 true，否则明确写 false，保证每次边沿只输出单周期脉冲；在代码使用的标识符包括
last_camera_capturer_trigger_timestamp_, camera_capturer_trigger_timestamp_,
camera_capturer_trigger_，请在相同代码块中将分支改为 if
(last_camera_capturer_trigger_timestamp_ != *camera_capturer_trigger_timestamp_)
{ *camera_capturer_trigger_ = true; } else { *camera_capturer_trigger_ = false;
} 并随后更新 last_camera_capturer_trigger_timestamp_ =
*camera_capturer_trigger_timestamp_;，同时保持对空指针的检查（如果有）一致。
In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp`:
- Around line 39-43: 这些角度控件（yaw_angle_number_, pitch_angle_number_,
bottom_yaw_angle_number_ 以及文件中另两处相同模式的实例）当前使用了 Float 路径导致通过
Float::set_value(double) 按 value*1000 编码并显示三位小数；请改为使用整数控件或整数接口：在创建这些控件时用对应的
Integer 类型（或 Shape::Integer 构造器）替代 Float/Float-based 构造，或者在赋值时调用
Integer::set_value(int) 并把 raw_angle
取整后传入，以确保显示为整数位且宽度计算（count_digits()）一致，修正由小数显示引起的布局错位。
- Around line 287-313: The Ctrl+E toggle in update_bottom_yaw_tracking_lines is
level-triggered causing rapid flips and the pink line remains visible when
tracking is off; change it to rising-edge detection by tracking the previous key
state (e.g. add a prev_ctrl_e_pressed_ bool) and only flip
bottom_yaw_tracking_enabled_ when ctrl_e_pressed is true and
prev_ctrl_e_pressed_ is false, and when enabling set
bottom_yaw_anchor_angle_rad_ = *bottom_yaw_angle_; when disabling immediately
hide the line (tracking_pink_line_.set_visible(false)) and return early so the
rest of the function doesn't update/force visibility; apply the same
edge-trigger fix to the other occurrence noted (lines ~521-522) where the same
variables keyboard_, bottom_yaw_tracking_enabled_, bottom_yaw_anchor_angle_rad_,
bottom_yaw_angle_, and tracking_pink_line_ are used.
---
Minor comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp`:
- Around line 62-66: The two init-angle constants kEInitViewerAngle and
kCtrlInitViewerAngle are both set to 0.38905 (outside the current clamp range
[0.68,1.17]), so Ctrl+E and E end up clamped to the same boundary and lose
distinct behavior; update the constants (or the clamp range) so the two
initialization angles are different and lie inside the allowed range, then
verify uses in the keyboard handling code (the block using last_keyboard_,
keyboard, viewer_init_angle_, viewer_reset_, scope_viewer_reset_) and the other
mentioned spots (around lines referenced 99-100 and 132-139) to ensure each
branch sets a distinct in-range value and the clamp no longer collapses them to
the same boundary.
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpp`:
- Around line 185-190: The scaling logic that caps AUTO rear-follow angular
velocity is misleading: update the implementation so the clamp upper bound is
parameterized (e.g., rear_follow_angular_scale_max) and used in the computation
that modifies angular_velocity (referencing measured_translational_speed,
translational_velocity_max, chassis_velocity_feedback_ready_,
chassis_velocity_), or if the 0.3 cap was intentional change the comment to
accurately state that AUTO is limited to 30% angular responsiveness; add the new
parameter to the controller config/defaults and use it in the std::clamp call
(or replace 0.3 with 1.0 if you intend no cap) and update the comment
accordingly.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp`:
- Line 152: The call to sort(velocities.begin(), velocities.end()) in
shooting_recorder.cpp should be made explicit to avoid ADL/macro conflicts:
replace the unqualified sort calls (found in the shooting recorder code where
velocities is a std::vector<double>) with std::sort(velocities.begin(),
velocities.end()); do this for all occurrences noted (around the spots
corresponding to the original lines 152, 185, 237) so the RMCSShootingRecorder
(or relevant function/method) uses the fully-qualified std::sort.
🪄 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: e34e9171-3dd7-42f9-bb84-97b0c41d0051
📒 Files selected for processing (28)
rmcs_ws/src/rmcs_bringup/config/mecanum-hero.yamlrmcs_ws/src/rmcs_bringup/config/omni-infantry.yamlrmcs_ws/src/rmcs_bringup/config/sentry.yamlrmcs_ws/src/rmcs_bringup/config/steering-hero-little.yamlrmcs_ws/src/rmcs_bringup/config/steering-hero.yamlrmcs_ws/src/rmcs_core/CMakeLists.txtrmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/controller/chassis/chassis_climber_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpprmcs_ws/src/rmcs_core/src/controller/chassis/hero_steering_wheel_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/player_viewer.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/precise_two_axis_gimbal_solver.hpprmcs_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/hero_friction_wheel_controller.cpprmcs_ws/src/rmcs_core/src/controller/shooting/hero_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/device/lk_motor.hpprmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpprmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpprmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpprmcs_ws/src/rmcs_core/src/referee/app/ui/shape/shape.hpprmcs_ws/src/rmcs_msgs/include/rmcs_msgs/shoot_condiction.hpp
💤 Files with no reviewable changes (1)
- rmcs_ws/src/rmcs_bringup/config/mecanum-hero.yaml
Comment on lines
+69
to
+90
| void update() override { | ||
| update_charging_power_limit(); | ||
|
|
||
| update_ui(); | ||
|
|
||
| using namespace rmcs_msgs; | ||
|
|
||
| auto switch_right = *switch_right_; | ||
| auto switch_left = *switch_left_; | ||
| auto keyboard = *keyboard_; | ||
| auto rotary_knob = *rotary_knob_; | ||
|
|
||
| if ((switch_left == Switch::UNKNOWN || switch_right == Switch::UNKNOWN) | ||
| || (switch_left == Switch::DOWN && switch_right == Switch::DOWN)) { | ||
| reset_power_control(); | ||
| return; | ||
| } | ||
|
|
||
| update_virtual_buffer_energy(); | ||
|
|
||
| boost_mode_ = keyboard.shift || rotary_knob < -0.9 || *auto_climb_active_; | ||
| update_control_power_limit(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp"
# 1) Show member declarations around likely locations
# We'll locate the variable names first to find exact declaration lines.
rg -n "virtual_buffer_energy_|chassis_power_limit_expected_|update_virtual_buffer_energy|update_control_power_limit|inf_" "$FILE" || true
# 2) Print around member declarations for the two variables
# Use line-number ranges extracted via rg matches by grepping just nearby context.
python3 - <<'PY'
import re, subprocess, os, textwrap, sys, json, pathlib
file_path="rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp"
vars=["virtual_buffer_energy_","chassis_power_limit_expected_","inf_"]
# get line numbers for declarations/usages
out=subprocess.check_output(["rg","-n",r"|".join(map(re.escape,vars)),file_path],text=True)
lines=[]
for line in out.strip().splitlines():
m=re.match(r"(\d+):(.*)", line)
if m:
lines.append(int(m.group(1)))
# pick unique sorted
uniq=sorted(set(lines))
# print snippets around each
for ln in uniq:
start=max(1,ln-20); end=ln+20
print("\n==== snippet around line",ln,"====")
# use sed -n
subprocess.run(["bash","-lc", f"sed -n '{start},{end}p' {file_path} | nl -ba -w2 -s':' | sed -n '1,45p'"], check=False)
PY
# 3) Print update() and the relevant called functions with line context
rg -n "void update\\(\\) override|update_virtual_buffer_energy\\(|update_control_power_limit\\(" "$FILE"
# Output smaller snippets
python3 - <<'PY'
import re, subprocess, sys
file_path="rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp"
def show(pattern, before=25, after=120):
out=subprocess.check_output(["rg","-n",pattern,file_path],text=True).strip().splitlines()
if not out:
print("No match for",pattern); return
for l in out:
ln=int(l.split(":")[0])
start=max(1,ln-before); end=ln+after
print("\n==== "+pattern+" at line "+str(ln)+" ====")
subprocess.run(["bash","-lc", f"sed -n '{start},{end}p' {file_path} | nl -ba -w2 -s':' | head -n 200"], check=False)
show(r"void update\\(\\) override", before=40, after=120)
show(r"update_virtual_buffer_energy\\(", before=40, after=160)
show(r"update_control_power_limit\\(", before=40, after=200)
PYRepository: Alliance-Algorithm/RMCS
Length of output: 3912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp"
# Show update() block fully
sed -n '50,110p' "$FILE" | cat -n
# Show update_virtual_buffer_energy()
sed -n '110,170p' "$FILE" | cat -n
# Show update_control_power_limit() and surrounding inf_
sed -n '170,240p' "$FILE" | cat -n
# Find all assignments to the two members and show nearby context
echo "---- assignments to virtual_buffer_energy_ ----"
rg -n "virtual_buffer_energy_\s*=" "$FILE" || true
echo "---- assignments to chassis_power_limit_expected_ ----"
rg -n "chassis_power_limit_expected_\s*=" "$FILE" || true
# Find constructors / member init lists (common patterns)
echo "---- constructors ----"
rg -n "hero_chassis_power_controller|PowerController|::hero_chassis_power_controller|\\bhero_chassis_power_controller\\b" "$FILE" || trueRepository: Alliance-Algorithm/RMCS
Length of output: 9356
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp"
# Constructor + member init area
sed -n '1,80p' "$FILE" | cat -n
# All calls to reset_power_control()
rg -n "reset_power_control\\(" "$FILE"
# Full class for virtual_buffer_energy_ / chassis_power_limit_expected_ assignments only
rg -n "virtual_buffer_energy_\\s*=|chassis_power_limit_expected_\\s*=" "$FILE"Repository: Alliance-Algorithm/RMCS
Length of output: 4194
关键：首次 update() 可能读到未初始化功率状态，且 LAUNCH_RAMP 存在 NaN 传播风险
- 当
update()未走reset_power_control()分支时，会先调用update_virtual_buffer_energy()；该函数在计算前会使用未初始化的virtual_buffer_energy_与chassis_power_limit_expected_（chassis_power_limit_expected_的赋值发生在随后update_control_power_limit()中），导致首次有效周期出现未定义行为，进而污染chassis_control_power_limit_/front_climber_control_power_limit_。 update_control_power_limit()在mode_ == LAUNCH_RAMP且boost_mode_ && *supercap_enabled_时令total_power_limit = inf_，随后用std::clamp(...)作为系数参与(*chassis_power_limit_referee_) + (inf_ - ref) * clamp计算；当clamp == 0时可能触发inf_ * 0 -> NaN，NaN 再继续在缓冲能量缩放与输出分配中传播。
至少先把启动状态显式初始化
- double virtual_buffer_energy_;
+ double virtual_buffer_energy_ = virtual_buffer_energy_limit_;
@@
- double chassis_power_limit_expected_;
+ double chassis_power_limit_expected_ = 0.0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@rmcs_ws/src/rmcs_core/src/controller/chassis/hero_chassis_power_controller.cpp`
around lines 69 - 90, On first update() the code may read uninitialized
virtual_buffer_energy_ and chassis_power_limit_expected_ (they are set in
update_control_power_limit()), and LAUNCH_RAMP logic can produce inf_ * 0 ->
NaN; fix by ensuring power state is initialized before any computation: call
reset_power_control() or initialize virtual_buffer_energy_ and
chassis_power_limit_expected_ before calling update_virtual_buffer_energy()
(e.g., move update_control_power_limit() earlier or perform an explicit
first-run init in update()), and guard the LAUNCH_RAMP branch in
update_control_power_limit() so operations with total_power_limit == inf_ never
multiply/divide by zero (e.g., detect isinf(total_power_limit) or clamp == 0 and
short-circuit the (inf - ref) * clamp computation to a safe value or fallback to
referee value). Ensure changes reference update(),
update_virtual_buffer_energy(), update_control_power_limit(),
virtual_buffer_energy_, chassis_power_limit_expected_, LAUNCH_RAMP, boost_mode_,
supercap_enabled_, total_power_limit, inf_, chassis_control_power_limit_, and
front_climber_control_power_limit_.
Comment on lines
+18
to
+19
| , heat_per_shot(get_parameter("heat_per_shot").as_int()) | ||
| , reserved_heat(get_parameter("reserved_heat").as_int()) { |
There was a problem hiding this comment.
为 heat_per_shot 增加正数校验。
limited_by_heat 的计算在 Line 38 直接除以 heat_per_shot。如果配置成 0 或负数，这里会直接崩掉，或者产生反向额度。构造函数里需要尽早拒绝非法参数。
💡 建议修正
HeroHeatController()
: Node(
get_component_name(),
rclcpp::NodeOptions{}.automatically_declare_parameters_from_overrides(true))
, heat_per_shot(get_parameter("heat_per_shot").as_int())
, reserved_heat(get_parameter("reserved_heat").as_int()) {
+ if (heat_per_shot <= 0)
+ throw std::runtime_error("\"heat_per_shot\" must be positive");
+ if (reserved_heat < 0)
+ throw std::runtime_error("\"reserved_heat\" must be non-negative");
register_input("/referee/shooter/cooling", shooter_cooling_);Also applies to: 37-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/hero_heat_controller.cpp`
around lines 18 - 19, The constructor initializes heat_per_shot and
reserved_heat but does not validate heat_per_shot, which is later used as a
divisor in limited_by_heat; add a parameter check in the constructor (after
reading heat_per_shot) to ensure heat_per_shot > 0 and throw a clear exception
(e.g., std::invalid_argument or an rclcpp-appropriate exception) with a
descriptive message if it is not, so the object fails fast and avoids
division-by-zero or negative-heat logic in limited_by_heat calculation.
Comment on lines
+216
to
+243
| void analysis3() { | ||
| double sum = 0.0; | ||
| for (const auto& v : velocities) { | ||
| sum += v; | ||
| } | ||
| velocity_ = sum / double(velocities.size()); | ||
|
|
||
| int excellence_count = 0; | ||
| int pass_count = 0; | ||
|
|
||
| for (const auto& v : velocities) { | ||
| if (v >= aim_velocity - 0.05 && v <= aim_velocity + 0.05) { | ||
| excellence_count += 1; | ||
| } | ||
| if (v >= aim_velocity - 0.1 && v <= aim_velocity + 0.1) { | ||
| pass_count += 1; | ||
| } | ||
| } | ||
| excellence_rate_ = double(excellence_count) / double(velocities.size()); | ||
| pass_rate_ = double(pass_count) / double(velocities.size()); | ||
|
|
||
| sort(velocities.begin(), velocities.end()); | ||
| velocity_max = velocities.back(); | ||
| velocity_min = velocities.front(); | ||
|
|
||
| range_ = velocities.back() - velocities.front(); | ||
| range2_ = velocities[int(velocities.size() - 2)] - velocities[1]; | ||
| } |
There was a problem hiding this comment.
先处理小样本，再访问 range2_。
update() 第一次收到数据就会调用 analysis3()，但这里无条件读 velocities[size-2] 和 velocities[1]。样本数是 1 或 2 时都会越界。
💡 建议修正
void analysis3() {
+ if (velocities.empty())
+ return;
+
double sum = 0.0;
for (const auto& v : velocities) {
sum += v;
}
velocity_ = sum / double(velocities.size());
@@
- range_ = velocities.back() - velocities.front();
- range2_ = velocities[int(velocities.size() - 2)] - velocities[1];
+ range_ = velocities.back() - velocities.front();
+ if (velocities.size() >= 3)
+ range2_ = velocities[velocities.size() - 2] - velocities[1];
+ else
+ range2_ = range_;
}📝 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
| void analysis3() { | |
| double sum = 0.0; | |
| for (const auto& v : velocities) { | |
| sum += v; | |
| } | |
| velocity_ = sum / double(velocities.size()); | |
| int excellence_count = 0; | |
| int pass_count = 0; | |
| for (const auto& v : velocities) { | |
| if (v >= aim_velocity - 0.05 && v <= aim_velocity + 0.05) { | |
| excellence_count += 1; | |
| } | |
| if (v >= aim_velocity - 0.1 && v <= aim_velocity + 0.1) { | |
| pass_count += 1; | |
| } | |
| } | |
| excellence_rate_ = double(excellence_count) / double(velocities.size()); | |
| pass_rate_ = double(pass_count) / double(velocities.size()); | |
| sort(velocities.begin(), velocities.end()); | |
| velocity_max = velocities.back(); | |
| velocity_min = velocities.front(); | |
| range_ = velocities.back() - velocities.front(); | |
| range2_ = velocities[int(velocities.size() - 2)] - velocities[1]; | |
| } | |
| void analysis3() { | |
| if (velocities.empty()) | |
| return; | |
| double sum = 0.0; | |
| for (const auto& v : velocities) { | |
| sum += v; | |
| } | |
| velocity_ = sum / double(velocities.size()); | |
| int excellence_count = 0; | |
| int pass_count = 0; | |
| for (const auto& v : velocities) { | |
| if (v >= aim_velocity - 0.05 && v <= aim_velocity + 0.05) { | |
| excellence_count += 1; | |
| } | |
| if (v >= aim_velocity - 0.1 && v <= aim_velocity + 0.1) { | |
| pass_count += 1; | |
| } | |
| } | |
| excellence_rate_ = double(excellence_count) / double(velocities.size()); | |
| pass_rate_ = double(pass_count) / double(velocities.size()); | |
| sort(velocities.begin(), velocities.end()); | |
| velocity_max = velocities.back(); | |
| velocity_min = velocities.front(); | |
| range_ = velocities.back() - velocities.front(); | |
| if (velocities.size() >= 3) | |
| range2_ = velocities[velocities.size() - 2] - velocities[1]; | |
| else | |
| range2_ = range_; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/shooting_recorder.cpp` around
lines 216 - 243, analysis3() assumes velocities has >=3 elements and
unconditionally accesses velocities[1] and velocities[size-2], causing OOB when
update() runs on 1–2 samples; fix by early-guarding on velocities.size(): if
velocities.empty() set velocity_, excellence_rate_, pass_rate_,
velocity_max/min, range_, range2_ to sensible defaults (e.g., 0) and return; if
velocities.size() == 1 set min=max=that value and range_/range2_ to 0; if
velocities.size() == 2 compute min/max and range_ normally but set range2_ to 0
(only compute velocities[int(velocities.size()-2)] - velocities[1] when
velocities.size() >= 3); also ensure all divisions by velocities.size() are
guarded to avoid divide-by-zero when computing velocity_, excellence_rate_, and
pass_rate_.
Comment on lines
+128
to
+132
| top_board_ = std::make_unique<TopBoard>( | ||
| *this, *command_component_, get_parameter("board_serial_top_board").as_string()); | ||
|
|
||
| bottom_board_ = std::make_unique<BottomBoard>( | ||
| *this, *command_component_, get_parameter("serial_bottom_rmcs_board").as_string()); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the class members + constructor assignments in the target file
FILE="rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp"
echo "== Member declarations (top_board_ / bottom_board_) =="
rg -n "top_board_|bottom_board_" "$FILE" || true
echo
echo "== Constructor assignment snippet =="
sed -n '110,160p' "$FILE" || true
echo
echo "== Pointer type declarations around members =="
# Show a bit more context around the first occurrence
FIRST_LINE=$(rg -n -m1 "top_board_|bottom_board_" "$FILE" | cut -d: -f1 || true)
if [[ -n "${FIRST_LINE:-}" ]]; then
START=$((FIRST_LINE-30))
END=$((FIRST_LINE+30))
sed -n "${START},${END}p" "$FILE"
fi
echo
echo "== Search for any make_unique/make_shared usage for these members =="
rg -n "make_unique<|make_shared<" "$FILE" || trueRepository: Alliance-Algorithm/RMCS
Length of output: 6021
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether the "latched camera trigger permanent-high bug" exists in little vs full-size
LITTLE="rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp"
FULL="rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp"
echo "== Locate TopBoard::update in little =="
rg -n "class TopBoard|TopBoard::update\\(|\\bupdate\\(\\)\\s*\\{|camera|trigger|latched" "$LITTLE" || true
echo
echo "== Snippet around TopBoard::update in little (first match) =="
LINE=$(rg -n -m1 "TopBoard::update\\(|class TopBoard" "$LITTLE" | head -n1 | cut -d: -f1 || true)
if [[ -n "${LINE:-}" ]]; then
START=$((LINE-80)); END=$((LINE+160))
sed -n "${START},${END}p" "$LITTLE"
fi
echo
echo "== Locate trigger/latch pattern in full-size steering-hero.cpp =="
rg -n "camera|trigger|latched|permanent|high" "$FULL" || true
echo
echo "== Snippet around likely camera/trigger code in full-size (context) =="
rg -n -C 3 "camera|trigger|latched|permanent|high" "$FULL" | head -n 200 || trueRepository: Alliance-Algorithm/RMCS
Length of output: 15886
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp"
echo "== Occurrences of camera_capturer_trigger_ in little =="
rg -n "camera_capturer_trigger_" "$FILE" || true
echo
echo "== Assignments to camera_capturer_trigger_ (rough) =="
rg -n "camera_capturer_trigger_\s*=" "$FILE" || true
echo
echo "== Assignments to false (if any) =="
rg -n "camera_capturer_trigger_.*false|false.*camera_capturer_trigger_" "$FILE" || trueRepository: Alliance-Algorithm/RMCS
Length of output: 907
steering-hero-little：修复 shared_ptr 与 make_unique 不匹配，并纠正相机触发“永久高电平”逻辑
- 编译错误（成员类型不匹配）：
top_board_/bottom_board_声明为std::shared_ptr（857-858），但构造处却用std::make_unique(...)赋值（128-132），无法通过编译。应改为std::make_shared(...)，或将成员类型改回std::unique_ptr。
可直接落地的修正
- top_board_ = std::make_unique<TopBoard>(
+ top_board_ = std::make_shared<TopBoard>(
*this, *command_component_, get_parameter("board_serial_top_board").as_string());
- bottom_board_ = std::make_unique<BottomBoard>(
+ bottom_board_ = std::make_shared<BottomBoard>(
*this, *command_component_, get_parameter("serial_bottom_rmcs_board").as_string());- 逻辑问题（置位后不复位）：
camera_capturer_trigger_只在时间戳变化时被置为true（334-336），在该文件中未见任何将其恢复为false的赋值，可能导致触发一直为高电平。需要在合适时机复位（例如输出一拍后清零/基于边沿触发状态机/明确ack后清零）。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp` around lines 128
- 132, top_board_ and bottom_board_ are declared as std::shared_ptr but
constructed with std::make_unique, causing a type mismatch; change the
construction to std::make_shared when assigning to top_board_ and bottom_board_
(or alternatively change their declarations to std::unique_ptr) so the pointer
types match (look for top_board_, bottom_board_ initialization). Separately,
camera_capturer_trigger_ is set true only on timestamp change (the assignment
near 334-336) and never reset, causing a permanent high level; add logic to
clear camera_capturer_trigger_ after producing the output pulse (e.g., reset it
after one cycle/after sending the trigger or on an explicit ack) or implement
edge-detect state so the trigger is true for only one iteration (modify the code
path that sets camera_capturer_trigger_ and the loop or function that emits the
hardware signal).
Comment on lines
122
to
+129
| top_board_ = std::make_unique<TopBoard>( | ||
| *this, *command_component_, get_parameter("board_serial_top_board").as_string()); | ||
| bottom_board_ = std::make_unique<BottomBoard>( | ||
| *this, *command_component_, get_parameter("board_serial_bottom_board").as_string()); | ||
|
|
||
| temperature_logging_timer_.reset(1000); | ||
| bottom_board_one_ = std::make_unique<BottomBoard_one>( | ||
| *this, *command_component_, get_parameter("board_serial_bottom_board_one").as_string()); | ||
|
|
||
| bottom_board_two_ = std::make_unique<BottomBoard_two>( | ||
| *this, *command_component_, get_parameter("board_serial_bottom_board_two").as_string()); |
There was a problem hiding this comment.
shared_ptr 成员却用 make_unique 初始化会导致编译失败。
rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp 中 top_board_/bottom_board_one_/bottom_board_two_ 为 std::shared_ptr，但在 122-129 行用 std::make_unique 赋值，类型不匹配。
可直接落地的修正
- top_board_ = std::make_unique<TopBoard>(
+ top_board_ = std::make_shared<TopBoard>(
*this, *command_component_, get_parameter("board_serial_top_board").as_string());
- bottom_board_one_ = std::make_unique<BottomBoard_one>(
+ bottom_board_one_ = std::make_shared<BottomBoard_one>(
*this, *command_component_, get_parameter("board_serial_bottom_board_one").as_string());
- bottom_board_two_ = std::make_unique<BottomBoard_two>(
+ bottom_board_two_ = std::make_shared<BottomBoard_two>(
*this, *command_component_, get_parameter("board_serial_bottom_board_two").as_string());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/hardware/steering-hero.cpp` around lines 122 - 129,
The member pointers top_board_, bottom_board_one_, and bottom_board_two_ are
std::shared_ptr but are being initialized with std::make_unique, causing a type
mismatch; replace std::make_unique with std::make_shared for each initialization
(creating shared_ptrs to TopBoard, BottomBoard_one, and BottomBoard_two
respectively) while keeping the same constructor arguments (*this,
*command_component_, get_parameter("board_serial_...").as_string()) so the types
match and compilation succeeds.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp (2)
113-116:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win这里的边界判断恒为真。
在正常的
lower_limit_ < upper_limit_配置下，x >= lower_limit_ || x <= upper_limit_对任意x都成立，所以/gimbal/player_viewer/delta_angle每帧都会被清零。若这里想判断“在区间内”或“在区间外”，条件需要改成对应的&&或取反。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp` around lines 113 - 116, 条件判断使用了逻辑或导致对任意角度均为真，使得 *viewer_delta_angle_by_mouse_wheel_ 每帧被清零；在包含符号 viewer_control_angle_, lower_limit_, upper_limit_, viewer_delta_angle_by_mouse_wheel_ 的代码中，将当前的 "norm_angle(*viewer_control_angle_) >= lower_limit_ || norm_angle(*viewer_control_angle_) <= upper_limit_" 改为 "&&" 如果你的意图是判断角度在区间内后清零，或者将整个表达式取反（加上 !(...)）如果你的意图是判断角度在区间外后清零；只需替换逻辑运算符以反映正确的“在区间内/在区间外”语义。
98-109:⚠️ Potential issue | 🟠 Major | ⚡ Quick win两个 reset 预置角都会被 clamp 成同一个下限。
Line 132 已写明当前可用范围是
[0.68, 1.17]，但kEInitViewerAngle = -0.065769、kCtrlInitViewerAngle = 0.38905都落在这个区间外。经过 Line 109 的 clamp 后，这两个分支都会收敛到lower_limit_，于是E和Ctrl+E不再有不同的复位位置。要么把常量改到同一角度域里，要么先归一化再 clamp。Also applies to: 132-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp` around lines 98 - 109, The reset branches (viewer_reset_, scope_viewer_reset_) assign angles (viewer_init_angle_, *scope_offset_angle_) that lie outside the allowed domain and then get forced to lower_limit_ by the final clamp; fix by normalizing/wrapping the angle into the same angular domain as lower_limit_/upper_limit_ before applying std::clamp in the block that updates *viewer_control_angle_ (the branch handling viewer_reset_, scope_viewer_reset_, and the else that adds *viewer_delta_angle_by_mouse_wheel_). Add or call a helper to normalize angles (e.g., wrap to [lower_limit_, upper_limit_) or to a consistent 0..2π range) for values like viewer_init_angle_ and kEInitViewerAngle/kCtrlInitViewerAngle (or alternatively change those constants to fall inside lower_limit_/upper_limit_); then apply std::clamp to the normalized value so E and Ctrl+E resets don’t both collapse to lower_limit_.rmcs_ws/src/rmcs_core/src/controller/shooting/heat_controller.cpp (1)
24-25:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
shooting_heat_被写入了，但从未注册输出。构造函数里只注册了
/gimbal/control_bullet_allowance/limited_by_heat，Line 45 却开始每帧解引用shooting_heat_。这会让新输出保持未绑定状态，轻则数据发不出去，重则在运行时直接踩空。请先把它注册到正确的 topic（并给默认值），或者在真正接线前移除这次写入。Also applies to: 45-45, 62-62
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/shooting/heat_controller.cpp` around lines 24 - 25, 构造函数里已经调用了 register_output(...) 为 "/gimbal/control_bullet_allowance/limited_by_heat" 注册了 control_bullet_allowance_，但成员 shooting_heat_ 每帧被写入（在更新/循环代码中，见对 shooting_heat_ 的解引用/赋值），却没有通过 register_output 注册到任何 topic，导致输出未绑定或运行时崩溃；请在构造函数中为 shooting_heat_ 调用 register_output 并提供合理的默认值（例如 register_output("/gimbal/shooting_heat", shooting_heat_, 0.0) 或类似），以确保输出已注册并有初始值，或者如果该输出暂未使用则移除对 shooting_heat_ 的写入；定位符：shooting_heat_, register_output(...), "/gimbal/control_bullet_allowance/limited_by_heat".rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp (2)
71-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
gimbal_mode_keyboard_在同一帧被覆盖掉了。Line 87 刚把
*gimbal_mode_设为键盘状态，Line 88 又无条件改成switch_right推导出的值，所以E/C只会改旁路状态，实际模式不会跟着切换；switch_encoder_to_imu_by_c在Switch::UP时也进不到 IMU 分支。这里需要把遥控开关和键盘状态合成为一个最终模式，而不是连续覆写。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp` around lines 71 - 89, The keyboard-derived mode (gimbal_mode_keyboard_) is being overwritten by the unconditional assignment from switch_right to *gimbal_mode_, so combine both sources into a single final mode instead of consecutive writes: compute a local final_mode (respecting gimbal_mode_keyboard_, the switch_encoder_to_imu_by_c flag, and switch_right/ Switch::UP) and then assign *gimbal_mode_ = final_mode; update logic around where encoder_init_pitch_ is set and where gimbal_mode_keyboard_ is toggled so the C-key forced IMU (switch_encoder_to_imu_by_c) and the hardware switch both influence final_mode correctly, and remove the unconditional second assignment to *gimbal_mode_.
90-108:⚠️ Potential issue | 🟠 Major | ⚡ Quick win切回 IMU 时没有释放 yaw brake 控制输出。
编码器路径会在 Line 196-197 写
/gimbal/yaw_brake/control_torque，但 IMU 分支没有重新赋值。这样从 ENCODER 切回 IMU 后，上一帧的制动力矩会继续挂在总线上，等于把旧执行命令带进了新模式。请在 IMU/disable 路径显式写回nan_或0.0。Also applies to: 196-200
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp` around lines 90 - 108, When switching into the IMU branch (when *gimbal_mode_ == GimbalMode::IMU and in the disable/IMU path handled by enter_imu_hold_current_pose()/update_imu_control() and encoder_gimbal_solver.update(...)), explicitly clear the yaw-brake control output that the encoder path writes to (the same variable used to publish /gimbal/yaw_brake/control_torque in the encoder path) by assigning nan_ or 0.0 to it so the previous encoder-frame brake torque isn’t left on the bus; add the assignment alongside setting *yaw_control_angle_shift_ = nan_ and *pitch_control_angle_ = nan_.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 89-97: The encoder-locked branch currently assumes
*bottom_yaw_angle_ is valid and feeds it into bottom_yaw_angle_pid_ and
bottom_yaw_velocity_pid_, which can propagate NaN if the sensor transiently
fails; before using *bottom_yaw_angle_ in the bottom_yaw_encoder_locked_ branch,
check std::isfinite(*bottom_yaw_angle_) and only run the angle/velocity PID
logic when finite; otherwise clear the lock (set bottom_yaw_encoder_locked_ =
false), set *bottom_yaw_control_torque_ = nan_ and clear/reset PID internal
state (e.g. call reset/clear on bottom_yaw_angle_pid_ and
bottom_yaw_velocity_pid_) so no polluted integrators remain.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp`:
- Around line 258-271: putter_timeout_count_ is only cleared on actual timeout
so it accumulates across successful shoots; modify set_preloading(),
set_shooting(), and reset_all_controls() to reset putter_timeout_count_ (e.g.,
set putter_timeout_count_ = 0) so the counter is cleared on normal transitions
and full reset, ensuring no premature "PUTTER TIMEOUT" after repeated successful
shoots; update the three methods (set_preloading, set_shooting,
reset_all_controls) to perform this reset alongside their existing state/log
updates.
- Around line 117-123: preloaded_ready_ is only set in the reverse-cooldown
branch and never cleared on state transitions, so it can stay true after
shooting/disable; update the logic so preloaded_ready_ is maintained on every
state change: either (preferred) stop storing mutable state and publish/assign
preloaded_ready_ as a derived boolean (preloaded_ready_ = (shoot_stage_ ==
ShootStage::PRELOADED)) wherever the topic/value is produced, or ensure all
state-transition functions (set_shooting(), set_preloading(),
reset_all_controls(), and the reverse-cooldown handlers around the other noted
branches) explicitly set *preloaded_ready_ = false when leaving PRELOADED; pick
one approach and apply consistently across the other affected spots referenced
in the comment.
---
Outside diff comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp`:
- Around line 71-89: The keyboard-derived mode (gimbal_mode_keyboard_) is being
overwritten by the unconditional assignment from switch_right to *gimbal_mode_,
so combine both sources into a single final mode instead of consecutive writes:
compute a local final_mode (respecting gimbal_mode_keyboard_, the
switch_encoder_to_imu_by_c flag, and switch_right/ Switch::UP) and then assign
*gimbal_mode_ = final_mode; update logic around where encoder_init_pitch_ is set
and where gimbal_mode_keyboard_ is toggled so the C-key forced IMU
(switch_encoder_to_imu_by_c) and the hardware switch both influence final_mode
correctly, and remove the unconditional second assignment to *gimbal_mode_.
- Around line 90-108: When switching into the IMU branch (when *gimbal_mode_ ==
GimbalMode::IMU and in the disable/IMU path handled by
enter_imu_hold_current_pose()/update_imu_control() and
encoder_gimbal_solver.update(...)), explicitly clear the yaw-brake control
output that the encoder path writes to (the same variable used to publish
/gimbal/yaw_brake/control_torque in the encoder path) by assigning nan_ or 0.0
to it so the previous encoder-frame brake torque isn’t left on the bus; add the
assignment alongside setting *yaw_control_angle_shift_ = nan_ and
*pitch_control_angle_ = nan_.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/player_viewer.cpp`:
- Around line 113-116: 条件判断使用了逻辑或导致对任意角度均为真，使得
*viewer_delta_angle_by_mouse_wheel_ 每帧被清零；在包含符号 viewer_control_angle_,
lower_limit_, upper_limit_, viewer_delta_angle_by_mouse_wheel_ 的代码中，将当前的
"norm_angle(*viewer_control_angle_) >= lower_limit_ ||
norm_angle(*viewer_control_angle_) <= upper_limit_" 改为 "&&"
如果你的意图是判断角度在区间内后清零，或者将整个表达式取反（加上
!(...)）如果你的意图是判断角度在区间外后清零；只需替换逻辑运算符以反映正确的“在区间内/在区间外”语义。
- Around line 98-109: The reset branches (viewer_reset_, scope_viewer_reset_)
assign angles (viewer_init_angle_, *scope_offset_angle_) that lie outside the
allowed domain and then get forced to lower_limit_ by the final clamp; fix by
normalizing/wrapping the angle into the same angular domain as
lower_limit_/upper_limit_ before applying std::clamp in the block that updates
*viewer_control_angle_ (the branch handling viewer_reset_, scope_viewer_reset_,
and the else that adds *viewer_delta_angle_by_mouse_wheel_). Add or call a
helper to normalize angles (e.g., wrap to [lower_limit_, upper_limit_) or to a
consistent 0..2π range) for values like viewer_init_angle_ and
kEInitViewerAngle/kCtrlInitViewerAngle (or alternatively change those constants
to fall inside lower_limit_/upper_limit_); then apply std::clamp to the
normalized value so E and Ctrl+E resets don’t both collapse to lower_limit_.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/heat_controller.cpp`:
- Around line 24-25: 构造函数里已经调用了 register_output(...) 为
"/gimbal/control_bullet_allowance/limited_by_heat" 注册了
control_bullet_allowance_，但成员 shooting_heat_ 每帧被写入（在更新/循环代码中，见对 shooting_heat_
的解引用/赋值），却没有通过 register_output 注册到任何 topic，导致输出未绑定或运行时崩溃；请在构造函数中为 shooting_heat_
调用 register_output 并提供合理的默认值（例如 register_output("/gimbal/shooting_heat",
shooting_heat_, 0.0) 或类似），以确保输出已注册并有初始值，或者如果该输出暂未使用则移除对 shooting_heat_
的写入；定位符：shooting_heat_, register_output(...),
"/gimbal/control_bullet_allowance/limited_by_heat".
🪄 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: 8fe2629a-a349-4a17-a97f-63ebf14b1889
📒 Files selected for processing (8)
rmcs_ws/src/rmcs_bringup/config/steering-hero-little.yamlrmcs_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/player_viewer.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/hardware/steering-hero-little.cpprmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp
🚧 Files skipped from review as they are similar to previous changes (3)
- rmcs_ws/src/rmcs_bringup/config/steering-hero-little.yaml
- rmcs_ws/src/rmcs_core/src/hardware/steering-hero-little.cpp
- rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp
Comment on lines
+117
to
+123
| if (!bullet_feeder_reverse_end_ && shoot_stage_ == ShootStage::PRELOADED) | ||
| // RCLCPP_INFO(get_logger(), "Reverse finished"); | ||
| { | ||
| *preloaded_ready_ = true; | ||
| } else { | ||
| *preloaded_ready_ = false; | ||
| } |
There was a problem hiding this comment.
preloaded_ready_ 没有跟着状态机回落。
它只在 Line 117-123 的 reverse cooldown 分支里被写一次，set_shooting()、set_preloading() 和 reset_all_controls() 都不会把它清回 false。第一次预装完成后，这个 topic 会在射击中/失能后继续保持 true，与真实机构状态脱节；而 rmcs_ws/src/rmcs_core/src/referee/app/ui/hero.cpp:99-105 正在直接订阅它做操作员反馈。最好在每次状态迁移时统一维护，或者直接按 shoot_stage_ == ShootStage::PRELOADED 推导输出。
Also applies to: 237-256, 258-271, 411-411
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp` around
lines 117 - 123, preloaded_ready_ is only set in the reverse-cooldown branch and
never cleared on state transitions, so it can stay true after shooting/disable;
update the logic so preloaded_ready_ is maintained on every state change: either
(preferred) stop storing mutable state and publish/assign preloaded_ready_ as a
derived boolean (preloaded_ready_ = (shoot_stage_ == ShootStage::PRELOADED))
wherever the topic/value is produced, or ensure all state-transition functions
(set_shooting(), set_preloading(), reset_all_controls(), and the
reverse-cooldown handlers around the other noted branches) explicitly set
*preloaded_ready_ = false when leaving PRELOADED; pick one approach and apply
consistently across the other affected spots referenced in the comment.
Comment on lines
+258
to
+271
| void set_preloading() { | ||
| RCLCPP_INFO(get_logger(), "PRELOADING"); | ||
| shoot_stage_ = ShootStage::PRELOADING; | ||
| } | ||
|
|
||
| void set_preloaded() { | ||
| RCLCPP_INFO(get_logger(), "PRELOADED"); | ||
| shoot_stage_ = ShootStage::PRELOADED; | ||
| } | ||
|
|
||
| void set_shooting() { | ||
| RCLCPP_INFO(get_logger(), "SHOOTING"); | ||
| shoot_stage_ = ShootStage::SHOOTING; | ||
| } |
There was a problem hiding this comment.
putter_timeout_count_ 会跨射击累积。
Line 323-329 只在真正超时后才把计数清零；正常回收到 set_preloading() 时，这个累计值会原样保留。这样多次正常射击后，下一次会越来越早触发“PUTTER TIMEOUT”，最后在健康路径上也被提前打断。请在 set_shooting() / set_preloading() / reset_all_controls() 里同步重置这个计数器。
Also applies to: 319-330
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/controller/shooting/putter_controller.cpp` around
lines 258 - 271, putter_timeout_count_ is only cleared on actual timeout so it
accumulates across successful shoots; modify set_preloading(), set_shooting(),
and reset_all_controls() to reset putter_timeout_count_ (e.g., set
putter_timeout_count_ = 0) so the counter is cleared on normal transitions and
full reset, ensuring no premature "PUTTER TIMEOUT" after repeated successful
shoots; update the three methods (set_preloading, set_shooting,
reset_all_controls) to perform this reset alongside their existing state/log
updates.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp (1)
272-277:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winIMU 模式下 top_yaw 和 bottom_yaw 的 NaN 处理不一致。
Line 273 直接使用
*control_angle_error_计算top_yaw_control_torque_，而 Line 276 通过bottom_yaw_control_error()对输入做了std::isfinite检查。当control_angle_error_为 NaN 时，top_yaw 会将 NaN 传入 PID 并输出 NaN 扭矩，而 bottom_yaw 则返回 NaN 但不会污染 PID 积分器（因为 PID 输入已是 NaN）。建议在 Line 273 之前也添加有限性检查，或在
before_updating中确保control_angle_error_始终有效。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around lines 272 - 277, top_yaw currently passes *control_angle_error_ directly into top_yaw_angle_pid_.update which can propagate NaN into top_yaw_velocity_pid_; mirror the bottom yaw handling by validating control_angle_error_ before using it (or ensure before_updating guarantees it's finite). Specifically, before computing top_yaw_control_torque_ call std::isfinite on *control_angle_error_ and if false use a safe fallback (e.g. 0.0 or skip the PID update) so top_yaw_angle_pid_.update and top_yaw_velocity_pid_.update never receive NaN (refer to top_yaw_control_torque_, top_yaw_velocity_pid_.update, top_yaw_angle_pid_.update, control_angle_error_, and before_updating).rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp (1)
81-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick winLine 81 的赋值被 Line 82 立即覆盖，属于无效代码。
*gimbal_mode_ = gimbal_mode_keyboard_;的结果会被下一行的switch_right == Switch::UP ? ...条件判断完全覆盖，导致gimbal_mode_keyboard_的状态永远不会反映到输出上。如果设计意图是让遥控器开关优先于键盘控制，建议删除 Line 81 或添加注释说明；如果是希望键盘能控制模式，则需要修改逻辑。
🔧 建议修复方案（如果 switch_right 应作为覆盖条件）
- *gimbal_mode_ = gimbal_mode_keyboard_; - *gimbal_mode_ = switch_right == Switch::UP ? GimbalMode::ENCODER : GimbalMode::IMU; + // Remote switch overrides keyboard mode selection + *gimbal_mode_ = switch_right == Switch::UP ? GimbalMode::ENCODER : GimbalMode::IMU;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp` around lines 81 - 82, The first assignment to *gimbal_mode_ from gimbal_mode_keyboard_ is immediately overwritten by the ternary using switch_right, so either remove the redundant assignment to gimbal_mode_keyboard_ (if the remote switch should have priority), or change the second line to only overwrite *gimbal_mode_ when the switch is actively selecting a mode (e.g., guard the switch_right == Switch::UP/other active values) so that gimbal_mode_keyboard_ can remain effective when the switch is neutral; update the logic around gimbal_mode_, gimbal_mode_keyboard_, switch_right, Switch::UP, GimbalMode::ENCODER and GimbalMode::IMU accordingly.
🧹 Nitpick comments (1)
rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp (1)
41-42: 💤 Low value注册的输入
gimbal_pitch_angle_和gimbal_pitch_raw_angle_未被使用。这两个输入在构造函数中注册（Lines 41-42），成员变量已声明（Lines 192-193），但在整个类中没有任何实际引用。如果是为后续功能预留，建议添加 TODO 注释；如果是遗留代码，建议移除。
Also applies to: 192-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp` around lines 41 - 42, The constructor registers inputs gimbal_pitch_angle_ and gimbal_pitch_raw_angle_ but these members (gimbal_pitch_angle_, gimbal_pitch_raw_angle_) are never referenced elsewhere in the HeroGimbalController class; either remove the register_input calls and the unused member variables, or keep them and add a clear TODO comment in the constructor next to register_input("/gimbal/pitch/angle", gimbal_pitch_angle_) and register_input("/gimbal/pitch/raw_angle", gimbal_pitch_raw_angle_) indicating these are reserved for future use and include a brief note what functionality will consume them (or add a // TODO TODO_USE_GIMBAL_PITCH comment), ensuring consistency between constructor registration and member declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 203-218: EngageYawBrake lacks timeout/sensor-validation: in the
EngageYawBrake branch (encoder_state_ == EncoderState::EngageYawBrake) validate
yaw_brake_velocity_ before using std::abs (or detect !std::isfinite) and add a
timeout counter (e.g., yaw_brake_engage_timeout_count_) similar to
ReleaseYawBrake; if velocity is invalid increment the timeout counter and once
it exceeds a defined limit (new constant like kYawBrakeEngageTimeoutCount) clear
yaw_brake_engage_count_, set *yaw_brake_control_torque_ = nan_, transition
encoder_state_ = EncoderState::BrakeLocked and reset the timeout counter,
otherwise keep the existing valid-velocity logic that increments
yaw_brake_engage_count_ and transitions when >= kYawBrakeEngageConfirmCount.
---
Outside diff comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp`:
- Around line 272-277: top_yaw currently passes *control_angle_error_ directly
into top_yaw_angle_pid_.update which can propagate NaN into
top_yaw_velocity_pid_; mirror the bottom yaw handling by validating
control_angle_error_ before using it (or ensure before_updating guarantees it's
finite). Specifically, before computing top_yaw_control_torque_ call
std::isfinite on *control_angle_error_ and if false use a safe fallback (e.g.
0.0 or skip the PID update) so top_yaw_angle_pid_.update and
top_yaw_velocity_pid_.update never receive NaN (refer to
top_yaw_control_torque_, top_yaw_velocity_pid_.update,
top_yaw_angle_pid_.update, control_angle_error_, and before_updating).
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp`:
- Around line 81-82: The first assignment to *gimbal_mode_ from
gimbal_mode_keyboard_ is immediately overwritten by the ternary using
switch_right, so either remove the redundant assignment to gimbal_mode_keyboard_
(if the remote switch should have priority), or change the second line to only
overwrite *gimbal_mode_ when the switch is actively selecting a mode (e.g.,
guard the switch_right == Switch::UP/other active values) so that
gimbal_mode_keyboard_ can remain effective when the switch is neutral; update
the logic around gimbal_mode_, gimbal_mode_keyboard_, switch_right, Switch::UP,
GimbalMode::ENCODER and GimbalMode::IMU accordingly.
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp`:
- Around line 41-42: The constructor registers inputs gimbal_pitch_angle_ and
gimbal_pitch_raw_angle_ but these members (gimbal_pitch_angle_,
gimbal_pitch_raw_angle_) are never referenced elsewhere in the
HeroGimbalController class; either remove the register_input calls and the
unused member variables, or keep them and add a clear TODO comment in the
constructor next to register_input("/gimbal/pitch/angle", gimbal_pitch_angle_)
and register_input("/gimbal/pitch/raw_angle", gimbal_pitch_raw_angle_)
indicating these are reserved for future use and include a brief note what
functionality will consume them (or add a // TODO TODO_USE_GIMBAL_PITCH
comment), ensuring consistency between constructor registration and member
declarations.
🪄 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: 5d79425b-2bc1-46ef-aa70-4e074144cd14
📒 Files selected for processing (2)
rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpprmcs_ws/src/rmcs_core/src/controller/gimbal/hero_gimbal_controller.cpp
Comment on lines
+203
to
+218
| } else if (encoder_state_ == EncoderState::EngageYawBrake) { | ||
| *top_yaw_control_angle_shift_ = 0.0; | ||
| *bottom_yaw_control_torque_ = nan_; | ||
| *yaw_brake_control_torque_ = kYawBrakeEngageTorque; | ||
|
|
||
| if (std::abs(*yaw_brake_velocity_) < kYawBrakeEngageVelocityThreshold) { | ||
| ++yaw_brake_engage_count_; | ||
| } else { | ||
| yaw_brake_engage_count_ = 0; | ||
| } | ||
|
|
||
| if (yaw_brake_engage_count_ >= kYawBrakeEngageConfirmCount) { | ||
| yaw_brake_engage_count_ = 0; | ||
| *yaw_brake_control_torque_ = nan_; | ||
| encoder_state_ = EncoderState::BrakeLocked; | ||
| } |
There was a problem hiding this comment.
EngageYawBrake 状态缺少超时保护，传感器异常时可能卡死。
与 ReleaseYawBrake 不同，EngageYawBrake 状态没有超时机制。如果 yaw_brake_velocity_ 持续返回 NaN 或非有限值，Line 208 的比较将始终为 false，导致 yaw_brake_engage_count_ 无法累加，状态机将永远停留在此状态，持续输出 kYawBrakeEngageTorque。
建议参考 ReleaseYawBrake 添加超时计数器，或在进入前校验传感器数据有效性。
🛡️ 建议添加超时保护
+ static constexpr int kYawBrakeEngageTimeoutCount = 500;
+ int yaw_brake_engage_elapsed_count_ = 0; // 需要添加为成员变量
+
} else if (encoder_state_ == EncoderState::EngageYawBrake) {
*top_yaw_control_angle_shift_ = 0.0;
*bottom_yaw_control_torque_ = nan_;
*yaw_brake_control_torque_ = kYawBrakeEngageTorque;
+ ++yaw_brake_engage_elapsed_count_;
+
if (std::abs(*yaw_brake_velocity_) < kYawBrakeEngageVelocityThreshold) {
++yaw_brake_engage_count_;
} else {
yaw_brake_engage_count_ = 0;
}
if (yaw_brake_engage_count_ >= kYawBrakeEngageConfirmCount) {
yaw_brake_engage_count_ = 0;
+ yaw_brake_engage_elapsed_count_ = 0;
*yaw_brake_control_torque_ = nan_;
encoder_state_ = EncoderState::BrakeLocked;
+ } else if (yaw_brake_engage_elapsed_count_ >= kYawBrakeEngageTimeoutCount) {
+ RCLCPP_WARN(get_logger(), "Yaw brake engage timed out.");
+ yaw_brake_engage_count_ = 0;
+ yaw_brake_engage_elapsed_count_ = 0;
+ *yaw_brake_control_torque_ = nan_;
+ encoder_state_ = EncoderState::Idle; // 或回退到 AlignTargetRawAngle
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rmcs_ws/src/rmcs_core/src/controller/gimbal/dual_yaw_controller.cpp` around
lines 203 - 218, EngageYawBrake lacks timeout/sensor-validation: in the
EngageYawBrake branch (encoder_state_ == EncoderState::EngageYawBrake) validate
yaw_brake_velocity_ before using std::abs (or detect !std::isfinite) and add a
timeout counter (e.g., yaw_brake_engage_timeout_count_) similar to
ReleaseYawBrake; if velocity is invalid increment the timeout counter and once
it exceeds a defined limit (new constant like kYawBrakeEngageTimeoutCount) clear
yaw_brake_engage_count_, set *yaw_brake_control_torque_ = nan_, transition
encoder_state_ = EncoderState::BrakeLocked and reset the timeout counter,
otherwise keep the existing valid-velocity logic that increments
yaw_brake_engage_count_ and transitions when >= kYawBrakeEngageConfirmCount.
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
2 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.
Integrate hardware, chassis, gimbal, shooting, and UI support for steering-hero and steering-hero-little. Also correct LK motor torque constants and retune the omni-infantry / sentry compensating parameters to keep mainline vehicle control output unchanged. Remove the unmaintained mecanum-hero configuration.
Known issues (not blocking this merge):
PR 80 - Steering-hero 主线支持集成
概述
将 steering-hero 与 steering-hero-little 平台的硬件、底盘、云台、射击与 UI 完整集成到主线；修正若干驱动/参数；移除不再维护的 mecanum-hero 配置。包含大量新增组件、配置文件、插件注册与第三方依赖版本更新，涉及 CAN/UART/IMU/TF 路径与控制回路的深度改动。
主要变更（按模块）
配置
构建依赖
硬件驱动
底盘控制器（新增/变更）
云台控制与求解器
射击系统
UI 与消息
插件注册
已知问题（非阻塞）
评审建议 / 风险提示