Deleted dependency to moveit

[ipa-nhg]

After a discussion with @ipa-fmw , we decided that the bringup packages should not require the installation of Moveit! packages and neither launch the cob_bringup/tools the creation of a Moveit! robot configuration. The tool collision_monitor should be moved to other repository (for example cob_control) or extra started.

[floweisshardt]

@ipa-nhg: please move/create a package for that in cob_manipulation before merging this. cob_control is no good alternative because it is a dependency of cob_bringup too.

[ipa-nhg]

What do you mean with a new package? The launch files already exist:

• https://github.com/ipa320/cob_manipulation/blob/indigo_dev/cob_collision_monitor/launch/collision_monitor.launch
• https://github.com/ipa320/cob_manipulation/blob/indigo_dev/cob_obstacle_distance_moveit/launch/obstacle_distance.launch

[ipa-nhg]

@ipa-fmw @ipa-mig can be merged?

[mgruhler]

CR; ok

Just one question: the deleted launch file contains a remapping, the linked on in cob_manipulation doesn't. Could this be a problem?

[ipa-nhg]

Just in case it could be a problem, I added the remap lines as comments ipa320/cob_manipulation#82
The remap topic name could also be define using an argument or a parameter, but @ipa-fxm should decide it...

[mathias-luedtke]

The collision monitor was a feature request by @ipa-tif!
It should prevent the robot from colliding with itself or the floor while moving the torso (or any other component).
And it does not help to move the package between repositories, the dependency on moveit will persist.
I don't see a good reason for banning moveit from the bringup.

or extra started.

It is a stand-alone node that requires some moveit libs for the actual collision checking.

CR: not okay ;)

[mgruhler]

Well, this depends on whether this should be automatically started with a roslaunch cob_bringup robot.launch or if it okay to start this seperatly. If a seperate launch file to be started additionally is okay, then this should solve the "dependency issue".

I agree that having it in a package that is started by cob_bringup does not make sense. But if I'm correct there has (once) been the guideline to have no "high-level" functionality such as navigation or manipulation in the bringup layer. Thus, intention to "ban moveit" from bringup is to keep dependencies smaller and stick to this guideline. Such high-level functionality should then be started from e.g. cob_manipulation.

However, I personally don't mind. cob_bringup has so many dependencies anyways, from my perspective it doesn't matter to add moveit as well. Given that this remains supported and is ported to new ROS distros within a reasonable period of time (wrt our fast switch-overs to new distros ;-) )

[mathias-luedtke]

The cob_collision_monitor is meant for use with the cob_safety_controller.
It only prevents self collisions, no high level functionalities are included.
It just reads joints_states and TF and publishes state validity information.

However, it needs a SRDF for the collision checking.
That's why it depends an cob_moveit_config.

[mgruhler]

The collision monitor also dependes on moveit_planning, amongst others...

[ipa-nhg]

@ipa-mdl @ipa-tif I understand that start the collision monitor by default is necessary, but on the other hand the actual status of the software (cob_bringup depending on cob_manipulation) doesn't fit with our architecture definition. Install moveit_core, moveit_ros_planning and moveit_ros_perception just to test the hardware is not ideal.
I can propose add the collision monitor to the cob-start script , if the users want to use the collision observer monitor they have to call the command cob-start to start the robot instead of roslaunch cob_bringup robot.launch . And cob_bringup will not have any dependency to moveit.

The main problem at the moment is that the cob_obstable_distance_moveit is not working properly, sometimes it stops the system because an "allowed" collision is detected, for example between base and torso. It can be solved modifying the SRDF and adding some disable_collisions tags. Also would be nice have a service to disable the collision monitor if it is necessary.

As the current version of cob_robots can not be used on the cob4 robots because of the wrong detection of collisions ( if we move the torso some degrees the collision monitor send a message to safety controller and we are not allow to move any more any component) I propose, and at least until we have new SRDF (or 3d model) files, comment the collision monitor lines here, and we will see if we find a better solution for the dependencies issue.

[mathias-luedtke]

cob_obstable_distance_moveit is unrelated to cob_collision_monitor.
cob_collision_monitor should run, but it should not be used in cob_safety_controller until the URDFs are correct.

The moveit dependencies are rather self-contained (except for the rviz stuff, which is not needed for collision monitoring).

the actual status of the software (cob_bringup depending on cob_manipulation)

We could move cob_moveit_config (without the launch files) to cob_robots.
And we could move cob_collision_monitor somewhere else, it just depends on the config.

[mathias-luedtke]

As discussed with @ipa-nhg:

• remove cob_obstable_distance_moveit from bringup
• add SRDF to cob_hardware_config (initially empty)
• load SRDF in https://github.com/ipa320/cob_robots/blob/indigo_dev/cob_hardware_config/common/upload_robot.launch
• update cob_collision_monitor to depend on cob_hardware_config
• update cob_moveit_config to use new SRDF
• fill SRDFs
• profit

[fmessmer]

Can't help it but to drop in this comment on my town day:
The solution looks good.
Still, cob_obstacle_distance_moveit is used for collission avoidance constraint in cob_twist_controller!
It also only needs the SRDF to initialize the PlanningSceneMonitor - so no depemdemcy to moveit_planning!
Please check with @ipa-bfb and @ipa-fxm-cm as they continued work on it!
They might want to send a PR for their changes...

[ipa-nhg]

Related to this pull request: #482 (already tested and merged)

[floweisshardt]

@ipa-nhg: what's the status here? close? merge?

[ipa-nhg]

This pull request can be closed, as soon as https://github.com/ipa320/cob_robots/pull/478/files is merged and the changes on cob_collision_monitor done.

[ipa-nhg]

Related to this discussion

#518
ipa320/cob_manipulation#86