search

moveit / moveit_ros

THIS REPO HAS MOVED TO https://github.com/ros-planning/moveit
69 stars 118 forks source link

initialize RobotModel even if URDF is empty #730

Closed v4hn closed 7 years ago

v4hn commented 7 years ago

There is no reason to have a null_ptr dangling here just because we didn't find a robot description. We can just initialize an empty RobotModel.

Without this patch a null_ptr is passed to getSharedStateMonitor here , while we could add null-checks to all usages of the robot model, I feel it's much more reasonable to simply generate an empty RobotModel. This still prints a ROS_ERROR because rdf_loader fails to find the rosparam.

Please cherry-pick to jade and kinetic.

Fixes https://github.com/ros-planning/moveit_commander/issues/32 .

rhaschke commented 7 years ago

+1

jonbinney commented 7 years ago

+1

davetcoleman commented 7 years ago

Shouldn't it just throw an error? What good is configuring a rdf_loader if you have no URDF?

v4hn commented 7 years ago

It does print an error. By far most MoveIt components do not abort on failure but try to push on after a ROS_ERROR. I decided to do that here too. Thus, nobody needs to check whether the URDF actually got loaded before they pass on the RobotModel to other data structures for initialization. Having null-pointers around when everyone expects a valid object is prone to error, because nobody remembers to check the pointer and usually there's nothing the calling code could do anyway when it receives a null-ptr.

This is an edge-case nobody but someone looking into MoveIt! for the first time cares about and the most simple way to avoid it is by never having a null ptr returned by getSharedRobotModel.

v4hn commented 7 years ago

@davetcoleman did you forget to cherry-pick? :)

davetcoleman commented 7 years ago

Ugh I just spent a good bit of time trying to figure out why my cherry-pick wasn't working - I thought you were reminding me to do it but you beat me to it

v4hn commented 7 years ago

lol,

yes, that was the plan. But an hour later I wanted to quit office for this week and wanted to make sure we don't forget about these pick because you didn't do them yet. So I picked them myself. Sorry, should have left a comment!