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

Move Joint Limits structures for use in controllers #462

Merged
merged 13 commits into from
Jul 8, 2022
Merged

Move Joint Limits structures for use in controllers #462

merged 13 commits into from
Jul 8, 2022

Conversation

destogl
Copy link
Member

@destogl destogl commented Jul 13, 2021

Working towards support for joint limits this is a structure we are suing in controllers to load the limits from yaml file.

We propose also to move joint limits structure into hardware_interface package to reduce number or dependencies in packages using it.

For example of it please check this commit.

@destogl destogl self-assigned this Jul 13, 2021
@destogl destogl marked this pull request as draft July 13, 2021 17:45
@destogl destogl mentioned this pull request Jul 13, 2021
Draft
@shonigmann
Copy link
Contributor

@destogl - Continuing the discussion on joint_limits from ros-controls/ros2_controllers#198, I agree it makes sense to have the joint_limits headers defined within the hardware interface, and I am happy to adopt this approach.

I know there has been some discussion about URDF vs Yaml config, and where limits should be defined. I agree that yaml config files should be a priority and should always be at least as strict as limits in the URDF (potentially set by the robot manufacturer). Perhaps it could be worth checking both the URDF and the Yaml file and throwing a warning or error if there is a discrepancy? But I think the current approach of only considering yaml is the best starting point.

One thing that I think would be nice to add eventually would be implementations of hard and soft enforce_limits() functions for the different interfaces. They are all simple enough functions but that I feel should probably live with the joint_limits headers for other controllers to reuse

shonigmann pushed a commit to shonigmann/ros2_controllers that referenced this pull request Jul 21, 2021
updated joint_limits use to match ros-controls/ros2_control#462; remo…
401dd9c 
…ved obsolete code and comments
@destogl destogl force-pushed the move-joint-limit-structures branch 3 times, most recently from a81d7a4 to b829d81 Compare September 7, 2021 08:18
@destogl destogl force-pushed the move-joint-limit-structures branch from 1439a38 to 54f1c3c Compare October 21, 2021 12:44
@henningkayser henningkayser mentioned this pull request Nov 10, 2021
Merged
@destogl destogl mentioned this pull request Mar 9, 2022
Closed
@AndyZe AndyZe mentioned this pull request Apr 5, 2022
Closed
2 tasks
@AndyZe
Copy link
Contributor

AndyZe commented Apr 6, 2022

I really want some of the functionality in this PR. I think it was a little too ambitious though, adding too many changes at once. I think I will take some of the code here and open a new PR just for reading the joint limits. Baby steps.

AndyZe
AndyZe approved these changes Apr 20, 2022
View reviewed changes
Copy link
Contributor

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

Thanks for simplifying the PR! It looks good other than some minor nitpicks.

joint_limits/include/joint_limits/joint_limits.hpp Show resolved Hide resolved
joint_limits/package.xml Outdated Show resolved Hide resolved
ros2_control/package.xml Outdated Show resolved Hide resolved
joint_limits/include/joint_limits/joint_limits_rosparam.hpp Show resolved Hide resolved
@destogl destogl marked this pull request as ready for review May 4, 2022 14:23
AndyZe
AndyZe approved these changes May 4, 2022
View reviewed changes
Copy link
Contributor

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

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

Looks good to me. I just notice one change that you forgot to push, I think.

ros2_control/package.xml Outdated Show resolved Hide resolved
saikishor
saikishor approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@saikishor saikishor left a comment

Choose a reason for hiding this comment

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

Looks good to me!

homalozoa
homalozoa suggested changes May 19, 2022
View reviewed changes
joint_limits/package.xml Show resolved Hide resolved
joint_limits/package.xml Show resolved Hide resolved
@bmagyar bmagyar force-pushed the move-joint-limit-structures branch from 6fd8dba to 991b610 Compare June 18, 2022 07:01
destogl and others added 11 commits July 6, 2022 15:35
@destogl
Using joint limits structure for the admittance controller.
f4e95da
@destogl
Ros2 control extensions rolling joint limits plugins (#5)
694cc6a 
* Added initial structures for joint-limit plugins.

* Move Ruckig limiter to package joint_limits_enforcement_plugins and make it working.

Co-authored-by: AndyZe <[email protected]>
@destogl
Use auto_declare for parameters to avoid crashing.
12aed7d
@destogl
Clean joint_limits package and copy tests into it. They are not worki…
d0ea4c5 
…ng yet.
@destogl @AndyZe
Update joint_limits/package.xml
eb6c7fc 
Co-authored-by: AndyZe <[email protected]>
@destogl
Final updates to according to reviews and trying to re-enable tests.
2e0e5bd
@destogl @AndyZe
Update ros2_control/package.xml
b409f7f 
Co-authored-by: AndyZe <[email protected]>
@destogl
Update joint_limits/package.xml
9f00962
@bmagyar @destogl
Fix building test & package name in launch script
6d5e658
@destogl
Repair tests after code changes.
a0e8cc5
@destogl
Make better interfaces for joint limits.
249d4c7
@destogl destogl force-pushed the move-joint-limit-structures branch from 76fd8d7 to 249d4c7 Compare July 6, 2022 13:45
bmagyar
bmagyar requested changes Jul 7, 2022
View reviewed changes
joint_limits/include/joint_limits/joint_limits.hpp Outdated Show resolved Hide resolved
joint_limits/include/joint_limits/joint_limits.hpp Outdated Show resolved Hide resolved
joint_limits/include/joint_limits/joint_limits.hpp Outdated Show resolved Hide resolved
@bmagyar bmagyar merged commit e68cbb1 into ros-controls:master Jul 8, 2022
@bmagyar bmagyar mentioned this pull request Jul 8, 2022
Merged
@destogl destogl deleted the move-joint-limit-structures branch March 7, 2023 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjahr sjahr sjahr left review comments

@AndyZe AndyZe AndyZe requested changes

@saikishor saikishor saikishor approved these changes

@homalozoa homalozoa homalozoa requested changes

@LukasMacha97 LukasMacha97 LukasMacha97 approved these changes

@bmagyar bmagyar bmagyar approved these changes

@livanov93 livanov93 Awaiting requested review from livanov93

@malapatiravi malapatiravi Awaiting requested review from malapatiravi

@TomoyaFujita2016 TomoyaFujita2016 Awaiting requested review from TomoyaFujita2016

Assignees

@destogl destogl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@destogl @shonigmann @AndyZe @bmagyar @saikishor @homalozoa @sjahr @LukasMacha97