Uh oh!
There was an error while loading. Please reload this page.
refactor(deformable-infantry): merge refactored chassis implementation and add pitch HUD - #74
Closed
Yukikaze2233 wants to merge 3 commits into
Closed
refactor(deformable-infantry): merge refactored chassis implementation and add pitch HUD#74Yukikaze2233 wants to merge 3 commits into
Yukikaze2233 wants to merge 3 commits into
Conversation
…an up dead code - Replace deformable_chassis.cpp with refactored version from refactor/deformable-infantry branch - Add IMU auto-calibration for pitch/roll offset - Inline JointFeedbackSource/JointIndex/JointFeedbackFrame types, removing dependency on deformable_joint_layer.hpp - Rename suspension PID variables to original naming convention (pitch_kp_, pitch_ki_, pitch_kd_, roll_kp_, roll_ki_, roll_kd_) - Remove unused SuspensionPhase enum - Remove 8 dead suspension output interfaces (suspension_mode/suspension_torque always false/NaN) - Fix config parameter names to match existing YAML config - Delete deformable_joint_layer.hpp (merged into deformable_chassis.cpp)
- Add pitch_hud.hpp widget (pitch scale with gimbal/chassis indicators) - Register /chassis/imu/pitch and /gimbal/pitch/angle inputs - Call pitch_hud_.update() in deformable_infantry_ui update loop
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
Walkthrough此 PR 将 deformable 底盘控制器的关节轨迹与悬架/姿态逻辑内联到 DeformableChassis，删除旧 joint 层头；新增 PitchHud UI、build 脚本 --link-default 支持及文档/补全；添加 OmniB 硬件插件与 bringup 配置，并在 steering 中添加 robot_status 服务。 更改内容底盘控制器整合重构
移除公有 joint 层头文件
俯仰 HUD UI 新增功能
构建脚本、补全与文档
Bringup 配置与 forwarding 调整
硬件插件与 Steering 变更
代码评审工作量估计🎯 4 (复杂) | ⏱️ ~75 分钟 可能相关的 PR
建议审阅者
诗歌
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
rmcs_ws/src/rmcs_core/src/controller/chassis/deformable_chassis.cpp (4)
38-45: ⚡ Quick win
JointFeedbackFrame::eso_z2/eso_z3字段从未被填充。
update_current_joint_feedback（line 522-561）只更新了motor_angles、physical_angles、physical_velocities、joint_torques，而eso_z2/eso_z3始终保留构造时的零值（line 43-44）。如果当前实现暂时不需要 ESO 状态反馈，建议先把这两项从结构体中删掉，避免下游误把零当作真实数据；如计划接入，请补上对应的register_input与读取分支。Also applies to: 419-425
🤖 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/deformable_chassis.cpp` around lines 38 - 45, The JointFeedbackFrame struct defines eso_z2 and eso_z3 but they are never populated in update_current_joint_feedback, so either remove these fields to avoid returning bogus zeros or implement proper input registration and assignment: if removing, delete eso_z2 and eso_z3 from JointFeedbackFrame; if keeping, add register_input calls for the ESO arrays and update the code in update_current_joint_feedback to read and assign the ESO values into JointFeedbackFrame::eso_z2 and ::eso_z3 (also ensure any other places that consume JointFeedbackFrame are updated accordingly). Use the identifiers JointFeedbackFrame, eso_z2, eso_z3, update_current_joint_feedback, and register_input to locate the related code.
446-459: ⚡ Quick win
refresh_requested_joint_targets_from_deploy_state_中的fill(min_angle_)与悬挂控制重复。当
suspension_requested_by_input_()为真时，这里会把requested_target_physical_angles_rad_全部填成min_angle_，并把current_target_physical_angles_rad_同步过去；但紧接着update_active_suspension_（line 582-631）在同一条件下又会基于base_target_angle = deg_to_rad(min_angle_)+ PID 修正整体重写current_target_physical_angles_rad_。第一次fill既不会被外部观察到（requested_target_physical_angles_rad_此后未再读），也掩盖了 deploy 状态原本的请求值，同时让两处的悬挂判断责任交叠，调试时容易误导。建议要么去掉这段悬挂分支，仅由update_active_suspension_负责悬挂目标；要么显式记录"请求来自 suspension"以便后续观测。Also applies to: 582-631
🤖 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/deformable_chassis.cpp` around lines 446 - 459, The code in refresh_requested_joint_targets_from_deploy_state_() overwrites requested_target_physical_angles_rad_ with deg_to_rad(min_angle_) when suspension_requested_by_input_() is true, duplicating responsibility with update_active_suspension_() and hiding the original deploy-requested targets; remove that fill branch so refresh_requested_joint_targets_from_deploy_state_() only copies deploy-state angles into requested_target_physical_angles_rad_ and leaves suspension handling to update_active_suspension_(), or alternatively replace the fill with setting a suspension_requested_flag_ (read by update_active_suspension_()) without mutating requested_target_physical_angles_rad_; touch the symbols requested_target_physical_angles_rad_, current_target_physical_angles_rad_, refresh_requested_joint_targets_from_deploy_state_(), update_active_suspension_(), suspension_requested_by_input_(), and min_angle_/base_target_angle accordingly.
374-382: ⚡ Quick win
joint_angle_deg是未被引用的代码，建议删除。在整个仓库中搜索结果显示，该方法仅在定义处出现一次，没有任何调用或引用。这疑似是从旧实现迁移后的遗留代码。直接删除可以减少代码复杂性和后续维护成本。
🤖 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/deformable_chassis.cpp` around lines 374 - 382, Remove the unused member function joint_angle_deg from deformable_chassis.cpp: delete the entire definition (the overload taking const InputInterface<double>& joint_angle, const InputInterface<double>& joint_encoder_angle, double joint_offset, double legacy_fixed_compensation) and any related forward declarations; ensure there are no remaining references to joint_angle_deg, then rebuild/tests to confirm no breakages. References to symbols in the removed code include JointFeedbackSource::kMotorAngle and wrap_deg—use these to locate the exact function to remove.
846-869: ⚡ Quick win删除未使用的
publish_current_joint_target_angles方法以消除代码重复
publish_current_joint_target_angles（第 846-869 行）与initialize_joint_target_states_from_feedback（第 803-819 行）的逻辑完全相同：都读取电机角度、计算物理角度，然后初始化相同的状态变量并设置joint_target_active_ = true。唯一的区别是前者从输入接口读取角度，后者接收作为参数。在整个代码库中没有找到对
publish_current_joint_target_angles的任何调用，而initialize_joint_target_states_from_feedback被run_joint_intent_pipeline_正确调用。建议删除publish_current_joint_target_angles以避免维护两份几乎相同的逻辑。🤖 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/deformable_chassis.cpp` around lines 846 - 869, The function publish_current_joint_target_angles duplicates initialize_joint_target_states_from_feedback and is unused; remove the publish_current_joint_target_angles definition (and any forward declaration) and any tests or references to it, keeping initialize_joint_target_states_from_feedback as the single source of truth used by run_joint_intent_pipeline_. Ensure you only delete the publish_current_joint_target_angles symbol (and related includes if any become unused) and run a build to verify no remaining references.rmcs_ws/src/rmcs_core/src/referee/app/ui/widget/pitch_hud.hpp (1)
48-55: ⚡ Quick win
tick_capacity_ = 25在常见配置下会被静默截断。按默认
half_span_deg = 30、tick_step_deg = 5需要 13 个刻度，能放下；但set_config仅约束tick_step_deg ≥ 1，因此当用户传入tick_step_deg = 1（甚至 2）时，所需刻度数为2 * 30 / 1 + 1 = 61，std::clamp(..., 1, 25)会直接截断到 25，HUD 仅覆盖配置范围的一小段且无任何告警。建议根据half_span_deg / tick_step_deg动态调整容量，或在set_config中按tick_capacity_反向 clamptick_step_deg下限以保证刻度始终覆盖完整 span。♻️ 一种最小改动方案：限制 tick_step_deg 以适配容量
void set_config(Config config) { config.tick_step_deg = std::max(config.tick_step_deg, 1.0); config.half_span_deg = std::max(config.half_span_deg, config.tick_step_deg); config.half_height_px = std::max<uint16_t>(config.half_height_px, 40); + // 保证刻度数不超过容量；否则按需放大 step 以覆盖完整 span+ const double min_step_for_capacity =+ 2.0 * config.half_span_deg / static_cast<double>(tick_capacity_ - 1);+ config.tick_step_deg = std::max(config.tick_step_deg, min_step_for_capacity); config_ = config; initialize_(); }🤖 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/widget/pitch_hud.hpp` around lines 48 - 55, The hard-coded tick_capacity_ (25) allows silent truncation in initialize_() when config_.tick_step_deg is small; fix by ensuring the configured tick_step_deg cannot demand more ticks than tick_capacity_—in set_config enforce a lower bound like tick_step_deg >= ceil(2*config_.half_span_deg/(tick_capacity_-1)) (use size/ceil math to compute), then recompute tick_count_ in initialize_() as you do now; alternatively, make tick_capacity_ dynamic (grow to required size before computing tick_count_). Update set_config and reference tick_capacity_, initialize_(), tick_count_, config_.half_span_deg and config_.tick_step_deg 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/deformable_chassis.cpp`:
- Around line 989-1017: The publish_nan_joint_targets() function currently
resets all joint target outputs but omits clearing *processed_encoder_angle_;
update publish_nan_joint_targets() (after the existing
angle/velocity/acceleration and error assignments) to set
*processed_encoder_angle_ = nan_ so that processed_encoder_angle_ is NaN when
feedback is unavailable (matching reset_all_controls() behavior) and prevents
downstream consumers from seeing stale encoder values.
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_core/src/controller/chassis/deformable_chassis.cpp`:
- Around line 38-45: The JointFeedbackFrame struct defines eso_z2 and eso_z3 but
they are never populated in update_current_joint_feedback, so either remove
these fields to avoid returning bogus zeros or implement proper input
registration and assignment: if removing, delete eso_z2 and eso_z3 from
JointFeedbackFrame; if keeping, add register_input calls for the ESO arrays and
update the code in update_current_joint_feedback to read and assign the ESO
values into JointFeedbackFrame::eso_z2 and ::eso_z3 (also ensure any other
places that consume JointFeedbackFrame are updated accordingly). Use the
identifiers JointFeedbackFrame, eso_z2, eso_z3, update_current_joint_feedback,
and register_input to locate the related code.
- Around line 446-459: The code in
refresh_requested_joint_targets_from_deploy_state_() overwrites
requested_target_physical_angles_rad_ with deg_to_rad(min_angle_) when
suspension_requested_by_input_() is true, duplicating responsibility with
update_active_suspension_() and hiding the original deploy-requested targets;
remove that fill branch so refresh_requested_joint_targets_from_deploy_state_()
only copies deploy-state angles into requested_target_physical_angles_rad_ and
leaves suspension handling to update_active_suspension_(), or alternatively
replace the fill with setting a suspension_requested_flag_ (read by
update_active_suspension_()) without mutating
requested_target_physical_angles_rad_; touch the symbols
requested_target_physical_angles_rad_, current_target_physical_angles_rad_,
refresh_requested_joint_targets_from_deploy_state_(),
update_active_suspension_(), suspension_requested_by_input_(), and
min_angle_/base_target_angle accordingly.
- Around line 374-382: Remove the unused member function joint_angle_deg from
deformable_chassis.cpp: delete the entire definition (the overload taking const
InputInterface<double>& joint_angle, const InputInterface<double>&
joint_encoder_angle, double joint_offset, double legacy_fixed_compensation) and
any related forward declarations; ensure there are no remaining references to
joint_angle_deg, then rebuild/tests to confirm no breakages. References to
symbols in the removed code include JointFeedbackSource::kMotorAngle and
wrap_deg—use these to locate the exact function to remove.
- Around line 846-869: The function publish_current_joint_target_angles
duplicates initialize_joint_target_states_from_feedback and is unused; remove
the publish_current_joint_target_angles definition (and any forward declaration)
and any tests or references to it, keeping
initialize_joint_target_states_from_feedback as the single source of truth used
by run_joint_intent_pipeline_. Ensure you only delete the
publish_current_joint_target_angles symbol (and related includes if any become
unused) and run a build to verify no remaining references.
In `@rmcs_ws/src/rmcs_core/src/referee/app/ui/widget/pitch_hud.hpp`:
- Around line 48-55: The hard-coded tick_capacity_ (25) allows silent truncation
in initialize_() when config_.tick_step_deg is small; fix by ensuring the
configured tick_step_deg cannot demand more ticks than tick_capacity_—in
set_config enforce a lower bound like tick_step_deg >=
ceil(2*config_.half_span_deg/(tick_capacity_-1)) (use size/ceil math to
compute), then recompute tick_count_ in initialize_() as you do now;
alternatively, make tick_capacity_ dynamic (grow to required size before
computing tick_count_). Update set_config and reference tick_capacity_,
initialize_(), tick_count_, config_.half_span_deg and config_.tick_step_deg
accordingly.
🪄 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: f58e52bf-ab93-4fef-812f-a8e70dfed31c
📒 Files selected for processing (4)
rmcs_ws/src/rmcs_core/src/controller/chassis/deformable_chassis.cpprmcs_ws/src/rmcs_core/src/controller/chassis/deformable_joint_layer.hpprmcs_ws/src/rmcs_core/src/referee/app/ui/deformable_infantry_ui.cpprmcs_ws/src/rmcs_core/src/referee/app/ui/widget/pitch_hud.hpp
💤 Files with no reviewable changes (1)
- rmcs_ws/src/rmcs_core/src/controller/chassis/deformable_joint_layer.hpp
Uh oh!
There was an error while loading. Please reload this page.
… chassis - Add deformable-infantry-omni-b: omni chassis + steering gimbal, no separate IMU board - Replace per-chassis calibrate topics with unified /rmcs/service/robot_status - Adopt two-frame alternating CAN transmission (even: 0x200+0x142, odd: 0x141) - Align BottomBoard constructor formatting across omni/omni-b/steering - Add --link-default to build-rmcs-cross for convenience symlinks - Add pitch HUD widget to referee UI
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Merge the refactored deformable chassis implementation from refactor/deformable-infantry and add a pitch angle HUD widget to the referee display.
Changes
变更概述
合并对可形变底盘（deformable chassis）的重构实现，新增裁判端俯仰角HUD小部件，并引入若干硬件/配置/构建脚本与机器人变体相关的改动（含 omni-b 变体、状态服务、CAN 传输节拍方案等）。
主要变更
核心控制器与接口
rmcs_core/src/controller/chassis/deformable_chassis.cpp
rmcs_core/src/controller/chassis/deformable_joint_layer.hpp
UI（裁判端）
硬件、变体与服务
配置与带来文件
构建脚本与文档
附加提交摘要（提交信息提到的其它更改）
影响范围与审查要点