fix(motion_velocity_smoother): improve backward trajectory handling #6079
+108
−46
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
Fixes #5976
Current support for backward trajectories is fairly limited and sometimes just happen to work "by accident" (e.g. some calculated data is wrong but mostly unused). In particular, I have found many backward driving issues with the
motion_velocity_smoother
node while implementing a new freespace planner.Most functions in
motion_velocity_smoother
have been implemented with forward trajectories in mind and don't apply to backward trajectories. The "trick" currently used to handle backward trajectories is to "flip" the trajectory points velocity before processing it, to do as if the vehicle was driving forward and not backward, and then flip the output back. I think the idea is valid, however it is not properly implemented: for example it causes steering values to be incorrect on backward trajectories.The "proper" way to implement the flipping trick is not just to flip the velocity but to flip everything: trajectory pose orientation, velocity, steering... but also the ego pose and velocity when we need to place ego vehicle on the flipped trajectory.
Note that the way the logic is implemented at the moment (local trajectory but global
is_reverse_
parameter) is error prone and difficult to maintain. It would be worth packing trajectory data and ego state inside a single ("flippable") struct so that sub functions could just assume everything goes forward.Related links
#5976
Tests performed
With the freespace planner in the planning simulator:
Notes for reviewers
Interface changes
None.
Effects on system behavior
On all trajectories:
motion_velocity_smoother
now ignores trajectories mixing forward/backward driving. AFAIK this is not supposed to be supported and would most likely give strange result. But now it is checked.On backward trajectory:
motion_velocity_smoother
output trajectory points values are now signed properlyExcept for these, there should be no other visible behavior change.
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.