turtlebot2_driver needs kobuki driver

nuclearsandwich commented 7 years ago

There was no dependency declared so this wasn't caught until the package tried to build.

There's no kobuki port for ROS 2 so I'm not sure what the ideal way to provide it will be.

http://build.ros2.org/job/Rbin_uX64__turtlebot2_drivers__ubuntu_xenial_amd64__binary/4/

15:26:53 CMake Error at CMakeLists.txt:14 (find_package):
15:26:53   By not providing "Findkobuki_driver.cmake" in CMAKE_MODULE_PATH this
15:26:53   project has asked CMake to find a package configuration file provided by
15:26:53   "kobuki_driver", but CMake did not find one.
15:26:53 
15:26:53   Could not find a package configuration file provided by "kobuki_driver"
15:26:53   with any of the following names:
15:26:53 
15:26:53     kobuki_driverConfig.cmake
15:26:53     kobuki_driver-config.cmake
15:26:53 
15:26:53   Add the installation prefix of "kobuki_driver" to CMAKE_PREFIX_PATH or set
15:26:53   "kobuki_driver_DIR" to a directory containing one of the above files.  If
15:26:53   "kobuki_driver" provides a separate development package or SDK, be sure it
15:26:53   has been installed.
15:26:53 
15:26:53

mikaelarguedas commented 7 years ago

Kobuki drive is a ros agnostic package. We can either release it as is and build it with catkin or change it's package.xml + cmakelists to be built with ament. Note this package depends on the ecl stack that is also catkin based and Ros agnostic AFAIK

nuclearsandwich commented 7 years ago

If they're ROS agnostic it's probably most straightforward to tweak the packages to bundle in an ament workspace since catkin and ament workspace setup files will collide if overlayed. I'll give this a shot.

clalancette commented 7 years ago

@nuclearsandwich I looked into this a while back. The entire ecl stack is indeed ROS-agnostic, but there are a lot of packages. It's somewhere on the order of 25 packages that you'll need to do for that stack, which is why I gave up on my effort at the time. The easier path will be to pull in the ros-kinetic-ecl* packages, but I'm not sure how easy that is either.

nuclearsandwich commented 7 years ago

The easier path will be to pull in the ros-kinetic-ecl* packages, but I'm not sure how easy that is either.

I don't have a full idea either, I think at a minimum the following things which I don't know the feasibility of would be necessary

importing the necessary kinetic packages into the ros2 buildfarm (there's docs on doing this that are of questionable comprehensiveness but they're intended for overlay farms rather than cross-rosdistro tomfoolery)
sourcing the kinetic catkin workspace from the ros2 workspace package. I'm not even sure if this will be sufficient since I don't know if catkin and ament will stack environments or if we'll have to write a separate shim setup file that sources both the true ROS 2 environment file and the kinetic one. I think trying to only source the kinetic workspace for the turtlebot2_demo packages will create bad environment stuff but that's mostly conjecture based on what I've seen of those scripts and I could be wrong.
injecting kinetic dependencies into r2b2. We can't leave the package.xml dependency key as kobuki_driver unless we define a rosdep key which resolves it to ros-kinetic-kobuki-driver which is probably not something we want in the upstream rosdep data. So end users, in addition to configuring the ROS 1 and ROS 2 apt repositories they'll need to drop in an additional ros2 support rosdep yaml containing the keys for bridging r2b2 and kinetic.

And that's just the stuff I can think of. @tfoote and @wjwwood is there any prior art on cross-rosdistro package interaction? Should we have an ecl_* migration party this afternoon?

nuclearsandwich commented 7 years ago

As discussed in today's ROS 2 meeting, the easiest win would be to make the ROS 1 repos accessible to the buildfarm, make manual changes to the release templates for turtlebot2_demo and ros1_bridge to source the ros-kinetic environment and depend on specific packages from that system.

By editing the templates directly, we bypass the need to add resolvable rosdep keys which could potentially interfere with installed ROS 1 systems in addition to being yet another user step for configuring ROS 2 but we sacrifice accuracy in the package.xml files on the system.

We also don't know how disparate environments (kinetic + r2b2 for turtlebot2_driver and ros1_bridge vs r2b2 alone for the rest of the release) will behave. We may need the entire release to be rebuilt with access to the kinetic environment, which could also expose problems.

