-
Notifications
You must be signed in to change notification settings - Fork 39
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 SubModelKinDynWrapper class #605
Add SubModelKinDynWrapper class #605
Conversation
Please, ping me again after #604 is merged and this is ready to be reviewed. Now I can see both PRs code and it is confusing 😊 |
a56a473
to
ef6fc89
Compare
c19baa4
to
47c6b65
Compare
Hi @S-Dafarra @GiulioRomualdi, the PR (#604) blocking this one has been merged. It is ready for review. |
src/Estimators/include/BipedalLocomotion/RobotDynamicsEstimator/SubModelKinDynWrapper.h
Outdated
Show resolved
Hide resolved
src/Estimators/include/BipedalLocomotion/RobotDynamicsEstimator/SubModelKinDynWrapper.h
Outdated
Show resolved
Hide resolved
src/Estimators/include/BipedalLocomotion/RobotDynamicsEstimator/SubModelKinDynWrapper.h
Show resolved
Hide resolved
Eigen::VectorXd motorTorques(numberOfJoints); | ||
Eigen::VectorXd frictionTorques(numberOfJoints); | ||
Eigen::VectorXd contactTorques(numberOfJoints); | ||
|
||
Eigen::VectorXd baseAcceleration(6); | ||
REQUIRE(kinDynSubModel.getBaseAcceleration(kinDyn, | ||
robotBaseAcceleration, | ||
robotJointAcceleration, | ||
baseAcceleration)); | ||
|
||
Eigen::VectorXd baseVelocity(6); | ||
baseVelocity = kinDynSubModel.getBaseVelocity(kinDyn); | ||
|
||
Eigen::VectorXd jointAcceleration(numberOfJoints); | ||
|
||
for (int i = 0; i < numberOfJoints; i++) | ||
{ | ||
motorTorques(i) = random_double(); | ||
frictionTorques(i) = random_double(); | ||
contactTorques(i) = random_double(); | ||
} | ||
|
||
REQUIRE(kinDynSubModel.inverseDynamics(motorTorques, | ||
frictionTorques, | ||
contactTorques, | ||
baseAcceleration, | ||
jointAcceleration)); |
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.
Does it make sense to run these tests with meaningful inputs to check if the output makes sense?
Tomorrow I will do a better review, but I noticed that you are using idyntree matrices and vectors in the interface can you use eigen or manif? |
@GiulioRomualdi @S-Dafarra I changed the code after your reviews. It's ready to be checked again. Thanks. |
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 just noticed a couple of other dynamic memory allocations. Apologies for not noticing it before.
1502d99
to
32cfd8c
Compare
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.
Minor comment
32cfd8c
to
a88121d
Compare
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.
In general, you may avoid using iDynTree::Vector
and Matrices
since all the KinDynComputations get and set methods take as input iDynTree::Span
and iDynTree::MatrixView
for (int idx = 0; idx < m_subModel.getJointMapping().size(); idx++) | ||
{ | ||
m_qj.setVal(idx, m_jointPositionModel[m_subModel.getJointMapping()[idx]]); | ||
m_dqj.setVal(idx, m_jointVelocityModel[m_subModel.getJointMapping()[idx]]); | ||
} | ||
|
||
if (!m_kinDyn.setRobotState(m_worldTBase, m_qj, m_baseVelocityiDyn, m_dqj, m_worldGravity)) | ||
{ | ||
blf::log()->error("{} Unable to set the robot state.", logPrefix); | ||
return false; | ||
} |
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.
You can use directly eigen vectors here
bool inverseDynamics(Eigen::VectorXd& motorTorqueAfterGearbox, | ||
Eigen::VectorXd& frictionTorques, | ||
Eigen::VectorXd& tauExt, | ||
Eigen::VectorXd& baseAcceleration, | ||
Eigen::VectorXd& jointAcceleration); |
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.
you can use the eigen ref
const Eigen::VectorXd& robotBaseAcceleration, | ||
const Eigen::VectorXd& robotJointAcceleration, |
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.
as above
* @param kinDynFullModel is the KinDynComputation object associated to a model. | ||
* @return baseVelocity the the base velocity of the sub-model. | ||
*/ | ||
Eigen::Ref<Eigen::VectorXd> getBaseVelocity(std::shared_ptr<iDynTree::KinDynComputations> kinDynFullModel); |
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.
This gives the user the possibility to change the base velocity is this wanted?
Eigen::Ref<Eigen::VectorXd> getBaseVelocity(std::shared_ptr<iDynTree::KinDynComputations> kinDynFullModel); | |
Eigen::Ref<const Eigen::VectorXd> getBaseVelocity(std::shared_ptr<iDynTree::KinDynComputations> kinDynFullModel) const; |
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 this should be a manif::SE3d::Tangent
object
#ifndef BIPEDAL_LOCOMOTION_ESTIMATORS_SUBMODELKINDYNWRAPPER_H | ||
#define BIPEDAL_LOCOMOTION_ESTIMATORS_SUBMODELKINDYNWRAPPER_H |
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.
#ifndef BIPEDAL_LOCOMOTION_ESTIMATORS_SUBMODELKINDYNWRAPPER_H | |
#define BIPEDAL_LOCOMOTION_ESTIMATORS_SUBMODELKINDYNWRAPPER_H | |
#ifndef BIPEDAL_LOCOMOTION_ESTIMATORS_SUB_MODEL_KINDYN_WRAPPER_H | |
#define BIPEDAL_LOCOMOTION_ESTIMATORS_SUB_MODEL_KINDYN_WRAPPER_H | |
std::map<const std::string, Eigen::VectorXd> m_dJnuList; /**< Accelerometer bias accelerations */ | ||
std::map<const std::string, Eigen::Matrix3d> m_accRworldList; /**< Rotation matrix of the | ||
accelerometer frame wrt world */ | ||
Eigen::VectorXd m_baseVelocity; /**< Velocity of the base of the sub-model */ |
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.
We use manif::SE3Tangentd for the velocity
std::map<const std::string, Eigen::MatrixXd> m_jacContactList; /**< Jacobians of the external | ||
contacts */ | ||
std::map<const std::string, Eigen::VectorXd> m_dJnuList; /**< Accelerometer bias accelerations */ | ||
std::map<const std::string, Eigen::Matrix3d> m_accRworldList; /**< Rotation matrix of the |
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.
std::map<const std::string, Eigen::Matrix3d> m_accRworldList; /**< Rotation matrix of the | |
std::map<const std::string, manif::SO3d> m_accRworldList; /**< Rotation matrix of the |
if (!m_kinDyn.getFreeFloatingMassMatrix(m_idynMassMatrix)) | ||
{ | ||
blf::log()->error("{} Unable to get the mass matrix of the sub-model.", logPrefix); | ||
return false; | ||
} | ||
m_massMatrix = iDynTree::toEigen(m_idynMassMatrix); |
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.
if (!m_kinDyn.getFreeFloatingMassMatrix(m_idynMassMatrix)) | |
{ | |
blf::log()->error("{} Unable to get the mass matrix of the sub-model.", logPrefix); | |
return false; | |
} | |
m_massMatrix = iDynTree::toEigen(m_idynMassMatrix); | |
if (!m_kinDyn.getFreeFloatingMassMatrix(m_massMatrix)) | |
{ | |
blf::log()->error("{} Unable to get the mass matrix of the sub-model.", logPrefix); | |
return false; | |
} |
m_FTBaseAcc.resize(m_numOfJoints); | ||
m_totalTorques.resize(m_numOfJoints); | ||
|
||
m_idynMassMatrix = iDynTree::FreeFloatingMassMatrix(m_subModel.getModel()); |
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.
m_idynMassMatrix = iDynTree::FreeFloatingMassMatrix(m_subModel.getModel()); |
iDynTree::VectorDynSize m_qj; | ||
iDynTree::VectorDynSize m_dqj; | ||
|
||
iDynTree::FreeFloatingMassMatrix m_idynMassMatrix; |
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.
iDynTree::FreeFloatingMassMatrix m_idynMassMatrix; |
a9f94be
to
1f2067f
Compare
@GiulioRomualdi I changed the code following your suggestions. I hope I got how to change it. |
1eb1edb
to
6861db0
Compare
…information of a sub-model
6861db0
to
9861bdb
Compare
Adding the
SubModelKinDynWrapper
class. It handles theKinDynComputation
object of a sub-model and keeps updated the dynamics and kinematics information.This PR is blocked by #604.