Skip to content
This repository has been archived by the owner on Oct 29, 2018. It is now read-only.

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

Open
mpomarlan opened this issue Jul 22, 2016 · 4 comments
Open

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

mpomarlan opened this issue Jul 22, 2016 · 4 comments
Assignees

Comments

@mpomarlan
Copy link
Contributor

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
Copy link
Contributor

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

  • The furniture, and everything that comes from the semantic map is defined in map already anyway (not explicitly, but implicitly)
  • The TF lookups to do are comparatively low in number (objects seen in the environment are mapped to map internally, too)

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-
Copy link
Member

gaya- commented Jul 25, 2016

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-
Copy link
Member

gaya- commented Jul 25, 2016

I can as well be a ros-warn statement.

@fairlight1337
Copy link
Contributor

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants