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.

Sign up for GitHub

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

Jump to bottom

fix(path_generator): deal with unintended input #336

Merged
merged 5 commits into from
Apr 9, 2025
Merged

fix(path_generator): deal with unintended input #336

merged 5 commits into from
Apr 9, 2025

Conversation

mitukou1109
Copy link
Contributor

@mitukou1109 mitukou1109 commented Mar 28, 2025 •
edited by kosuke55
Loading

Description

This PR fixes the behavior of path_generator's utility functions for unintended input.

  • get_first_intersection_arc_length() returns std::nullopt if either of the input lanelet sequence or the line strings generated during the process is empty
  • get_path_bounds() returns std::nullopt if the input lanelet sequence is empty
  • crop_line_string() returns the input line string as is if it consists of less than 3 points
  • crop_line_string() returns the input line string as is if the start of the crop bound is negative or the start is larger than the end
  • get_arc_length_on_bounds() returns 0 if the input arc length is negative
  • get_arc_length_on_centerline() returns 0 if the input arc length is negative

How was this PR tested?

2025/04/09 https://evaluation.tier4.jp/evaluation/reports/9e238d3c-7940-526d-be8e-27b70954f684/?project_id=prd_jt 96/96

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 28, 2025 •
edited
Loading

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

@mitukou1109 mitukou1109 mentioned this pull request Mar 28, 2025
Merged
youtalk
youtalk reviewed Mar 31, 2025
View reviewed changes
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

Please add unit tests that try unintended inputs.

@mitukou1109
Copy link
Contributor Author

@youtalk

Please add unit tests that try unintended inputs.

The tests will be added by #268.

@youtalk youtalk added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Apr 3, 2025
youtalk
youtalk approved these changes Apr 3, 2025
View reviewed changes
Copy link
Member

@youtalk youtalk left a comment

Choose a reason for hiding this comment

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

LGTM

@mitukou1109 mitukou1109 force-pushed the fix/path_generator_utils branch from a4a213e to 02c0d7c Compare April 3, 2025 03:57
@mitukou1109
Copy link
Contributor Author

@youtalk @kosuke55 I found autoware::trajectory::Trajectory segfaults when the input has less than 2 points. I fixed it by 02c0d7c, so could you review again?

@kosuke55
Copy link
Contributor

kosuke55 commented Apr 3, 2025 •
edited
Loading

@mitukou1109

I found autoware::trajectory::Trajectory segfaults when the input has less than 2 points. I fixed it by 02c0d7c, so could you review again?

thanks!
@yhisaki please check autoware_trjaectory side too in another PR
->

  auto trajectory = autoware::trajectory::Trajectory<geometry_msgs::msg::Point>::Builder()
                      .set_xy_interpolator<trajectory::interpolator::Linear>()
                      .build(line_string);

already return nullopt when input size is less than2 points. so no need to fix more.

kosuke55
kosuke55 reviewed Apr 3, 2025
View reviewed changes
Copy link
Contributor

@kosuke55 kosuke55 left a comment

Choose a reason for hiding this comment

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

LGTM

@kosuke55
Copy link
Contributor

kosuke55 commented Apr 3, 2025

now we can't run evaluator becase autoware setup is broken

@codecov Codecov
Copy link

codecov bot commented Apr 3, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 4.34783% with 22 lines in your changes missing coverage. Please review.

Project coverage is 18.42%. Comparing base (4cb18f5) to head (bb74f8f).
Report is 153 commits behind head on main.

Files with missing lines Patch % Lines
planning/autoware_path_generator/src/utils.cpp 4.34% 13 Missing and 9 partials ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##             main     #336       +/-   ##
===========================================
- Coverage   78.75%   18.42%   -60.33%     
===========================================
  Files          11       97       +86     
  Lines         193     4694     +4501     
  Branches       73      924      +851     
