Add functions to set pick and place goal fields

[mvieth]

Description

Add functions to set fields in the pick and place goals, namely place_eef and allow_gripper_support_collision. Also some minor formatting changes by clang-format.
See also issue #631

[welcome]

Thanks for helping in improving MoveIt!

[BryceStevenWilley]

LGTM, with suggested changes.

[rhaschke]

In general I approve these changes as well.
@mvieth, please consider Bryce's comments and also add an appropriate python wrapper in wrap_python_move_group.cpp.
Unfortunately, clang-format requires to revert some of your formatting improvements.

[rhaschke]

Having thought a little bit more about this PR, I suggest to remove the extra member variables and instead pass those flags as arguments to the corresponding constructGoal() and pick() / place() functions - providing the old hard-coded values as defaults. My reasoning for this is, that those flags usually will change from call to call. What do you think @BryceStevenWilley?

[BryceStevenWilley]

I don't think those parameters should change every call. Whatever is providing the goals targets won't change it's transform math, so Place EEF shouldn't change. And, at least for every pick or every place, collision with support should also stay the same. But in general, I agree that less saved state in the class is better, so I'm okay with that implementation as well.

[mvieth]

Maybe it would even make sense to expose the constructGoal() functions for the Pickup and Place Goals? There are more fields and options a user might want access to (e.g. attached_object_touch_links, minimize_object_distance, allowed_touch_objects), and making setter functions/optional arguments for all of them would not be so elegant. Instead, the goals could be optional arguments for the pick/place functions, so that a user could call the constructGoal()function, then change any option in the goal, and pass it on to pick/place.

[rhaschke]

Maybe it would even make sense to expose the constructGoal()

I like this idea. Maybe the function names should become more descriptive if exposed, i.e. constructPickGoal and constructPlaceGoal.

[mvieth]

Ok, I pushed the proposed changes. Looking forward to your feedback @rhaschke @BryceStevenWilley . I did not remove the setPlaceEEF() and setAllowGripperTouchSupport() functions yet because they do not conflict with the new pick() and place() functions, but could do so.

[rhaschke]

I did not remove the setPlaceEEF() and setAllowGripperTouchSupport()

My original intention was to get rid off the extra state variables 😉
I pushed some extra commits to your PR branch to reflect my vision of this PR.
You can now decide whether to add those place_eef, allow_touching_support arguments to constructPickGoal and constructPlaceGoal.

I addressed the remaining issues myself. Awaiting feedback.

[mvieth]

I would change this line and the previous one to how they originally were, with goal.allow_gripper_support_collision = true (same for constructPlaceGoal).

You can now decide whether to add those place_eef, allow_touching_support arguments to constructPickGoal and constructPlaceGoal

I would say that's not necessary any more because a user could just use constructPickupGoal and change the place_eef and allow_gripper_support_collision fields in the goal before calling pick(PickupGoal)

[rhaschke]

I would change this line and the previous one to how they originally were, with goal.allow_gripper_support_collision = true

I'm fine with this. Didn't noticed the original state.

[rhaschke]

As @mvieth seems to be happy with the current state, I can approve the PR in its present state.

[v4hn]

I approve as long as someone cleans up the history before merging.

[welcome]

Congrats on getting your first MoveIt! pull request merged and improving open source robotics!

[v4hn]

Thanks Robert, I wasn't sure if you are ok with squashing your commits too.