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 degraded status chainiksolverpos #358

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

craigirobot
Copy link

@craigirobot craigirobot commented Sep 28, 2021

The nr position solver currently produces a non-convergence error, if the Cartesian pose does not converge to the desired value within the specified maximum number of iterations. However, the lack of convergence could be due to the singularity avoidance being active leaving the solver in a "degraded" state. So that it cannot achieve the desired pose.

This commit adds a degraded flag which is checked, when the maximum number of iterations is reached. If it is degraded, then the solver error is set to E_DEGRADED and the user can then decide whether it should be allowed to continue. Otherwise, the error is set to E_MAX_ITERATIONS_EXCEEDED which would normally terminate the process.
In addition, functions were added to set/get the input parameters (maxiter, eps) and retrieve other data from the solver (twist error, number of iterations, velocity solver status).

ccarigna added 2 commits September 28, 2021 13:59
The position solver was causing an IK failure if it did not converge after the maximum number of iterations was exceeded.  However, when singularity avoidance is active (degraded), the solution may not converge but the IK should be allowed to continue.  So a degraded flag was added and is checked when the maximum number of iterations is exceeded.  If it is degraded, the error is set to E_DEGRADED to allow it to continue.  Otherwise, it is set to E_MAX_ITERATIONS_EXCEEDED which will cause the IK to fail.  Added functions to set the input parameters (maxiter, eps) and to retrieve the twist error, number of iterations performed, and the velocity solver status.
…singular

If the maximum iterations is reached but the velocity solver returns a singular status, it is now flagged as degraded and allowed to continue.  Because it was singular, it could not reduce the error to zero which is why it kept iterating to the limit.  This does not constitute a failure of the inverse kinematics.
@MatthijsBurgh
Copy link
Collaborator

Please follow code styler more strictly. Please add more documentations what the new variables are used for.

@snrkiwi
Copy link
Contributor

snrkiwi commented Oct 11, 2021

Please follow code styler more strictly. Please add more documentations what the new variables are used for.

@MatthijsBurgh Where is the code style documented? It seems like over the years that no really style has been enforced here. Are there particular things you're looking for now?

@MatthijsBurgh
Copy link
Collaborator

@snrkiwi there is indeed no strict guide. I am aiming for the ros1 style but with 4 space indent

@snrkiwi
Copy link
Contributor

snrkiwi commented Oct 11, 2021

Ok, it'll be good to have a convention. Is there somewhere in the doc's that we can document this, and add a URL to the ROS1 style (along with any modifications)?

@MatthijsBurgh
Copy link
Collaborator

We opened an issue a while back, #258

@craigirobot
Copy link
Author

I am not certain where this stands. Are there specific code style issues that need to be addressed to move this forward? I tried to follow the style that was already being used even though I did not always agree with it. Note that there is already documentation of the new variables in the header file, but I can expand upon that if desired.

orocos_kdl/src/chainiksolverpos_nr.cpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.cpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.cpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.cpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.cpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.cpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.hpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.hpp Outdated Show resolved Hide resolved
orocos_kdl/src/chainiksolverpos_nr.hpp Outdated Show resolved Hide resolved
/**
* Get delta twist from last call to CartToJnt()
*/
void getDeltaTwist(KDL::Twist& _delta_twist)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reason why not to just simple return?

Copy link
Author

@craigirobot craigirobot Nov 24, 2021

Choose a reason for hiding this comment

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

Do you mean something like this?

Twist& getDeltaTwist()
{
    return delta_twist;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, indeed. But then get a const reference.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, so this?

const Twist& getDeltaTwist()const
{
return delta_twist;
}

Copy link
Author

Choose a reason for hiding this comment

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

I just added this commit.

craigirobot and others added 9 commits November 24, 2021 10:09
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
Co-authored-by: Matthijs van der Burgh <matthijs.vander.burgh@live.nl>
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.

3 participants