Add admittance controller

[pac48]

This PR adds an admittance controller that conforms to the chained controller interface. Additionally, this addition depends on chained JTC. The admittance_controller_parameters.hpp header file is generated using the generate_parameter_library and will not be checked in for the final version.

[progtologist]

Should this be broken down into separate PRs?

[progtologist]

Regarding the autogenerated parameter struct (and the gen_param_struct repo), this appears to be a common misconception from the habits of ROS1. You shouldn't be having member variables for storing the values of parameters in ROS2. You also shouldn't be creating a callback to just store the values when it gets called. The base class already handles all of those things, it has a map for all declared parameters, and you query that map with the get_parameter calls. Sure, a call to a map is not as fast as accessing a member variable, but adding extra callbacks with all those if statements is not fast either.

[pac48]

Regarding the autogenerated parameter struct (and the gen_param_struct repo), this appears to be a common misconception from the habits of ROS1. You shouldn't be having member variables for storing the values of parameters in ROS2. You also shouldn't be creating a callback to just store the values when it gets called. The base class already handles all of those things, it has a map for all declared parameters, and you query that map with the get_parameter calls. Sure, a call to a map is not as fast as accessing a member variable, but adding extra callbacks with all those if statements is not fast either.

There are two main benefits of using a struct instead of the API directly: 1) all the parameters can easily be copied before the control loop starts 2) errors will be caught at compile time instead of runtime. The first mentioned benefit is a design requirement. Additionally, for this controller, all of the parameters still need to be declared which is lots of boiler plate. As far as the if statements being slow, that code runs on the non-real-time thread. It shouldn't impact the control loop.

[mergify[bot]]

This pull request is in conflict. Could you fix it @pac48?

[mergify[bot]]

This pull request is in conflict. Could you fix it @pac48?

[bmagyar]

Out of curiosity, does it not depend also on https://github.com/ros-controls/ros2_control/pull/462 ?

[pac48]

Out of curiosity, does it not depend also on ros-controls/ros2_control#462 ?

I meant to remove any dependencies on the joint limits interface because it is not strictly necessary for functionality. If users to need to enforce limits, then can do it by writing a kinematics plugin for now. I thought that once the joint limits interface is finalized, joint limits support could be added to all the controllers in another PR.

[mergify[bot]]

This pull request is in conflict. Could you fix it @pac48?

[codecov-commenter]

Codecov Report

Merging #370 (e9c7432) into master (e7f9962) will decrease coverage by 5.79%. The diff coverage is 20.65%.

@@            Coverage Diff             @@
##           master     #370      +/-   ##
==========================================
- Coverage   35.78%   29.98%   -5.80%     
==========================================
  Files         189        7     -182     
  Lines       17570      737   -16833     
  Branches    11592      422   -11170     
==========================================
- Hits         6287      221    -6066     
+ Misses        994      161     -833     
+ Partials    10289      355    -9934     

Flag	Coverage Δ
unittests	29.98% <20.65%> (-5.80%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...de/diff_drive_controller/diff_drive_controller.hpp	100.00% <ø> (ø)
...ontroller/test/test_load_diff_drive_controller.cpp	12.50% <0.00%> (ø)
diff_drive_controller/src/odometry.cpp	42.16% <11.11%> (ø)
...ive_controller/test/test_diff_drive_controller.cpp	17.62% <12.08%> (ø)
diff_drive_controller/src/speed_limiter.cpp	46.55% <13.33%> (ø)
...troller/include/diff_drive_controller/odometry.hpp	20.00% <20.00%> (ø)
...iff_drive_controller/src/diff_drive_controller.cpp	32.67% <24.89%> (ø)
...include/joint_trajectory_controller/trajectory.hpp
..._broadcaster/test/test_joint_state_broadcaster.cpp
...test/test_load_joint_group_velocity_controller.cpp
... and 192 more

[AndyZe]

This may be too nitpicky, but should this package be renamed to end_effector_admittance? Somebody else may come along and want to implement admittance at the joint level, which this package doesn't do

[destogl]

This may be too nitpicky, but should this package be renamed to end_effector_admittance? Somebody else may come along and want to implement admittance at the joint level, which this package doesn't do

Isn't it admittance control always in Cartesian space, i.e., end-effector? What do you man on joint level? Isn't that then impedance control?

Oh, I see now… I would not rename package, but then maybe class/plugin. If we ever have some other type of admittance control, it should come into this package. We are growing number of packages recently, which I don't think makes much sense. We should rather reduce number of packages, for example, for forwarding controllers.

[mergify[bot]]

This pull request is in conflict. Could you fix it @pac48?

[mergify[bot]]

This pull request is in conflict. Could you fix it @pac48?

[AndyZe]

@destogl I can't respond to some of your individual comments because they weren't left as part of a github review. I guess I'll do it here instead:

I would still go without temporary variables. They are not necessary if used only once.

Something like this Eigen::Matrix<double, 3, 3> rot_world_sensor = has a fixed size so allocating it usually would not require any extra overhead. It's allocated in the same step as the rest of the stack memory for this function. It does help with readability though. Explanation from Gijs here:

https://github.com/ros-controls/ros2_control/issues/668#issuecomment-1062676446

[nbbrooks]

It looks like these videos Paul took of the admittance controller in action on a UR-5e haven't been shared here yet - enjoy!

• Compliant on 1 axis (EEF vertical axis)
• Compliant trajectory
• Compliant in orientation
• Compliant in all DOF

[destogl]

@Mergifyio backport humble

[mergify[bot]]

backport humble

❌ No backport have been created

• Backport to branch humble failed: Branch not found