sdformat icon indicating copy to clipboard operation
sdformat copied to clipboard

Better way to (not) specify `//joint/axis/xyz` when it's not used?

Open EricCousineau-TRI opened this issue 4 years ago • 1 comments

Came up here: https://github.com/robotlocomotion/drake/pull/13824#pullrequestreview-462497185

For certain joint types, like @type="ball", it doesn't really make sense to have //axis/xyz specified?

However, when I look around, sometimes it is, sometimes it isn't?

  • Where it isn't -- I think this is correct?: https://github.com/osrf/sdformat/blob/f94626a8898b0a2898ae65892d76fd1031456133/test/sdf/joint_complete.sdf#L67-L71
  • Where it is -- I think this is wrong?: https://github.com/osrf/gazebo/blob/884d4d57b52dbe873ebec0e9bbb8a95b10ea4b6b/test/worlds/ball_joint_test.world#L145-L166

I understand //axis is used to set properties like dynamics (I think?), so I think //axis is OK-ish (even if it's a bit of a misnomer, since it's for multiple axes), but I think it should then require only one value of //axis/xyz (or if possible, no value)?

At present, the tutorial has no mention of this case, nor any recommendation (e.g. don't, or expect a warning, or ...): http://sdformat.org/tutorials?tut=spec_model_kinematics&cat=specification&#jointaxis

\cc @azeey @scpeters

EricCousineau-TRI avatar Aug 06 '20 23:08 EricCousineau-TRI

Correct, a ball joint shouldn't need an //axis/xyz element at all. I was going to say that it shouldn't need an //axis at all, but I could imagine how //axis/dynamics/damping and //axis/dynamics/friction could be applied to any relative rotational motion for a ball joint, so I won't go quite that far.

According to the spec, //joint/axis is not required, but //axis/xyz is required, which is not compatible with what we're talking about here. So I think we should set required = 0 for //axis/xyz in the following line to support this use case:

https://github.com/osrf/sdformat/blob/master/sdf/1.8/joint.sdf#L53

scpeters avatar Aug 07 '20 00:08 scpeters