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.

Sign up for GitHub

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

Jump to bottom

Add admittance controller #370

Merged
merged 128 commits into from
Oct 10, 2022
Merged

Add admittance controller #370

merged 128 commits into from
Oct 10, 2022

Conversation

pac48
Copy link
Contributor

@pac48 pac48 commented Jun 28, 2022 •
edited
Loading

This PR adds an admittance controller that conforms to the chained controller interface. Additionally, this depends on a PR to control_msgs.

destogl and others added 25 commits May 16, 2022 20:56
@destogl
Make forward controller chainable.
0206ee6 
Enabling chanable mode for forward command controllers.

Fix all bugs in chained-controllers mode.

Remove debug output from forwarding controller.

Updated example ffwd chainable controller for new structure.
@destogl
Enabling chanable mode for forward command controllers.
43f39a0
@destogl
Fix all bugs in chained-controllers mode.
29c4393
@destogl
Remove debug output from forwarding controller.
6f2f956
@destogl
Make forward controller chainable.
704ba5d
@destogl
Enabling chanable mode for forward command controllers.
6759bb5
@destogl
Fix all bugs in chained-controllers mode.
7afdefc
@destogl
Remove debug output from forwarding controller.
7c03f79
@destogl
Some compile fixes
f8556ea
@destogl
Fix some small issues.
d77b4b6
@destogl
Update to use class.
778ed13
@pac48
added admittance controller
39b640e
@pac48
merge fwd chainable controller
be2f308
@pac48
merged additional from fwd chainable controllers
4f3d709
@pac48
fixed interface memory bug
fcc2be2
@pac48
changed position/velocity reference fields
0aac28e
@pac48
added gravity compensation
7033048
@pac48
replaced Eigen 3.4 API with Eigen 3.3
1a266bc
@pac48
clean up and TODOs
8dc9b37
@pac48
removed reshaped operation and fix forward velocity
1563113
@pac48
code cleanup and refactored gravity compensation
5d59427
@pac48
replaced parameter handler with auto-generated struct
18097a2
@pac48
dynamic parameter updating enabled
dfc27bf
@pac48
enable/disable parameter update during execution + fill state message
2db281f
@pac48
remove unused code
9990dca
@pac48 pac48 requested a review from destogl June 28, 2022 05:00
@mergify
Copy link
Contributor

mergify bot commented Aug 27, 2022

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

@AndyZe
Copy link
Contributor

AndyZe commented Sep 23, 2022 •
edited
Loading

@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:

ros-controls/ros2_control#668 (comment)

@nbbrooks
Copy link
Contributor

nbbrooks commented Oct 3, 2022

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

sjahr
sjahr approved these changes Oct 7, 2022
View reviewed changes
Copy link

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

I tested this PR with the admittance controller demos from ros-controls/ros2_control_demos#206 and it build and worked well for me 👍

@destogl destogl mentioned this pull request Oct 10, 2022
Open
destogl
destogl reviewed Oct 10, 2022
View reviewed changes
admittance_controller/test/6d_robot_description.hpp

#include <string>

namespace ros2_control_test_assets
Copy link
Member

Choose a reason for hiding this comment

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

Move this to the assets' folder in the ros2_control repository.

@destogl destogl mentioned this pull request Oct 10, 2022
Open
2 tasks
@destogl destogl merged commit 9be3bfe into ros-controls:master Oct 10, 2022
@destogl
Copy link
Member

destogl commented Oct 11, 2022 •
edited
Loading

@Mergifyio backport humble

@mergify
Copy link
Contributor

mergify bot commented Oct 11, 2022

backport humble

❌ No backport have been created

  • Backport to branch humble failed: Branch not found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@destogl destogl destogl approved these changes

@AndyZe AndyZe AndyZe approved these changes

@sjahr sjahr sjahr approved these changes

@bmagyar bmagyar Awaiting requested review from bmagyar

@harderthan harderthan Awaiting requested review from harderthan

@progtologist progtologist Awaiting requested review from progtologist

@rosterloh rosterloh Awaiting requested review from rosterloh

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@pac48 @progtologist @bmagyar @codecov-commenter @AndyZe @destogl @nbbrooks @sjahr @Schulze18