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.

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

Acceleration Limited Smoothing Plugin for Servo #2651

Merged
merged 30 commits into from
Jan 19, 2024

Conversation

pac48
Copy link
Contributor

@pac48 pac48 commented Jan 13, 2024

Description

This PR adds a new plugin called AccelerationLimitedPlugin under the online_signal_smoothing::SmoothingBaseClass interface. The purpose of the plugin is to prevent the robot's acceleration limits from being violated by instantaneous changes to the servo command topics. The plugin is not meant to act as a reference tracking controller, rather it's designed to be a layer between the hardware and command interface that prevents infeasible motions.

There are three cases considered for acceleration-limiting illustrated in the following figures:
Screenshot from 2024-01-12 16-45-36
In the figures, the red arrows show the displacement that will occur given the current velocity. The blue line shows the displacement between the current position and the desired position. The black dashed line shows the maximum possible displacements that are within the acceleration limits. The green line shows the acceleration commands that will be used by this acceleration-limiting plugin.

  • Figure A: The desired position is within the acceleration limits. The next commanded point will be exactly the desired point.
  • Figure B: The line between the current position and the desired position intersects the acceleration limits, but the reference position is not within the bounds. The next commanded point will be the point on the displacement line that is closest to the reference.
  • Figure C: Neither the displacement line intersects the acceleration limits nor does the reference point lie within the limits. In this case, the next commanded point will be the one that minimizes the robot's velocity while maintaining its direction.

Case B can be solved via linear or quadratic program, hence I added OSQP as a dependency. This approach is compared to a more naive approach (where the required acceleration is scaled to avoid any limits from being violated) on a square tracing servoing motion (Left is this plugin).

Screenshot from 2024-01-12 16-45-56

The same motions are also shown as functions of time in the following plots. Importantly, the velocity of the left plot goes to zero in order to take the 90-degree turn.

Screenshot from 2024-01-12 16-46-08

Testing instructions:

  1. Launch demo_ros_api.launch.py from moveit_serov
  2. Run demo_ros_api.launch.py from servo_keyboard_input
  3. Use arrow keys to move the robot. You should see smoother motion compared to without the plugin.

The update to the ros2_control usage may have broken this example. So to test, you will have to revert that commit.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference

@pac48 pac48 marked this pull request as draft January 13, 2024 00:18
Copy link

codecov bot commented Jan 13, 2024

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (e1782bc) 50.92% compared to head (05e0918) 50.74%.

Files Patch % Lines
...nline_signal_smoothing/src/acceleration_filter.cpp 97.34% 4 Missing ⚠️
moveit_ros/moveit_servo/src/servo.cpp 83.34% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2651      +/-   ##
==========================================
- Coverage   50.92%   50.74%   -0.17%     
==========================================
  Files         391      392       +1     
  Lines       32590    32549      -41     
==========================================
- Hits        16592    16513      -79     
- Misses      15998    16036      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

*********************************************************************/

/* Author: Paul Gesel
Description: applies smoothing by limiting the acceleration between consecutive commands
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add more details to description

Copy link
Contributor

Choose a reason for hiding this comment

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

If you feel particularly ambitious, the images in your PR ASCIIfied would be great here. Or otherwise, to add it to a README in this folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added ASCII diagrams


#include <cstddef>

// Auto-generated
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment referring to moveit_acceleration_parameters.hpp? If yes, maybe it can be removed since we're using generate_parameters all over the moveit2 codebase

Copy link
Contributor Author

@pac48 pac48 Jan 17, 2024

Choose a reason for hiding this comment

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

Good point. I am removing it.

*********************************************************************/

/* Author: Paul Gesel
Description: applies smoothing by limiting the acceleration between consecutive commands
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Description: applies smoothing by limiting the acceleration between consecutive commands

Usually, we only have a description in the header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing

osqp_update_bounds(work_, data_->l.data(), data_->u.data());
};

size_t num_constraints = num_joints_ + 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add more comments to describe the computation steps

moveit_ros/moveit_servo/launch/demo_pose.launch.py Outdated Show resolved Hide resolved
@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch from d351f44 to aaf3af4 Compare January 17, 2024 18:45
pac48 and others added 10 commits January 17, 2024 11:46
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>
Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch from aaf3af4 to aa77647 Compare January 17, 2024 18:46
@pac48 pac48 marked this pull request as ready for review January 17, 2024 18:46
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch from aa77647 to d28596c Compare January 17, 2024 19:42
pac48 and others added 3 commits January 17, 2024 13:35
Co-authored-by: Sebastian Jahr <sebastian.jahr@tuta.io>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch from 225f1e0 to 231f02a Compare January 17, 2024 21:09
pac48 added 3 commits January 17, 2024 14:14
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 requested a review from sjahr January 17, 2024 22:54
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Looks awesome!

My only real comment besides small stuff is that the variable naming and amount of comments in the "meat" of the OSQP code could use some attention.

Oh yeah, also one of the unit tests seems to be failing: https://github.com/ros-planning/moveit2/actions/runs/7562781804/job/20593931009?pr=2651#step:12:9070

moveit_core/CMakeLists.txt Outdated Show resolved Hide resolved
moveit_core/online_signal_smoothing/CMakeLists.txt Outdated Show resolved Hide resolved
*********************************************************************/

/* Author: Paul Gesel
Description: applies smoothing by limiting the acceleration between consecutive commands
Copy link
Contributor

Choose a reason for hiding this comment

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

If you feel particularly ambitious, the images in your PR ASCIIfied would be great here. Or otherwise, to add it to a README in this folder.

moveit_ros/moveit_servo/launch/demo_ros_api.launch.py Outdated Show resolved Hide resolved
pac48 added 2 commits January 18, 2024 08:38
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch from ea7ce4c to 7b4c6e3 Compare January 18, 2024 15:58
pac48 and others added 6 commits January 18, 2024 11:50
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 requested a review from sea-bass January 18, 2024 20:30
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

I think it's almost there!

moveit_core/online_signal_smoothing/CMakeLists.txt Outdated Show resolved Hide resolved
moveit_ros/moveit_servo/launch/demo_ros_api.launch.py Outdated Show resolved Hide resolved
pac48 and others added 2 commits January 18, 2024 14:50
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
sea-bass
sea-bass previously approved these changes Jan 18, 2024
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

LGTM -- but let's wait until @sjahr is also good.

@sea-bass
Copy link
Contributor

@sea-bass sea-bass dismissed their stale review January 18, 2024 22:41

Clang tidy

@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch 2 times, most recently from be703cd to 87bd0a8 Compare January 18, 2024 23:32
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 force-pushed the pr-add-acceleration-based-smoothing branch from 87bd0a8 to 05e0918 Compare January 18, 2024 23:47
@sjahr sjahr merged commit cba873a into moveit:main Jan 19, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants