Merged
Conversation
Contributor
总体概述本 PR 实现了从 OpenVinoNet 模型接口到新型 OpenVINO 插件式 ArmorDetection 架构的重大重构，支持多种模型（TongJiYoloV5、ShenZhen0526、ShenZhen0708），替换 ArmorInferResult 为数据驱动的 InferResultAdapter，并重构运行时控制流。 变更列表
序列图sequenceDiagram
participant User
participant ArmorDetection
participant ModelCompiler as compile_model_with
participant OpenVINO as ov::Core
participant Explainer as ExplainInterface
participant Result as InferResult
User->>ArmorDetection: initialize(yaml)
activate ArmorDetection
ArmorDetection->>ArmorDetection: parse model_location
ArmorDetection->>ModelCompiler: compile_model_with(model_type)
activate ModelCompiler
ModelCompiler->>OpenVINO: read_model(location)
OpenVINO-->>ModelCompiler: CompiledModel
deactivate ModelCompiler
ModelCompiler-->>ArmorDetection: compiled_model
ArmorDetection->>ArmorDetection: create ExplainInterface
deactivate ArmorDetection
User->>ArmorDetection: sync_detect(image)
activate ArmorDetection
ArmorDetection->>ArmorDetection: preprocess (ROI, resize)
ArmorDetection->>OpenVINO: infer(input_tensor)
OpenVINO-->>ArmorDetection: raw_output
ArmorDetection->>Explainer: explain_infer_result(output)
activate Explainer
Explainer->>Explainer: parse + sigmoid
Explainer->>Explainer: apply NMS
Explainer-->>Result: Armor2D vector
deactivate Explainer
ArmorDetection-->>User: Armor2Ds
deactivate ArmorDetection
代码审核工作量评估🎯 4 (复杂) | ⏱️ ~45 分钟 可能相关的 PR
建议标签
建议审核人
庆祝诗
✨ 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: 6
🧹 Nitpick comments (6)
src/utility/model/common_model.hpp (2)
108-123:noexcept标记与可能抛出异常的at()调用不一致
partial_shape()和shape()方法标记为noexcept，但内部调用的dimensions.at()在接收到无效维度字符时会抛出std::runtime_error。虽然
TensorLayout构造时已验证chars只包含有效字符（N/C/W/H），实际上不会触发异常，但从防御性编程角度，建议：
- 移除
noexcept标记，或- 在
Dimensions::at()中使用assert替代throw（因为这是编程错误而非运行时错误）♻️ 可选：使用 assert 替代 throw
constexpr auto at(char dimension) const -> Value { switch (dimension) { case 'N': return N; case 'C': return C; case 'W': return W; case 'H': return H; default: - throw std::runtime_error("Wrong dimension char, valid: N, C, W, H"); + assert(false && "Wrong dimension char, valid: N, C, W, H"); + return 0; // unreachable, for compiler satisfaction } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/model/common_model.hpp` around lines 108 - 123, The methods partial_shape and shape are marked noexcept but call Dimensions::at(), which may throw; remove the noexcept specifier from both partial_shape(const Dimensions&) and shape(const Dimensions&) to reflect potential exceptions (or alternatively change Dimensions::at() to use assert instead of throwing if you prefer treating invalid chars as programmer errors); ensure references to chars and the TensorLayout constructor validation remain intact and update any callers/tests if they relied on noexcept.
76-76: 静态断言消息与检查条件不一致断言检查
description.length() == 5，但错误消息说"必须恰好 4 个字符"。虽然这在技术上是正确的（StaticString的length()包含 null 终止符），但可能造成困惑。📝 建议：使消息更清晰
- static_assert(description.length() == 5, "Layout description must be exactly 4 characters"); + static_assert(description.length() == 5, "Layout description must be exactly 4 characters (length includes null terminator)");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/model/common_model.hpp` at line 76, The static_assert uses the condition description.length() == 5 but its message says "must be exactly 4 characters", causing confusion; update the static_assert message to match the check (e.g., "Layout description must be exactly 5 characters (including null terminator)") or otherwise clarify the expected length (mentioning the null terminator) next to the static_assert in common_model.hpp where description.length() is validated.src/runtime.cpp (1)
21-21: 使用实验性头文件<experimental/scope>
<experimental/scope>是 Library Fundamentals TS v3 的一部分，尚未标准化。不同编译器的支持程度可能不同（GCC 需要 libstdc++ 且可能需要特定版本）。如果需要更广泛的编译器兼容性，可考虑使用简单的 RAII 包装类或第三方库如
gsl::finally。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime.cpp` at line 21, The code currently includes the non-standard header <experimental/scope>, which is not portable; remove that include and replace uses of its facilities with a portable alternative—either use the C++23 standard std::scope_exit if your target supports it, or add a small RAII helper (e.g., a ScopeGuard/Finally class) to perform the same cleanup semantics; update any code referencing the experimental APIs to use std::scope_exit or your ScopeGuard class (same function/variable names where applicable) so the project builds across compilers without relying on <experimental/scope>.test/model_infer.cpp (1)
44-58:ExpectedCorners与Armor2D命名不一致
ExpectedCorners使用lt/lb/rb/rt（left-top/left-bottom/right-bottom/right-top），而Armor2D使用tl/tr/br/bl（top-left/top-right/bottom-right/bottom-left）。这种命名差异可能导致混淆。在
assert_sync_infer_with_expected中（lines 106-109），变量命名已经对齐到 Armor2D 的风格，但ExpectedCorners本身的字段和方法仍使用不同的命名。♻️ 建议：统一命名风格
struct ExpectedCorners { - float lt_x; - float lt_y; - float lb_x; - float lb_y; - float rb_x; - float rb_y; - float rt_x; - float rt_y; - - auto lt() const noexcept -> cv::Point2f { return { lt_x, lt_y }; } - auto lb() const noexcept -> cv::Point2f { return { lb_x, lb_y }; } - auto rb() const noexcept -> cv::Point2f { return { rb_x, rb_y }; } - auto rt() const noexcept -> cv::Point2f { return { rt_x, rt_y }; } + float tl_x; + float tl_y; + float bl_x; + float bl_y; + float br_x; + float br_y; + float tr_x; + float tr_y; + + auto tl() const noexcept -> cv::Point2f { return { tl_x, tl_y }; } + auto bl() const noexcept -> cv::Point2f { return { bl_x, bl_y }; } + auto br() const noexcept -> cv::Point2f { return { br_x, br_y }; } + auto tr() const noexcept -> cv::Point2f { return { tr_x, tr_y }; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/model_infer.cpp` around lines 44 - 58, ExpectedCorners uses lt/lb/rb/rt naming while Armor2D uses tl/tr/br/bl causing inconsistency; rename the struct fields (lt_x, lt_y, lb_x, lb_y, rb_x, rb_y, rt_x, rt_y) to tl_x, tl_y, bl_x, bl_y, br_x, br_y, tr_x, tr_y and rename the accessor methods lt(), lb(), rb(), rt() to tl(), bl(), br(), tr() (preserve signatures like auto tl() const noexcept -> cv::Point2f), then update all usages (e.g., in assert_sync_infer_with_expected and test constructions) to the new symbols to keep naming consistent with Armor2D.src/utility/model/armor_detection.hpp (2)
52-55: 让这个头文件自包含。这一版新引入了
std::span、std::array、std::ranges::*、std::memcpy、std::is_trivially_copyable_v和std::size_t，但本头没有显式包含对应标准库头。现在能编译的话也是依赖传递包含，后面只要 include 顺序一变就容易炸。📦 建议补齐直接依赖
`#include` "utility/robot/armor.hpp" +#include <algorithm> +#include <array> +#include <cstddef> +#include <cstring> +#include <iterator> +#include <span> +#include <type_traits> `#include` <opencv2/core/types.hpp>Also applies to: 58-69, 126-135
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/model/armor_detection.hpp` around lines 52 - 55, The header is not self-contained: functions like unsafe_from and types data_type/precision_type use std::span, std::array, std::ranges utilities, std::memcpy, std::is_trivially_copyable_v and std::size_t but the header omits their headers; update the top of src/utility/model/armor_detection.hpp to explicitly include the direct standard headers (<span> or <cstddef>/<span> as appropriate), <array>, <ranges> (if ranges algorithms are used), <cstring> for memcpy, <type_traits> for is_trivially_copyable_v and <cstddef> for size_t so unsafe_from, related blocks (lines ~58-69 and ~126-135) compile independently of include order and transitive includes.
7-8: 把模板契约前置，不要让错误散落在各个 accessor 里。这个类型现在既依赖固定字段名，也依赖
length()/memcpy()的布局约定，但这些前提都没有在模板入口声明。后面只要有新的ResultData字段名或布局稍微变化，错误就会在某个成员函数里以很长的模板回溯形式出现。建议用concept或类级static_assert把必需成员和布局约束集中起来。Also applies to: 15-55, 126-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/model/armor_detection.hpp` around lines 7 - 8, The template InferResultAdapter currently assumes the template parameter (e.g., ResultData) exposes specific fields and a contiguous memcpy-able layout, causing cryptic errors inside accessors; add a template-level contract — either a C++20 concept (e.g., ResultDataLike) or class-level static_asserts inside InferResultAdapter — that checks for required members/methods (presence of length() or length member, required named fields accessed by the accessors), that the type is std::is_trivially_copyable and sizeof matches expected element size/stride or that data() returns a contiguous pointer, and fail early with clear messages; update InferResultAdapter's template declaration to use the concept or include the static_asserts so violations are reported at instantiation rather than deep inside member functions (refer to InferResultAdapter and the accessors that rely on length()/memcpy()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@config/config.yaml`:
- Around line 39-43: Rename the misspelled YAML key "avaliable_models" to
"available_models" in the config (change the key under the model list), and
search the codebase for any references to "avaliable_models" and update them to
"available_models" (or add a compatibility fallback reading "avaliable_models"
if immediate rename would break runtime). Ensure the config consumer (the code
that reads model_location / available_models) uses the corrected key name.
In `@src/module/identifier/armor_detection.cpp`:
- Around line 278-288: sync_detect is declared noexcept but calls
generate_openvino_request and request.infer() which may throw; either remove
noexcept from sync_detect or wrap the body in a try/catch that catches
std::exception and other throwables and returns std::nullopt on error. Locate
sync_detect and implement one of these fixes referencing
generate_openvino_request, request.infer(), and explain_infer_result so thrown
exceptions won't escape; ensure any caught exception is logged or handled as
appropriate and the function returns std::nullopt.
- Around line 194-248: The function generate_openvino_request is marked noexcept
but calls operations that can throw (ov::Tensor construction and
openvino_model.create_infer_request()); remove the noexcept specifier from
generate_openvino_request or wrap the body in try/catch and convert exceptions
into std::unexpected<string> error results. Specifically, catch std::exception
(and any OpenVINO-specific exceptions) around the ov::Tensor creation and the
call to openvino_model.create_infer_request(), and on any catch return
std::unexpected with a clear error message; keep references to symbols
input_tensor, ov::Tensor, openvino_model.create_infer_request, request and
input_layout/input_dimensions to locate the changes.
- Around line 81-127: The explain(...) function is marked noexcept but performs
operations that may throw (e.g., boxes.push_back, cv::dnn::NMSBoxes,
final_result.reserve/push_back); either remove the noexcept specifier from the
explain method signature (the override of model_type::explain) or wrap the
entire body in a try-catch that catches std::exception (and optionally ...) and
returns an empty Armor2Ds on error; if choosing try-catch, ensure you catch
exceptions before they unwind (catch const std::exception& e and return {} —
optionally log e.what()) and maintain the Armor2Ds return semantics, and if
removing noexcept, update the declaration to drop noexcept so exceptions can
propagate normally.
In `@src/runtime.cpp`:
- Around line 240-249: AutoAimState::target (DeviceId) is left as UNKNOWN
because runtime.cpp creates AutoAimState without setting target and
predictor::Snapshot does not expose the device id; add a public getter (e.g.,
device_id() or getDeviceId()) to predictor::Snapshot (the constructor already
takes DeviceId) so callers can obtain the source DeviceId, update set_identity()
to reset or propagate target where appropriate, and modify the runtime.cpp block
that builds AutoAimState to assign state.target = snapshot.device_id() (or call
the new getter) before committing; reference Snapshot, its constructor, existing
public methods ekf_x()/time_stamp()/predict_at()/predicted_armors(),
set_identity(), and AutoAimState::target when making the changes.
In `@src/utility/model/armor_detection.hpp`:
- Line 13: The member `data` in armor_detection.hpp must be value-initialized to
avoid leaking previous-frame or uninitialized contents when an early return
occurs; change its declaration (in the class that currently has `data_type
data;`) to default-initialize (zero/empty) and ensure any adapter reuse sees a
clean `data`. Also modify `unsafe_from()` to return an explicit status (e.g.,
bool/optional/result) instead of silently returning on failure so callers can
handle errors; update call sites that rely on `bounding_rect()` / `armor_*()` to
check the new status before accessing `data`.
---
Nitpick comments:
In `@src/runtime.cpp`:
- Line 21: The code currently includes the non-standard header
<experimental/scope>, which is not portable; remove that include and replace
uses of its facilities with a portable alternative—either use the C++23 standard
std::scope_exit if your target supports it, or add a small RAII helper (e.g., a
ScopeGuard/Finally class) to perform the same cleanup semantics; update any code
referencing the experimental APIs to use std::scope_exit or your ScopeGuard
class (same function/variable names where applicable) so the project builds
across compilers without relying on <experimental/scope>.
In `@src/utility/model/armor_detection.hpp`:
- Around line 52-55: The header is not self-contained: functions like
unsafe_from and types data_type/precision_type use std::span, std::array,
std::ranges utilities, std::memcpy, std::is_trivially_copyable_v and std::size_t
but the header omits their headers; update the top of
src/utility/model/armor_detection.hpp to explicitly include the direct standard
headers (<span> or <cstddef>/<span> as appropriate), <array>, <ranges> (if
ranges algorithms are used), <cstring> for memcpy, <type_traits> for
is_trivially_copyable_v and <cstddef> for size_t so unsafe_from, related blocks
(lines ~58-69 and ~126-135) compile independently of include order and
transitive includes.
- Around line 7-8: The template InferResultAdapter currently assumes the
template parameter (e.g., ResultData) exposes specific fields and a contiguous
memcpy-able layout, causing cryptic errors inside accessors; add a
template-level contract — either a C++20 concept (e.g., ResultDataLike) or
class-level static_asserts inside InferResultAdapter — that checks for required
members/methods (presence of length() or length member, required named fields
accessed by the accessors), that the type is std::is_trivially_copyable and
sizeof matches expected element size/stride or that data() returns a contiguous
pointer, and fail early with clear messages; update InferResultAdapter's
template declaration to use the concept or include the static_asserts so
violations are reported at instantiation rather than deep inside member
functions (refer to InferResultAdapter and the accessors that rely on
length()/memcpy()).
In `@src/utility/model/common_model.hpp`:
- Around line 108-123: The methods partial_shape and shape are marked noexcept
but call Dimensions::at(), which may throw; remove the noexcept specifier from
both partial_shape(const Dimensions&) and shape(const Dimensions&) to reflect
potential exceptions (or alternatively change Dimensions::at() to use assert
instead of throwing if you prefer treating invalid chars as programmer errors);
ensure references to chars and the TensorLayout constructor validation remain
intact and update any callers/tests if they relied on noexcept.
- Line 76: The static_assert uses the condition description.length() == 5 but
its message says "must be exactly 4 characters", causing confusion; update the
static_assert message to match the check (e.g., "Layout description must be
exactly 5 characters (including null terminator)") or otherwise clarify the
expected length (mentioning the null terminator) next to the static_assert in
common_model.hpp where description.length() is validated.
In `@test/model_infer.cpp`:
- Around line 44-58: ExpectedCorners uses lt/lb/rb/rt naming while Armor2D uses
tl/tr/br/bl causing inconsistency; rename the struct fields (lt_x, lt_y, lb_x,
lb_y, rb_x, rb_y, rt_x, rt_y) to tl_x, tl_y, bl_x, bl_y, br_x, br_y, tr_x, tr_y
and rename the accessor methods lt(), lb(), rb(), rt() to tl(), bl(), br(), tr()
(preserve signatures like auto tl() const noexcept -> cv::Point2f), then update
all usages (e.g., in assert_sync_infer_with_expected and test constructions) to
the new symbols to keep naming consistent with Armor2D.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eef19187-fe34-4421-9a55-2645f0fb77fe
⛔ Files ignored due to path filters (1)
models/tongji-yolov5.binis excluded by!**/*.bin
📒 Files selected for processing (23)
.vscode/settings.jsonconfig/config.yamlmodels/shenzhen-0526.onnxmodels/shenzhen-0708.onnxmodels/tongji-yolov5.xmlsrc/kernel/pose_estimator.cppsrc/module/identifier/armor_detection.cppsrc/module/identifier/model.cppsrc/module/identifier/model.hppsrc/module/identifier/models/shenzhen.hppsrc/module/identifier/models/shenzhen_0526.hppsrc/module/identifier/models/shenzhen_0708.hppsrc/module/identifier/models/tongji_yolov5.hppsrc/runtime.cppsrc/utility/model/armor_detection.hppsrc/utility/model/common_model.hppsrc/utility/robot/armor.hppsrc/utility/string.hpptest/CMakeLists.txttest/model_infer.cpptest/solve_pnp.cpptool/CMakeLists.txttool/ov_model_status.cpp
💤 Files with no reviewable changes (3)
- .vscode/settings.json
- src/module/identifier/model.cpp
- src/module/identifier/model.hpp
Comment on lines
+39
to
+43
| avaliable_models: | ||
| - "tongji-yolov5.xml" | ||
| - "shenzhen-0526.onnx" | ||
| - "shenzhen-0708.onnx" | ||
| model_location: "shenzhen-0526.onnx" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 搜索代码中对此配置键的引用
rg -n "avaliable_models|available_models" --type cpp --type hpp
rg -n "avaliable_models|available_models" -g "*.cpp" -g "*.hpp"Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 106
🏁 Script executed:
#!/bin/bash
# 在所有文件中搜索配置键的引用
rg -i "avaliable_models|available_models" --type-list | head -20Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 468
🏁 Script executed:
#!/bin/bash
# 不限制文件类型，搜索所有引用
rg "avaliable_models|available_models" -nRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 120
🏁 Script executed:
#!/bin/bash
# 搜索YAML配置加载的代码
rg -n "config.yaml|avaliable_models|available_models" -g "*.py" -g "*.cpp" -g "*.cc"Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 204
🏁 Script executed:
#!/bin/bash
# 查看config.yaml的完整内容和相关模型配置的使用
cat config/config.yaml | head -50Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1115
🏁 Script executed:
#!/bin/bash
# 搜索代码中如何访问YAML配置，特别是模型相关的配置
rg -n "model|Model" config/config.yaml | head -20Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 143
🏁 Script executed:
#!/bin/bash
# 查找config.yaml的加载和使用
rg -A 5 -B 5 "config.yaml" -g "*.cpp" -g "*.cc" -g "*.h" | head -100Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1282
🏁 Script executed:
#!/bin/bash
# 搜索所有可能的Python配置加载代码
rg -n "avaliable_models|available_models" -g "*.py"Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 61
🏁 Script executed:
#!/bin/bash
# 搜索YAML相关的加载代码
rg -n "YAML\.Load|yaml\.load|config\[" -g "*.cpp" -g "*.py" | head -30Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 260
拼写错误：avaliable_models 应为 available_models
配置键 avaliable_models 存在拼写错误，正确拼写为 available_models。虽然当前代码未引用此键，但应修复此拼写错误以避免未来使用此配置时出现问题。
🐛 修复建议
- avaliable_models:
+ available_models:
- "tongji-yolov5.xml"
- "shenzhen-0526.onnx"
- "shenzhen-0708.onnx"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| avaliable_models: | |
| - "tongji-yolov5.xml" | |
| - "shenzhen-0526.onnx" | |
| - "shenzhen-0708.onnx" | |
| model_location: "shenzhen-0526.onnx" | |
| available_models: | |
| - "tongji-yolov5.xml" | |
| - "shenzhen-0526.onnx" | |
| - "shenzhen-0708.onnx" | |
| model_location: "shenzhen-0526.onnx" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/config.yaml` around lines 39 - 43, Rename the misspelled YAML key
"avaliable_models" to "available_models" in the config (change the key under the
model list), and search the codebase for any references to "avaliable_models"
and update them to "available_models" (or add a compatibility fallback reading
"avaliable_models" if immediate rename would break runtime). Ensure the config
consumer (the code that reads model_location / available_models) uses the
corrected key name.
Comment on lines
+81
to
+127
| auto explain(ov::InferRequest& finished_request) const noexcept -> Armor2Ds override { | ||
| using result_type = typename model_type::Result; | ||
| using precision_type = typename result_type::precision_type; | ||
|
|
||
| auto tensor = finished_request.get_output_tensor(); | ||
| auto& shape = tensor.get_shape(); | ||
|
|
||
| const auto rows = static_cast<std::size_t>(shape.at(1)); | ||
| const auto cols = static_cast<std::size_t>(shape.at(2)); | ||
| if (cols != result_type::length()) { | ||
| return {}; | ||
| } | ||
|
|
||
| auto parsed_results = std::vector<result_type> {}; | ||
| auto scores = std::vector<float> {}; | ||
| auto boxes = std::vector<cv::Rect> {}; | ||
|
|
||
| const auto* data = tensor.data<precision_type>(); | ||
| for (std::size_t row = 0; row < rows; row++) { | ||
| auto line = result_type {}; | ||
| line.unsafe_from(std::span { data + row * cols, cols }); | ||
| line.confidence() = util::sigmoid(line.confidence()); | ||
|
|
||
| if (line.confidence() > min_confidence) { | ||
| parsed_results.push_back(line); | ||
| scores.push_back(line.confidence()); | ||
| boxes.push_back(line.bounding_rect()); | ||
| } | ||
| } | ||
|
|
||
| auto kept_points = std::vector<int> {}; | ||
| cv::dnn::NMSBoxes(boxes, scores, score_threshold, nms_threshold, kept_points); | ||
|
|
||
| auto final_result = Armor2Ds {}; | ||
| final_result.reserve(kept_points.size()); | ||
|
|
||
| for (auto idx : kept_points) { | ||
| auto armor = cast_to_armor_result(parsed_results[idx]); | ||
| armor.tl *= 1.f / adapt_scaling; | ||
| armor.tr *= 1.f / adapt_scaling; | ||
| armor.br *= 1.f / adapt_scaling; | ||
| armor.bl *= 1.f / adapt_scaling; | ||
| armor.center = (armor.tl + armor.tr + armor.br + armor.bl) * 0.25f; | ||
| final_result.push_back(armor); | ||
| } | ||
| return final_result; | ||
| } |
Contributor
There was a problem hiding this comment.
noexcept 函数中存在可能抛出异常的操作
explain() 函数标记为 noexcept，但以下操作可能抛出异常：
- Line 107:
boxes.push_back()- 内存分配失败 - Line 112:
cv::dnn::NMSBoxes()- OpenCV 函数可能抛出异常 - Line 115, 124:
final_result.reserve()/push_back()- 内存分配
如果异常发生，程序将调用 std::terminate() 而非优雅处理。
🛡️ 建议：移除 noexcept 或添加 try-catch
- auto explain(ov::InferRequest& finished_request) const noexcept -> Armor2Ds override {
+ auto explain(ov::InferRequest& finished_request) const -> Armor2Ds override {或者包装异常处理：
auto explain(ov::InferRequest& finished_request) const noexcept -> Armor2Ds override {
+ try {
// ... existing code ...
+ } catch (...) {
+ return {};
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| auto explain(ov::InferRequest& finished_request) const noexcept -> Armor2Ds override { | |
| using result_type = typename model_type::Result; | |
| using precision_type = typename result_type::precision_type; | |
| auto tensor = finished_request.get_output_tensor(); | |
| auto& shape = tensor.get_shape(); | |
| const auto rows = static_cast<std::size_t>(shape.at(1)); | |
| const auto cols = static_cast<std::size_t>(shape.at(2)); | |
| if (cols != result_type::length()) { | |
| return {}; | |
| } | |
| auto parsed_results = std::vector<result_type> {}; | |
| auto scores = std::vector<float> {}; | |
| auto boxes = std::vector<cv::Rect> {}; | |
| const auto* data = tensor.data<precision_type>(); | |
| for (std::size_t row = 0; row < rows; row++) { | |
| auto line = result_type {}; | |
| line.unsafe_from(std::span { data + row * cols, cols }); | |
| line.confidence() = util::sigmoid(line.confidence()); | |
| if (line.confidence() > min_confidence) { | |
| parsed_results.push_back(line); | |
| scores.push_back(line.confidence()); | |
| boxes.push_back(line.bounding_rect()); | |
| } | |
| } | |
| auto kept_points = std::vector<int> {}; | |
| cv::dnn::NMSBoxes(boxes, scores, score_threshold, nms_threshold, kept_points); | |
| auto final_result = Armor2Ds {}; | |
| final_result.reserve(kept_points.size()); | |
| for (auto idx : kept_points) { | |
| auto armor = cast_to_armor_result(parsed_results[idx]); | |
| armor.tl *= 1.f / adapt_scaling; | |
| armor.tr *= 1.f / adapt_scaling; | |
| armor.br *= 1.f / adapt_scaling; | |
| armor.bl *= 1.f / adapt_scaling; | |
| armor.center = (armor.tl + armor.tr + armor.br + armor.bl) * 0.25f; | |
| final_result.push_back(armor); | |
| } | |
| return final_result; | |
| } | |
| auto explain(ov::InferRequest& finished_request) const -> Armor2Ds override { | |
| using result_type = typename model_type::Result; | |
| using precision_type = typename result_type::precision_type; | |
| auto tensor = finished_request.get_output_tensor(); | |
| auto& shape = tensor.get_shape(); | |
| const auto rows = static_cast<std::size_t>(shape.at(1)); | |
| const auto cols = static_cast<std::size_t>(shape.at(2)); | |
| if (cols != result_type::length()) { | |
| return {}; | |
| } | |
| auto parsed_results = std::vector<result_type> {}; | |
| auto scores = std::vector<float> {}; | |
| auto boxes = std::vector<cv::Rect> {}; | |
| const auto* data = tensor.data<precision_type>(); | |
| for (std::size_t row = 0; row < rows; row++) { | |
| auto line = result_type {}; | |
| line.unsafe_from(std::span { data + row * cols, cols }); | |
| line.confidence() = util::sigmoid(line.confidence()); | |
| if (line.confidence() > min_confidence) { | |
| parsed_results.push_back(line); | |
| scores.push_back(line.confidence()); | |
| boxes.push_back(line.bounding_rect()); | |
| } | |
| } | |
| auto kept_points = std::vector<int> {}; | |
| cv::dnn::NMSBoxes(boxes, scores, score_threshold, nms_threshold, kept_points); | |
| auto final_result = Armor2Ds {}; | |
| final_result.reserve(kept_points.size()); | |
| for (auto idx : kept_points) { | |
| auto armor = cast_to_armor_result(parsed_results[idx]); | |
| armor.tl *= 1.f / adapt_scaling; | |
| armor.tr *= 1.f / adapt_scaling; | |
| armor.br *= 1.f / adapt_scaling; | |
| armor.bl *= 1.f / adapt_scaling; | |
| armor.center = (armor.tl + armor.tr + armor.br + armor.bl) * 0.25f; | |
| final_result.push_back(armor); | |
| } | |
| return final_result; | |
| } |
🧰 Tools
🪛 Cppcheck (2.20.0)
[error] 110-110: Unhandled exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
[error] 118-118: Unhandled exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/module/identifier/armor_detection.cpp` around lines 81 - 127, The
explain(...) function is marked noexcept but performs operations that may throw
(e.g., boxes.push_back, cv::dnn::NMSBoxes, final_result.reserve/push_back);
either remove the noexcept specifier from the explain method signature (the
override of model_type::explain) or wrap the entire body in a try-catch that
catches std::exception (and optionally ...) and returns an empty Armor2Ds on
error; if choosing try-catch, ensure you catch exceptions before they unwind
(catch const std::exception& e and return {} — optionally log e.what()) and
maintain the Armor2Ds return semantics, and if removing noexcept, update the
declaration to drop noexcept so exceptions can propagate normally.
Comment on lines
+194
to
+248
| auto generate_openvino_request(const Image& image) noexcept | ||
| -> std::expected<ov::InferRequest, std::string> { | ||
|
|
||
| const auto& origin_mat = image.details().mat; | ||
| if (origin_mat.empty()) [[unlikely]] { | ||
| return std::unexpected { "Empty image mat" }; | ||
| } | ||
| const auto& raw_results = result.value(); | ||
|
|
||
| auto result_armors = std::vector<Armor2D> {}; | ||
| for (const auto& raw_armor : raw_results) { | ||
| auto armor = Armor2D {}; | ||
| armor.genre = raw_armor.armor_genre(); | ||
| armor.color = raw_armor.armor_color(); | ||
| auto segmentation = origin_mat; | ||
| if (config.use_roi_segment) { | ||
| auto action_success = false; | ||
| do { | ||
| const auto cols = config.roi_cols; | ||
| const auto rows = config.roi_rows; | ||
| if (cols > origin_mat.cols) break; | ||
| if (rows > origin_mat.rows) break; | ||
|
|
||
| armor.tl = raw_armor.tl(); | ||
| armor.tr = raw_armor.tr(); | ||
| armor.bl = raw_armor.bl(); | ||
| armor.br = raw_armor.br(); | ||
| const auto x = (origin_mat.cols - cols) / 2; | ||
| const auto y = (origin_mat.rows - rows) / 2; | ||
| const auto rect = cv::Rect2i { x, y, cols, rows }; | ||
|
|
||
| armor.confidence = raw_armor.confidence; | ||
| action_success = true; | ||
| segmentation = origin_mat(rect); | ||
| } while (false); | ||
|
|
||
| if (!action_success) { | ||
| return std::unexpected { "Failed to segment image" }; | ||
| } | ||
| } | ||
|
|
||
| constexpr auto devices = DeviceIds::kLargeArmor(); | ||
| armor.shape = devices.contains(armor.genre) ? ArmorShape::LARGE : ArmorShape::SMALL; | ||
| const auto rows = static_cast<int>(input_dimensions.H); | ||
| const auto cols = static_cast<int>(input_dimensions.W); | ||
| auto input_tensor = ov::Tensor { | ||
| ov::element::u8, | ||
| input_layout.shape(input_dimensions), | ||
| }; | ||
| { | ||
| adapt_scaling = std::min(static_cast<float>(1. * cols / segmentation.cols), | ||
| static_cast<float>(1. * rows / segmentation.rows)); | ||
|
|
||
| result_armors.push_back(armor); | ||
| const auto scaled_w = static_cast<int>(1. * segmentation.cols * adapt_scaling); | ||
| const auto scaled_h = static_cast<int>(1. * segmentation.rows * adapt_scaling); | ||
|
|
||
| auto input_mat = cv::Mat { rows, cols, CV_8UC3, input_tensor.data() }; | ||
| input_mat.setTo(cv::Scalar::all(0)); | ||
|
|
||
| auto input_roi = cv::Rect2i { 0, 0, scaled_w, scaled_h }; | ||
| cv::resize(segmentation, input_mat(input_roi), { scaled_w, scaled_h }); | ||
| } | ||
| return result_armors; | ||
|
|
||
| auto request = openvino_model.create_infer_request(); | ||
| request.set_input_tensor(input_tensor); | ||
|
|
||
| return request; | ||
| } |
Contributor
There was a problem hiding this comment.
generate_openvino_request 中存在可能抛出异常的操作
此函数标记为 noexcept，但以下操作可能抛出异常：
- Line 226-229:
ov::Tensor构造函数可能抛出 - Line 244:
openvino_model.create_infer_request()可能抛出 OpenVINO 异常
建议移除 noexcept 标记或添加异常处理。
🛡️ 建议修复
- auto generate_openvino_request(const Image& image) noexcept
+ auto generate_openvino_request(const Image& image)
-> std::expected<ov::InferRequest, std::string> {🧰 Tools
🪛 Cppcheck (2.20.0)
[error] 228-228: Unhandled exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/module/identifier/armor_detection.cpp` around lines 194 - 248, The
function generate_openvino_request is marked noexcept but calls operations that
can throw (ov::Tensor construction and openvino_model.create_infer_request());
remove the noexcept specifier from generate_openvino_request or wrap the body in
try/catch and convert exceptions into std::unexpected<string> error results.
Specifically, catch std::exception (and any OpenVINO-specific exceptions) around
the ov::Tensor creation and the call to openvino_model.create_infer_request(),
and on any catch return std::unexpected with a clear error message; keep
references to symbols input_tensor, ov::Tensor,
openvino_model.create_infer_request, request and input_layout/input_dimensions
to locate the changes.
Comment on lines
+278
to
288
| auto sync_detect(const Image& image) noexcept -> std::optional<Armor2Ds> { | ||
| auto result = generate_openvino_request(image); | ||
| if (!result.has_value()) { | ||
| return std::nullopt; | ||
| } | ||
|
|
||
| auto request = std::move(result.value()); | ||
| request.infer(); | ||
|
|
||
| return explain_infer_result(request); | ||
| } |
Contributor
There was a problem hiding this comment.
sync_detect 中调用可能抛出异常的方法
sync_detect 标记为 noexcept，但调用了 generate_openvino_request 和 request.infer()，这些都可能抛出异常。
🛡️ 建议修复
- auto sync_detect(const Image& image) noexcept -> std::optional<Armor2Ds> {
+ auto sync_detect(const Image& image) -> std::optional<Armor2Ds> {
+ try {
auto result = generate_openvino_request(image);
if (!result.has_value()) {
return std::nullopt;
}
auto request = std::move(result.value());
request.infer();
return explain_infer_result(request);
+ } catch (...) {
+ return std::nullopt;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| auto sync_detect(const Image& image) noexcept -> std::optional<Armor2Ds> { | |
| auto result = generate_openvino_request(image); | |
| if (!result.has_value()) { | |
| return std::nullopt; | |
| } | |
| auto request = std::move(result.value()); | |
| request.infer(); | |
| return explain_infer_result(request); | |
| } | |
| auto sync_detect(const Image& image) -> std::optional<Armor2Ds> { | |
| try { | |
| auto result = generate_openvino_request(image); | |
| if (!result.has_value()) { | |
| return std::nullopt; | |
| } | |
| auto request = std::move(result.value()); | |
| request.infer(); | |
| return explain_infer_result(request); | |
| } catch (...) { | |
| return std::nullopt; | |
| } | |
| } |
🧰 Tools
🪛 Cppcheck (2.20.0)
[error] 279-279: Unhandled exception thrown in function declared not to throw exceptions.
(throwInNoexceptFunction)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/module/identifier/armor_detection.cpp` around lines 278 - 288,
sync_detect is declared noexcept but calls generate_openvino_request and
request.infer() which may throw; either remove noexcept from sync_detect or wrap
the body in a try/catch that catches std::exception and other throwables and
returns std::nullopt on error. Locate sync_detect and implement one of these
fixes referencing generate_openvino_request, request.infer(), and
explain_infer_result so thrown exceptions won't escape; ensure any caught
exception is logged or handled as appropriate and the function returns
std::nullopt.
Comment on lines
+240
to
249
| auto state = AutoAimState {}; | ||
| state.timestamp = Clock::now(); | ||
| state.gimbal_takeover = true; | ||
| state.shoot_permitted = true; | ||
| state.yaw = control_cmd->yaw; | ||
| state.pitch = control_cmd->pitch; | ||
| if (!feishu.commit(state)) { | ||
| action_throttler.dispatch( | ||
| feishu_commit_label, [&] { rclcpp_node.warn("Commit auto_aim_state failed"); }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 查找 AutoAimState.target 的使用情况
rg -n "\.target" --type=cpp -C3 | grep -i "auto.*aim.*state\|autoaimstate" -i -A3 -B3Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Find AutoAimState definition in the codebase
find . -type f -name "*.hpp" -o -name "*.h" | xargs grep -l "AutoAimState" | head -5Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Look at the actual runtime.cpp file around the mentioned lines
cat -n src/runtime.cpp | sed -n '235,255p'Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 990
🏁 Script executed:
#!/bin/bash
# Search for all usages of target field in cpp files
rg "target\s*=" --type=cpp -C2 | head -100Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1163
🏁 Script executed:
#!/bin/bash
# Read the AutoAimState definition from context.hpp
cat -n src/utility/shared/context.hpp | grep -A 20 "struct AutoAimState"Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 724
🏁 Script executed:
#!/bin/bash
# Search more specifically for AutoAimState.target usage
rg "AutoAimState.*target|\.target.*AutoAim" --type=cpp -C5Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Also check what fields are accessed/set on AutoAimState in the codebase
rg "state\." --type=cpp src/runtime.cpp -C2 | head -80Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1447
🏁 Script executed:
#!/bin/bash
# Check what information the tracker exposes about the current target
rg "class Tracker|struct Tracker" --type=hpp -A 30 | head -100Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Look for any public methods on tracker that might give us target info
rg "tracker\." src/runtime.cpp -B 2 -A 2Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Check if snapshot has target/armor information
rg "snapshot->" src/runtime.cpp -B 1 -A 1 | head -60Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 229
🏁 Script executed:
#!/bin/bash
# Find Tracker class definition
find . -name "tracker.hpp" -o -name "tracker.h" | xargs cat | head -150Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 727
🏁 Script executed:
#!/bin/bash
# Look for any methods on tracker that return the tracked target
rg "decide|get_target|current_armor|tracked" --type=cpp -C 3 src/kernel/tracker.cpp | head -100Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 843
🏁 Script executed:
#!/bin/bash
# Check if predictor::Snapshot has target information
rg "class Snapshot|struct Snapshot" --type=cpp -A 20 | head -80Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 2806
🏁 Script executed:
#!/bin/bash
# Check if Snapshot has a device() getter method
rg "device()" src/module/predictor/snapshot.hpp -B 2 -A 2Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Look at full Snapshot public interface
cat src/module/predictor/snapshot.hpp | grep -A 100 "class Snapshot {" | head -60Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 797
🏁 Script executed:
#!/bin/bash
# Check how the snapshot is used elsewhere - maybe there's a pattern for getting device
rg "snapshot\." src/ --type=cpp -B 1 -A 1 | grep "device\|target" -iRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 482
🏁 Script executed:
#!/bin/bash
# Check if Snapshot::Impl::device is exposed through any public method
rg "pimpl->device|->device\(" src/module/predictor/snapshot.cppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Verify no other code accesses snapshot device info
rg "device_id|device\|target" src/runtime.cpp -C 2Repository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 61
🏁 Script executed:
#!/bin/bash
# Check the complete Snapshot header to see all public methods
cat src/module/predictor/snapshot.hppRepository: Alliance-Algorithm/rmcs_auto_aim_v2
Length of output: 1037
AutoAimState::target 字段未被设置，且缺乏获取目标信息的接口
AutoAimState 结构体中的 target 字段（DeviceId 类型，定义于 context.hpp:31）在此代码段中保持默认值 DeviceId::UNKNOWN，未被赋值。
更深层的问题是，即使想要填充此字段，也无法从可用的数据源中获取：predictor::Snapshot 虽在构造时接收 DeviceId 参数，但其公共接口中不包含获取该值的方法（仅暴露 ekf_x()、time_stamp()、predict_at()、predicted_armors()）。
建议：
- 若
target字段是必需的，则需在Snapshot中添加公共 getter 方法来暴露设备 ID - 同时考虑在
set_identity()方法中包含对target字段的重置逻辑（目前该方法不涉及target）
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/runtime.cpp` around lines 240 - 249, AutoAimState::target (DeviceId) is
left as UNKNOWN because runtime.cpp creates AutoAimState without setting target
and predictor::Snapshot does not expose the device id; add a public getter
(e.g., device_id() or getDeviceId()) to predictor::Snapshot (the constructor
already takes DeviceId) so callers can obtain the source DeviceId, update
set_identity() to reset or propagate target where appropriate, and modify the
runtime.cpp block that builds AutoAimState to assign state.target =
snapshot.device_id() (or call the new getter) before committing; reference
Snapshot, its constructor, existing public methods
ekf_x()/time_stamp()/predict_at()/predicted_armors(), set_identity(), and
AutoAimState::target when making the changes.
| auto rt() const noexcept { return Point { rt_x, rt_y }; } | ||
| auto lb() const noexcept { return Point { lb_x, lb_y }; } | ||
| } corners; | ||
| data_type data; |
Contributor
There was a problem hiding this comment.
不要在短输入上静默保留旧结果。
Line 54 直接返回，但 Line 13 的 data 也没有做值初始化。适配器复用时，这里会把上一帧甚至未初始化的内容继续暴露给 bounding_rect() / armor_*()，而调用方也拿不到失败信号。建议把 data 默认置零，并让 unsafe_from() 返回状态。
🩹 建议修改
- data_type data;
+ data_type data{};
- auto unsafe_from(std::span<const precision_type> raw) noexcept -> void {
+ [[nodiscard]] auto unsafe_from(std::span<const precision_type> raw) noexcept -> bool {
static_assert(std::is_trivially_copyable_v<data_type>);
- if (raw.size() < length()) return;
+ if (raw.size() < length()) {
+ data = {};
+ return false;
+ }
std::memcpy(&data, raw.data(), sizeof(data_type));
+ return true;
}Also applies to: 52-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utility/model/armor_detection.hpp` at line 13, The member `data` in
armor_detection.hpp must be value-initialized to avoid leaking previous-frame or
uninitialized contents when an early return occurs; change its declaration (in
the class that currently has `data_type data;`) to default-initialize
(zero/empty) and ensure any adapter reuse sees a clean `data`. Also modify
`unsafe_from()` to return an explicit status (e.g., bool/optional/result)
instead of silently returning on failure so callers can handle errors; update
call sites that rely on `bounding_rect()` / `armor_*()` to check the new status
before accessing `data`.
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.
No description provided.