===========================================
+ Hits          152      865      +713     
- Misses         11     3716     +3705     
- Partials       30      113       +83
Flag Coverage Δ
differential 18.42% <4.34%> (?)
total ?

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kosuke55 kosuke55 force-pushed the fix/path_generator_utils branch from 6915692 to 50612a6 Compare April 7, 2025 07:17
@kosuke55
Copy link
Contributor

kosuke55 commented Apr 7, 2025 •
edited
Loading

2025/04/07 https://evaluation.tier4.jp/evaluation/reports/1134b8b8-c9ff-5ea2-8770-fcf3a8bdb96b/?project_id=prd_jt

-> sorry, please don't care this result

@kosuke55 kosuke55 force-pushed the fix/path_generator_utils branch from 50612a6 to b586fd9 Compare April 8, 2025 03:00
@kosuke55
Copy link
Contributor

kosuke55 commented Apr 8, 2025

2025/04/08 https://evaluation.tier4.jp/evaluation/reports/3f0121df-ad7e-5ea7-9b18-fc149a2bad5e/?project_id=prd_jt

@mitukou1109
Copy link
Contributor Author

mitukou1109 commented Apr 8, 2025 •
edited by kosuke55
Loading

Changed lanelet2_extension version to 0.6.4
2025/04/08 https://evaluation.tier4.jp/evaluation/reports/3531b0f8-7c05-5bdc-8a8f-c22f0080dbdc?project_id=prd_jt failed

@kosuke55
Copy link
Contributor

kosuke55 commented Apr 9, 2025 •
edited
Loading

after https://github.com/tier4/pilot-auto/pull/1297
2025/04/09 https://evaluation.tier4.jp/evaluation/reports/9c5f4510-fdf4-5c04-89bc-d480dcd9948a/?project_id=prd_jt

@mitukou1109 @kosuke55
prevent segfault
1e523f5 
Signed-off-by: mitukou1109 <mitukou1109@gmail.com>
@mitukou1109 @kosuke55
fix self-intersection search range
12462f2 
Signed-off-by: mitukou1109 <mitukou1109@gmail.com>
@mitukou1109 @kosuke55
define behavior for unintended input
eaf60fa 
Signed-off-by: mitukou1109 <mitukou1109@gmail.com>
@mitukou1109 @kosuke55
prevent segfault
b0bc22a 
Signed-off-by: mitukou1109 <mitukou1109@gmail.com>
@mitukou1109 @kosuke55
check builder output instead of input size
bb74f8f 
Signed-off-by: mitukou1109 <mitukou1109@gmail.com>
@kosuke55 kosuke55 force-pushed the fix/path_generator_utils branch from b586fd9 to bb74f8f Compare April 9, 2025 01:30
@kosuke55
Copy link
Contributor

kosuke55 commented Apr 9, 2025 •
edited
Loading

2025/04/09 https://evaluation.tier4.jp/evaluation/reports/9e238d3c-7940-526d-be8e-27b70954f684/?project_id=prd_jt 96/96

kosuke55
kosuke55 approved these changes Apr 9, 2025
View reviewed changes
Copy link
Contributor

@kosuke55 kosuke55 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM ! all scenario tests passed

@kosuke55 kosuke55 merged commit 047696c into autowarefoundation:main Apr 9, 2025
24 of 27 checks passed
@mitukou1109 mitukou1109 deleted the fix/path_generator_utils branch April 10, 2025 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@youtalk youtalk youtalk approved these changes

@kosuke55 kosuke55 kosuke55 approved these changes

@sasakisasaki sasakisasaki Awaiting requested review from sasakisasaki sasakisasaki is a code owner

@satoshi-ota satoshi-ota Awaiting requested review from satoshi-ota satoshi-ota is a code owner

@takayuki5168 takayuki5168 Awaiting requested review from takayuki5168 takayuki5168 is a code owner

Assignees
No one assigned
Labels
run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mitukou1109 @kosuke55 @youtalk