jhu-lcsr-forks / rtt_ros_integration

Orocos-ROS integration libraries and tools
1 stars 2 forks source link

rosbuild support #9

Closed meyerj closed 11 years ago

meyerj commented 11 years ago

This is a follow up of #8 as rosbuild support is not really within the original scope.

Right, we can do this, but part of the goal of all of this is to remove all of the catkin/ros-specific stuff from the Orocos upstream, aside from lightweight package.xml files. If we added the ${CATKIN_DEVEL_PREFIX} variable to the normal orocos_generate_package() macro, then that would involve putting catkin code into the Orocos upstream.

I would vote for not totally ban everything that has to do with ROS/catkin from upstream rtt, as long as no ROS libraries are used and there are no differences in the binaries. But to check if certain cmake variables are defined and to create additional files or use other default paths in the UseOROCOS-RTT macros would be okay if that makes life easier...

Concerning orocos_create_package(), I propose the following behavior:

I would like to keep rosbuild support if possible. The differences in the required cmake code and the rtt_ros service are not so big and it would be convenient for existing projects. It would be okay to require some changes in the manifests and in cmake, but I don't see a good reason to drop rosbuild support totally.

To be clear, you mean support for rosbuild packages to depend on catkin-based orocos packages, right?

Yes, I should have been clearer here. rosbuild support in my opinion includes:

  1. All orocos/rtt_ros_integration packages are plain cmake or catkin. This is already the case thanks to your work.
  2. rosbuild packages can find_package() and use orocos like any other cmake project. Orocos cmake macros orocos_component(), orocos_library(), ... should use rosbuild_add_library() if rosbuild is detected (based on the ROSBUILD_init_called variable?).
  3. rosbuild packages can find_package(rtt_roscomm) and call ros_generate_rtt_typekit() and ros_generate_rtt_service_proxies() on rosbuild message packages. This replaces the former rosbuild_include(rtt_rosnode GenerateRTTtypekit) call.
  4. orocos_use_package()/orocos_find_package() or any equivalent macro in package rtt_ros should use rospack to find the package and its dependencies and set the PKG_CONFIG_PATH environment variable for rosbuild packages found. For catkin or installed orocos packages we do not need that as their configuration files are already in the PKG_CONFIG_PATH. The real work is still done by pkg_search_module() afterwards.
  5. The rtt_ros service should be able to import rosbuild packages.

2.-4. have already been implemented in the rosbuild_support branches of meyerj/rtt and meyerj/rtt_ros_integration (still testing). I did not look into 5. so far, but as the rtt_ros service already uses rospack to find packages it shouldn't be too complicated to also check if a manifest.xml file exists and use that one instead of package.xml (or only if no package.xml file exists). Recursive plugin dependencies could still be declared via <export><plugin_depend>...</plugin_depend></export> if this is preferred.

psoetens commented 11 years ago

Could you first confirm that: 1- Hydro has no rosbuild support so this proposal is only for Groovy or older ? 2- You don't want to call rosbuild_init() in your package's CMakeLists.xml file (so UseOrocos has to call it) ? 3- The UseOrocos logic now checks for the ROS_PACKAGE_PATH, if the current dir is in the ros package path and no catkin is detected, it's a rosbuild package...

meyerj commented 11 years ago

1- Hydro has no rosbuild support so this proposal is only for Groovy or older ?

I am talking about hydro. rosbuild support will not be dropped totally. The only thing is that it's not possible to release rosbuild packages any more on the ROS buildfarm (see Tully's anwer on ROS-users here: https://code.ros.org/lurker/attach/4@20130822.014202.19ef68d7.attach). In the current beta release building rosbuild packages from source works without problems. There are plans to drop rosbuild support in ROS indigo...

Of course it would be possible to also use and release the new catkin/hydro-devel version of Orocos/rtt_ros_integration in groovy. But as there will be changes at least in the cmake API this might not be a good idea. However, that's another topic.

2- You don't want to call rosbuild_init() in your package's CMakeLists.xml file (so UseOrocos has to call it) ?

No, it is the user's responsibility to call rosbuild_init() and decide if he wants to use rosbuild or not. UseOrocos should not decide anything.

3- The UseOrocos logic now checks for the ROS_PACKAGE_PATH, if the current dir is in the ros package path and no catkin is detected, it's a rosbuild package...

