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

Fix linker error to gst_base_sink_get_type #26

Conversation

RFRIEDM-Trimble
Copy link
Contributor

@RFRIEDM-Trimble RFRIEDM-Trimble commented May 29, 2022

This should actually fix the #12 by properly linking gstreamer libs.

  • Use PkgConfig populated variables correctly
  • Stop linking to strings that aren't targets
  • Add alias library for gscam_node

Note: ament_target_dependencies is on its way out in favor of modern cmake calls to target_link_libraries.
ament/ament_cmake#292

It's out of scope of this bug fix, but upgrading to modern cmake in this library would be favorable to make it easier and more robust to consume.

References:

* Use PkgConfig populated variables correctly
* Stop linking to strings that aren't targets
* Add alias library for gscam_node
@RFRIEDM-Trimble
Copy link
Contributor Author

CI failed on rolling due to this failure:

 -- Found ament_cmake: 1.2.1 (/opt/ros/rolling/share/ament_cmake/cmake)
  -- Found Python3: /usr/bin/python3.8 (found version "3.8.10") found components: Interpreter 
  -- Override CMake install command with custom implementation using symlinks instead of copying resources
  CMake Error at CMakeLists.txt:35 (find_package):
    By not providing "Findcamera_calibration_parsers.cmake" in
    CMAKE_MODULE_PATH this project has asked CMake to find a package
    configuration file provided by "camera_calibration_parsers", but CMake did
    not find one.
  
    Could not find a package configuration file provided by
    "camera_calibration_parsers" with any of the following names:
  
      camera_calibration_parsersConfig.cmake
      camera_calibration_parsers-config.cmake

In the galactic build, it is found here:

-- Found camera_calibration_parsers: 2.3.1 (/opt/ros/galactic/share/camera_calibration_parsers/cmake)

Will try to get a rolling environment set up to see if I can reproduce this issue.

@clydemcqueen
Copy link
Owner

clydemcqueen commented May 30, 2022

Hi, thanks for this contribution! LGTM+++

I'm learning a bunch of CMake by reading the changes the you made and diving around to understand the syntax. I am very open to any/all changes you'd like to propose.

CI failed on rolling due to this failure:

I think this is a bug in the CI, I am still building Rolling in Focal, not Jammy. See:

- docker_image: rostooling/setup-ros-docker:ubuntu-focal-ros-rolling-ros-base-latest

I will merge this PR as-is and then fix CI in a separate PR.

@clydemcqueen clydemcqueen merged commit 157b0ee into clydemcqueen:main May 30, 2022
@RFRIEDM-Trimble
Copy link
Contributor Author

Hi, thanks for this contribution! LGTM+++

I'm learning a bunch of CMake by reading the changes the you made and diving around to understand the syntax. I am very open to any/all changes you'd like to propose.

CI failed on rolling due to this failure:

I think this is a bug in the CI, I am still building Rolling in Focal, not Jammy. See:

- docker_image: rostooling/setup-ros-docker:ubuntu-focal-ros-rolling-ros-base-latest

I will merge this PR as-is and then fix CI in a separate PR.

Excellent! Thanks for merging. Hopefully the links I supplied are useful for your learning.

I tried replacing the libraries in ament_target_dependencies to another call of target_link_libraries as one should, however the upstream libraries don't support alias targets and Config mode exports (yet). I've filed the issue upstream here, so if I can get traction on that and implement it, then the modern cmake support can also be improved here.

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.

2 participants