-
Notifications
You must be signed in to change notification settings - Fork 301
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 joint limiter interface plugins to enforce limits defined in the URDF #1526
base: master
Are you sure you want to change the base?
Add joint limiter interface plugins to enforce limits defined in the URDF #1526
Conversation
This pull request is in conflict. Could you fix it @saikishor? |
This pull request is in conflict. Could you fix it @saikishor? |
61a0949
to
876a52c
Compare
* @param dt The time step. | ||
* @return The position limits. | ||
*/ | ||
std::pair<double, double> compute_position_limits( |
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 have a suggestion from a code reader's point of view. What do you think about creating a struct with the min
and max
members instead? Using std::pair
results later in the use of first
and second
accessors, which does not describe the purpose of the variable.
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.
Hello!
As these are helper methods alone. I want to keep them simple. Making a new struct and passing them around is something I wanted to avoid. That's the main reason I avoided using such new structs
This pull request is in conflict. Could you fix it @saikishor? |
82107d2
to
7b6c4ff
Compare
c4da2d0
to
f088099
Compare
f088099
to
51a69af
Compare
@@ -70,55 +70,58 @@ class JointLimiterInterface | |||
// TODO(destogl): get limits from URDF |
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 think this is not needed anymore. Am I right? Do we want to get here limits from URDF? This might make sense to parse URDF limits also in the controller if this are required.
return eff_limits; | ||
} | ||
|
||
std::pair<double, double> compute_acceleration_limits( |
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.
Don't we have to consider here position and velocity limits too?
…rfacesData> and enforce hard limtis
1d96350
to
2f91440
Compare
This PR includes the changes of the PR from @destogl #971, and on top of that it extends the proposed methods to be able to integrate and be able to provide an interface that will allow enforcing the joint limits in position, velocity, effort, acceleration, etc. The proposed interface can accept different types of data in the enforce methods with the help of templating and this can be helpful to extend to different applications in the future.
This PR is done as a part of #1466 to be able to integrate the limiters within the
ResourceManager
Closes #971