Update ParabolicSmoother/ParabolicTimer to support Splines #324
Conversation
Codecov Report
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
+ Coverage 81.04% 81.08% +0.03%
==========================================
Files 204 205 +1
Lines 6005 6010 +5
==========================================
+ Hits 4867 4873 +6
+ Misses 1138 1137 -1
|
| /// \param _inputTraj The untimed *spline* trajectory for the arm to process. | ||
| /// \param _rng Random number generator. | ||
| virtual std::unique_ptr<aikido::trajectory::Spline> postprocess( | ||
| const std::unique_ptr<trajectory::Spline>& _inputTraj, |
There was a problem hiding this comment.
I don't believe postprocess wants to take the ownership of trajectories. Could we change this to just const reference type as well as the above Interpolated version? If this introduces too much work and out of this PR's scope, we could do this as a separate PR.
There was a problem hiding this comment.
Sure! These classes are pretty new, so changing the API shouldn’t break things too much.
| const std::unique_ptr<trajectory::Spline>& _inputTraj, | ||
| const aikido::common::RNG* _rng) | ||
| { | ||
| if (!_rng) |
There was a problem hiding this comment.
Could we change the type of _rng to const reference if we throw an exception for nullptr anyway?
…ajectory instead of pointer.
…onalrobotics/aikido into sniyaz/spline_postprocessor
|
@jslee02, responded to your code review comments! 😄 |
|
@sniyaz Thanks for the change! It would be great to have tests for this new functions. 😈 |
|
@jslee02 I've added new tests for ParabolicTimer that also test Spline trajectories. |
Magi requires that we have post processors support both Interpolated and Spline trajectories. This is a small PR that makes the parabolic post processor classes support the new methods added in #302.
Magi requires that we have post processors support both Interpolated and Spline trajectories. This is a small PR that makes the parabolic post processor classes support the new methods added in #302.