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(planning_evaluator): fix crashes with empty trajectories #6129

Conversation

maxime-clem
Copy link
Contributor

@maxime-clem maxime-clem commented Jan 22, 2024

Description

Crashes were caused by subtracting 1 from unsigned integer (size_t), causing crashes with empty trajectories (where the size = 0).

To reproduce the crash, you can publish an empty trajectory with the following command:

ros2 topic pub /planning/scenario_planning/trajectory autoware_auto_planning_msgs/msg/Trajectory "{header: {stamp: now, frame_id: "base_link"}}"

Tests performed

PSim

Effects on system behavior

Not applicable.

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: Maxime CLEMENT <maxime.clement@tier4.jp>
@maxime-clem maxime-clem added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Jan 22, 2024
@github-actions github-actions bot added the component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) label Jan 22, 2024
@kyoichi-sugahara
Copy link
Contributor

@maxime-clem
Thanks!
How did you detect this bug?

Copy link
Contributor

@kyoichi-sugahara kyoichi-sugahara left a comment

Choose a reason for hiding this comment

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

LGTM!

@maxime-clem
Copy link
Contributor Author

@maxime-clem Thanks! How did you detect this bug?

@TakaHoribe found this bug (#6126).
I added instructions on how to reproduce the crash in the PR description.

Copy link

codecov bot commented Jan 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (19009f7) 14.61% compared to head (f112e4c) 14.61%.
Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6129   +/-   ##
=======================================
  Coverage   14.61%   14.61%           
=======================================
  Files        1860     1860           
  Lines      127417   127417           
  Branches    37286    37286           
=======================================
+ Hits        18621    18622    +1     
  Misses      87884    87884           
+ Partials    20912    20911    -1     
Flag Coverage Δ *Carryforward flag
differential 58.69% <100.00%> (?)
total 14.61% <ø> (-0.01%) ⬇️ Carriedforward from 19009f7

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@maxime-clem maxime-clem merged commit 19f4484 into autowarefoundation:main Jan 24, 2024
33 of 36 checks passed
@maxime-clem maxime-clem deleted the fix/planning_evaluator/crash-with-empty-traj branch January 24, 2024 21:56
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:evaluator Evaluation tools for planning, localization etc. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants