RobotLocomotion / drake

Model-based design and verification for robotics.
https://drake.mit.edu
Other
3.25k stars 1.26k forks source link

Improved diagnostic for parsing non-tree-ordered joints #19034

Open jwnimmer-tri opened 1 year ago

jwnimmer-tri commented 1 year ago

This error can occur when parsing a URDF:

RuntimeError: When creating a model, an attempt was made to add two inboard joints to the same body; this is not allowed. One possible cause might be attempting to weld a robot to World somewhere other than its base link; see Drake issue #17429 for discussion and work-arounds, e.g., reversing some joint parent/child directions. Another possible cause might be attempting to form a kinematic loop using joints; to create a loop, consider using a LinearBushingRollPitchYaw instead of a joint

https://github.com/RobotLocomotion/drake/blob/022a1efdd8b5c052b791412066b9bbd80d42bf55/multibody/tree/multibody_tree_topology.h#L677

Would it be possible to print out the name of the body that has two inboard joints, and the name of these two inboard joints? I think that would help debugging this error.

jwnimmer-tri commented 1 year ago

The challenge here is that class MultibodyTreeTopology does not know about names, only the integer indices.

Assigning to +@amcastro-tri for suggestions on how to approach a solution.

Shall we add names into topology data structures? Or pointers to names?

Or provide a callback function to the topology so it can ask about names for error messages?

Or change add_mobilizer to use something like std::expected to return the error message instead of throwing it, so that the caller (MbT) can add more context to the error message?

rpoyner-tri commented 1 year ago

Do we have access to std::expected yet? Looks all C++23 to me.

jwnimmer-tri commented 1 year ago

That's why I qualified the citation with "something like".

rpoyner-tri commented 1 year ago

\cc @sherm1 does a case like this have any influence on your proposed mb-tree restructuring work?

sherm1 commented 1 year ago

I am currently (post-vacation that is) reworking the MbP graph analysis code, which will now live at a higher level than the MultibodyTreeTopology stuff and hence have access to MbP objects with friendly names. Hopefully the need for these messages will be reduced since we'll be able to deal with reversed joints and ultimately loops, but I'll make sure it produces helpful messages for cases it can't deal with.

It may also be possible to tweak MultibodyTreeTopology to improve its error messages now but the medium-term plan is for that to disappear altogether when I get the new graph analysis code up.

amcastro-tri commented 1 year ago

Shall we add names into topology data structures?

That is not a bad idea at all. However, Sherm's work will essentially do that in a much cleaner revisited structure that'll allow us to cleanly reason about directed graphs and trees. We could add the names now if a high priority, but my vote would be to wait for Sherm's work to land.