-
Notifications
You must be signed in to change notification settings - Fork 0
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
[JTC] Move PID to controller plugin #2
Conversation
0c2f441
to
71b6efb
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.
I'll review rest later. So far it looks spot on
if (use_closed_loop_pid_adapter_) | ||
{ | ||
trajectory_msgs::msg::JointTrajectory traj; | ||
traj.points.push_back(state_current_); |
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.
@christophfroehlich Maybe also adding the state_desired_
would help
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.
computeGains
takes a full trajectory, here it is only initialized with the same points as used for set_hold_position
As mentioned in the description, an update for dynamic reconfigure is missing.
But there is also a call missing for receiving a new trajectory, I'll push an update here.
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 pushed a new update
lock_cmd_joint_names = get_node()->add_pre_set_parameters_callback( | ||
[this](std::vector<rclcpp::Parameter> & parameters) | ||
{ | ||
for (auto & parameter : parameters) | ||
{ | ||
if (parameter.get_name() == "command_joints") | ||
{ | ||
RCLCPP_ERROR( | ||
get_node()->get_logger(), | ||
"The parameter 'command_joints' is read-only. You can't change it."); | ||
parameter = rclcpp::Parameter("command_joints", command_joint_names_); | ||
} | ||
} | ||
}); |
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.
@christophfroehlich Shouldn't this be set before the set_parameter
line?
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.
no because I wouldn't be able to set the parameter then. it should just "lock" the parameter afterwards
8b48b19
to
d22d5ff
Compare
de738e6
to
4927435
Compare
Up to now there were only two possibilities of controlling the system with JTC:
The aim of this PR is to create an interface for supporting new control strategies, and enable even creating custom controller plugins by users. This is done by plugins of base class
TrajectoryControllerBase
, loaded by the pluginlib.Together with the change of #3 it will even be supported to integrate state-space controller where the system output does not have the same size as the system input: See the following swing-up of a cart-pole from this demo, where I implemented a LQR as plugin
swingup_lqr_gh.mp4
Why to change JTC in this way:
The API was designed around the following methods:
computeControlLawNonRT
: called when a new trajectory is received. This may take some time (e.g., calculating the gains for a LQR by solving Riccati equations). This blocks the execution of the new trajectory by JTC until the calculation is finished (seeis_ready()
). The user has to implement a realtime-buffer to switch the old control law to the new one, once it is started (seestart()
). In case of any abort (update trajectory withset_hold_position()
), there is acomputeControlLawRT
which needs to finish in the RT loop.updateGainsRT
: A fast update within the RT loop, e.g., update of the PID gains from dynamic reconfigured ROS parameters.computeCommands
: Evaluate the control law inside the RT loop with earlier calculated/updated gains.Notes:
joints
andcommand_joints
parameter #3, the map for the gains should becommand_joints
. But most of the time this parameter is empty and copied over fromjoints
in JTC. Because it is a static read-only variable, it is not possible to override the parameter for the plugin from JTC -> I had to make it reconfigurable from the generate_parameter_library, but added a callback afterwards to prohibit a later change from the user. If anyone knows a better way to solve this, please let me know.The behavior of existing configurations should not break with the proposed changes.