-
Notifications
You must be signed in to change notification settings - Fork 284
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 an option to skip interpolation in the joint trajectory controller #374
Conversation
b80481a
to
37db860
Compare
joint_trajectory_controller/include/joint_trajectory_controller/interpolation_methods.hpp
Outdated
Show resolved
Hide resolved
joint_trajectory_controller/src/joint_trajectory_controller.cpp
Outdated
Show resolved
Hide resolved
joint_trajectory_controller/src/joint_trajectory_controller.cpp
Outdated
Show resolved
Hide resolved
time_before_traj_msg_, state_before_traj_msg_, first_point_timestamp, first_point_in_msg, | ||
sample_time, output_state); | ||
// If interpolation is disabled, just forward the next waypoint | ||
if (interpolation_method == joint_trajectory_controller::InterpolationMethod::NONE) |
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.
Shouldn't we check this before calling sample in the update
method?
What I understand, we will anyway have also ruckig based interpolation this means that we would need some base class for interpolations. If so we can then just decide at the begining in the configure which interpolation should be used and if non is used then we have default "passthrough" interpolation.
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 should also reduce number of checks.
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 don't think Ruckig is going to be a drop-in replacement for the spline interpolation that's currently there. It requires position/vel/accel inputs. What if there is only a position
interface active? Then Ruckig will not have vel/accel input. So we will often still need to run some type of interpolation before Ruckig.
I'm trying to figure out the logic re. your comment on update
...
This pull request is in conflict. Could you fix it @AndyZe? |
c95cc2a
to
1d489b1
Compare
1d489b1
to
0527751
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.
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.
Few comments to increase maintainability and clarity in the code. Sorry, guys if I am starting to be too picky, and please tell me if I am drifting away in my requests/concerns.
joint_trajectory_controller/include/joint_trajectory_controller/interpolation_methods.hpp
Show resolved
Hide resolved
joint_trajectory_controller/include/joint_trajectory_controller/interpolation_methods.hpp
Show resolved
Hide resolved
joint_trajectory_controller/src/joint_trajectory_controller.cpp
Outdated
Show resolved
Hide resolved
17ad373
to
fb384bc
Compare
fb384bc
to
4c9f59a
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.
Thanks for all the follow-ups!
…os-controls#374) * Introduce `InterpolationMethods` structure * Use parameters to define interpolation use in JTC
This should be useful if a user has a trajectory that is already interpolated to a fine degree, or if the low-level controllers just don't handle fine waypoint spacing well.
To use, just set the parameter in the yaml file. If the parameter is unset, it defaults to the previous method (which is spline-based). The default spline-based method doesn't obey joint limits (position/vel/accel/jerk), which is another reason to use this.