Skip to content

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

Merged
creeper5820 merged 29 commits intomainfrom
feat/predictor-replica-ballistics
Feb 25, 2026
Merged

Fix: address issues in #25 regarding ballistics calculation#30
creeper5820 merged 29 commits intomainfrom
feat/predictor-replica-ballistics

Conversation

@heyeuu
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@creeper5820 creeper5820 merged commit 867483f into main Feb 25, 2026
2 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in RMCS Auto Aim V2 Feb 25, 2026
@coderabbitai coderabbitai bot mentioned this pull request Mar 11, 2026
@heyeuu heyeuu deleted the feat/predictor-replica-ballistics branch March 20, 2026 08:37
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: Done

Development

Successfully merging this pull request may close these issues.

2 participants