Moveit: [moveit_setup_assistant] Build failure around RobotModel (URDF?)

[mikaelarguedas]

Description

moveit_setup_assistant not building on the following platforms: Ubuntu Yakkety, Ubuntu Zesty, Debian Stretch

Your environment

• ROS Distro: Lunar
• OS Version: Ubuntu 16.10, Ubuntu 17.04, Debian 9
• Source
• If source, which git commit or tag? https://github.com/ros-planning/moveit/commit/4da41e12207669037f527f1abaae16f6eca09cc9

Steps to reproduce

Run a prerelease test (below code for Yakkety)

mkdir -p /tmp/prerelease_job
cd /tmp/prerelease_job
generate_prerelease_script.py \
  https://raw.githubusercontent.com/ros-infrastructure/ros_buildfarm_config/production/index.yaml \
  lunar default ubuntu yakkety amd64 \
  moveit \
  --level 1 \
  --output-dir ./

Expected behaviour

Code should build without error or warning

Actual behaviour

Code fails to build with error pasted below

Backtrace or Console output

Sample below,
Full log available here

[ 11%] Building CXX object CMakeFiles/moveit_setup_assistant_tools.dir/src/tools/moveit_config_data.cpp.o
/tmp/catkin_workspace/src/moveit/moveit_setup_assistant/src/tools/moveit_config_data.cpp: In member function ‘moveit::core::RobotModelConstPtr moveit_setup_assistant::MoveItConfigData::getRobotModel()’:
/tmp/catkin_workspace/src/moveit/moveit_setup_assistant/src/tools/moveit_config_data.cpp:186:83: error: no matching function for call to ‘moveit::core::RobotModel::RobotModel(boost::shared_ptr<urdf::Model>&, srdf::ModelSharedPtr&)’
     robot_model_.reset(new robot_model::RobotModel(urdf_model_, srdf_->srdf_model_));
                                                                                   ^
In file included from /tmp/catkin_workspace/install_isolated/include/moveit/planning_scene/planning_scene.h:40:0,
                 from /tmp/catkin_workspace/src/moveit/moveit_setup_assistant/include/moveit/setup_assistant/tools/moveit_config_data.h:45,
                 from /tmp/catkin_workspace/src/moveit/moveit_setup_assistant/src/tools/moveit_config_data.cpp:37:
/tmp/catkin_workspace/install_isolated/include/moveit/robot_model/robot_model.h:72:3: note: candidate: moveit::core::RobotModel::RobotModel(const ModelInterfaceSharedPtr&, const ModelConstSharedPtr&)
   RobotModel(const urdf::ModelInterfaceSharedPtr& urdf_model, const srdf::ModelConstSharedPtr& srdf_model);
   ^~~~~~~~~~
/tmp/catkin_workspace/install_isolated/include/moveit/robot_model/robot_model.h:72:3: note:   no known conversion for argument 1 from ‘boost::shared_ptr<urdf::Model>’ to ‘const ModelInterfaceSharedPtr& {aka const std::shared_ptr<urdf::ModelInterface>&}’
/tmp/catkin_workspace/install_isolated/include/moveit/robot_model/robot_model.h:68:7: note: candidate: moveit::core::RobotModel::RobotModel(const moveit::core::RobotModel&)
 class RobotModel
       ^~~~~~~~~~
/tmp/catkin_workspace/install_isolated/include/moveit/robot_model/robot_model.h:68:7: note:   candidate expects 1 argument, 2 provided
/tmp/catkin_workspace/src/moveit/moveit_setup_assistant/src/tools/moveit_config_data.cpp: In member function ‘void moveit_setup_assistant::MoveItConfigData::updateRobotModel()’:
/tmp/catkin_workspace/src/moveit/moveit_setup_assistant/src/tools/moveit_config_data.cpp:204:81: error: no matching function for call to ‘moveit::core::RobotModel::RobotModel(boost::shared_ptr<urdf::Model>&, srdf::ModelSharedPtr&)’
   robot_model_.reset(new robot_model::RobotModel(urdf_model_, srdf_->srdf_model_));

[130s]

This is yet another blocker for #486 Lunar release. Anyone?

Both files that are mentioned in the error log, moveit_setup_assistant/src/tools/moveit_config_data.cpp and moveit_core/robot_model/src/robot_model.cpp have not been updated since Jan 2017 on Kinetic and no issue reported. So could be something else that's affecting.

[mikaelarguedas]

Yakkety Zesty and Stretch now ship with liburdfdom v1.0 while xenial was shipping v0.4.
Very likely the conversion of URDF to use std::shared_ptr instead of boost::shared_ptr is one of the culprits. There may be more API to update to use the new major version of urdfdom though.

[davetcoleman]

@mikaelarguedas yes, it is because urdfdom_headers switched to std::shared_ptrs but robot_model::urdf is still using boost. The fix for lunar (once I found it) is simple but because we have not branched for lunar I will need to use compiler flags to allow this change to still work with kinetic. Unless someone opposes that approach?

I started a related C++11 discussion here

[davetcoleman]

@jspricke @rhaschke do you have a recommendation for the best way to use boost:: for kinetic and std:: for lunar for urdf::model? It seems like adding URDF_TYPEDEF_CLASS_POINTER(Model) to the list here would do the trick, but it might be too late to modify that file given that Lunar is already released?

Is there a maco to simply detect the ROS release being used?

[marcoesposito1988]

@davetcoleman

Yes, you can do something like this (this is checking for Lunar):

#if ROS_VERSION_MINIMUM(1, 13, 0) 
  const std::shared_ptr<urdf::ModelInterface>& urdf_model = rdf_loader.getURDF();
#else
   const boost::shared_ptr<urdf::ModelInterface>& urdf_model = rdf_loader.getURDF();
#endif

Or you can also check ROS_VERSION_MAJOR and ROS_VERSION_MINOR

[v4hn]

Yes you can check for the ROS version, but this assumes people don't run "mixed" setups, e.g. building lunar modules on top of kinetic, which are not uncommon with real robot setups.

Wherever possible, please prefer checking the version of the affected dependency directly, even if this adds another check to the cmake build description.

[v4hn]

It seems like adding URDF_TYPEDEF_CLASS_POINTER(Model) to the list here would do the trick

How? This macro creates boost shared ptrs and until now this is the same in ROS lunar, doesn't it?

[davetcoleman]

How? This macro creates boost shared ptrs and until now this is the same in ROS lunar, doesn't it?

Ooops, I meant we should use that macro in this header where they are std::shared_ptrs - but that wouldn't work either actually.

I think the better solution would be to add urdf::Model to the compatibility layer. I haven't fully tested this yet, but here's the proposal

[davetcoleman]

I believe the solution we need is to change urdfdom as well as this simple one-liner in MoveIt.

[mikaelarguedas]

awesome thanks @davetcoleman for investigating and submitting a fix, we're running prerelease tests as we speak and will hopefully be able to release this into lunar (and kinetic?) in the next few cycles (at @sloretz and @clalancette discretion)

[130s]

Fixed via #542 thanks to @davetcoleman