feat(autoware_motion_utils): set zero velocity after stop in resample trajectory #8768
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
component:common
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
yhisaki
requested review from
rej55,
kosuke55,
soblin,
satoshi-ota,
taikitanaka3,
TakaHoribe,
takayuki5168 and
tkimura4
as code owners
yhisaki
added
the
tag:run-build-and-test-differential
takayuki5168
reviewed
Sep 5, 2024
Comment on lines
621
to
625
| if (stop_point_found_in_v_lon) { | ||
| v_lon.push_back(0.0); | ||
| } else { | ||
| v_lon.push_back(curr_pt.longitudinal_velocity_mps); | ||
| } |
There was a problem hiding this comment.
Please add a comment on the reason why this implementation is required.
Also, can you create a unit test for this time's bug?
There was a problem hiding this comment.
I added test and comment!
… trajectory Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>
Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>
Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>
yhisaki
force-pushed
the
set-zero-velocity-after-stop-awf
branch
from
9c046bf
to
0795a05
Compare
Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>
takayuki5168
approved these changes
Sep 6, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8768 +/- ##
==========================================
- Coverage 25.02% 24.93% -0.10%
==========================================
Files 1322 1325 +3
Lines 98047 98423 +376
Branches 37795 37885 +90
==========================================
+ Hits 24536 24540 +4
- Misses 71020 71392 +372
Partials 2491 2491
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Autumn60
pushed a commit
to tier4/autoware.universe
that referenced
this pull request
Sep 8, 2024
… trajectory (autowarefoundation#8768) * feat(autoware_motion_utils): set zero velocity after stop in resample trajectory Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * fix unit test Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * simplify implementation Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * update comment and add test Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> --------- Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> (cherry picked from commit 1a8fbc4)
Autumn60
added a commit
to tier4/autoware.universe
that referenced
this pull request
Sep 8, 2024
… trajectory (autowarefoundation#8768) * feat(autoware_motion_utils): set zero velocity after stop in resample trajectory Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * fix unit test Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * simplify implementation Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * update comment and add test Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> --------- Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> Signed-off-by: Autumn60 <harada.akiro@gmail.com>
Autumn60
added a commit
to tier4/autoware.universe
that referenced
this pull request
Sep 9, 2024
… trajectory (#1522) feat(autoware_motion_utils): set zero velocity after stop in resample trajectory (autowarefoundation#8768) * feat(autoware_motion_utils): set zero velocity after stop in resample trajectory * fix unit test * simplify implementation * update comment and add test --------- Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> Signed-off-by: Autumn60 <harada.akiro@gmail.com>
emuemuJP
pushed a commit
to arayabrain/autoware.universe.origin
that referenced
this pull request
Sep 10, 2024
… trajectory (autowarefoundation#8768) * feat(autoware_motion_utils): set zero velocity after stop in resample trajectory Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * fix unit test Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * simplify implementation Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> * update comment and add test Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> --------- Signed-off-by: Y.Hisaki <yhisaki31@gmail.com> Signed-off-by: emuemuJP <k.matsumoto.0807@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR modifies the resampling of longitudinal_velocity_mps.
The algorithm has been updated to handle data points with embedded zero values differently. For example, when interpolating a sequence like [10, 10, 10, 0, 10, 0, 0, 0], the new algorithm will first transform the data to [10, 10, 10, 0, 0, 0, 0, 0] by setting all values after the first zero point to zero. Interpolation is then performed on this modified sequence.
Motivation for the Change:
When interpolating sequences like [10, 10, 10, 0, 10, 0, 0, 0] using zero-order hold, there were cases where the middle zero point was ignored due to the parameters of the zero-order hold. This resulted in interpolations like [10, 10, 10, 10, 10, 0, 0, 0], where the middle point that should have been zero was not.
This issue led to a bug in this ticket, where points that should have been zero were not being set to zero as expected. The proposed change aims to address this problem by ensuring that all values after the first detected zero point are set to zero before interpolation.
Related links
How was this PR tested?
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.