-
Notifications
You must be signed in to change notification settings - Fork 70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add exportFirstBaseLinkAdditionalFrameAsFakeURDFBase option to ModelExporter #567
Add exportFirstBaseLinkAdditionalFrameAsFakeURDFBase option to ModelExporter #567
Conversation
I am not documenting this in the changelog as it is an additional option of the ModelExporter class, that it is first introduced in v0.12, and for which there is a changelog entry. |
* Supported formats: urdf. | ||
*/ | ||
bool exportFirstBaseLinkAdditionalFrameAsFakeURDFBase{true}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Model-kdl-urdf-parser relationship based question.
I do not quite understand what might happen if the base link has more than one additional frames and you do not want the first additional link as the fake base link?
I see here https://github.com/traversaro/idyntree/blob/b780bd770a2a2960833cea8abf159a3e576f73c0/src/model_io/urdf/src/URDFModelExport.cpp#L350
we are handling only one additional frame scenario.
Maybe we should create an issue for this to be handled later if it's not necessary now.
And by this feature in the current PR, we handle additionally handle a case if we do not want the first additional frame to be default fake-base.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not quite understand what might happen if the base link has more than one additional frames and you do not want the first additional link as the fake base link?
In that case, baseFakeLinkFrameIndex
remains set to FRAME_INVALID_INDEX
, and all the additional frames of the base link get exported as child fake links in https://github.com/traversaro/idyntree/blob/b780bd770a2a2960833cea8abf159a3e576f73c0/src/model_io/urdf/src/URDFModelExport.cpp#L394 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
In that case,
baseFakeLinkFrameIndex
remains set toFRAME_INVALID_INDEX
, and all the additional frames of the base link get exported as child fake links
Do you think this goes in the documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approved the PR anyways because it was just my personal doubt. Maybe I was a bit confused with the Model and ModelExporter differences ;D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
In that case,
baseFakeLinkFrameIndex
remains set toFRAME_INVALID_INDEX
, and all the additional frames of the base link get exported as child fake linksDo you think this goes in the documentation?
Added in 1db7d57 .
Even if the fast majority of URDF models embed a "fake" base link in the form of a parent URDF link with no inertia to workaround ros/kdl_parser#27 , some models existing in the wild, such as the ROS-Industrial KUKA robots (see for example https://github.com/ros-industrial/kuka_experimental/blob/indigo-devel/kuka_kr16_support/urdf/kr16_2.urdf) actually have a root URDF link with a mass and inertia, and just add additional frames as child frames, i.e. :
to generate URDF models in this form, it is necessary to add an explicit option to the URDF ModelExporter, to decide how to generate the URDF "fake link" corresponding to the first additional frame of the base link.