JenniferBuehler / gazebo-pkgs

A collection of tools and plugins for Gazebo
BSD 3-Clause "New" or "Revised" License
207 stars 104 forks source link

Publish grasp events on a latched topic (implements #36) #50

Closed corot closed 3 years ago

corot commented 3 years ago

This allows to emulate a physical grasped/no-grasped check, as described on #36

JenniferBuehler commented 3 years ago

Thank you for this PR :)

The issue to publish grasp events is a feature I've been wanting to add for quite some time now, and never found the free time to actually do it.

The plan however is to keep this package ROS-independent. I'm aware that the CMakeLists.txt already contains catkin stuff, but the code itself does not depend on ROS. There are users who want to use this plugin standalone with Gazebo only. So my intention was to publish events as gazebo messages (not ros messages), which then can be translated to ROS messages in a separate node, if desired.

Not sure you'll find the time to make those adjustments, and I certainly wouldn't expect it. The options I see now:

(1) Provide your code in a branch, and mention that this is available in the WIKI or (2) I can use your PR as starting point when I find the time (hopefully soon) and adjust it so that gazebo messages are used instead or (3) You can do the adjustments, and we move forwards with this PR and eventually merge it or (4) Both (1) and (2 or 3)

Which would you prefer? Thanks again for your efforts! Jennifer

corot commented 3 years ago

Thank you for this PR :)

No, thanks for your plugins! I suppose you mean publishing sim_event.proto, or maybe tactile.proto msgs instead of ROS ones, right? I have zero experience on that, but can try. Then u read with something like this in my ROS node, right?

JenniferBuehler commented 3 years ago

Yes, that's right. You can also add new Gazebo messages which could then be equivalent to your ros message, see this tutorial. That's maybe easier and it can include all info you need (robot and object involved).

The ROS node would roughly look like the one in your link, but you'd of course subscribe to the topic with your gazebo message, and in the callback you would then translate to the ROS message and publish it via a ros::Publisher.

I'd be happy to include the ROS code too, but then it would have to be in a separate package (within this repo though, but in a separate directory and with it's own CMakeLists.txt and package.xml) so that it's separated from the code in gazebo-grasp-plugin (could eg be called gazebo-grasp-plugin-ros).

Thank you so much for your effort! Let me know if I can do anything to help.

corot commented 3 years ago

Finally I found some time to remake the PR with gazebo instead of ROS msgs.

But I need some help; looks like I create properly the protobuff msg GraspEvent, but when I try to use it from outside of the workspace containing this pkg, client pkgs subscribing to the topic cannot find the msg definition. Protobuf creates a header in ./build/gazebo_grasp_plugin/msgs/grasp_event.pb.h, but obviously this is not visible outside of the current workspace (and to make it visible you must add this to the include_directories, what looks a bit hackish).

I followed this tutorial , but they don't explain how to export properly the new message .pb.h header. In the worst case, we can make a link to ./build/gazebo_grasp_plugin/msgs/grasp_event.pb.h within the include directory, but pretty sure this is not he right way. @JenniferBuehler, do u have experience with custom pb msgs?

corot commented 3 years ago

btw, I add a 2nd commit in case we want to roll back to the ROS msg, but I would prefer to squash them into one before merging

JenniferBuehler commented 3 years ago

Finally I found some time to remake the PR with gazebo instead of ROS msgs.

But I need some help; looks like I create properly the protobuff msg GraspEvent, but when I try to use it from outside of the workspace containing this pkg, client pkgs subscribing to the topic cannot find the msg definition. Protobuf creates a header in ./build/gazebo_grasp_plugin/msgs/grasp_event.pb.h, but obviously this is not visible outside of the current workspace (and to make it visible you must add this to the include_directories, what looks a bit hackish).

I followed this tutorial , but they don't explain how to export properly the new message .pb.h header. In the worst case, we can make a link to ./build/gazebo_grasp_plugin/msgs/grasp_event.pb.h within the include directory, but pretty sure this is not he right way. @JenniferBuehler, do u have experience with custom pb msgs?

I will look at this when I test it :)

JenniferBuehler commented 3 years ago

Gnawkhhh the weekend just melted away... Sorry I'll try to make some time in the evenings this week

JenniferBuehler commented 3 years ago

Hello, just giving you a heads up that I started testing. Sorry it took me ages. I'm making a few changes to the build files (should answer your question about accessibility) and will post a PR to your branch with the changes. Please have a few more days patience, I'm doing it bit by bit in free time slots. Cheers

corot commented 3 years ago

I'm making a few changes to the build files (should answer your question about accessibility) and will post a PR to your branch with the changes. Please have a few more days patience, I'm doing it bit by bit in free time slots.

Great! tell me if u need any help

JenniferBuehler commented 3 years ago

I followed this tutorial , but they don't explain how to export properly the new message .pb.h header. In the worst case, we can make a link to ./build/gazebo_grasp_plugin/msgs/grasp_event.pb.h within the include directory, but pretty sure this is not he right way.

The problem is that we are compiling with ROS, and install doesn't get executed in devel space. It does if you run in install space (catkin config --install if you're using the catkin tools). It's ugly, but unfortunately in devel space we'll need to keep using the build directory as include - see my PR to your branch. Maybe there is a better solution but I would have to look into it, and I don't think it's that important at the moment to get it perfect.

corot commented 3 years ago

Hope this is what u wanted: 1 commit with the gazebo events + yours with the ROS relay looks good?

just in case, we want to check the history, I pushed the separated commits in this branch

JenniferBuehler commented 3 years ago

Awesome, this is it! Thank you so much for your hard work :) Finally we have a long-awaited feature!