Skip to content
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

fix(stop_line): z position of stop point #2239

Conversation

TomohitoAndo
Copy link
Contributor

@TomohitoAndo TomohitoAndo commented Nov 8, 2022

Signed-off-by: Tomohito Ando tomohito.ando@tier4.jp

Description

Currently z position of stop line is used as the z position of inserted point, but it causes z position error.
(e.g. when there is a stop line in a slope.)
That resulted in strange pitch due to the high elevation angle.

z position of stop point is calculated here, so I think this calculation is not needed.

before this change:
I visualized the output path poses from stop_line module.
image

after this change:
image

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.

  • There are no open discussions or they are tracked via tickets.

After all checkboxes are checked, anyone who has write access can merge the PR.

Signed-off-by: Tomohito Ando <tomohito.ando@tier4.jp>
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Base: 11.16% // Head: 11.15% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (09bb8f6) compared to base (7829c07).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2239      +/-   ##
==========================================
- Coverage   11.16%   11.15%   -0.01%     
==========================================
  Files        1200     1200              
  Lines       86184    86205      +21     
  Branches    20787    20805      +18     
==========================================
  Hits         9620     9620              
- Misses      66439    66459      +20     
- Partials    10125    10126       +1     
Flag Coverage Δ *Carryforward flag
differential 4.23% <ø> (?)
total 11.13% <ø> (ø) Carriedforward from 7829c07

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...ocity_planner/src/scene_module/stop_line/scene.cpp 0.00% <ø> (ø)

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

@TomohitoAndo
Copy link
Contributor Author

@takayuki5168
Could you take a look at this because you modified this last time? 🙏 (and as a code owner @purewater0901 )

@@ -64,7 +64,6 @@ bool StopLineModule::modifyPathVelocity(

const auto stop_point_idx = stop_point->first;
auto stop_pose = stop_point->second;
stop_pose.position.z = (stop_line_[0].z() + stop_line_[1].z()) / 2.0;

Copy link
Contributor

Choose a reason for hiding this comment

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

When committing this line, I just followed the original implementation before my PR.
Currently, z position is interpolated from the path. So it makes sense to remove this line.

Copy link
Contributor

@takayuki5168 takayuki5168 left a comment

Choose a reason for hiding this comment

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

LGTM

@TomohitoAndo
Copy link
Contributor Author

@purewater0901
Could you approve this as a code owner if there are no ploblems?

Copy link
Contributor

@purewater0901 purewater0901 left a comment

Choose a reason for hiding this comment

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

LGTM

@TomohitoAndo TomohitoAndo merged commit 5d7748c into autowarefoundation:main Nov 8, 2022
HansRobo pushed a commit to HansRobo/autoware.universe that referenced this pull request Dec 16, 2022
fix: z position of stop point

Signed-off-by: Tomohito Ando <tomohito.ando@tier4.jp>

Signed-off-by: Tomohito Ando <tomohito.ando@tier4.jp>
Signed-off-by: Kotaro Yoshimoto <pythagora.yoshimoto@gmail.com>
YoshiRi pushed a commit to YoshiRi/autoware.universe that referenced this pull request Jan 11, 2023
fix: z position of stop point

Signed-off-by: Tomohito Ando <tomohito.ando@tier4.jp>

Signed-off-by: Tomohito Ando <tomohito.ando@tier4.jp>
Signed-off-by: yoshiri <yoshiyoshidetteiu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants