cram2 / cram_moveit

DEPRECATED. Use https://github.com/cram2/cram repository instead.
0 stars 2 forks source link

CollisionObject message definition has changed. How to react? #3

Open mpomarlan opened 8 years ago

mpomarlan commented 8 years ago

Previously, the CollisionObject message contained vectors of geometry_msgs/PoseStamped that was used to place meshes, primitive shapes etc. The message definition has changed however, and now those are vectors of geometry_msgs/Pose.

This impacts cram-moveit:add-collision-object, as the serialization of the collision object produced garbage that moveit couldn't interpret very well.

A quick fix was to ignore stamps in the arguments to add-collision-object, effectively limiting the function to place objects relative to the global frame. The advantage of the fix is that it's simple and doesn't need tf lookups.

A cleaner fix is to have add-collision-object convert the poses it gets into poses in the global frame. I'd like to avoid this, as it needs tf lookups, but if there's public demand for it, that's the way we'll go.

fairlight1337 commented 8 years ago

We could add the TF conversion, keeping in mind that:

Actually I fail to see a valid case for TF frames being different from the fixed frame at the moment. Some might be present in odom_combined for reasons beyond my comprehension currently, but that's a very simple transformation.

There would not be any actual transformation happening for frames equal to the fixed frame, although we would do an extra call to the TF module. TF1 is CRAM-internal and transformations should be looked up locally (possibly with wait), no? With TF2 that would conclude in an actionlib service call, being a lot of overhead.

So, bottom line is: I'd vote for doing the implicit conversion, as that should almost never happen. But if it does (for whatever reasons), we're safe, and the overhead is kind of manageable. This keeps the system compatible and flexible; an assertion (or whatever else punishment measure) would be a bit of an overkill I guess, forcing people into our own shoes.

E.g. grasping relative to other links like torso_lift_link or so would benefit from this; although I don't do that, but other people might.

gaya- commented 8 years ago

I can see very well why some people would like to avoid the "map" frame, as simple actions like moving an arm, that only need robot's own frames, could break if the localization doesn't work. I don't mind converting or not converting. But, good point about assertions, I vote for adding assertions that the frame that is going to be ignored has to be the fixed frame. It's better when your compiler tells you you're using the wrong frame than you spending days debugging wrong coordinates because you forgot to make a tf lookup.

gaya- commented 8 years ago

I can as well be a ros-warn statement.

fairlight1337 commented 8 years ago

I'd agree on the ros-warn. I had discussions with a couple of people that were not using a fixed frame other than odom_combined, and were doing grasping always relative to the robot's own links; perception results never goes through a fixed frame for them, but is transformed into something like torso_lift_link.

This is mostly done due to sensor/localization drift, but also happens when no mobile base is used.