Skip to content

refactor: split aruco detector to ros and lib - #13

Open
AntTra wants to merge 15 commits into
mainfrom
refactor/lib-ros
Open

refactor: split aruco detector to ros and lib#13
AntTra wants to merge 15 commits into
mainfrom
refactor/lib-ros

Conversation

@AntTra

@AntTra AntTra commented Mar 9, 2026

Copy link
Copy Markdown

No description provided.

@codecov

codecov Bot commented Mar 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 55 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (3656250) to head (9be7306).

Files with missing lines Patch % Lines
aruco-detector/src/lib/aruco_file_logger.cpp 0.00% 34 Missing ⚠️
aruco-detector/src/ros/aruco_detector_ros.cpp 0.00% 18 Missing ⚠️
aruco-detector/src/lib/aruco_detector.cpp 0.00% 3 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main     #13   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          4       4           
  Lines        282     291    +9     
  Branches      28      29    +1     
=====================================
- Misses       282     291    +9     
Flag Coverage Δ
unittests 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../include/aruco_detector/ros/aruco_detector_ros.hpp 0.00% <ø> (ø)
aruco-detector/src/lib/aruco_detector.cpp 0.00% <0.00%> (ø)
aruco-detector/src/ros/aruco_detector_ros.cpp 0.00% <0.00%> (ø)
aruco-detector/src/lib/aruco_file_logger.cpp 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jorgenfj jorgenfj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dictionary_map declared in aruco_detector_ros.hpp should be moved to /lib.
Also logging functionality should be moved to lib/ and be made ros independent.

I also have a few pointers for a more fine-grained refactor. Let me know if that is wanted

@kluge7 kluge7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As part of this pull request, this issue will also be implemented: #11

@jorgenfj

Copy link
Copy Markdown
Contributor

As part of this pull request, this issue will also be implemented: #11

Aha! You have completed all your tasks and have been awared with more work!

Good job

@jorgenfj

Copy link
Copy Markdown
Contributor

I think it makes sense to make some of the cluttered return types into structs instead of tuples.
We should have one simple function to detect aruco markers that is just a wrapper around the OpenCV call. Then we can make the other functions like estimate pose and estimate board pose, input the marker corners and ids from the initial detect aruco markers call. The estimate pose and estimate board pose should return the vortex::utils::types::Pose type ideally

@AntTra
AntTra force-pushed the refactor/lib-ros branch from 6f89401 to 62d5054 Compare March 25, 2026 15:44
@AntTra
AntTra force-pushed the refactor/lib-ros branch from 62d5054 to bc3019a Compare March 25, 2026 15:45
@AntTra
AntTra force-pushed the refactor/lib-ros branch from 027614e to f300672 Compare April 17, 2026 20:37
@AntTra
AntTra force-pushed the refactor/lib-ros branch from ae5ad1d to 97a54e2 Compare April 19, 2026 18:25
@jorgenfj

jorgenfj commented May 2, 2026

Copy link
Copy Markdown
Contributor

Does it make sense to split standard aruco marker detection and the board detection into two separate classes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor ArUco detector to separate ROS interface and core library Make ArUco logging thresholds and blacklist configurable through params

4 participants