New grasp pose generator stage

[bostoncleek]

This PR introduces a new stage called GraspProvider. This stage uses grasping_msgs::GraspPlanningAction to communicate with an action server external to MTC. The action server is used to provide grasp candidates to MTC. For example the action server can use machine learning pipelines to sample grasps given a point cloud or depth image. This feature removes complex dependencies from MTC that would otherwise be required to generate grasp candidates.

The GraspProvider stage sends a request for grasps. If a time out is reached or no grasps are found then planning fails. Otherwise the provided grasps will be used by MTC.

Closes #188

[codecov[bot]]

Codecov Report

Merging #196 (7c47532) into master (e923fbc) will decrease coverage by 0.77%. The diff coverage is 1.74%.

@@            Coverage Diff             @@
##           master     #196      +/-   ##
==========================================
- Coverage   54.63%   53.86%   -0.76%     
==========================================
  Files         102      106       +4     
  Lines        7852     7967     +115     
==========================================
+ Hits         4289     4291       +2     
- Misses       3563     3676     +113     

Impacted Files	Coverage Δ
...clude/moveit/task_constructor/stages/action_base.h	0.00% <0.00%> (ø)
...de/moveit/task_constructor/stages/grasp_provider.h	0.00% <0.00%> (ø)
core/src/stages/grasp_provider.cpp	1.00% <1.00%> (ø)
core/src/stages/action_base.cpp	7.70% <7.70%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e923fbc...7c47532. Read the comment docs.

[mamoll]

The intent is to add a base class that can be used to implement support for GPD and Dex-Net. These grasp synthesis pipelines have a whole slew of dependencies, so the GPD/Dex-Net support would likely live in a separate packages (say, moveit_task_constructor_gpd, and moveit_task_constructor_dexnet).

[mamoll]

@rhaschke @v4hn perhaps it's helpful to see how this code can be used. See https://github.com/PickNikRobotics/deep_grasp_demo/blob/master/src/gpd_pick_place_task.cpp for an example.

[bostoncleek]

It might be useful to check out the implementation and documentations for the demos.

[bostoncleek]

Can I get a review please? @v4hn @rhaschke

[lianghongzhuo]

Looks good for me when I test this with our ur5 robot.

[JStech]

I'm using this for a project and I was surprised to see it's still not merged. @v4hn and @rhaschke could you please take a look at this PR?

[JStech]

I had to rebase this to get it to build on Noetic (it was a very easy rebase). Here is my branch.

[rhaschke]

Thanks @JStech for providing the rebased branch link. Currently, there is so much other work, that I cannot catch up with all PR reviews. As this one is a larger one, it will probably take some more time... Please be patient (and continue using your own branch meanwhile).

[lianghongzhuo]

@JStech 's rebase link works for me, (ubuntu 18, ros melodic with moveit master branch)

[davetcoleman]

@bostoncleek think this could get merged now?

[bostoncleek]

@bostoncleek think this could get merged now?

@davetcoleman This was ready to be merged back on August 11th. It is a matter of the maintainers pressing the merge button!

Looks like it does need a rebase though. I will squash and rebase today.

[v4hn]

It's not just merge, but also review (and possibly fixup). I'm sorry for not getting around to do it. There is always more pressing issues and often two crying kids in the background since Corona homeoffice.

Thanks for rebasing in advance!

[bostoncleek]

It's not just merge, but also review (and possibly fixup). I'm sorry for not getting around to do it. There is always more pressing issues and often two crying kids in the background since Corona homeoffice. Thanks for rebasing in advance!

Any change requests will be fixed so please make a comment on what you would like.

[v4hn]

As I said, I did not have the time to look through this in detail, and I'm sorry for that.

My biggest concern, as I can recall from discussion with @henningkayser and @lianghongzhuo was that the use-case to receive candidate poses in an any-time fashion through feedback and spawn solutions based on them continuously should be well supported, even if you did not use that approach in your project. Such an approach might be valuable for projects like this too.

[bostoncleek]

As I said, I did not have the time to look through this in detail, and I'm sorry for that.

My biggest concern, as I can recall from discussion with @henningkayser and @lianghongzhuo was that the use-case to receive candidate poses in an any-time fashion through feedback and spawn solutions based on them continuously should be well supported, even if you did not use that approach in your project. Such an approach might be valuable for projects like this too.

Don't have the time? ... its World MoveIt Day! of course you have the time. Grow the project a little don't be afraid to let others make a contribution towards your work.

If it doesn't happen today I will see you on World MoveIt Day 2022!

[JStech]

the use-case to receive candidate poses in an any-time fashion through feedback and spawn solutions based on them continuously should be well supported, even if you did not use that approach in your project.

This sounds more like a request for an enhancement. Why block merging useful code because we can imagine even more useful code?

[davetcoleman]

@bostoncleek could you fix the conflict? @JafarAbdi would be a good candidate to review and merge this, given his extensive work with MTC

[rhaschke]

I shortly looked into this PR and I was surprised by the many unrelated commits. @bostoncleek, it would be great if you could rebase and cleanup the history.

[bostoncleek]

@rhaschke @JafarAbdi I squashed the commits and now it should be easy for you to review. Conflict is also resolved.

[bostoncleek]

@rhaschke Thank you for the feedback. I will work on the change requests.

[v4hn]

Thanks for the review @rhaschke ! @bostoncleek I will wait for his feedback to be addressed and review afterwards. If I'll have further requests for changes then I will patch them myself and you got my official permission to kick me if I did not do it by the maintainer meeting in April. :muscle: (Of course assuming you addressed the feedback before :smile: )

[bostoncleek]

@v4hn Thank you for offering to patch future requests yourself. I am addressing the larger issues now so hopefully it will be set to merge in soon.

[bostoncleek]

@v4hn I have addressed @rhaschke requests. Please let me know if there are other patches that need to be made.

[lianghongzhuo]

any updates?

[lianghongzhuo]

@bostoncleek Could you rebase this PR? There is some upstream change that makes this PR unable to compile. Like add this https://github.com/ros-planning/moveit_task_constructor/commit/437cc550f2a68895de56f025e2507770bff2faca

[v4hn]

There is some upstream change that makes this PR unable to compile. Like add this https://github.com/ros-planning/moveit_task_constructor/commit/437cc550f2a68895de56f025e2507770bff2faca

Hongzhuo, I stated above that I will fix this up and merge it, and so I will do that. If I wouldn't spend some of my time fixing other bugs you cannot find yourself in our setups, I also would have more time for things like this. (And there are numerous other current issues/patches active in MoveIt/MTC). In the meanwhile feel free to use the PR branch if you need the stage.

[bostoncleek]

@lianghongzhuo yes I will rebase

[bostoncleek]

This is failing because of clang-tidy.

error: unknown warning option '-Wno-unused-but-set-parameter'; did you mean '-Wno-unused-parameter'? [clang-diagnostic-unknown-warning-option]

I don't see where add_compile_options() are set using a call to -Wno.

Can a maintainer provide help?

[v4hn]

I rebased.

#274 (which was not included here yet) recently made the CI configuration quite a bit stricter and addressed this particular error. Let's see whether new ones pop up now. :-)