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.

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 inertial parameters regressor method to KinDynComputations #480

Merged
merged 5 commits into from
Sep 12, 2018

Conversation

traversaro
Copy link
Member

In particular, add the inverseDynamicsInertialParametersRegressor that
computes the (6+nrOfDofs) \times (10*nrOfLinks) matrix that maps the inertial parameters
to the part of inverse dynamics that depends on inertial parameters,
and the generalizedExternalForces that computes the contribution of external
forces to inverse dynamics (useful because those are not included in the
torques computed by the regressor).

I would still like to add some more docs, but nevertheless the code and the tests should be complete.

@traversaro
Copy link
Member Author

traversaro commented Sep 10, 2018

With this commit, the KinDynComputations class should now have feature parity w.r.t. to the old DynamicsComputations class.

@traversaro traversaro changed the title Add inertial parameters regressors method to KinDynComputations Add inertial parameters regressor method to KinDynComputations Sep 10, 2018
// Check that ok is actually true

KinDynComputations kinDynComp;
kinDynComp.loadRobotModel("model.urdf");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not work!

kinDynComp.loadRobotModel(mdlLoader.model());

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 have toooo many failed attempts to get good documentation around, I should just thrown them our. :D
Anyway, nice catch!

/**
* Compute the getNrOfDOFS()+6 vector of generalized external forces.
*
* The semantics of baseAcc, the base part of baseForceAndJointTorques
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO using "variable name style", i.e. baseAcc and baseForceAndJointTorques in a function that does not accept these variables is a bit confusing.
Same for the inverseDynamicsInertialParametersRegressor method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's a left over, thanks for catching that.

@traversaro
Copy link
Member Author

Pushed the docs of the new method, and some general improvements of the docs of KinDynComputations. The documentation will probably need many more improvements, but this is a first step. Example:

regressor

@traversaro
Copy link
Member Author

I think I need to stop in trying to fix all the docs otherwise I will do that for one month. : )
Feel free to review @francesco-romano
@prashanthr05 I also tagged you for a review, but I think I will merge, feel free to comment also once the PR has been merged.

In particular, add the inverseDynamicsInertialParametersRegressor that
computes the (6+nrOfDofs) \times (10*nrOfLinks) matrix  that maps the inertial parameters
to the part of inverse dynamics that depends on inertial parameters,
and the generalizedExternalForces that computes the contribution of external
forces to inverse dynamics (useful because those are not included in the
torques computed by the regressor).
Cleanup also documentation for KinDynComputations, to simplify the
process of adding docs for the new method.
@traversaro traversaro merged commit 986a5d6 into robotology:devel Sep 12, 2018
@traversaro traversaro deleted the addRegressor branch September 12, 2018 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants