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

iCubGenova09: Add FT sensor to right upper leg #218

Merged
merged 5 commits into from
Feb 17, 2022

Conversation

mfussi66
Copy link
Member

@mfussi66 mfussi66 commented Feb 11, 2022

As per title, this PR adds the FT sensor on the right leg of iCub3 to reflect the addition on the real robot.

Its transformation relative to the root link is the following:

-0.866025  -0.5        0
-0.5        0.866025   0
 0          0         -1

Which corresponds to the transform in the CAD, and therefore it corresponds to the way it is mounted:
MicrosoftTeams-image

cc @S-Dafarra @pattacini

Copy link
Member

@Nicogene Nicogene left a comment

Choose a reason for hiding this comment

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

@mfussi66 thanks for the PR, the CI is failing because there is a mismatch between the expected number of FT and the actual number since you added two sensors :

icub-model-test : odd number of F/T sensor found in the model
Could you please fix the test accordingly?

@Nicogene
Copy link
Member

Actually the problem is that the left leg sensor is missing:

if( nrOfFTSensors % 2 == 1 )
{
std::cerr << "icub-model-test : odd number of F/T sensor found in the model" << std::endl;
return false;
}

@mfussi66
Copy link
Member Author

mfussi66 commented Feb 11, 2022

Actually the problem is that the left leg sensor is missing:

Yep @Nicogene , only the right one was mounted on the robot. I guess changing the test to check against zero should suffice.

@Nicogene
Copy link
Member

Yep @Nicogene , only the right one was mounted on the robot. I guess changing the test to check against zero should suffice.

Yes, then if it is intentional it is fine to change the test

@mfussi66
Copy link
Member Author

Additionally, I think I'll need to change the check on the correct orientation of the sensor:

bool checkFTSensorsAreCorrectlyOrientedV3(iDynTree::KinDynComputations & comp)
{
iDynTree::Rotation rootLink_R_sensorFrameLeftArmExpected =
iDynTree::Rotation(-0.267012, -0.961047, 0.0713662,
-0.960459, 0.271447, 0.0619303,
-0.0788901, -0.0520081, -0.995526);
iDynTree::Rotation rootLink_R_sensorFrameRightArmExpected =
iDynTree::Rotation(-0.267012, 0.961047, 0.0713662,
0.960459, 0.271447, -0.0619303,
-0.0788901, 0.0520081, -0.995526);
iDynTree::Rotation rootLink_R_sensorFrameExpectedFoot =
iDynTree::Rotation(-0.5, 0.866025, 0,
-0.866025, -0.5, 0,
0, 0, 1);
bool ok = checkFTSensorIsCorrectlyOriented(comp, rootLink_R_sensorFrameLeftArmExpected, "l_arm_ft_sensor");
ok = checkFTSensorIsCorrectlyOriented(comp, rootLink_R_sensorFrameRightArmExpected, "r_arm_ft_sensor") && ok;
// l_leg_ft_sensor and r_leg_ft_sensor frames seems to be wrong(see https://github.com/robotology/icub-models/issues/71)
ok = checkFTSensorIsCorrectlyOriented(comp, rootLink_R_sensorFrameExpectedFoot, "l_foot_rear_ft_sensor") && ok;
ok = checkFTSensorIsCorrectlyOriented(comp, rootLink_R_sensorFrameExpectedFoot, "r_foot_rear_ft_sensor") && ok;
ok = checkFTSensorIsCorrectlyOriented(comp, rootLink_R_sensorFrameExpectedFoot, "l_foot_front_ft_sensor") && ok;
ok = checkFTSensorIsCorrectlyOriented(comp, rootLink_R_sensorFrameExpectedFoot, "r_foot_front_ft_sensor") && ok;
return ok;
}

@mfussi66 mfussi66 marked this pull request as draft February 11, 2022 16:18
@GiulioRomualdi
Copy link
Member

cc @isorrentino

@mfussi66 mfussi66 marked this pull request as ready for review February 14, 2022 14:46
@mfussi66 mfussi66 requested a review from Nicogene February 14, 2022 14:46
@traversaro
Copy link
Member

@mfussi66 are we waiting for something or can we merge?

@mfussi66
Copy link
Member Author

@mfussi66 are we waiting for something or can we merge?

I was waiting for the approval from @Nicogene, on my side I think we can merge.

Copy link
Member

@Nicogene Nicogene left a comment

Choose a reason for hiding this comment

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

LGTM !

@pattacini
Copy link
Member

Shall we merge?

@traversaro traversaro merged commit 9c2ef28 into robotology:master Feb 17, 2022
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.

5 participants