Skip to content

Comments

Fix: address issues in #25 regarding ballistics calculation#30

Open
heyeuu wants to merge 29 commits intomainfrom
feat/predictor-replica-ballistics
Open

Fix: address issues in #25 regarding ballistics calculation#30
heyeuu wants to merge 29 commits intomainfrom
feat/predictor-replica-ballistics

Conversation

@heyeuu
Copy link
Member

@heyeuu heyeuu commented Feb 10, 2026

修复:解决 #25 中关于弹道计算的问题

核心改动

本 PR 通过简化重力处理和重构运行时管道,修复并优化了弹道计算相关逻辑,提升可维护性与错误隔离。

弹道计算与配置

  • 从配置文件 config/config.yaml 中移除可配置的重力参数 g
  • 在 fire_control 模块内部删除了 FireControl::Impl::Config 中的 g 字段以及 TrajectoryParamsg 成员。
  • 在 trajectory_solution 中将重力改为常量内置(新增私有常量 kGravity = 9.81),不再作为函数参数传递。
  • 更新方法签名:Estimate(v0, pitch, d, k, g)Estimate(v0, pitch, d, k),并在垂直速度更新中用 kGravity 替代原来的 g 参数。

运行时结构重构

  • 将原有单体控制流程(Identify -> PnP -> Track -> Fire -> Feishu)重构为一系列阶段化、可组合的 lambda:
    • fetch_control_state、detect_armors、solve_pnp、track_armors、execute_fire_control、visualize_detection、commit_result、visualize_prediction
  • 引入统一的节流/错误上报机制(action_throttler),在各阶段对失败或缺失数据进行稳妥处理并提前返回,降低嵌套复杂度并提高可读性。
  • 在本地运行时添加对控制状态的友好降级处理与限频日志提示。

其他变更

  • 新增 .gitattributes,统一行尾与文件类型文本属性(.c/.cpp/.h/.hpp/.sh/.py/.json/.yaml/.md 等)。
  • ekf.hpp 中将 #include <eigen3/Eigen/Dense> 替换为 <eigen3/Eigen/Geometry> 并新增 <eigen3/Eigen/Cholesky>,调整头文件依赖,未改变公开接口。

对外 API 与行为影响

  • 所有变更均为内部实现调整;无对外导出/公共 API 的修改。
  • 功能行为保持一致:弹道求解逻辑行为不变(仅将重力来源由配置参数改为常量),但配置中不再可调节 g。

备注

  • 相关文件:config/config.yaml、src/kernel/fire_control.cpp、src/module/fire_control/trajectory_solution.{cpp,hpp}、src/runtime.cpp、src/utility/math/kalman_filter/ekf.hpp、.gitattributes

creeper5820 and others added 28 commits February 10, 2026 11:32
Enabled real-time 3D visualization of predicted armor plates in the world coordinate system, allowing for intuitive debugging via Foxglove Studio.
@heyeuu heyeuu added this to the 预测及解算模块 milestone Feb 10, 2026
@heyeuu heyeuu self-assigned this Feb 10, 2026
@heyeuu heyeuu added the bug Something isn't working label Feb 10, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

Walkthrough

本次变更移除可配置的重力参数并改为私有常数,将火控轨迹估计相关签名及实现调整;同时把 src/runtime.cpp 的单体控制流重构为模块化 lambda 管道;新增 .gitattributes;并调整了 Eigen 头文件包含。无导出 API 行为变化。

Changes

Cohort / File(s) Summary
重力参数移除与常数化
config/config.yaml, src/kernel/fire_control.cpp, src/module/fire_control/trajectory_solution.hpp, src/module/fire_control/trajectory_solution.cpp
从配置与参数结构中移除重力 g,将轨迹估计接口去掉 g 参数并在类内引入私有常量 kGravity = 9.81,相应调用处和参数结构同步更新。注意参数结构与方法签名改变。
控制流重构(Runtime 管道化)
src/runtime.cpp
将先前嵌套的 Feishu / 识别 / PnP / 跟踪 / 火控 流程拆分为一组模块化 lambda(fetch_control_state、detect_armors、solve_pnp、track_armors、execute_fire_control、visualize_detection、commit_result、visualize_prediction),引入阶段化错误处理与节流器逻辑,最多影响控制流理解与测试。
仓库配置
.gitattributes
新增文件,配置行尾及各类文件的文本属性(C/C++ 源头文件、脚本强制 LF、配置/文档标记为 text)。
依赖头文件调整
src/utility/math/kalman_filter/ekf.hpp
#include <eigen3/Eigen/Dense> 改为 #include <eigen3/Eigen/Geometry> 并新增 #include <eigen3/Eigen/Cholesky>,仅变更包含以便使用几何/分解类型。

Sequence Diagram(s)

sequenceDiagram
    participant Camera as 摄像机/输入帧
    participant Runtime as Runtime 管道
    participant Detection as 装甲识别
    participant PnP as PnP 求解器
    participant Tracker as 追踪器
    participant FireCtrl as 火控 系统
    participant Feishu as Feishu 服务

    Camera->>Runtime: 新帧
    Runtime-->>Runtime: fetch_control_state()
    Runtime->>Detection: detect_armors()
    Detection-->>Runtime: 2D 检测结果
    Runtime->>PnP: solve_pnp(2D 检测)
    PnP-->>Runtime: 3D 装甲位姿
    Runtime->>Tracker: track_armors(3D 数据)
    Tracker-->>Runtime: 跟踪快照
    Runtime->>FireCtrl: execute_fire_control(跟踪数据)
    FireCtrl-->>Runtime: 射击解
    Runtime->>Feishu: commit_result(自动瞄准状态)
    Runtime->>Runtime: visualize_detection()/visualize_prediction()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • creeper5820

Poem

🐰 我是小兔写提交,常数代替了配置心,
管道取代嵌套困,模块分明步轻盈。
少了可调多了稳,绘图跟踪任君行,
提交一跳庆新改,胡萝卜请别忘给〜 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately describes the main objective: fixing ballistics-related issues by removing the hardcoded gravity parameter and refactoring the trajectory calculation system to use a constant instead.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/predictor-replica-ballistics

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
src/utility/math/kalman_filter/ekf.hpp (1)

2-3: Eigen/Geometry 在此文件中确实多余,建议移除。

验证确认:

  • ekf.hpp 不使用任何 Geometry 类型(Quaternion、AngleAxis、Transform 等)
  • 仅使用 Eigen::Matrix 类型和 ldlt() 分解
  • Eigen/Cholesky 已隐式包含 Eigen/Core,满足矩阵类型需求
  • 下游文件(如 ekf_parameter.hpp)在需要 Geometry 时自行包含该头文件,不依赖通过 ekf.hpp 的传递包含

移除 Eigen/Geometry 不会对下游消费者造成影响,可以减少不必要的编译开销。

建议修改
-#include <eigen3/Eigen/Geometry>
-#include <eigen3/Eigen/Cholesky>
+#include <eigen3/Eigen/Cholesky>

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
src/module/fire_control/trajectory_solution.cpp (1)

6-13: ⚠️ Potential issue | 🟡 Minor

文档注释中仍引用已移除的参数 g

第 11 行 @param g 重力加速度 对应的参数已被移除(改为内部常量 kGravity),但注释未同步更新。

🔧 建议修复
 /**
  * `@brief` 弹道解算 (考虑空气阻力迭代)
  * `@param` v0 子弹初速度 (m/s)
  * `@param` target_d 水平距离 (m)
  * `@param` target_h 目标竖直高度 (m) - 目标在枪口上方为正
- * `@param` g 重力加速度
  * `@param` params 系数
  * 参考资料:https://zhuanlan.zhihu.com/p/1970271417149920247
  */
🤖 Fix all issues with AI agents
In `@src/utility/math/kalman_filter/ekf.hpp`:
- Line 2: 在 ekf.hpp 中当前只包含 <eigen3/Eigen/Geometry>,但后续在 S.ldlt().solve()
的模板实例化(例如在 robot_state.cpp 中)需要 Cholesky/LDLT 支持;将头文件改为包含
<eigen3/Eigen/Dense>(或同时保留 Geometry 并新增 <eigen3/Eigen/Cholesky>)以确保
LDLT/Cholesky 分解可用,从而修复 S.ldlt().solve() 的编译错误。
🧹 Nitpick comments (1)
src/module/fire_control/trajectory_solution.hpp (1)

32-35: 常量命名中的拼写问题(已有代码)

kMaxPitchThreoldkHeightErrorThreoldkEstimateTimeOutThreold 中 "Threold" 应为 "Threshold"。这是预先存在的拼写问题,建议在方便时统一修正。

The call to `S.ldlt().solve()` at line 133 requires the Cholesky
module, which is not provided by `Eigen/Geometry`.

- Re-add `Eigen/Cholesky` to ensure the template can be instantiated.
- Resolve the compilation error in `robot_state.cpp` that occurs when
  `Eigen/Dense` is not previously included in the chain.
- Maintain `Eigen/Geometry` for rotation and quaternion operations.
@heyeuu heyeuu requested a review from creeper5820 February 10, 2026 06:09
@heyeuu heyeuu moved this from Todo to In progress in RMCS Auto Aim V2 Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants