export find_ros1_package cmake by Karsten1987 · Pull Request #164 · ros2/ros1_bridge

Karsten1987 · 2019-02-13T20:02:18Z

Signed-off-by: Karsten Knese karsten@openrobotics.org

I believe this is the correct fix for exporting the find_ros1_package.cmake. This allows other packages to use this macro straight after finding the ros1_bridge` package.
see: ros2/rosbag2#90 (comment)

please advise how to test this patch. Also I would appreciate if this could be backported to crystal.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

dirk-thomas · 2019-02-13T20:14:37Z

please advise how to test this patch.

You can test it with the referenced rosbag2 PR which shouldn't need the include calls anymore.

Karsten1987 · 2019-02-13T22:14:25Z

in order to compile these packages, the ros1_bridge has to be compiled. Which CI job could do that?

dirk-thomas · 2019-02-13T22:18:55Z

https://ci.ros2.org/job/ci_packaging_linux/

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2019-02-14T00:41:54Z

In order to use the cmake macros, the pkg-config dependency has to be exported with it.
I am not sure, but when interpreting rep140 correctly, the pkg-config dependency should be exported via build_export_depend. Please advise if not.

dirk-thomas · 2019-02-14T21:07:25Z

buildtool_export_depend - "tool" because pkg-config needs to be present on the host system rather than on the target system in case of cross compilation.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2019-02-22T04:38:54Z

comments are addressed and CI was setup here:

putting this in review

dirk-thomas · 2019-02-22T17:33:19Z

cmake/find_ros1_package.cmake

+    # call ament_package() to avoid ament_tools treating this
+    # as a plain CMake pkg
+    ament_package()
+    return()


This CMake extra file can't call ament_package(). Instead the find_package(PkgConfig) call should be REQUIRED.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2019-02-22T23:30:16Z

new CI:

dirk-thomas · 2019-02-22T23:43:08Z

CMakeLists.txt


-ament_package()
+ament_package(
+  CONFIG_EXTRAS cmake/find_ros1_package.cmake cmake/find_ros1_interface_packages.cmake


Nitpick: maybe move the duplicated arguments into a variable?

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

dirk-thomas · 2019-02-26T17:53:07Z

CMakeLists.txt

 find_package(std_msgs REQUIRED)

 # find ROS 1 packages
+list(APPEND ros1_cmake_macros cmake/find_ros1_package.cmake cmake/find_ros1_interface_packages.cmake)


Since the variable ros1_cmake_macros doesn't exist yet use set() instead. Based on the usage I would also suggest to name the variable cmake_extra_files.

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 · 2019-02-27T00:37:01Z

latest CI:

merging :)

* export find_ros1_package cmake Signed-off-by: Karsten Knese <karsten@openrobotics.org> * export pkg-config dependency Signed-off-by: Karsten Knese <karsten@openrobotics.org> * use buildtool_export_depend Signed-off-by: Karsten Knese <karsten@openrobotics.org> * make pkg-config required Signed-off-by: Karsten Knese <karsten@openrobotics.org> * also export cmake macros if ros1 is not found Signed-off-by: Karsten Knese <karsten@openrobotics.org> * set cmake files in variable Signed-off-by: Karsten Knese <karsten@openrobotics.org> * use set instead of list Signed-off-by: Karsten Knese <karsten@openrobotics.org>

* export find_ros1_package cmake Signed-off-by: Karsten Knese <karsten@openrobotics.org> * export pkg-config dependency Signed-off-by: Karsten Knese <karsten@openrobotics.org> * use buildtool_export_depend Signed-off-by: Karsten Knese <karsten@openrobotics.org> * make pkg-config required Signed-off-by: Karsten Knese <karsten@openrobotics.org> * also export cmake macros if ros1 is not found Signed-off-by: Karsten Knese <karsten@openrobotics.org> * set cmake files in variable Signed-off-by: Karsten Knese <karsten@openrobotics.org> * use set instead of list Signed-off-by: Karsten Knese <karsten@openrobotics.org> Signed-off-by: Dhananjay Sathe <dhananjay.sathe@rapyuta-robotics.com>

Karsten1987 self-assigned this Feb 13, 2019

Karsten1987 added the in progress Actively being worked on (Kanban column) label Feb 13, 2019

export find_ros1_package cmake

2c5ff62

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 force-pushed the export_cmake_file branch from fa1d4d1 to 2c5ff62 Compare February 13, 2019 20:03

export pkg-config dependency

81c939d

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 mentioned this pull request Feb 14, 2019

check ros1 deps correctly on all packages DensoADAS/rosbag2#1

Merged

This was referenced Feb 19, 2019

removed dependency to ros1_bridge package ros2/rosbag2#90

Merged

Crystal Patch Release 3 ros2/ros2#656

Closed

use buildtool_export_depend

5e02ce1

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

Karsten1987 added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Feb 22, 2019

dirk-thomas reviewed Feb 22, 2019

View reviewed changes

Karsten1987 added 2 commits February 22, 2019 14:15

make pkg-config required

d5e2f02

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

also export cmake macros if ros1 is not found

4eb0929

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

dirk-thomas reviewed Feb 22, 2019

View reviewed changes

Karsten1987 mentioned this pull request Feb 23, 2019

Post rosbridge pkgs ros2/ci#249

Merged

This comment has been minimized.

Sign in to view

set cmake files in variable

0769a4f

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

dirk-thomas reviewed Feb 26, 2019

View reviewed changes

use set instead of list

587ab84

Signed-off-by: Karsten Knese <karsten@openrobotics.org>

dirk-thomas approved these changes Feb 26, 2019

View reviewed changes

Karsten1987 merged commit 70c3735 into master Feb 27, 2019

Karsten1987 removed the in review Waiting for review (Kanban column) label Feb 27, 2019

Karsten1987 deleted the export_cmake_file branch February 27, 2019 00:37

Karsten1987 mentioned this pull request Mar 4, 2019

Failed to load entry point 'play': cannot import name 'NODE_NAME_PREFIX' ros2/rosbag2#92

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

export find_ros1_package cmake#164

export find_ros1_package cmake#164
Karsten1987 merged 7 commits intomasterfrom
export_cmake_file

Karsten1987 commented Feb 13, 2019

Uh oh!

dirk-thomas commented Feb 13, 2019

Uh oh!

Karsten1987 commented Feb 13, 2019

Uh oh!

dirk-thomas commented Feb 13, 2019

Uh oh!

Karsten1987 commented Feb 14, 2019

Uh oh!

dirk-thomas commented Feb 14, 2019

Uh oh!

Karsten1987 commented Feb 22, 2019

Uh oh!

dirk-thomas Feb 22, 2019

Uh oh!

Karsten1987 commented Feb 22, 2019

Uh oh!

dirk-thomas Feb 22, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

dirk-thomas Feb 26, 2019

Uh oh!

Karsten1987 commented Feb 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Karsten1987 commented Feb 13, 2019

Uh oh!

dirk-thomas commented Feb 13, 2019

Uh oh!

Karsten1987 commented Feb 13, 2019

Uh oh!

dirk-thomas commented Feb 13, 2019

Uh oh!

Karsten1987 commented Feb 14, 2019

Uh oh!

dirk-thomas commented Feb 14, 2019

Uh oh!

Karsten1987 commented Feb 22, 2019

Uh oh!

dirk-thomas Feb 22, 2019

Choose a reason for hiding this comment

Uh oh!

Karsten1987 commented Feb 22, 2019

Uh oh!

dirk-thomas Feb 22, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

dirk-thomas Feb 26, 2019

Choose a reason for hiding this comment

Uh oh!

Karsten1987 commented Feb 27, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants