-
Notifications
You must be signed in to change notification settings - Fork 3
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
Move the linkFrames in case of the axis of the joint does not pass through the csys #106
Conversation
Right now I am handling the Moreover, I am always invoking |
src/creo2urdf/src/Creo2Urdf.cpp
Outdated
// Convert modelToExport in a URDF-compatible model (using the default base link) | ||
iDynTree::Model modelToExportURDFCompatible; | ||
|
||
bool ok = iDynTree::moveLinkFramesToBeCompatibleWithURDFWithGivenBaseLink(mdl, modelToExportURDFCompatible); |
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 am a bit afraid in doing this unconditionally, as it could hide some bugs in the assignment of the frames in the models in which all the link frames are assigned. Perhaps we could do this only if link frames are not assigned, or have an explicit option to enable this mode that is off on models that do not need it?
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.
Perhaps we could do this only if link frames are not assigned
Personally I'm in favour of this as it would avoid complicating the yaml.
@mfussi66 @traversaro in the latest commit I changed a little bit the state flow:
|
src/creo2urdf/src/Creo2Urdf.cpp
Outdated
idyn_model.visualSolidShapes().getLinkSolidShapes()[idyn_model.getLinkIndex(urdf_child_link_name)][0]->setLink_H_geometry(oldChild_H_newChild.inverse() * link_H_visual_solid_shape); | ||
// Check if the axis is aligned with the link frame | ||
if ( parent_link_frame == "CSYS" || idyn_axis.getDistanceBetweenAxisAndPoint(iDynTree::Position::Zero()) > 1e-7 ) { | ||
printToMessageWindow("The axis " + axis_name + " of "+ joint_name+ " is not aligned with the link frame " + parent_link_frame + ", the axis will be moved to the link frame in order to be urdf-compliant", c2uLogLevel::WARN); |
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 am a bit dubious about printing this as a warning. In the case of iCub 2.7, this is expected, and adding warning that can't be fixed is typically a good way to ensure that people actually do not check warnings. Anyhow, this is non blocking.
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.
Actually that if is not correct, because that distance will be never > 0, I put the PR in draft until I fix it
0939a54
to
eb10745
Compare
@mfussi66, @traversaro I managed to correctly handle when moving the frames, only when it is not user-specified (i.e. Creo2urdf exports both ergocub and iCub correctly now. Moreover, I removed the warning, since there is already plenty of and they can be confusing for the user. Merging! |
Awesome! I'll try that with other models |
This feature is available in the latest release: |
It fixes:
Thanks heap to @traversaro who added the required feature to idyntree.
This version of
creo2urdf
is able to export the urdf from the mistreated icub 2.5 simulation model, see: