Open
Conversation
…ings - Refactor armor_detection logic for better performance. - Increase camera frame rate to 120 FPS in configuration. - Add logic to handle single-frame false positives (misidentification).
- Remove redundant aerodynamic coefficients to simplify the model. - Add `yaw_offset` and `pitch_offset` for improved alignment control.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 33 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough本次提交将预测器/快照与 RobotState 抽象化为后端接口，新增常规/前哨后端实现；重构追踪器/Decider、FireControl 与射击评估器，调整组件间数据流及运行时提交逻辑，并同步更新配置、测试和若干工具接口（NaN 初始值、EKF P() 访问器、mahalanobis 等）。 Changes
Sequence Diagram(s)sequenceDiagram
participant Runtime
participant Tracker as Decider/Tracker
participant Predictor as Snapshot/RobotState Backend
participant FireControl
participant Feishu
Runtime->>Tracker: submit detected armors, timestamp
Tracker->>Predictor: ensure/update backend (initialize/predict/update)
Predictor-->>Tracker: Snapshot (kinematics, predicted_armors)
Tracker-->>Runtime: decide (allow_takeover, target_id, tracking_confirmed, snapshot)
Runtime->>FireControl: solve(snapshot, control=tracking_confirmed, current_yaw)
FireControl-->>Runtime: Result (yaw,pitch,shoot_permitted)
Runtime->>Feishu: commit_state(next_state)
Feishu-->>Runtime: commit result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 分钟 Possibly related PRs
Suggested reviewers
诗
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
src/utility/math/mahalanobis.hpp (1)
11-16: 建议明确这是“平方马氏距离”而非“马氏距离”。
Line 14计算的是二次型d²。当前函数名容易误导阈值使用，建议重命名为mahalanobis_distance_sq（或改为返回sqrt(d²)并同步调用方阈值）。♻️ 建议修改示例（保留 d² 语义）
template <class TVec, class TMat> -inline auto mahalanobis_distance(TVec const& innovation, TMat const& covariance) +inline auto mahalanobis_distance_sq(TVec const& innovation, TMat const& covariance) -> std::optional<double> { auto solved = covariance.ldlt().solve(innovation); - auto distance = innovation.dot(solved); - if (!std::isfinite(distance)) return std::nullopt; - return distance; + auto distance_sq = innovation.dot(solved); + if (!std::isfinite(distance_sq) || distance_sq < 0.0) return std::nullopt; + return distance_sq; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/math/mahalanobis.hpp` around lines 11 - 16, The function mahalanobis_distance computes the quadratic form (d²) but is named as if it returned the actual Mahalanobis distance; rename the function to mahalanobis_distance_sq to reflect it returns the squared distance (update the function declaration/definition and all call sites that use mahalanobis_distance), or alternatively change the implementation to return std::sqrt(distance) and update threshold logic; ensure references to the function, any comments/docs, and threshold comparisons (e.g., uses of the return value) are updated accordingly; the change affects the function that computes solved = covariance.ldlt().solve(innovation) and distance = innovation.dot(solved).src/kernel/fire_control.hpp (1)
18-23: 给shoot_permitted补默认值。这是这次新增的公共返回字段；如果后面出现
Result result;这种默认构造路径，它会保持未定义状态。直接在声明处初始化为false会更稳妥。🔧 建议修改
struct Result { double pitch; double yaw; double horizon_distance; - bool shoot_permitted; + bool shoot_permitted { false }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/kernel/fire_control.hpp` around lines 18 - 23, Result 结构体中的成员 shoot_permitted 当前未初始化，会在默认构造（例如 `Result result;`）时成为未定义值；在 `struct Result` 的声明处将 `shoot_permitted` 直接初始化为 `false`（例如 `bool shoot_permitted = false;`）以确保默认构造时为确定性值，定位标识符：Result、shoot_permitted。src/module/debug/action_throttler.hpp (1)
67-70: 避免直接改FramerateCounter的内部字段。
reset()这里直接清last_reach_interval_timestamp，等于把ActionThrottler绑定到了FramerateCounter的内部实现细节上。后面如果FramerateCounter再增加状态，这里的重置很容易变得不完整。更稳妥的做法是给FramerateCounter提供显式reset()，或者在这里重建一个新的计数器并重新设置 interval。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module/debug/action_throttler.hpp` around lines 67 - 70, The reset() implementation mutates FramerateCounter internals (metronome.last_reach_interval_timestamp) making ActionThrottler dependent on FramerateCounter internals; instead add and call an explicit FramerateCounter::reset() (or recreate the metronome instance) from ActionThrottler::reset() so the counter's full state is properly reset; update reset() to call limit.reset() and then either metronome.reset() (preferred) or replace metronome with a newly-constructed FramerateCounter configured with the existing interval, and remove direct writes to last_reach_interval_timestamp.src/module/fire_control/shoot_evaluator.cpp (1)
32-33: 建议使用deg2rad工具函数替代魔法数字 57.3。代码中使用
57.3作为180/π的近似值进行角度转换，但该近似值精度有限（实际值约为 57.2957795...）。项目中已经有utility/math/angle.hpp提供的deg2rad函数，建议统一使用。♻️ 建议的修改
+#include "utility/math/angle.hpp" // 如果尚未包含 + // 在 Impl 中 - double first_tolerance_ { 4.0 / 57.3 }; - double second_tolerance_ { 2.0 / 57.3 }; + double first_tolerance_ { util::deg2rad(4.0) }; + double second_tolerance_ { util::deg2rad(2.0) };同样适用于
initialize()中的第 45-46 行：- first_tolerance_ = config.first_tolerance / 57.3; - second_tolerance_ = config.second_tolerance / 57.3; + first_tolerance_ = util::deg2rad(config.first_tolerance); + second_tolerance_ = util::deg2rad(config.second_tolerance);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module/fire_control/shoot_evaluator.cpp` around lines 32 - 33, Replace the hardcoded angle conversion magic number 57.3 by using the project's deg2rad helper: update the member initializers first_tolerance_ and second_tolerance_ to use deg2rad(4.0) and deg2rad(2.0) respectively, and similarly replace the conversions in initialize() (the two occurrences noted around lines 45-46) to call deg2rad(...) from utility/math/angle.hpp; ensure the header is included if not already and use the deg2rad symbol to improve precision and consistency.src/module/predictor/robot_state.cpp (1)
45-49:ensure_backend后的空检查可能冗余在
ensure_backend(armors.front())调用后，如果make_backend总是返回有效指针，则第 48 行的backend ?检查是多余的。如果make_backend可能返回nullptr，建议在ensure_backend中添加断言或处理此情况。♻️ 建议简化
auto update(std::span<Armor3D const> armors) -> bool { if (armors.empty()) return false; ensure_backend(armors.front()); - return backend ? backend->update(armors) : false; + return backend->update(armors); }或者在
ensure_backend中添加后置条件检查：auto ensure_backend(Armor3D const& armor) -> void { if (backend) return; backend = make_backend(armor.genre, pending_time_stamp); + // assert(backend && "make_backend should never return nullptr"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module/predictor/robot_state.cpp` around lines 45 - 49, The null-check after ensure_backend in update is likely redundant; either guarantee backend is non-null by modifying ensure_backend (or make_backend) to never return nullptr and add a postcondition/assertion there (e.g., assert(backend) or throw on failure), or keep defensive behavior by having ensure_backend return a bool and early-return on failure; update the code paths in update (the update method, ensure_backend function, and make_backend) accordingly so you only have one clear responsibility for handling a missing backend and remove the redundant "backend ?" conditional if you enforce a non-null postcondition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/module/debug/visualization/armor_visualizer.cpp`:
- Around line 112-145: The Marker timestamps are created with a static
rclcpp_clock (RCL_STEADY_TIME), causing time-source mismatch with RViz; replace
usage of rclcpp_clock and current_time with the node's clock (e.g., call
node->get_clock()->now() where current_time is set) or set the stamp to zero
when constructing markers inside make_marker so header.stamp uses the ROS time
domain; update all places using rclcpp_clock (the static rclcpp_clock variable
and the current_time assignment) and ensure make_marker receives the
node-derived time (or a zero time) before publishing via rclcpp_pub->publish so
RViz/TF compare times from the same clock.
- Around line 117-139: The markers currently use only armor.id which causes ID
collisions across devices; change all places that build marker IDs (calls to
make_marker and the current_ids/previous_ids keys) to combine armor.genre
(device ID) and armor.id into a single stable unique ID (e.g.,
(static_cast<uint32_t>(armor.genre) << 8) | armor.id or by appending device ID
into the marker namespace), and apply the same composition for both ADD and
DELETE markers and when inserting/looking up IDs in current_ids and previous_ids
so add/delete logic remains consistent.
In `@src/module/fire_control/aim_point_chooser.cpp`:
- Around line 29-37: The file uses std::atan2 and multiple std::abs calls inside
choose_armor (and surrounding functions) but does not explicitly include
<cmath>; add an explicit `#include` <cmath> at the top of
src/module/fire_control/aim_point_chooser.cpp so that functions like std::atan2
and std::abs are guaranteed to be available regardless of transitive includes
and toolchain/include-order changes.
In `@src/module/predictor/backend/robot_state_backend.hpp`:
- Line 44: 文件中打开的命名空间是 `namespace rmcs::predictor`（在顶部），但结尾注释写成了 `// namespace
rmcs::predictor::detail`; 将结尾的注释改为与实际声明一致为 `// namespace
rmcs::predictor`，或者如果意图是关闭 `rmcs::predictor::detail`，则修改开头的 `namespace` 声明以包含
`::detail`（检查并统一 `namespace rmcs::predictor` 与结尾注释中的命名空间名以保持一致）。
In `@src/module/predictor/outpost/robot_state.cpp`:
- Around line 424-428: The implementation defines both OutpostRobotState()
noexcept and OutpostRobotState(Clock::time_point stamp) noexcept but the header
only declares the parameterized ctor; update the class declaration in
robot_state.hpp to also declare the default constructor OutpostRobotState()
noexcept (matching the implementation) so the declarations and definitions
align, keeping the explicit/ noexcept qualifiers consistent with the existing
OutpostRobotState(Clock::time_point) declaration and retaining the pimpl/Impl
usage.
In `@src/module/predictor/regular/ekf_parameter.hpp`:
- Around line 17-18: This header uses std::numbers::pi in functions armor_yaw
and H but does not include the <numbers> header; add a direct `#include` <numbers>
at the top of the file so the header compiles standalone and does not rely on
transitive includes, ensuring references to std::numbers::pi in armor_yaw and H
resolve.
In `@src/module/predictor/regular/robot_state.cpp`:
- Around line 69-70: The convergence check uses min_updates = 3 but compares
with update_count using '>' so it only returns true after the 4th update; change
the comparison in the return expression that references r_ok, l_ok, update_count
and min_updates from 'update_count > min_updates' to 'update_count >=
min_updates' so that "at least 3 updates" is treated correctly.
- Around line 73-79: get_snapshot() and distance() currently read ekf.x even
when the object has been marked uninitialized after timeout; change both to
first check the object's uninitialized flag (the same flag set by reset) and if
not initialized return a safe empty value: for get_snapshot() return
Snapshot::empty(...) using the same device/color/armor_num/time_stamp context,
and for distance() return a sentinel (e.g. NaN via
std::numeric_limits<double>::quiet_NaN()) without accessing ekf.x; ensure no
direct reads of ekf.x occur when the uninitialized flag is true.
---
Nitpick comments:
In `@src/kernel/fire_control.hpp`:
- Around line 18-23: Result 结构体中的成员 shoot_permitted 当前未初始化，会在默认构造（例如 `Result
result;`）时成为未定义值；在 `struct Result` 的声明处将 `shoot_permitted` 直接初始化为 `false`（例如
`bool shoot_permitted = false;`）以确保默认构造时为确定性值，定位标识符：Result、shoot_permitted。
In `@src/module/debug/action_throttler.hpp`:
- Around line 67-70: The reset() implementation mutates FramerateCounter
internals (metronome.last_reach_interval_timestamp) making ActionThrottler
dependent on FramerateCounter internals; instead add and call an explicit
FramerateCounter::reset() (or recreate the metronome instance) from
ActionThrottler::reset() so the counter's full state is properly reset; update
reset() to call limit.reset() and then either metronome.reset() (preferred) or
replace metronome with a newly-constructed FramerateCounter configured with the
existing interval, and remove direct writes to last_reach_interval_timestamp.
In `@src/module/fire_control/shoot_evaluator.cpp`:
- Around line 32-33: Replace the hardcoded angle conversion magic number 57.3 by
using the project's deg2rad helper: update the member initializers
first_tolerance_ and second_tolerance_ to use deg2rad(4.0) and deg2rad(2.0)
respectively, and similarly replace the conversions in initialize() (the two
occurrences noted around lines 45-46) to call deg2rad(...) from
utility/math/angle.hpp; ensure the header is included if not already and use the
deg2rad symbol to improve precision and consistency.
In `@src/module/predictor/robot_state.cpp`:
- Around line 45-49: The null-check after ensure_backend in update is likely
redundant; either guarantee backend is non-null by modifying ensure_backend (or
make_backend) to never return nullptr and add a postcondition/assertion there
(e.g., assert(backend) or throw on failure), or keep defensive behavior by
having ensure_backend return a bool and early-return on failure; update the code
paths in update (the update method, ensure_backend function, and make_backend)
accordingly so you only have one clear responsibility for handling a missing
backend and remove the redundant "backend ?" conditional if you enforce a
non-null postcondition.
In `@src/utility/math/mahalanobis.hpp`:
- Around line 11-16: The function mahalanobis_distance computes the quadratic
form (d²) but is named as if it returned the actual Mahalanobis distance; rename
the function to mahalanobis_distance_sq to reflect it returns the squared
distance (update the function declaration/definition and all call sites that use
mahalanobis_distance), or alternatively change the implementation to return
std::sqrt(distance) and update threshold logic; ensure references to the
function, any comments/docs, and threshold comparisons (e.g., uses of the return
value) are updated accordingly; the change affects the function that computes
solved = covariance.ldlt().solve(innovation) and distance =
innovation.dot(solved).
🪄 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: 2cfd39eb-c499-4368-b17b-235842827dc0
📒 Files selected for processing (47)
CMakeLists.txtconfig/config.yamlpackage.xmlsrc/component.cppsrc/kernel/fire_control.cppsrc/kernel/fire_control.hppsrc/kernel/tracker.cppsrc/kernel/tracker.hppsrc/module/debug/action_throttler.hppsrc/module/debug/visualization/armor_visualizer.cppsrc/module/fire_control/aim_point_chooser.cppsrc/module/fire_control/aim_point_chooser.hppsrc/module/fire_control/shoot_evaluator.cppsrc/module/fire_control/shoot_evaluator.hppsrc/module/fire_control/trajectory_solution.cppsrc/module/fire_control/trajectory_solution.hppsrc/module/predictor/backend/robot_state_backend.cppsrc/module/predictor/backend/robot_state_backend.hppsrc/module/predictor/backend/snapshot_backend.hppsrc/module/predictor/outpost/armor_layout.hppsrc/module/predictor/outpost/ekf_parameter.hppsrc/module/predictor/outpost/robot_state.cppsrc/module/predictor/outpost/robot_state.hppsrc/module/predictor/outpost/snapshot.cppsrc/module/predictor/outpost/snapshot.hppsrc/module/predictor/regular/ekf_parameter.hppsrc/module/predictor/regular/robot_state.cppsrc/module/predictor/regular/robot_state.hppsrc/module/predictor/regular/snapshot.cppsrc/module/predictor/regular/snapshot.hppsrc/module/predictor/robot_state.cppsrc/module/predictor/robot_state.hppsrc/module/predictor/snapshot.cppsrc/module/predictor/snapshot.hppsrc/module/tracker/decider.cppsrc/module/tracker/decider.hppsrc/module/tracker/state.hppsrc/runtime.cppsrc/utility/math/kalman_filter/ekf.hppsrc/utility/math/mahalanobis.hppsrc/utility/model/armor_detection.hppsrc/utility/robot/constant.hppsrc/utility/shared/context.hpptest/CMakeLists.txttest/action_throttler.cpptest/feishu_test.cpptest/model_infer.cpp
💤 Files with no reviewable changes (1)
- src/module/tracker/state.hpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/module/tracker/decider.cpp (1)
263-285:⚠️ Potential issue | 🟡 Minor移除未使用的优先级模式定义。
mode1和mode2在此文件中定义但从未使用。请移除这些未使用的常量定义。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module/tracker/decider.cpp` around lines 263 - 285, Remove the unused PriorityMode constants mode1 and mode2 from decider.cpp: these locals ({ DeviceId::HERO, ... } and { DeviceId::ENGINEER, ... } definitions) are defined but never referenced, so delete the declarations of mode1 and mode2 to avoid unused symbol warnings and dead code; search for the identifiers mode1 and mode2 and remove their full const PriorityMode definitions (including the DeviceId entries) and ensure no other code depends on them.
🧹 Nitpick comments (1)
src/module/predictor/regular/ekf_parameter.hpp (1)
18-25: 建议把装甲板数量的判定统一成一个来源。现在同一套几何分支一部分走
armor_num(device)（armor_yaw()/h_armor_z()），另一部分走显式armor_num（h_armor_xyz()/H()）。当前调用方在src/module/predictor/regular/robot_state.cpp和src/module/predictor/regular/snapshot.cpp里是对齐的，但这会把h()和H()的一致性变成隐式约束，后续只改一边时很容易把观测函数和 Jacobian 改散。建议全链路只保留一个来源：要么全部从device推导，要么统一显式传armor_num。Also applies to: 146-158, 202-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/module/predictor/regular/ekf_parameter.hpp` around lines 18 - 25, The code mixes two sources for the armor plate count (calling armor_num(device) inside armor_yaw() / h_armor_z() but passing an explicit armor_num into h_armor_xyz() / H()), which risks divergence; pick one source and make it consistent across the chain: either (A) change h_armor_xyz(...) and H(...) to accept DeviceId (or otherwise call armor_num(device) internally) so all geometry functions derive armor count from device (update all call sites in robot_state.cpp and snapshot.cpp accordingly), or (B) change armor_yaw(DeviceId,const EKF::XVec,int) and h_armor_z(DeviceId,const EKF::XVec,int) to accept an explicit armor_num parameter and use that instead of calling armor_num(device) (and update callers to pass the same explicit armor_num used for h_armor_xyz/H). Ensure function signatures and all callers for armor_yaw, h_armor_z, h_armor_xyz, and H are updated together so there's a single source of truth for armor count.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/module/predictor/backend/robot_state_backend.hpp`:
- Around line 3-5: This header (robot_state_backend.hpp) uses std::uint8_t in
its public API but does not include <cstdint>, so add an explicit `#include`
<cstdint> at the top of the file to ensure std::uint8_t is defined and the
header is self-contained (don’t rely on transitive includes).
In `@src/module/predictor/regular/robot_state.cpp`:
- Around line 51-55: 当前实现会在 update_single(...) 内对 update_count 增量，导致批量调用
update(std::span<Armor3D const> armors) 时每融合一块装甲都增加一次 update_count，从而破坏
is_converged() 关于“至少 3 次时序更新”的语义；请把对 update_count 的自增逻辑从 update_single(...)
中移除，并在 update(std::span<Armor3D const> armors) 中在循环结束后（当 fused 为
true）只做一次自增，这样每次成功的批量更新只增加一次 update_count，保持 is_converged() 的预期行为。确保删除或注释掉
update_single 中的增量并在 update 中在确认 fused 后执行 update_count++（或等价赋值）。
---
Outside diff comments:
In `@src/module/tracker/decider.cpp`:
- Around line 263-285: Remove the unused PriorityMode constants mode1 and mode2
from decider.cpp: these locals ({ DeviceId::HERO, ... } and {
DeviceId::ENGINEER, ... } definitions) are defined but never referenced, so
delete the declarations of mode1 and mode2 to avoid unused symbol warnings and
dead code; search for the identifiers mode1 and mode2 and remove their full
const PriorityMode definitions (including the DeviceId entries) and ensure no
other code depends on them.
---
Nitpick comments:
In `@src/module/predictor/regular/ekf_parameter.hpp`:
- Around line 18-25: The code mixes two sources for the armor plate count
(calling armor_num(device) inside armor_yaw() / h_armor_z() but passing an
explicit armor_num into h_armor_xyz() / H()), which risks divergence; pick one
source and make it consistent across the chain: either (A) change
h_armor_xyz(...) and H(...) to accept DeviceId (or otherwise call
armor_num(device) internally) so all geometry functions derive armor count from
device (update all call sites in robot_state.cpp and snapshot.cpp accordingly),
or (B) change armor_yaw(DeviceId,const EKF::XVec,int) and
h_armor_z(DeviceId,const EKF::XVec,int) to accept an explicit armor_num
parameter and use that instead of calling armor_num(device) (and update callers
to pass the same explicit armor_num used for h_armor_xyz/H). Ensure function
signatures and all callers for armor_yaw, h_armor_z, h_armor_xyz, and H are
updated together so there's a single source of truth for armor count.
🪄 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: 95f390de-bc20-4ebb-99c9-b5baa93d7b64
📒 Files selected for processing (6)
src/module/debug/visualization/armor_visualizer.cppsrc/module/fire_control/aim_point_chooser.cppsrc/module/predictor/backend/robot_state_backend.hppsrc/module/predictor/regular/ekf_parameter.hppsrc/module/predictor/regular/robot_state.cppsrc/module/tracker/decider.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- src/module/fire_control/aim_point_chooser.cpp
- src/module/debug/visualization/armor_visualizer.cpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/module/predictor/backend/robot_state_backend.hpp`:
- Around line 42-43: The factory make_robot_state_backend currently only takes
RobotStateBackendKind and a time_point but must also accept and forward the
DeviceId so backends can hold the real device identifier; update the signature
of make_robot_state_backend to include a DeviceId parameter and propagate that
DeviceId into the concrete backend constructors, ensure IRobotStateBackend
implementations store the DeviceId and pass it down to RegularRobotState (so
RegularRobotState::Impl::initialize can call
EKFParameters::P_initial_dig(device), h(device, ...) and H(device, ...)) while
keeping armor-based logic limited to Armor3D/ArmorGenre for armor_num
derivation; search for make_robot_state_backend, RobotStateBackendKind,
IRobotStateBackend::Clock::time_point, DeviceId,
RegularRobotState::Impl::initialize, EKFParameters::P_initial_dig, h and H to
apply the change and update callers accordingly.
In `@src/module/predictor/regular/robot_state.cpp`:
- Around line 3-5: This file uses std::numeric_limits<double>::infinity() (see
usage in robot_state.cpp) but doesn't include <limits>; add a direct include for
<limits> at the top of robot_state.cpp so the code doesn't rely on transitive
includes (i.e., insert `#include` <limits> alongside the existing includes to
ensure compilation is robust).
🪄 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: b04cf086-36d5-4681-a2d0-ebdb95d44851
📒 Files selected for processing (2)
src/module/predictor/backend/robot_state_backend.hppsrc/module/predictor/regular/robot_state.cpp
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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
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.
前哨站预测能力集成 — 自动瞄准系统架构升级 PR（Feat/outpost-prediction）
一、总体目标
引入前哨站（Outpost）机器人预测与追踪能力，重构预测/快照/追踪/火控等模块以支持多类型机器人、提升解耦性与可配置性，并增加射击判定逻辑以降低误击风险。
二、核心架构变化
三、前哨站（Outpost）实现要点
四、常规机器人（Regular）预测改动
五、火控（FireControl）重构与新增 ShootEvaluator
六、瞄准点选择器（AimPointChooser）
七、追踪决策（Decider）与 Tracker 集成
八、自动瞄准组件与上下文（AutoAim / Control）
九、运行时（runtime）整合
十、调试与可视化改进
十一、工具库与常量
十二、配置（config/config.yaml）主要变更
十三、测试
十四、公共 API 与兼容性影响（重要变更）
十五、审查重点与风险评估
十六、变更量与审查建议