-
Notifications
You must be signed in to change notification settings - Fork 508
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
Add a version of TOTG computeTimeStamps() for a fixed num waypoints #1771
Conversation
moveit_core/trajectory_processing/src/time_optimal_trajectory_generation.cpp
Outdated
Show resolved
Hide resolved
15abc2a
to
e4e8d4d
Compare
Codecov ReportBase: 50.92% // Head: 50.93% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1771 +/- ##
==========================================
+ Coverage 50.92% 50.93% +0.01%
==========================================
Files 378 378
Lines 31671 31680 +9
==========================================
+ Hits 16126 16132 +6
- Misses 15545 15548 +3
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a useful feature of people know what they're doing
...jectory_processing/include/moveit/trajectory_processing/time_optimal_trajectory_generation.h
Show resolved
Hide resolved
...jectory_processing/include/moveit/trajectory_processing/time_optimal_trajectory_generation.h
Outdated
Show resolved
Hide resolved
6c09d46
to
f382ffe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the documentation is good now. If you don't like the small clarification of your comment I suggested, feel free to reject it
...jectory_processing/include/moveit/trajectory_processing/time_optimal_trajectory_generation.h
Outdated
Show resolved
Hide resolved
Co-authored-by: Sebastian Jahr <sebastian.jahr@tuta.io>
private: | ||
bool doTimeParameterizationCalculations(robot_trajectory::RobotTrajectory& trajectory, | ||
const Eigen::VectorXd& max_velocity, | ||
const Eigen::VectorXd& max_acceleration) const; | ||
|
||
const double path_tolerance_; | ||
const double resample_dt_; | ||
double resample_dt_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AndyZe it's actually error-prone that resample_dt_
is being changed implicitly by the new computeTimeStamps()
function. The other member functions have been const
for a reason, namely to allow repeatable results. Now the results of the old computeTimeStamps()
depends on if the new function has been called and whatever num_waypoints
has been used. Please fix that, either by implementing a free function that creates two instances of TOTG (one for each resample_dt), or by providing a private computeTimeStampsInternal
function that accepts resample_dt
as argument so that you can call that one twice without changing the member variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll fix this soon
Fixes #1769