First version by MatthijsBurgh · Pull Request #1 · tue-robotics/yolo_onnx_ros

MatthijsBurgh · 2025-06-24T19:22:13Z

No description provided.

…ssion in case of multiple inputs (to avoid necessary initializations).

…rence)

…ages

… ed_sensor_integration

…ctoringYolo

…he executable)

Copilot

Pull request overview

This PR transforms a standalone YOLOv8 ONNX inference C++ implementation into a ROS (Robot Operating System) package called yolo_onnx_ros. The changes refactor the codebase to follow ROS conventions, improve modularity, add comprehensive testing, and update build configuration for catkin integration.

Key Changes:

Restructured code into ROS package with proper header organization and catkin build system
Refactored member variables to use trailing underscore naming convention
Added unit tests using Google Test framework for YOLO inference functionality

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
CMakeLists.txt	Complete rebuild system migration from standalone CMake to catkin with testing support and automated model downloads
package.xml	New ROS package manifest defining dependencies and metadata
src/yolo_inference.cpp	Core inference logic with member variable renaming, input validation, and dimension order fixes
include/yolo_onnx_ros/yolo_inference.hpp	Header refactored with trailing underscore convention and CUDA macro updates
src/detection.cpp	New detection utilities extracted from main, including YAML parsing and initialization functions
include/yolo_onnx_ros/detection.hpp	New header exposing detection API functions
src/main.cpp	Simplified main function using new detection API
test/yolo_test.cpp	New comprehensive unit tests for YOLO inference pipeline
include/yolo_onnx_ros/config.hpp.in	CMake-generated configuration template for CUDA feature detection
data/open-images-v7.yaml	Reference dataset configuration with 601 class definitions
README.md	Updated documentation for ROS integration and CUDA version requirements
.vscode/*	VSCode configuration files for development environment
.github/workflows/main.yml	GitHub Actions CI workflow using TUe robotics infrastructure

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-25T21:16:51Z

src/detection.cpp

+                cv::RNG rng(cv::getTickCount());
+                cv::Scalar color(rng.uniform(0, 256), rng.uniform(0, 256), rng.uniform(0, 256));
+
+                cv::rectangle(img, re.box, color, 3);


The function draws on a const-qualified cv::Mat. The img parameter is passed as const cv::Mat& (line 8), but cv::rectangle modifies the image. This will cause a compilation error. Change the parameter to cv::Mat& to allow modification.

Copilot · 2025-11-25T21:16:51Z

test/yolo_test.cpp

+        cv::randu(testImage_realistic, cv::Scalar(0, 0, 0), cv::Scalar(255, 255, 255));
+
+        // Setup common parameters
+        std::tie(yolo, params) = Initialize("./yolo11m.onnx");


The Initialize function expects a std::filesystem::path but is called with a C-string literal. While this may work due to implicit conversion, it's more explicit to construct the path: Initialize(std::filesystem::path(\"./yolo11m.onnx\")).

Copilot · 2025-11-25T21:16:51Z

src/yolo_inference.cpp


 template<typename N>
-char* YOLO_V8::TensorProcess(clock_t& starttime_1, const cv::Mat& iImg, N& blob, std::vector<int64_t>& inputNodeDims,
+char* YOLO_V8::TensorProcess(clock_t& starttime_1, const cv::Mat&, N& blob, std::vector<int64_t>& inputNodeDims,


The unnamed parameter (const cv::Mat&) is confusing. If the parameter is intentionally unused, name it with a comment explaining why, or use [[maybe_unused]] attribute: [[maybe_unused]] const cv::Mat& iImg.

Suggested change

char* YOLO_V8::TensorProcess(clock_t& starttime_1, const cv::Mat&, N& blob, std::vector<int64_t>& inputNodeDims,

char* YOLO_V8::TensorProcess(clock_t& starttime_1, [[maybe_unused]] const cv::Mat& iImg, N& blob, std::vector<int64_t>& inputNodeDims,

Copilot · 2025-11-25T21:16:52Z

src/yolo_inference.cpp

-        cv::dnn::NMSBoxes(boxes, confidences, rectConfidenceThreshold, iouThreshold, nmsResult);
-        for (int i = 0; i < nmsResult.size(); ++i)
+        cv::dnn::NMSBoxes(boxes, confidences, rectConfidenceThreshold_, iouThreshold_, nmsResult);
+        for (uint i = 0; i < nmsResult.size(); ++i)


Using uint instead of size_t for vector iteration is non-standard. Replace with size_t or use a range-based for loop to avoid type mismatch warnings and ensure portability.

Suggested change

for (uint i = 0; i < nmsResult.size(); ++i)

for (size_t i = 0; i < nmsResult.size(); ++i)

Copilot · 2025-11-25T21:16:52Z

src/yolo_inference.cpp


        DL_RESULT result;
-        for (int i = 0; i < this->classes.size(); i++)
+        for (uint i = 0; i < this->classes.size(); i++)


Using uint instead of size_t for vector iteration is non-standard. Replace with size_t or use a range-based for loop to avoid type mismatch warnings and ensure portability.

Suggested change

for (uint i = 0; i < this->classes.size(); i++)

for (size_t i = 0; i < this->classes.size(); i++)

Copilot · 2025-11-25T21:16:52Z

include/yolo_onnx_ros/detection.hpp

+#include "yolo_onnx_ros/yolo_inference.hpp"
+
+#include <filesystem>
+


The commented-out #define LOGGING suggests this is used for conditional compilation, but there's no documentation explaining when/why a developer should enable it. Add a comment explaining its purpose and usage.

Suggested change

// Uncomment the following line to enable additional logging output for debugging purposes.

// When LOGGING is defined, diagnostic messages will be printed to help trace execution and troubleshoot issues.

// Enable this during development or debugging; leave it commented out in production for better performance and less verbosity.

Copilot · 2025-11-25T21:16:53Z

src/yolo_inference.cpp

    {
-        int signalResultNum = outputNodeDims[1];//84
-        int strideNum = outputNodeDims[2];//8400
+        int signalResultNum = outputNodeDims[1]; // Should be 605 for OIV7 (4 bbox + 601 classes)


The comment mentions OIV7 with 605 (4 bbox + 601 classes), but this appears to be dataset-specific. Add clarification that this value varies by dataset (e.g., 84 for COCO: 4 bbox + 80 classes).

Suggested change

int signalResultNum = outputNodeDims[1]; // Should be 605 for OIV7 (4 bbox + 601 classes)

int signalResultNum = outputNodeDims[1]; // Dataset-specific: e.g., 605 for OIV7 (4 bbox + 601 classes), 84 for COCO (4 bbox + 80 classes)

…(flexible onnx dims) on muptiple pretrained datasets (flexible yaml reading files)

MatthijsBurgh and others added 30 commits June 24, 2025 21:16

WIP: not compiling yet because of dependency

b5b5ec1

(actions) add CI

e8a2678

Include with <> for onnx header

a1e06ed

Updates gtests on the CMakeFile and made Preprocess public method

42e6125

Created initial tests and gitignore file

9d60544

Created ONNX Yolo run tests

3914fc6

Renamed tests

0280491

Small test update

2e9fb55

Use test Fixtures

38ad695

Updated readme file

723a133

Commented out the classification parts

686ea7f

Add inference engine calling functionality

2cac602

Use inference to call the function from a single point

a4038fc

Used Initilialize function to create a session outside the running se…

deb5227

…ssion in case of multiple inputs (to avoid necessary initializations).

Updated ReadmeFile

d5bcc12

Working switch between cuda and non cuda; code not working

22da61d

Made the yolo a catkin package

63e4f11

Renamed inference files to detection

1742156

Create single point of contact for Initialization and Detection (infe…

ef1f775

…rence)

Refactored test according to detection.h way of inference

8a9cfc6

Aligned Preprocessing and ONNX inference height width dimensions

34a82df

Fixed the results vector in order not too keep boxes from previous im…

2318110

…ages

Change CMake to be catkin package and small overall changes to run on…

afb8972

… ed_sensor_integration

Merge remote-tracking branch 'origin/first_version' into feature/Refa…

06439ac

…ctoringYolo

fix main.cpp

ec2c4c7

Correct logic for cuda; Not compiling

266ebda

Ignore onnx models

5c01ab4

Make linking conditional based on CUDA

acd9b51

Fix conditional cuda

38032f7

(actions) allow manual trigger

8e69019

Fix header locations

ad9cf19

MatthijsBurgh force-pushed the first_version branch from 4ecaf3c to ad9cf19 Compare October 28, 2025 10:33

Added optional argument for images (you need to provide it only for t…

90afade

…he executable)

IasonTheodorou force-pushed the first_version branch from c4770a4 to 90afade Compare October 28, 2025 17:25

MatthijsBurgh added 6 commits November 16, 2025 16:41

fix(cmake): linking to gmock

5b8f5c4

Add google-mock test dep

dda02a7

chore(test): results doesn't need to be in the class

0759554

Remove useless test, which breaks the tests

f4e6bed

Fix linking of test

1cb8161

model name needs to be provided to the test

96ef5cf

IasonTheodorou force-pushed the first_version branch from b87a311 to 6226976 Compare November 25, 2025 19:57

MatthijsBurgh marked this pull request as ready for review November 25, 2025 21:14

MatthijsBurgh requested review from IasonTheodorou and Copilot and removed request for IasonTheodorou November 25, 2025 21:15

Copilot AI reviewed Nov 25, 2025

View reviewed changes

IasonTheodorou and others added 3 commits November 25, 2025 22:21

Expanded implementation to be able to support multiple YOLO versions …

0935746

…(flexible onnx dims) on muptiple pretrained datasets (flexible yaml reading files)

Fix yolo tests

18f019a

Fix catkin_lint test

cf92346

MatthijsBurgh force-pushed the first_version branch from f669f5e to cf92346 Compare November 25, 2025 21:21

IasonTheodorou previously approved these changes Nov 25, 2025

View reviewed changes

Add missing include guard

2311a75

MatthijsBurgh dismissed IasonTheodorou’s stale review via 2311a75 November 26, 2025 09:03

MatthijsBurgh requested a review from IasonTheodorou November 27, 2025 12:26

Resolved some PR comments

e6e5bc5

MatthijsBurgh force-pushed the first_version branch from 2e3dba4 to e6e5bc5 Compare November 27, 2025 12:27

MatthijsBurgh enabled auto-merge November 27, 2025 12:27

IasonTheodorou approved these changes Nov 27, 2025

View reviewed changes

MatthijsBurgh merged commit 3d32ce6 into master Nov 27, 2025
2 checks passed

IasonTheodorou deleted the first_version branch November 27, 2025 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

First version#1

First version#1
MatthijsBurgh merged 51 commits intomasterfrom
first_version

MatthijsBurgh commented Jun 24, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Copilot AI Nov 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	char* YOLO_V8::TensorProcess(clock_t& starttime_1, const cv::Mat&, N& blob, std::vector<int64_t>& inputNodeDims,
	char* YOLO_V8::TensorProcess(clock_t& starttime_1, [[maybe_unused]] const cv::Mat& iImg, N& blob, std::vector<int64_t>& inputNodeDims,

	for (uint i = 0; i < nmsResult.size(); ++i)
	for (size_t i = 0; i < nmsResult.size(); ++i)

	for (uint i = 0; i < this->classes.size(); i++)
	for (size_t i = 0; i < this->classes.size(); i++)

		#include "yolo_onnx_ros/yolo_inference.hpp"

		#include <filesystem>

+// Uncomment the following line to enable additional logging output for debugging purposes.
+// When LOGGING is defined, diagnostic messages will be printed to help trace execution and troubleshoot issues.
+// Enable this during development or debugging; leave it commented out in production for better performance and less verbosity.

	int signalResultNum = outputNodeDims[1]; // Should be 605 for OIV7 (4 bbox + 601 classes)
	int signalResultNum = outputNodeDims[1]; // Dataset-specific: e.g., 605 for OIV7 (4 bbox + 601 classes), 84 for COCO (4 bbox + 80 classes)

Comments

Conversation

MatthijsBurgh commented Jun 24, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants