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(obstacle_stop_planner): add checking of point height #2296

Merged
merged 21 commits into from
Feb 15, 2023

Conversation

lchojnack
Copy link
Contributor

@lchojnack lchojnack commented Nov 15, 2022

Description

This PR adds checking of a point height. If a point is lower than the vehicle's height, then the point is in collision with the vehicle. This allows the EGO to drive when an obstacle is above the EGO e.g. tree branches and leaves.

Parameters are added in PR autowarefoundation/autoware_launch#150

Related to:

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.

@github-actions github-actions bot added the component:planning Route planning, decision-making, and navigation. (auto-assigned) label Nov 15, 2022
@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Base: 11.82% // Head: 11.57% // Decreases project coverage by -0.25% ⚠️

Coverage data is based on head (4d41ceb) compared to base (4c6107a).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2296      +/-   ##
==========================================
- Coverage   11.82%   11.57%   -0.25%     
==========================================
  Files        1319     1320       +1     
  Lines       92194    92561     +367     
  Branches    24785    24334     -451     
==========================================
- Hits        10900    10714     -186     
- Misses      69934    70665     +731     
+ Partials    11360    11182     -178     
Flag Coverage Δ *Carryforward flag
differential 0.00% <0.00%> (?)
total 11.68% <0.00%> (-0.14%) ⬇️ Carriedforward from 1cbe5c3

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

Impacted Files Coverage Δ
control/mpc_lateral_controller/src/mpc_utils.cpp 57.27% <ø> (ø)
...lanner/include/obstacle_avoidance_planner/node.hpp 0.00% <ø> (ø)
planning/obstacle_avoidance_planner/src/node.cpp 0.12% <ø> (+<0.01%) ⬆️
...ner/include/obstacle_stop_planner/debug_marker.hpp 0.00% <ø> (ø)
...top_planner/include/obstacle_stop_planner/node.hpp 0.00% <ø> (ø)
...lanning/obstacle_stop_planner/src/debug_marker.cpp 0.00% <0.00%> (ø)
planning/obstacle_stop_planner/src/node.cpp 0.00% <0.00%> (ø)
...anning/obstacle_stop_planner/src/planner_utils.cpp 0.00% <0.00%> (ø)
...nning_simulator/simple_planning_simulator_core.cpp 37.42% <0.00%> (-1.96%) ⬇️
planning/route_handler/src/route_handler.cpp 11.45% <0.00%> (-0.14%) ⬇️
... and 50 more

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.

Copy link
Contributor

@maxime-clem maxime-clem left a comment

Choose a reason for hiding this comment

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

The idea seems good, expect for the case where ego is about to drive up a slope.
I believe in that case we can encounter an obstacle that is currently at a higher z than the vehicle height, but that will still collide with the ego vehicle if it follows its trajectory.
image

What do you think ?

planning/obstacle_stop_planner/src/planner_utils.cpp Outdated Show resolved Hide resolved
@xmfcx xmfcx self-requested a review November 23, 2022 13:30
@github-actions github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Nov 29, 2022
@lchojnack
Copy link
Contributor Author

I've updated the PR. I've added a polyhedron for visualization (topic: /planning/scenario_planning/lane_driving/motion_planning/obstacle_stop_planner/debug/marker) and colliding point cloud (topic: /planning/scenario_planning/lane_driving/motion_planning/obstacle_stop_planner/output/point_cloud/debug).

I've tested the PR in some scenarios, and I think it works now. Printscreens below
Screenshot_20221129_101853
Screenshot_20221128_153850
Screenshot_20221128_153753
Screenshot_20221128_153701

@lchojnack lchojnack changed the title fix(obstacle_stop_planner): Add checking of point height. fix(obstacle_stop_planner): add checking of point height Nov 29, 2022
@xmfcx xmfcx added the priority:high High urgency and importance. label Nov 29, 2022
@xmfcx xmfcx added this to the Bus ODD Nov-Dec Milestone milestone Nov 29, 2022
Copy link
Contributor

@maxime-clem maxime-clem left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. I think there are some debugging code remaining in the code that need to be removed, otherwise everything looks good.
One thing still missing is an update to the obstacle_stop_planner documentation but this can be updated in a separate PR.
I am now testing the PR with some scenarios to make sure there are no big issue. Once they finish and once the remaining minor code issues are fixed, I will approve the PR.

planning/obstacle_stop_planner/src/node.cpp Outdated Show resolved Hide resolved
planning/obstacle_stop_planner/src/node.cpp Outdated Show resolved Hide resolved
@lchojnack
Copy link
Contributor Author

@maxime-clem
I've applied your review hints.
In several days I plan to test the PR with the ITRI simulations, to be sure that the PR resolves the issue #2213

Copy link
Contributor

@maxime-clem maxime-clem left a comment

Choose a reason for hiding this comment

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

The code looks good and the behavior in simulation is as expected.

@xmfcx
Copy link
Contributor

xmfcx commented Feb 7, 2023

@lchojnack could you rebase and resolve the conflicts with the planning/obstacle_stop_planner/src/node.cpp please? Then we can proceed to merge it.

Signed-off-by: Lukasz Chojnacki <lukasz.chojnacki@robotec.ai>
@github-actions github-actions bot added the type:documentation Creating or refining documentation. (auto-assigned) label Feb 8, 2023
@lchojnack
Copy link
Contributor Author

lchojnack commented Feb 13, 2023

@xmfcx I've resolved conflicts and I've also tested the implemented functionality. The feature is working as expected. I've tested it using planning simulator.

I think that would be good to test the feature implemented in PR #2647 in order to be sure that during resolving a conflict in planning/obstacle_stop_planner/src/node.cpp the chattering prevention mechanism wasn't broken.

I think the PR requires a review from one of the code owners @satoshi-ota @shmpwk @taikitanaka3 @tkimura4

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2023

@satoshi-ota could you check if this PR breaks any functionality that was added in your previous PR #2647 ?

… another part of code

Signed-off-by: Lukasz Chojnacki <lukasz.chojnacki@robotec.ai>
@satoshi-ota
Copy link
Contributor

@lchojnack @xmfcx It looks good to me from the point of view of chattering functionality.

Copy link
Contributor

@xmfcx xmfcx left a comment

Choose a reason for hiding this comment

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

will resolve the last simple conflicts and merge

@xmfcx xmfcx enabled auto-merge (squash) February 15, 2023 13:23
@lchojnack
Copy link
Contributor Author

@xmfcx Thank you for resolving the last simple conflicts.

This is just a friendly reminder. This PR should be merged together with parameters PR autowarefoundation/autoware_launch#150

@xmfcx xmfcx merged commit 722f4cd into autowarefoundation:main Feb 15, 2023
nabetetsu pushed a commit to xygyo77/autoware.universe that referenced this pull request Mar 1, 2023
…ndation#2296)

Signed-off-by: Łukasz Chojnacki <lukasz.chojnacki@robotec.ai>
1222-takeshi pushed a commit to 1222-takeshi/autoware.universe that referenced this pull request Mar 6, 2023
…ndation#2296)

Signed-off-by: Łukasz Chojnacki <lukasz.chojnacki@robotec.ai>
@brkay54
Copy link
Member

brkay54 commented Apr 18, 2023

Hello @lchojnack @maxime-clem , sorry to disturb you. I am working on z-axis filtering for predicted objects so I want to ask you that how to arrange the z-axis coordinate of the obstacle in planning simulator? I set the value in dummy objects but it is not working 😞

@lchojnack
Copy link
Contributor Author

lchojnack commented Apr 18, 2023

Hello @brkay54 ,
I've just checked on the latest autoware on awsim-stable branch hash 984e35c, and the feature works as expected.

Please make sure:

  1. This parameter is True
  2. There are green box markers on topic
    /planning/scenario_planning/lane_driving/motion_planning/obstacle_stop_planner/debug/marker
    and collision point cloud on topic
    /planning/scenario_planning/lane_driving/motion_planning/obstacle_stop_planner/debug/collision_pointcloud as in the picture below

image

I've used the following command to test
ros2 launch autoware_launch planning_simulator.launch.xml map_path:=$HOME/autoware_map/elevation_test/ vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit

@brkay54
Copy link
Member

brkay54 commented Apr 18, 2023

Thank you for your answer. If I understood correctly, you used awsim for these tests (setting dummy obstacle). I did not use awsim, just I am using planning simulator.
image

When I set this value to higher, obstacle position on Z axis did not change.

Also, if you don't mind, can you share your test map with me?

Thank you! @lchojnack

@lchojnack
Copy link
Contributor Author

@brkay54

If I understood correctly, you used awsim for these tests (setting dummy obstacle).

No, I use planning simulation for testing using the command
ros2 launch autoware_launch planning_simulator.launch.xml map_path:=$HOME/autoware_map/elevation_test/ vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit

I just have ready to run 'clean' autoware (without any local changes) on awsim-stable branch hash 984e35c.
I use 2D Dummy Bus for testing, and sometimes I modify its H vehicle height as below

image

Also, if you don't mind, can you share your test map with me?

Please use the map from the tutorial. I've just tested and it also works as expected using command
ros2 launch autoware_launch planning_simulator.launch.xml map_path:=$HOME/autoware_map/sample-map-planning vehicle_model:=sample_vehicle sensor_model:=sample_sensor_kit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:planning Route planning, decision-making, and navigation. (auto-assigned) priority:high High urgency and importance. type:documentation Creating or refining documentation. (auto-assigned)
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

8 participants