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

Bad Inertia Test (False Positive) #32

Open
guru-florida opened this issue Dec 30, 2019 · 3 comments
Open

Bad Inertia Test (False Positive) #32

guru-florida opened this issue Dec 30, 2019 · 3 comments

Comments

@guru-florida
Copy link

guru-florida commented Dec 30, 2019

Take a closer look at the code for this test:
https://github.com/ros/kdl_parser/blob/melodic-devel/kdl_parser/test/test_inertia_rpy.cpp

Torques output arrays are zero-size
Test does not properly size the torques_1 or torques_2 JntArrays. Therefor solver.CartToJnt(...) call actually returns -4 (E_SIZE_MISMATCH) because the first thing CartToJnt() method does is check that all the JntArray have the same size (matching number of joints). Unfortunately, the error value from CartToJnt() is not captured or checked/asserted.

False Positive
This test passes because the two torques_1 and 2 arrays are black (0 elements) and therefor are still considered equal but the CartToJnt() method is not actually tested.

Recommendations:

  • Before the CartToJnt() call, add torques_1.resize(chain.getNrOfJoints())
  • Also Wrap the solver.CartToJnt() call in an ASSERT() checking that it returns KDL::SolverI::E_NOERROR
  • perhaps also ensure that at least 1 torques_ array is non-zero?

For reference, CartToJnt() method code:
http://docs.ros.org/jade/api/orocos_kdl/html/chainidsolver__recursive__newton__euler_8cpp_source.html

Thanks!

@clalancette
Copy link
Collaborator

I know we are approaching a year since this was opened, so sorry about that. If you'd be willing to open a pull request to fix the test as you think it should be, we can iterate there. Thanks for opening this issue.

@guru-florida
Copy link
Author

Sure, I can do that. I have some other tickets to attend to first but I will schedule this out.

@guru-florida
Copy link
Author

Feel free to assign this to me.

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

No branches or pull requests

2 participants