Inconsistency with axis parsing in SdfParser

[mxgrey]

I've noticed that readRevoluteJoint, readPrismaticJoint, readScrewJoint, and readUniversalJoint are all parsing "axis" elements, but only readRevoluteJoint is checking for a "use_parent_model_frame" element. This inconsistency seems very suspicious to me.

Is that tag something unique to Revolute Joints? I can't imagine why it would be. I'm going to make a function for parsing axis elements and use that function for each joint parser in order to have better consistency among them. Stop me if there are some irregularities I should know about.

[jslee02]

It should be applied to every joint type. I added it for RevoluteJoint for quick test and forgot to add it for other joint types.

[mxgrey]

Okay, I'm making a separate function for axis parsing and then I'll have it be used in every joint type. That should help code complexity and maintainability.

[jslee02]

Sounds good!

[mxgrey]

On a somewhat related note: is there a particular motive for having the SoftSdfParser separate from the SdfParser? Is the format for the soft SDFs any different than some new element types being introduced to SDF? Are they being kept separate only because the soft SDF is experimental?

[jslee02]

In the first place, DART had separate classes and parsers to support soft body dynamics such as SoftBodyNode, SoftSkeleton, SoftWorld, SoftSkelParser, and SoftSdfParser. Later we decided to merge them into the original classes except for SoftBodyNode, and SoftSdfParser is just left to be merged to SdfParser which hasn't been done yet.

SDF already has new element types to support DART's soft body node. However, SoftSdfParser and SDF has little bit different elements, and the places in the structure are also different. So we need to adapt to SDF's one.

[mxgrey]

I've found a lot of copy/pasting in the SoftSdfParser implementation, so I've cut that down heavily with a little bit of templating and std::function. Now SoftSdfParser will be a much simpler extension of SdfParser.

I'm going to leave the task of updating to the latest SDF format to someone else, since I don't use soft bodies myself.

[jslee02]

Sure. I created an issue #371 for it.

[mxgrey]

I think this issue can be closed, because it has been addressed in pull request #369 which is just pending cleanup and review at this point.