I am aware of this logic. My proposal is to just "capture" the rosbuild macros if they are defined. It's easy to check if either the ROSBUILD_init_called variable is true or if a macro has been defined:

if(COMMAND rosbuild_add_library)    # ... or if(ROSBUILD_init_called)
  rosbuild_add_library(...)
else()
  add_library(...)
endif()

If the user called rosbuild_init() before include(UseOROCOS-RTT.cmake) he will get rosbuild support. Otherwise he won't.

psoetens commented 11 years ago

On Thu, Aug 29, 2013 at 12:49 PM, Johannes Meyer notifications@github.comwrote:

1- Hydro has no rosbuild support so this proposal is only for Groovy or older ?

I am talking about hydro. rosbuild support will not be dropped totally. The only thing is that it's not possible to release rosbuild packages any more on the ROS buildfarm (see Tully's anwer on ROS-users here: https://code.ros.org/lurker/attach/4@20130822.014202.19ef68d7.attach). In the current beta release building rosbuild packages from source works without problems. There are plans to drop rosbuild support in ROS indigo...

Of course it would be possible to also use and release the new catkin/hydro-devel version of Orocos/rtt_ros_integration in groovy. But as there will be changes at least in the cmake API this might not be a good idea. However, that's another topic.

Hmm, yes, I was confused. This is only '2.7' + Hydro.

2- You don't want to call rosbuild_init() in your package's CMakeLists.xml file (so UseOrocos has to call it) ?

No, it is the user's responsibility to call rosbuild_init() and decide if he wants to use rosbuild or not. UseOrocos should not decide anything.

3- The UseOrocos logic now checks for the ROS_PACKAGE_PATH, if the current dir is in the ros package path and no catkin is detected, it's a rosbuild package...

I am aware of this logic. My proposal is to just "capture" the rosbuild macros if they are defined. It's easy to check if either the ROSBUILD_init_called variable is true or if a macro has been defined:

if(COMMAND rosbuild_add_library) # ... or if(ROSBUILD_init_called) rosbuild_add_library(...) else() add_library(...) endif()

If the user called rosbuild_init() before include(UseOROCOS-RTT.cmake) he will get rosbuild support. Otherwise he won't.

Fine for me.

Peter

jbohren commented 11 years ago

There are some other details that were removed when I removed rosbuild support in my fork, see the details here:

https://github.com/meyerj/rtt/commit/8d465b341005238a9dd6fbd03632d7619aca148e#commitcomment-3974459

https://github.com/meyerj/rtt/commit/8d465b341005238a9dd6fbd03632d7619aca148e#commitcomment-3974458

jbohren commented 11 years ago

@meyerj I can make the changes, I;ll pull yours in and then re-add the stuff I removed. I'll close this issue when I'm done, if we still need to re-iterate, we can re-open it.

meyerj commented 11 years ago

@jbohren As you want. I can also do it, but not before tomorrow afternoon. The rosbuild_support branch is really only a draft and some of my proposed changes here is still missing (e.g. write a pkg-config file in CATKIN_DEVEL_PREFIX). If you don't mind, perhaps it would be easiest if you give me push access to jhu-lcsr-forks so that we can work in the same repo? I would only commit stuff to the main branches that we agreed upon in the discussions.

jbohren commented 11 years ago

@meyerj

@jbohren As you want. I can also do it, but not before tomorrow afternoon. The rosbuild_support branch is really only a draft and some of my proposed changes here is still missing (e.g. write a pkg-config file in CATKIN_DEVEL_PREFIX). If you don't mind, perhaps it would be easiest if you give me push access to jhu-lcsr-forks so that we can work in the same repo? I would only commit stuff to the main branches that we agreed upon in the discussions.

Yeah I can definitely give you access. Consider it done. I'm over here on US east coast time so I've got some time to work on it today (:

jbohren commented 11 years ago

There are a lot of topics in this issue, some of them have to do with rtt_ros_integration, and others require changes in rtt, itself.

Most of rosbuild support was resurrected via the following commits (along with catkin support): https://github.com/jhu-lcsr-forks/rtt/compare/d607a06...3c47308?w=1

Then, orocos_use_package() was split into orocos_use_package() and orocos_find_package(): https://github.com/jhu-lcsr-forks/rtt/commit/9236de4404e6ecfe1280a43a2128221593a904a7

jbohren commented 11 years ago

Further discussion of modifications to the rtt package concerning catkin support should happen here: https://github.com/jhu-lcsr-forks/rtt/issues/4