Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

image_proc's TrackMarkerNode requires OpenCV 4.5.1 or greater #1030

Closed
tedsteiner opened this issue Sep 4, 2024 · 4 comments
Closed

image_proc's TrackMarkerNode requires OpenCV 4.5.1 or greater #1030

tedsteiner opened this issue Sep 4, 2024 · 4 comments
Labels

Comments

@tedsteiner
Copy link
Contributor

The TrackMarkerNode includes the OpenCV header <opencv2/core/quaternion.hpp>. Currently the image_proc CMakeLists.txt file specifies that OpenCV 3.2.0 is the minimum required version. However, quaternion.hpp wasn't added to OpenCV until version 4.5.1. The result is a compiler error on earlier versions of OpenCV, such as those provided by Ubuntu 20.04 (4.2.0).

At first I thought this was maybe just a header left unintentionally because it's a bit subtle, but here's where the OpenCV quaternion header is used:

      // Convert angle-axis to quaternion
      cv::Quatd q = cv::Quatd::createFromRvec(rvecs[0]);
      pose.pose.orientation.x = q.x;
      pose.pose.orientation.y = q.y;
      pose.pose.orientation.z = q.z;
      pose.pose.orientation.w = q.w;
      pub_->publish(pose);

Is there maybe some other code that already exists somewhere to do this conversion without using the cv::Quatd class?

This causes a build error when trying to build ROS 2 Jazzy from source with the perception variant on Ubuntu 20.04 with the Apt-provided version of libopencv-dev. The original merge request was here: #930

@mikeferguson
Copy link
Member

A couple thoughts:

  • We should fix the opencv version requirement on the Jazzy/rolling branches to be correct - that is an oversight
  • Jazzy targets 24.04, 20.04 is pretty old for us to also target.
  • It looks like even 22.04 has opencv 4.5.4 - so that meets the minimum all the way back to Humble (which, didn't actually get this feature backported)

@mikeferguson
Copy link
Member

Note: this is not to say we wouldn't merge a PR that addressed this so it would run on 20.04 - it's just not something the maintainers are likely to find time to tackle

@tedsteiner
Copy link
Contributor Author

Thanks for the quick response! I appreciate that 20.04 is pretty old to still support.

Instead of using OpenCV for this conversion, how about using TF2, which is already a dependency of image_pipeline (both depth_image_proc and image_rotate)? It looks like this function would do the same thing: https://docs.ros.org/en/jazzy/p/tf2/generated/classtf2_1_1Quaternion.html#_CPPv4N3tf210Quaternion10QuaternionERK7Vector3RK9tf2Scalar.

If so, it should be a pretty simple change. But I don't use TrackMarkerNode and it doesn't appear to have unit tests so if I did make the change I wouldn't be able to verify that it has the same behavior.

@hacker1024
Copy link

On a similar note, OpenCV > 4.6.0 doesn't work with this node either, as the ArUco API changed.

mikeferguson pushed a commit that referenced this issue Oct 29, 2024
The OpenCV quaternion class was not added until OpenCV 4.5.1, so it's
less widely available than the TF2 conversion. This change allows a
source build of the ROS 2 "perception" variant on Ubuntu 20.04 without a
custom source build of OpenCV.

Addresses issue
#1030
mergify bot pushed a commit that referenced this issue Oct 31, 2024
The OpenCV quaternion class was not added until OpenCV 4.5.1, so it's
less widely available than the TF2 conversion. This change allows a
source build of the ROS 2 "perception" variant on Ubuntu 20.04 without a
custom source build of OpenCV.

Addresses issue
#1030

(cherry picked from commit 4993e05)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants