added rosidl_runtime c and cpp depencencies #100

ahcorde · 2020-03-04T15:08:47Z

This PR is related to the changes introduced in this PR ros2/rosidl#442. The full process can be followed here ros2/rosidl#443

@wjwwood @dirk-thomas

Signed-off-by: ahcorde ahcorde@gmail.com

Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde · 2020-03-06T10:24:35Z

rosidl_runtime_c headers are used only for testing as you can see in the CMakeLists.txt. In the package.xml the dependency will appear as <test_depend>

The python templates use headers here.
https://github.com/ahcorde/rosidl_python/blob/ahcorde/rosidl_runtime/rosidl_generator_py/resource/_msg_pkg_typesupport_entry_point.c.em#L13

Headers which are located here rosidl_runtime_c

rosidl_generator_py/CMakeLists.txt

rosidl_generator_py/package.xml

Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde · 2020-03-30T07:37:05Z

Included rosidl_generator_c and rosidl_generator_cpp as <buildtool_depend>, this is need it here because this is a generator package.

dirk-thomas · 2020-03-30T22:01:01Z

Included rosidl_generator_c and rosidl_generator_cpp as <buildtool_depend>, this is need it here because this is a generator package.

I don't understand the rational here: why does the Python generator need the C and C++ generators?

Also I don't see any reference to rosidl_generator_cpp in CMake after it is being find_package()-ed.

ahcorde · 2020-03-31T08:01:33Z

rosidl_generator_py need to call an extension from rosidl_typesupport_c that uses rosidl_generator_cas you can see below:

CMake Error at /home/ahcorde/ros2_idl/install/ament_cmake_target_dependencies/share/ament_cmake_target_dependencies/cmake/ament_target_dependencies.cmake:52 (message):
 ament_target_dependencies() the passed package name 'rosidl_generator_c'
 was not found before
Call Stack (most recent call first):
/home/ahcorde/ros2_idl/install/rosidl_typesupport_c/share/rosidl_typesupport_c/cmake/rosidl_typesupport_c_generate_interfaces.cmake:132 (ament_target_dependencies)
/home/ahcorde/ros2_idl/install/ament_cmake_core/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
/home/ahcorde/ros2_idl/install/rosidl_cmake/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)

and rosidl_typesupport_cpp

  The dependency target "rosidl_generator_py_custom__cpp" of target
  "rosidl_generator_py_custom__rosidl_typesupport_introspection_cpp" does not
  exist.
Call Stack (most recent call first):
/home/ahcorde/rosidl_compile/install/ament_cmake_core/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
home/ahcorde/rosidl_compile/install/rosidl_cmake/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)
  CMakeLists.txt:50 (rosidl_generate_interfaces)

To make use of the CMakeLists.txt functions and macros we should use find_package(), right?

the description of rosidl_typesupport_c and rosidl_typesupport_cpp say:

Generate the type support for C messages.
Generate the type support for C++ messages.

Should we consider rosidl_typesupport_c and rosidl_typesupport_cpp as a generators ? and export in this packages these two depencencies as <buildtool_depend>

Other DDS typesupports such as rosidl_typesupport_fastrtps_c and rosidl_typesupport_cpp need to <buildtool_depend> the generator dependency

rosidl_generator_py/package.xml

dirk-thomas · 2020-03-31T18:17:40Z

Also I don't see any reference to rosidl_generator_cpp in CMake after it is being find_package()-ed.

I still don't see a rational why this is needed / added.

dirk-thomas · 2020-03-31T18:20:15Z

rosidl_generator_py need to call an extension from rosidl_typesupport_c that uses rosidl_generator_c as you can see below:

If this package only uses rosidl_typesupport_c it should also only depend / find_package() it. The fact that transitively might require rosidl_generator_c is something this package shouldn't need to know about. rosidl_typesupport_c needs to provide all transitive dependencies automatically.

…ll transitive dependencies Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde · 2020-04-01T10:31:37Z

Removed rosidl_generator_c and rosidl_generator_cpp dependencies because rosidl_typesupport_c and rosidl_typesupport_cpp will provide the transitive dependencies automatically.

rosidl_typesupport: ros2/rosidl_typesupport@56d3e7c

rosidl_typesupport_fastrtps: ros2/rosidl_typesupport_fastrtps@7791764

ahcorde · 2020-04-02T16:26:05Z

I'm having some issues compiling the tests for this package (rosidl_generator_py) with the current status of Splitted rosidl_generator_c and rosidl_generator_cpp in two: rosidl_generator_x and rosidl_runtime_x.

I'm able to compile message packages such as: std_msgs and run some demos such as: demo_nodes_cpp/talker or demo_nodes_py/talker.py but inside this package the following line is failing

rosidl_python/rosidl_generator_py/CMakeLists.txt

Lines 48 to 54 in d5c17af

    
           rosidl_generate_interfaces(${PROJECT_NAME}_custom 
        
             ${test_interface_files_MSG_FILES} 
        
             # Cases not covered by test_interface_files 
        
             msg/StringArrays.msg 
        
             ADD_LINTER_TESTS 
        
             SKIP_INSTALL 
        
           )

with the following error

CMake Error at /home/ahcorde/rosidl_compile/install/rosidl_typesupport_introspection_cpp/share/rosidl_typesupport_introspection_cpp/cmake/rosidl_typesupport_introspection_cpp_generate_interfaces.cmake:113 (add_dependencies):
  The dependency target "rosidl_generator_py_custom__cpp" of target
  "rosidl_generator_py_custom__rosidl_typesupport_introspection_cpp" does not
  exist.
Call Stack (most recent call first):
  /home/ahcorde/rosidl_compile/install/ament_cmake_core/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
  /home/ahcorde/rosidl_compile/install/rosidl_cmake/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)
  CMakeLists.txt:48 (rosidl_generate_interfaces)


CMake Error at /home/ahcorde/rosidl_compile/install/rosidl_typesupport_fastrtps_cpp/share/rosidl_typesupport_fastrtps_cpp/cmake/rosidl_typesupport_fastrtps_cpp_generate_interfaces.cmake:163 (add_dependencies):
  The dependency target "rosidl_generator_py_custom__cpp" of target
  "rosidl_generator_py_custom__rosidl_typesupport_fastrtps_cpp" does not
  exist.
Call Stack (most recent call first):
  /home/ahcorde/rosidl_compile/install/ament_cmake_core/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
  /home/ahcorde/rosidl_compile/install/rosidl_cmake/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)
  CMakeLists.txt:48 (rosidl_generate_interfaces)


CMake Error at /home/ahcorde/rosidl_compile/install/rosidl_typesupport_fastrtps_c/share/rosidl_typesupport_fastrtps_c/cmake/rosidl_typesupport_fastrtps_c_generate_interfaces.cmake:163 (add_dependencies):
  The dependency target "rosidl_generator_py_custom__cpp" of target
  "rosidl_generator_py_custom__rosidl_typesupport_fastrtps_c" does not exist.
Call Stack (most recent call first):
  /home/ahcorde/rosidl_compile/install/ament_cmake_core/share/ament_cmake_core/cmake/core/ament_execute_extensions.cmake:48 (include)
  /home/ahcorde/rosidl_compile/install/rosidl_cmake/share/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake:286 (ament_execute_extensions)
  CMakeLists.txt:48 (rosidl_generate_interfaces)

Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde · 2020-04-03T12:42:33Z

These two lines fix the issue

rosidl_python/rosidl_generator_py/package.xml

Lines 42 to 43 in 614d373

    
           <test_depend>rosidl_generator_c</test_depend> 
        
           <test_depend>rosidl_generator_cpp</test_depend>

dirk-thomas · 2020-04-03T16:50:14Z

These two lines fix the issue

Can you please provide a rational why the additional test dependency on rosidl_generator_cpp is needed now?

ahcorde · 2020-04-03T17:02:10Z

the test inside rosidl_generator_py uses rosidl_generate_interfaces, this should be used with rosidl_default_generators but at this point this package is not build yet

Inside this macro the following funtion is called:

  ament_execute_extensions("rosidl_generate_idl_interfaces")

This will call all the typesupports (rosidl_typesupport_c, rosidl_typesupport_fastrtps_c, rosidl_typesupport_connext_c) which need to link also with the cpp libraries

dirk-thomas · 2020-04-03T17:07:46Z

So what has changed in the tested branches that makes this change necessary where it wasn't before?

ahcorde · 2020-04-03T17:14:12Z

the packages typesupports and others were exporting rosidl_generator_cpp and rosidl_generator_c which contain both things headers and generators. Right now, we should explicitily add these dependencies

ahcorde · 2020-04-10T10:23:48Z

Updated all repos involved in this change.
Skipping ros1_bridge, qt_*, rqt_*, and rviz_* packages.
Testing against test_rclcpp and rosbag2_tests.

Linux
Linux-aarch64
macOS
Windows

One warning made the builds unstable but there is a (PR pending)

CMake Deprecation Warning at /home/jenkins-agent/workspace/ci_linux-aarch64/ws/install/ament_cmake_export_interfaces/share/ament_cmake_export_interfaces/cmake/ament_export_interfaces.cmake:37 (message):
ament_export_interfaces() is deprecated, use ament_export_targets() instead
Call Stack (most recent call first):
CMakeLists.txt:85 (ament_export_interfaces)

