-
Notifications
You must be signed in to change notification settings - Fork 682
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
feat(obstacle_cruise_planner): prior generation of collision distance #5619
feat(obstacle_cruise_planner): prior generation of collision distance #5619
Conversation
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5619 +/- ##
==========================================
+ Coverage 15.32% 15.35% +0.02%
==========================================
Files 1721 1721
Lines 118559 118523 -36
Branches 37995 38078 +83
==========================================
+ Hits 18169 18199 +30
+ Misses 79657 79569 -88
- Partials 20733 20755 +22
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
std::optional<double> calcCollisionDistanceForStopObstacle( | ||
const std::vector<TrajectoryPoint> & traj_points, const std::vector<Polygon2d> & traj_polys, | ||
const Obstacle & obstacle) const; |
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.
std::optional<double> calcCollisionDistanceForStopObstacle( | |
const std::vector<TrajectoryPoint> & traj_points, const std::vector<Polygon2d> & traj_polys, | |
const Obstacle & obstacle) const; |
Seems this function declaration is not required.
const std::vector<TrajectoryPoint> & traj_points, const std::vector<Polygon2d> & traj_polys, | ||
const Obstacle & obstacle) | ||
{ | ||
auto obstacle_polygon = tier4_autoware_utils::toPolygon2d(obstacle.pose, obstacle.shape); |
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.
auto obstacle_polygon = tier4_autoware_utils::toPolygon2d(obstacle.pose, obstacle.shape); | |
const auto obstacle_polygon = tier4_autoware_utils::toPolygon2d(obstacle.pose, obstacle.shape); |
for (size_t col_i = 0; col_i < traj_polys.size(); ++col_i) { | ||
if (boost::geometry::intersects(traj_polys.at(col_i), obstacle_polygon)) { | ||
try { | ||
double prev_index_dist = |
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.
double prev_index_dist = | |
const double prev_index_dist = |
auto obstacle_polygon = tier4_autoware_utils::toPolygon2d(obstacle.pose, obstacle.shape); | ||
for (size_t col_i = 0; col_i < traj_polys.size(); ++col_i) { | ||
if (boost::geometry::intersects(traj_polys.at(col_i), obstacle_polygon)) { | ||
try { |
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.
Why do you use try here?
I think we can handle the corner case without using try, which is a better implementation
@@ -401,12 +373,13 @@ std::vector<TrajectoryPoint> PlannerInterface::generateStopTrajectory( | |||
|
|||
stop_planning_debug_info_.set( | |||
StopPlanningDebugInfo::TYPE::STOP_CURRENT_OBSTACLE_DISTANCE, | |||
closest_obstacle_dist - abs_ego_offset); // TODO(murooka) | |||
closest_stop_obstacle->dist_to_collide); // TODO(murooka) |
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.
Is this correct? Seems dist_to_collide
does not consider the following distances
- the distance from the path start point and the ego position
- the distance from the base link to the ego front position.
: TargetObstacleInterface(arg_uuid, arg_stamp, arg_pose, arg_lon_velocity, arg_lat_velocity), | ||
shape(arg_shape), | ||
collision_point(arg_collision_point) | ||
collision_point(arg_collision_point), |
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.
Is it hard to remove the collision_point here so that the developer will not get confused about which (collision point or dist_to_collide) to use?
If this still exists here, it will be used for some calculation by mistake.
Actually, the nearest search is applied to this collision point, which is not expected.
motion_utils::findNearestSegmentIndex(planner_data.traj_points, stop_obstacle.collision_point); |
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.
If this variable has to exist for the debug information, we'd better change the variable name to collision_point_for_debug
for example so that the developer can understand the reason for this variable to exist.
double dist_to_collide_on_ref_traj = | ||
motion_utils::calcSignedArcLength(planner_data.traj_points, 0, ego_segment_idx) + | ||
closest_stop_obstacle->dist_to_collide; |
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.
This cannot consider the detailed ego position since ego_segment_idx
means the resampled index for the ego position.
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.
Also, the distance from the base link to the ego front position (abs_ego_offset
) seems not considered with this PR's change.
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
…autowarefoundation#5619) * set goal extension param as unused * change to use collision point * fill out z coordinate value * handle with the backward paths Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
…autowarefoundation#5619) * set goal extension param as unused * change to use collision point * fill out z coordinate value * handle with the backward paths Signed-off-by: Yuki Takagi <yuki.takagi@tier4.jp>
Description
BEFORE:

The current implementation calculate the stop point by the closest collision point.
This cause inappropriate behavior for looped trajectories.
AFTER:

The followings are achieved by this PR,
goal_extension_length
andgoal_extension_interval
may be no longer used andsafe_distance_margin
anddecimate_trajectory_step_length
are used instead.This PR similar to #5512, but this PR maintain more compatibility.
In other words, the function that included in the above PR This PR contains the similar feature to #4952, and therefore approaching on curve is now provided as default setting. is obsoleted.
Related links
Tests performed
psim tests and Tier4 internal tests was performed in success.
No degrade directly related to this PR founds.
https://evaluation.tier4.jp/evaluation/reports/52d22759-d1af-5896-b016-3b45e73d4138?project_id=prd_jt
Notes for reviewers
While no longer used parameters remain, I will remove these parameters after this PR has been agreed upon.
Interface changes
No interface changes.
Effects on system behavior
Minor bugs mentioned above have been fixed.
The slow down planning and the cruise planning may not changed.
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.