Skip to content
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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 27, 2019
Merged

Add exportFirstBaseLinkAdditionalFrameAsFakeURDFBase option to ModelExporter #567

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b780bd7
Add exportFirstBaseLinkAdditionalFrameAsFakeURDFBase option to ModelE…
traversaro Aug 27, 2019
1db7d57
Update URDFModelExport.cpp
traversaro Aug 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/model_io/urdf/include/iDynTree/ModelIO/ModelExporter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ class ModelExporterOptions
*/
std::string baseLink;

/**
* Select if the first additional frame of the base link is exported as fake base link.
*
* The URDF exporter by default exports the first additional frame of the base link as
* a parent "fake" link to the actual base link, as a workaround for https://github.com/ros/kdl_parser/issues/27).
* By setting this option to false, is possible to disable this behaviour, for more info see iDynTree::ModelExporter docs.
* This option is ignored in non-URDF exporter.
*
* Default value: true.
* Supported formats: urdf.
*/
bool exportFirstBaseLinkAdditionalFrameAsFakeURDFBase{true};

Copy link
Contributor

@prashanthr05 prashanthr05 Aug 27, 2019 •
edited
Loading

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.

Copy link
Member Author

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 .

Copy link
Contributor

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 to FRAME_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?

Copy link
Contributor

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

Copy link
Member Author

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 to FRAME_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?

Added in 1db7d57 .

/**
* Constructor.
*/
Expand Down Expand Up @@ -84,9 +97,10 @@ class ModelExporterOptions
*
* Furthermore, it is widespread use in URDF models to never use a real link (with mass) as the root link of a model, mainly
* due to workaround a bug in official %KDL parser used in ROS (see https://github.com/ros/kdl_parser/issues/27 for more info). For this reason,
* if the selected base_link has at least one additional frame, the first additional frame of the base link is added as a **parent** fake URDF link,
* if the selected base_link has at least one additional frame, by default the first additional frame of the base link is added as a **parent** fake URDF link,
* instead as a **child** fake URDF link as done with the rest of %iDynTree's additional frames. If no additional frame is available for the base link,
* the base link of the URDF will have a mass, and will generate a warning then used with the ROS's [`kdl_parser`](https://github.com/ros/kdl_parser) .
* This behaviour can be disabled by setting to false the `exportFirstBaseLinkAdditionalFrameAsFakeURDFBase` attribute of ModelExporterOptions.
*
*/
class ModelExporter
Expand Down
5 changes: 3 additions & 2 deletions src/model_io/urdf/src/URDFModelExport.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,11 +345,12 @@ bool URDFStringFromModel(const iDynTree::Model & model,
}

// If the base link has at least an additional frame, add it as parent URDF link
// as a workaround for https://github.com/ros/kdl_parser/issues/27
// as a workaround for https://github.com/ros/kdl_parser/issues/27, unless
// options.exportFirstBaseLinkAdditionalFrameAsFakeURDFBase is set to false
FrameIndex baseFakeLinkFrameIndex = FRAME_INVALID_INDEX;
std::vector<FrameIndex> frameIndices;
ok = model.getLinkAdditionalFrames(baseLinkIndex, frameIndices);
if (ok && frameIndices.size() >= 1) {
if (ok && frameIndices.size() >= 1 && options.exportFirstBaseLinkAdditionalFrameAsFakeURDFBase) {
baseFakeLinkFrameIndex = frameIndices[0];
ok = ok && exportAdditionalFrame(model.getFrameName(baseFakeLinkFrameIndex),
model.getFrameTransform(baseFakeLinkFrameIndex),
Expand Down