In MacOS

projectroot.cppcheck

Windows

some failures in test_rclcpp

added rosidl_runtime c and cpp depencencies

6581615

Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde mentioned this pull request Mar 4, 2020

Splitted rosidl_generator_c and rosidl_generator_cpp in two: rosidl_generator_x and rosidl_runtime_x ros2/rosidl#443

Closed

ahcorde added 2 commits March 4, 2020 17:54

Removed rosidl_generator_c dependency

c78938c

Signed-off-by: ahcorde <ahcorde@gmail.com>

Fixed rosidl_runtime dependencies

1c81b45

Signed-off-by: ahcorde <ahcorde@gmail.com>

wjwwood reviewed Mar 12, 2020

View reviewed changes

rosidl_generator_py/CMakeLists.txt Outdated Show resolved Hide resolved

dirk-thomas reviewed Mar 12, 2020

View reviewed changes

rosidl_generator_py/CMakeLists.txt Outdated Show resolved Hide resolved

rosidl_generator_py/package.xml Show resolved Hide resolved

ahcorde added 3 commits March 12, 2020 23:59

Fixed rosidl_runtime_c dependencies

f180b5e

Signed-off-by: ahcorde <ahcorde@gmail.com>

Fixed rosidl_runtime_c dependencies

79a0b4c

Signed-off-by: ahcorde <ahcorde@gmail.com>

Removed rosidl_runtime_c from test_depend

e60791a

Signed-off-by: ahcorde <ahcorde@gmail.com>

ahcorde requested review from dirk-thomas and wjwwood March 19, 2020 14:39

wjwwood approved these changes Mar 20, 2020

View reviewed changes

Added rosidl_generator_x to rosidl_generator_py

dea8232

Signed-off-by: ahcorde <ahcorde@gmail.com>

dirk-thomas reviewed Mar 31, 2020

View reviewed changes

rosidl_generator_py/package.xml Show resolved Hide resolved

removed rosidl_generator_x dependencies rosidl_typesupport provides a…

d5c17af

…ll transitive dependencies Signed-off-by: ahcorde <ahcorde@gmail.com>

This was referenced Apr 1, 2020

added rosidl_runtime_c depencency ros2/rosidl_typesupport_fastrtps#32

Merged

added rosidl_runtime c depencency ros2/rosidl_typesupport#58

Merged

dirk-thomas approved these changes Apr 1, 2020

View reviewed changes

Fixed rosidl_generator_py test

614d373

Signed-off-by: ahcorde <ahcorde@gmail.com>

dirk-thomas approved these changes Apr 3, 2020

View reviewed changes

wjwwood approved these changes Apr 3, 2020

View reviewed changes

Merge remote-tracking branch 'osrf/master' into ahcorde/rosidl_runtime

9b511b5

ahcorde merged commit dda5ada into ros2:master Apr 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added rosidl_runtime c and cpp depencencies #100

added rosidl_runtime c and cpp depencencies #100

ahcorde commented Mar 4, 2020

ahcorde commented Mar 6, 2020

ahcorde commented Mar 30, 2020

dirk-thomas commented Mar 30, 2020

ahcorde commented Mar 31, 2020 •

edited

Loading

dirk-thomas commented Mar 31, 2020

dirk-thomas commented Mar 31, 2020

ahcorde commented Apr 1, 2020

ahcorde commented Apr 2, 2020

ahcorde commented Apr 3, 2020

dirk-thomas commented Apr 3, 2020

ahcorde commented Apr 3, 2020

dirk-thomas commented Apr 3, 2020

ahcorde commented Apr 3, 2020

ahcorde commented Apr 10, 2020

added rosidl_runtime c and cpp depencencies #100

added rosidl_runtime c and cpp depencencies #100

Conversation

ahcorde commented Mar 4, 2020

ahcorde commented Mar 6, 2020

ahcorde commented Mar 30, 2020

dirk-thomas commented Mar 30, 2020

ahcorde commented Mar 31, 2020 • edited Loading

dirk-thomas commented Mar 31, 2020

dirk-thomas commented Mar 31, 2020

ahcorde commented Apr 1, 2020

ahcorde commented Apr 2, 2020

ahcorde commented Apr 3, 2020

dirk-thomas commented Apr 3, 2020

ahcorde commented Apr 3, 2020

dirk-thomas commented Apr 3, 2020

ahcorde commented Apr 3, 2020

ahcorde commented Apr 10, 2020

ahcorde commented Mar 31, 2020 •

edited

Loading