Skip to content

Fix: TOTG can return vels/accels greater than the limits#3394

Merged
AndyZe merged 6 commits intomoveit:masterfrom
AndyZe:andyz/totg_jump
Apr 6, 2023
Merged

Fix: TOTG can return vels/accels greater than the limits#3394
AndyZe merged 6 commits intomoveit:masterfrom
AndyZe:andyz/totg_jump

Conversation

@AndyZe
Copy link
Copy Markdown
Member

@AndyZe AndyZe commented Apr 3, 2023 •
edited
Loading

This is a 2-line bug fix for this test failure:

/home/andy/ws_moveit/src/moveit/moveit_core/trajectory_processing/test/test_time_optimal_trajectory_generation.cpp:395: Failure
The difference between trajectory.getAcceleration(time)[0] and max_accelerations[0] is 4, which exceeds 1e-3, where
trajectory.getAcceleration(time)[0] evaluates to 32,
max_accelerations[0] evaluates to 28, and
1e-3 evaluates to 0.001.
Time: 0.01
/home/andy/ws_moveit/src/moveit/moveit_core/trajectory_processing/test/test_time_optimal_trajectory_generation.cpp:395: Failure
The difference between trajectory.getAcceleration(time)[0] and max_accelerations[0] is 28, which exceeds 1e-3, where
trajectory.getAcceleration(time)[0] evaluates to 0,
max_accelerations[0] evaluates to 28, and
1e-3 evaluates to 0.001.
Time: 0.02

@AndyZe AndyZe requested a review from mlautman as a code owner April 3, 2023 21:38
@AndyZe AndyZe marked this pull request as draft April 3, 2023 21:38
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2023 •
edited
Loading

Codecov Report

Patch coverage: 85.72% and project coverage change: -0.72 ⚠️

Comparison is base (c7d8a19) 61.79% compared to head (b25f9b8) 61.06%.

❗ Current head b25f9b8 differs from pull request most recent head 19fa271. Consider uploading reports for the commit 19fa271 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3394      +/-   ##
==========================================
- Coverage   61.79%   61.06%   -0.72%     
==========================================
  Files         380      380              
  Lines       33605    33616      +11     
==========================================
- Hits        20762    20524     -238     
- Misses      12843    13092     +249
Impacted Files Coverage Δ
...ry_processing/time_optimal_trajectory_generation.h 50.00% <ø> (-50.00%) ⬇️
...cessing/src/time_optimal_trajectory_generation.cpp 42.46% <85.72%> (-47.48%) ⬇️

... and 4 files with indirect coverage changes

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 in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@AndyZe AndyZe force-pushed the andyz/totg_jump branch from 658c471 to 92a188b Compare April 4, 2023 14:23
@AndyZe AndyZe force-pushed the andyz/totg_jump branch from 92a188b to b787b68 Compare April 4, 2023 14:44
@AndyZe AndyZe force-pushed the andyz/totg_jump branch from 759f693 to d778f9d Compare April 4, 2023 15:27
@AndyZe AndyZe force-pushed the andyz/totg_jump branch from f0ca8c6 to d8fee48 Compare April 4, 2023 19:08
@AndyZe AndyZe force-pushed the andyz/totg_jump branch from 7ea902c to b25f9b8 Compare April 4, 2023 19:50
@AndyZe AndyZe marked this pull request as ready for review April 5, 2023 14:37
AndyZe
AndyZe commented Apr 5, 2023
View reviewed changes
moveit_core/trajectory_processing/src/time_optimal_trajectory_generation.cpp
const double acceleration =
2.0 * (it->path_pos_ - previous->path_pos_ - time_step * previous->path_vel_) / (time_step * time_step);

time_step = time - previous->time_;
Copy link
Copy Markdown
Member Author

@AndyZe AndyZe Apr 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Note that time_step is already calculated a few lines above.

I think this deleted line was intended to improve accuracy but it caused the bug where getAcceleration() returned values over the limit.

The accuracy should still be good if TOTG is run with a small timestep. The default timestep is 0.001. https://github.com/ros-planning/moveit/blob/9fd645d76d0a4fbdbf55f8aecb09fd5a31a63e1a/moveit_core/trajectory_processing/src/time_optimal_trajectory_generation.cpp#L994

@AndyZe AndyZe changed the title Debugging a TOTG failure Fix: TOTG can return vels/accels greater than the limits Apr 5, 2023
@AndyZe AndyZe mentioned this pull request Apr 5, 2023
Merged
@AndyZe AndyZe merged commit 1eb3252 into moveit:master Apr 6, 2023
@AndyZe AndyZe deleted the andyz/totg_jump branch April 6, 2023 15:01
@cgeering cgeering mentioned this pull request Mar 12, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sjahr sjahr sjahr approved these changes

@mlautman mlautman Awaiting requested review from mlautman

@mikeferguson mikeferguson Awaiting requested review from mikeferguson

@henningkayser henningkayser Awaiting requested review from henningkayser

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AndyZe @sjahr