feat: add UiRobotCenterOverlay for flight referee UI#87
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough新增AutoAimUi瞄准UI插件组件及其配置绑定;重构rmcs_executor更新线程为带TDigest统计的周期调度;新增rmcs_utility下的TDigest统计摘要与ThreadConfig线程配置解析/应用工具;更新CMake的C++标准为23;简化main.cpp参数读取逻辑。 ChangesAutoAimUi瞄准UI组件接入
执行器线程调度统计与工具库
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant rmcs_executor
participant AutoAimUi
participant TF
participant UI图形
rmcs_executor->>AutoAimUi: update()
AutoAimUi->>AutoAimUi: 校验robot_center是否有限/非零
alt 数据无效
AutoAimUi->>UI图形: hide_all()
else 数据有效
AutoAimUi->>TF: lookup_transform(相机位姿)
TF-->>AutoAimUi: 相机变换
AutoAimUi->>AutoAimUi: reproject()计算像素坐标
alt 坐标越界或NaN
AutoAimUi->>UI图形: hide_all()
else 坐标在屏幕内
AutoAimUi->>UI图形: 更新圆环颜色/半径
AutoAimUi->>UI图形: 按should_shoot显示/隐藏十字线
end
end
sequenceDiagram
participant Executor
participant ThreadConfig
participant thread_main
participant TDigest
Executor->>Executor: 校验update_rate并读取thread_config
Executor->>ThreadConfig: 构造并解析spec
Executor->>thread_main: 启动线程(period, thread_config)
thread_main->>ThreadConfig: apply_to_current_thread()
loop 每轮迭代
thread_main->>thread_main: execute_update_iteration()
thread_main->>TDigest: 记录start_lateness/update_duration
thread_main->>thread_main: calculate_next_iteration_time()
alt 达到上报周期
thread_main->>TDigest: 合并并计算分位数
thread_main->>thread_main: 输出累计与窗口统计日志
end
end
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/referee/app/ui/robot_center_overlay.cpp`:
- Around line 80-87: The boundary check in robot_center_overlay.cpp lets values
like u/v near the maximum screen edge pass, but RobotCenterOverlay::hide_all and
the later std::lround conversion can still produce out-of-range pixel indices.
Update the coordinate handling in the overlay update path so the rounded results
are clamped to the valid screen range before being stored in the ui/vi pixel
variables, using the existing screen_width and screen_height bounds as the
reference.
🪄 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: 0b31389a-4df4-475e-a6f9-0d2414333535
📒 Files selected for processing (3)
rmcs_ws/src/rmcs_bringup/config/flight.yamlrmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/referee/app/ui/robot_center_overlay.cpp
|
|
||
| static constexpr uint16_t screen_width = 1920, screen_height = 1080; | ||
|
|
||
| static constexpr double fx_ = 730.7267062695; |
There was a problem hiding this comment.
常量的命名规则为:kXxxxXxxx,比如:kFy, kCx
| , cross_right_{Shape::Color::GREEN, 2, 0, 0, 0, 0, false} { | ||
| offset_x_ = get_parameter_or("offset_x", 0.000); | ||
| offset_y_ = get_parameter_or("offset_y", 0.000); | ||
| offset_z_ = get_parameter_or("offset_z", 0.202); |
| } | ||
|
|
||
| void update() override { | ||
| using namespace Eigen; |
| const double r4 = r2 * r2; | ||
| const double r6 = r4 * r2; | ||
|
|
||
| const double radial = 1.0 + k1_ * r2 + k2_ * r4 + k3_ * r6; |
There was a problem hiding this comment.
将重投影提取成一个函数 reproject( ... )
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
955cc52 to
2953667
Compare
- Rename camera constants to kXxxxXxxx convention - Move shape construction to member definition site - Change offset_z default to 0 - Remove using namespace Eigen, use explicit Eigen:: prefix - Extract reproject() function for coordinate projection pipeline
- Add optional `thread_config` support for the executor update thread - Log cumulative and 5s window timing stats for start lateness, update duration, skipped cycles, and top component costs - Vendor a local `TDigest` utility for percentile reporting - Switch affected packages to C++23 for `std::format` and `std::expected` - Allow starting the executor with an empty `components` list for testing
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
rmcs_ws/src/rmcs_executor/src/executor.hpp (1)
224-231: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value大幅落后时会重复输出累计统计日志(仅观感问题)。
当
current_time相对next_report_time落后超过一个上报间隔(例如长时间停顿导致跨越多个 5s 窗口)时,while循环会执行多次:首次迭代已reset_window_stats,后续迭代log_window_stats因executed_count == 0提前返回,但log_cumulative_stats仍会重复打印相同的累计内容。功能无碍,可按需在追补next_report_time时跳过中间空窗口。🤖 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_executor/src/executor.hpp` around lines 224 - 231, The issue is in log_due_reports, where the while loop can emit the same cumulative stats multiple times when current_time is far behind stats.next_report_time. Update the catch-up logic in log_due_reports so it advances stats.next_report_time across missed 5s intervals without repeatedly calling log_cumulative_stats for empty windows, or otherwise ensure only one cumulative log is produced per actual report cycle while still preserving the window reset behavior.
🤖 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/referee/app/ui/auto_aim.cpp`:
- Around line 66-71: The clamp_x and clamp_y lambdas in auto_aim.cpp are
clamping in uint16_t, which can wrap negative int values to the screen maximum
instead of 0. Update these helpers to first call std::clamp on the int input
using an int range, then cast the clamped result to std::uint16_t so negative
coordinates stay pinned to the left/top edge.
In `@rmcs_ws/src/rmcs_executor/CMakeLists.txt`:
- Around line 6-7: The CMakeLists.txt currently only sets the C++23 standard,
but it does not enforce the minimum compiler/standard library versions needed
for std::expected and std::format. Update the build configuration around
CMAKE_CXX_STANDARD/CMAKE_CXX_STANDARD_REQUIRED to also validate the toolchain
(for example in the rmcs_executor target or top-level CMake logic) and fail
early unless GCC/libstdc++ or libc++ meets the required minimum versions. Add
the same requirement to CI checks so the build cannot proceed on unsupported
environments.
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_executor/src/executor.hpp`:
- Around line 224-231: The issue is in log_due_reports, where the while loop can
emit the same cumulative stats multiple times when current_time is far behind
stats.next_report_time. Update the catch-up logic in log_due_reports so it
advances stats.next_report_time across missed 5s intervals without repeatedly
calling log_cumulative_stats for empty windows, or otherwise ensure only one
cumulative log is produced per actual report cycle while still preserving the
window reset behavior.
🪄 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: 24865d3a-6280-4160-bdc3-37c7f83e3ab7
📒 Files selected for processing (9)
rmcs_ws/src/rmcs_bringup/config/flight.yamlrmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/referee/app/ui/auto_aim.cpprmcs_ws/src/rmcs_executor/CMakeLists.txtrmcs_ws/src/rmcs_executor/src/executor.hpprmcs_ws/src/rmcs_executor/src/main.cpprmcs_ws/src/rmcs_utility/CMakeLists.txtrmcs_ws/src/rmcs_utility/include/rmcs_utility/tdigest.hpprmcs_ws/src/rmcs_utility/include/rmcs_utility/thread_config.hpp
| constexpr auto clamp_x = [](int x) { | ||
| return std::clamp<std::uint16_t>(x, 0, kScreenW - 1); | ||
| }; | ||
| constexpr auto clamp_y = [](int y) { | ||
| return std::clamp<std::uint16_t>(y, 0, kScreenH - 1); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 确认 clamp 辅助函数的调用方确实会产生负实参
sed -n '62,105p' rmcs_ws/src/rmcs_core/src/referee/app/ui/auto_aim.cppRepository: Alliance-Algorithm/RMCS
Length of output: 1998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and inspect the relevant region with line numbers.
git ls-files 'rmcs_ws/src/rmcs_core/src/referee/app/ui/auto_aim.cpp'
echo '---'
cat -n rmcs_ws/src/rmcs_core/src/referee/app/ui/auto_aim.cpp | sed -n '62,105p'
echo '---'
# Show all clamp_x/clamp_y references in the file for call-site context.
rg -n 'clamp_[xy]\(' rmcs_ws/src/rmcs_core/src/referee/app/ui/auto_aim.cppRepository: Alliance-Algorithm/RMCS
Length of output: 1969
先在 int 域内 clamp,避免负坐标回绕到屏幕最大边缘 std::clamp<std::uint16_t> 会先把 int 实参转成 uint16_t,所以靠近左上角时的负值会被夹到上界而不是 0。改成先 std::clamp(x, 0, ...),再转换为 uint16_t。
🤖 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/auto_aim.cpp` around lines 66 - 71,
The clamp_x and clamp_y lambdas in auto_aim.cpp are clamping in uint16_t, which
can wrap negative int values to the screen maximum instead of 0. Update these
helpers to first call std::clamp on the int input using an int range, then cast
the clamped result to std::uint16_t so negative coordinates stay pinned to the
left/top edge.
| set(CMAKE_CXX_STANDARD 23) | ||
| set(CMAKE_CXX_STANDARD_REQUIRED ON) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
minimum GCC and Clang versions for std::expected and std::format support
💡 Result:
The availability of std::expected and std::format depends primarily on the underlying standard library (libstdc++ for GCC, libc++ for Clang) rather than the compiler executable itself. std::format (C++20) - GCC: Supported starting with GCC 13 (via libstdc++) [1][2]. - Clang: Supported starting with Clang 14 when using libc++ [2]. Note that Clang users often encounter issues when using older versions of libstdc++; using -stdlib=libc++ is recommended for best compatibility [3][2][4]. std::expected (C++23) - GCC: Supported starting with GCC 13 (via libstdc++) [5]. - Clang: Supported in libc++ (the LLVM C++ standard library) [6]. Be aware that Clang often defaults to using the host system's libstdc++ (e.g., from an older GCC installation), which may lack support for C++23 features like std::expected even if the Clang version is modern [3][7][8]. Explicitly configuring Clang to use libc++ with the -stdlib=libc++ flag is often required to access this feature [3][7][8]. General Requirements For both features, you must compile with the appropriate C++ standard flag (e.g., -std=c++20 or -std=c++23) [9][2]. If you are using Clang and encounter "header not found" or "no template named" errors despite having a modern Clang version, it is typically because the linked standard library (often libstdc++) is outdated [3][8][4]. Switching to libc++ is the standard solution for Clang users on Linux to ensure compatibility with modern C++ library features [8][2].
Citations:
- 1: https://stackoverflow.com/questions/63724059/does-gcc-support-c20-stdformat
- 2: https://openillumi.com/en/en-std-format-gcc-clang-msvc-version-guide/
- 3: https://stackoverflow.com/questions/77634762/using-clang-with-c23-and-gcc-candiate-12-cannot-find-expected-header
- 4: https://stackoverflow.com/questions/79161039/clang-and-libstdc-compatibility-c20-format-header
- 5: https://en.cppreference.com/w/cpp/compiler_support/23
- 6: https://reviews.llvm.org/D124516
- 7: std::expected is unexpected disabled when parsing cpp file drivered by g++ clangd/clangd#1986
- 8:
std::expectedand related types are not implemented. llvm/llvm-project#62801 - 9: https://clang.llvm.org/cxx_status.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target CMake file and the headers mentioned in the comment.
for f in \
rmcs_ws/src/rmcs_executor/CMakeLists.txt \
rmcs_ws/src/rmcs_executor/include/thread_config.hpp \
rmcs_ws/src/rmcs_executor/include/executor.hpp
do
echo "===== $f ====="
wc -l "$f"
echo
done
echo "===== CMakeLists ====="
cat -n rmcs_ws/src/rmcs_executor/CMakeLists.txt
echo
echo "===== thread_config.hpp (relevant lines) ====="
rg -n "std::expected|std::format|`#include`" rmcs_ws/src/rmcs_executor/include/thread_config.hpp
echo
echo "===== executor.hpp (relevant lines) ====="
rg -n "std::expected|std::format|`#include`" rmcs_ws/src/rmcs_executor/include/executor.hpp
echo
echo "===== package metadata mentions ====="
rg -n "ubuntu|jammy|focal|humble|iron|jazzy|gcc|clang|c\\+\\+23|c\\+\\+20|std::expected|std::format" rmcs_ws/src/rmcs_executor -g '!**/build/**' -g '!**/install/**' -g '!**/log/**'Repository: Alliance-Algorithm/RMCS
Length of output: 1969
🌐 Web query:
libc++ std::expected minimum version and libstdc++ std::expected GCC version; libc++ std::format minimum version and libstdc++ std::format GCC version
💡 Result:
Support for std::expected and std::format in libc++ and libstdc++ is as follows: libc++: - std::expected: First released in libc++ 16.0 [1]. - std::format: First released in libc++ 14.0 (as part of the C++20 format library implementation) [2][3]. libstdc++: - std::expected: First supported in GCC 12 (libstdc++) [4][5]. - std::format: First supported in GCC 13 (libstdc++) [6]. Note: For both libraries, these features require compiling with appropriate C++ standard flags (typically -std=c++23 for std::expected and -std=c++20 or later for std::format) [4][6]. Earlier versions of libc++ required the -fexperimental-library flag to enable these features before they were considered stable [1][7].
Citations:
- 1: https://discourse.llvm.org/t/abi-break-in-libc-for-a-17-x-guidance-requested/74483
- 2: https://releases.llvm.org/19.1.0/projects/libcxx/docs/Status/Format.html
- 3: https://releases.llvm.org/15.0.0/projects/libcxx/docs/Status/Format.html
- 4: https://forums.raspberrypi.com/viewtopic.php?t=377738
- 5: https://sourceware.org/pipermail/gcc-patches/2025-August/693116.html
- 6: https://gcc.gnu.org/pipermail/libstdc++/2022-November/054991.html
- 7: https://discourse.llvm.org/t/std-format-not-available-in-libc-15/66137
补充 C++23 标准库最低版本约束
std::expected 需要 libstdc++(GCC 12+) 或 libc++ 16+,std::format 需要 libstdc++(GCC 13+) 或 libc++ 14+。Ubuntu 22.04 默认 GCC 11/libstdc++ 不满足要求,CI 和部署镜像若不升级编译器/标准库会直接编译失败;建议把最低工具链版本写入构建约束或 CI 检查中。
🤖 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_executor/CMakeLists.txt` around lines 6 - 7, The
CMakeLists.txt currently only sets the C++23 standard, but it does not enforce
the minimum compiler/standard library versions needed for std::expected and
std::format. Update the build configuration around
CMAKE_CXX_STANDARD/CMAKE_CXX_STANDARD_REQUIRED to also validate the toolchain
(for example in the rmcs_executor target or top-level CMake logic) and fail
early unless GCC/libstdc++ or libc++ meets the required minimum versions. Add
the same requirement to CI checks so the build cannot proceed on unsupported
environments.
feat: add UiRobotCenterOverlay for flight referee UI
本次变更主要为飞行裁判 UI 新增
UiRobotCenterOverlay/AutoAimUi相关能力,并配套完善执行器与运行时基础设施:AutoAimUi组件插件,实现基于/tf相机位姿、robot_center和should_shoot的屏幕投影与准星绘制逻辑;当输入无效或投影越界时隐藏图形,满足条件时显示圆环、十字线及状态颜色切换。rmcs_executor升级为 C++23,并重构执行线程调度:加入更新频率校验、线程配置应用、周期统计与 5 秒窗口的性能上报。ThreadConfig,支持解析并应用线程名、CPU 亲和力、调度策略、优先级和 nice。TDigest统计实现,用于记录更新耗时、迟到分布等运行指标。main中组件参数读取逻辑改为直接获取,不再显式在缺失时抛出异常。