@wjwwood offered an infodump of ways this could end badly and (hopefully) possible mitigations. I'm dead tired now but trying out our easy win is top of my todo list for tomorrow.

wjwwood commented 7 years ago

@wjwwood offered an infodump of ways this could end badly and (hopefully) possible mitigations. I'm dead tired now but trying out our easy win is top of my todo list for tomorrow.

Thanks for the reminder, I had forgotten.

By editing the templates directly, we bypass the need to add resolvable rosdep keys which could potentially interfere with installed ROS 1 systems in addition to being yet another user step for configuring ROS 2

Well actually there are two things we a changing by patch (as I understood it):

the rules file to source the /opt/ros/kinetic/setup.bash file
the control file to inject the missing dependency on the ROS 1 package(s)

The second could be handle differently, where we modify bloom to check a whitelist (or more generically the ROS 1 rosdistro list) to see if a key is a released package in the rosdistro. If so it could transform it according to a different rule (e.g. ros-kinetic-<pkg> versus our normal ros-r2b2-<pkg>). We avoid all of this more generic, tricky logic by injecting the dependency directly, but we also need to either modify or remove the dependency in the package.xml so bloom doesn't complain that it cannot convert it. (which is what I guess you meant by "but we sacrifice accuracy in the package.xml files on the system.")

We also don't know how disparate environments (kinetic + r2b2 for turtlebot2_driver and ros1_bridge vs r2b2 alone for the rest of the release) will behave. We may need the entire release to be rebuilt with access to the kinetic environment, which could also expose problems.

This is basically all I was going to say. There are two possible sets of issues:

sourcing the kinetic file only in the two special packages might result in them linking to a different version of some common dependency like console_bridge
source the kinetic file in all might result in all our packages using the ROS 1 version of an overlapping dependency (again like console_bridge) which might not have patches we need for ROS 2, breaking things

I still think your described approach of "only source kinetic for the special packages and see what happens" and "manually remove the dependencies on ROS 1 packages from the package.xml and manually inject them into the debian control file" is the best approach to take for now. And if we run into issues, adjust as necessary.

Hopefully that's helpful and not adding more confusion. Good luck and let me know if I can help in some way!

nuclearsandwich commented 7 years ago

We avoid all of this more generic, tricky logic by injecting the dependency directly, but we also need to either modify or remove the dependency in the package.xml so bloom doesn't complain that it cannot convert it. (which is what I guess you meant by "but we sacrifice accuracy in the package.xml files on the system.")

The dependency on kobuki driver is already missing from the package.xml which is why this issue wasn't raised earlier in release preparation. I didn't know we needed it until now.

clalancette commented 7 years ago

I just tested out the binary package here. In good news, it installs just fines, and I can launch it, and it seems to attach to the robot. In bad news, I can't seem to send any commands to it over the joystick; that particular functionality works when building from source, so something must not be entirely correct when it gets built/installed as a binary. I haven't looked deeper into the reason it doesn't work yet.

dhood commented 7 years ago

sorry for the accidental close there...

nuclearsandwich commented 7 years ago

so something must not be entirely correct when it gets built/installed as a binary. I haven't looked deeper into the reason it doesn't work yet.

:sob:

nuclearsandwich commented 7 years ago

@clalancette I've assigned you as well as a gentle bump to help me look into the packaged kobuki_node behavior.

clalancette commented 7 years ago

It looks like what is happening is that the kobuki_node gets created (it shows up in ros2 node list), but none of the topics are being created (neither publishers nor subscribers). This explains the failure, but I don't understand why that would happen. I'll look into it some more.

clalancette commented 7 years ago

I still don't understand. When I build just kobuki_node from source, it then publishes the correct topics, but then doesn't subscribe to the cmd_vel topic, and thus driving it around doesn't really work. It looks like everything else (joy_node, teleop_node) is properly subscribing and publishing, so I'm not sure why kobuki_node is special here.

nuclearsandwich commented 7 years ago

Don't mind me. @dhood apparently this issue is cursed with accidental closures

nuclearsandwich commented 7 years ago

Reopening until we have a chance to test a built package. Thanks for investigating and resolving the issue @clalancette.

clalancette commented 7 years ago

@nuclearsandwich I think we are pretty good on this issue now; should we close this out?

nuclearsandwich commented 7 years ago

Yeah! If the recent debs are working I think we're good to go.

ros2 / rosdistro

turtlebot2_driver needs kobuki driver #5