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

Fix Ruckig termination condition #1963

Merged
merged 1 commit into from
Mar 10, 2023

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Feb 21, 2023

Description

Previously I didn't understand the difference between ruckig::Finished and ruckig::Working so I focused only on ruckig::Finished. There's not much documentation but here it is: https://github.com/pantor/ruckig/blob/master/include/ruckig/result.hpp

This corrects that error and adds a comment. Fixes #1960

This should significantly speed up Ruckig-smoothed trajectories. I'm seeing speedups in the range of 50% although it depends on the specific robot configuration.

Fixes #1960. It is a port of this MoveIt1 PR: moveit/moveit#3348

@swiz23 @mechwiz @nbbrooks

Plots. The performance is close to optimal but not quite optimal. This makes sense because the iterative process for multiple waypoints involves extending durations in 10% increments -- so it could be up to 10% non-optimal.

J1, vel limit=2.18 rad/s, accel limit=3.75 rad/s^2, jerk limit=100
j1

J3, vel limit=2.18, accel limit=2.5, jerk limit=100
j3

@codecov
Copy link

codecov bot commented Feb 21, 2023

Codecov Report

Patch coverage: 31.04% and project coverage change: -0.14 ⚠️

Comparison is base (c27cb30) 50.31% compared to head (5ac13f4) 50.16%.

❗ Current head 5ac13f4 differs from pull request most recent head 572014b. Consider uploading reports for the commit 572014b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1963      +/-   ##
==========================================
- Coverage   50.31%   50.16%   -0.14%     
==========================================
  Files         374      374              
  Lines       31358    31354       -4     
==========================================
- Hits        15774    15726      -48     
- Misses      15584    15628      +44     
Impacted Files Coverage Δ
...rajectory_processing/src/ruckig_traj_smoothing.cpp 59.60% <31.04%> (-25.55%) ⬇️
...nning_scene_monitor/src/planning_scene_monitor.cpp 45.26% <0.00%> (-0.43%) ⬇️
...e/collision_detection_fcl/src/collision_common.cpp 73.76% <0.00%> (-0.22%) ⬇️
moveit_core/robot_state/src/robot_state.cpp 48.57% <0.00%> (+0.62%) ⬆️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp 79.17% <0.00%> (+1.14%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@swiz23
Copy link
Contributor

swiz23 commented Feb 21, 2023

Can you link to the issue that this is fixing?

@AndyZe AndyZe marked this pull request as draft February 21, 2023 17:51
@AndyZe AndyZe marked this pull request as ready for review February 22, 2023 15:43
@AndyZe AndyZe force-pushed the andyz/ruckig_duration branch 2 times, most recently from c0f73b5 to 5ac13f4 Compare February 24, 2023 20:40
@AndyZe
Copy link
Member Author

AndyZe commented Feb 24, 2023

@swiz23 I got rid of runRuckigInBatches() for now so this should definitely fix #1960. I'll put some plots in the description now, too.

@mergify
Copy link

mergify bot commented Mar 7, 2023

⚠️ The sha of the head commit of this PR conflicts with #1990. Mergify cannot evaluate rules on this PR. ⚠️

@henningkayser henningkayser added the backport-humble Mergify label that triggers a PR backport to Humble label Mar 10, 2023
@henningkayser henningkayser merged commit 605d28b into moveit:main Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble Mergify label that triggers a PR backport to Humble
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruckig trajectory smoothing plugin: duration is extended unnecessarily
3 participants