RobotLocomotion / drake

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

mbp: Errors about redundant / reversed weld joints should provide more information #9939

Open EricCousineau-TRI opened 6 years ago

EricCousineau-TRI commented 6 years ago

Ran into a case where an IIWA SDF had a world-weld joint, but we accidentally added a secondary / redundant weld.

The error message from MultibodyTreeTopolgoy is verbose, but doesn't point to the frames in question (which is honestly the most useful information).

The easiest solution is to print out the frame index, and instruct the user to use get_frame(index).name() to figure out the user-friendly bits.

EricCousineau-TRI commented 4 years ago

@hongkai-dai ran into this recently: Slack: https://drakedevelopers.slack.com/archives/C2WBPQDB7/p1604601033080200

I tried to construct a pendulum using Pendulum.urdf https://github.com/RobotLocomotion/drake/blob/master/examples/pendulum/Pendulum.urdf. I found I can call plant.WeldFrames(plant.world_frame(), plant.GetFrameByName("base")), but if I reverse the order of the two frames as plant.WeldFrames(plant.GetFrameByName("base"), plant.world_frame()), then I got the error

    plant.Finalize()
RuntimeError: This multibody tree already has a mobilizer connecting these two frames. More than one mobilizer between two frames is not allowed

I am wondering if plant.WeldFrames(A, B) has enforced that if A, B are adjacent links, then A has to be the parent of B?

(less verbose, but still a bit obtuse for world welding / parent-child mismatch?)

I won't be able to address this - @amcastro-tri I'm assigning to ya, but if you don't think it's actionable, feel free to close!

sherm1 commented 4 years ago

Related: #13040

EricCousineau-TRI commented 3 years ago

Running into this again in Anzu; in this case, doing a dual-arm setup, and I'm playing with how I glue stuff together (topology) to make calibration (touch-point) a bit simpler for readers to digest.

\cc @siyuanfeng-tri

amcastro-tri commented 3 years ago

Thanks Eric. cc'ing @joemasterjohn since eventually we'll clean up the whole tree vs model topology.

sherm1 commented 2 years ago

Closing this -- #17429 is close enough.

jwnimmer-tri commented 2 years ago

Eric's original post (and the follow-up posts) were about adding two welds, where the complaint is that the second weld is redundant. #17429 is about welding to the middle of a model, which is a different exception path. They don't seem like the same issue to me?

sherm1 commented 2 years ago

My thought is that if we can properly handle the analysis that leads us to use a reverse mobilizer to fix #17429 that analysis will also detect cases where we can't. But I think you're right that we should leave this open until we can verify that the error messages are tolerable.