-
Notifications
You must be signed in to change notification settings - Fork 649
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
build(behavior_path_external_request_lane_change_module): prefix package and namespace with autoware_ #6636
build(behavior_path_external_request_lane_change_module): prefix package and namespace with autoware_ #6636
Conversation
9b080aa
to
fb85a73
Compare
fb85a73
to
dc486f9
Compare
2bce890
to
a2c712d
Compare
@xmfcx @mitsudome-r @rej55 @kosuke55 @shmpwk @TomohitoAndo @tkimura4 @zulfaqar-azmi-t4 This is now ready for review, please have a look at it when you have a moment. Thanks. |
90eac3d
to
b2a6bc8
Compare
I've reverted any performance or any other improvements detected by clang-tidy, the CI will now fail because of the clang-tidy check, but now the PR only conntains the prefix changes. |
Pull request was converted to draft
Back to draft because I'm going to rename folders and move headers to a separate folder (see autowarefoundation/autoware#4569 (comment)) |
@xmfcx this is now ready for review again. |
I've ran with this branch, it compiled and planning simulator worked alright. Then to see if I was testing the correct things, I've renamed the contents of https://github.com/autowarefoundation/autoware.universe/blob/main/planning/behavior_path_external_request_lane_change_module/plugins.xml and added random characters. The planning sim demo worked fine again. Probably this is not being launched then. The default is false. Then I looked for the references of this plugin, since it should be called from somewhere else. (searched for Here are the remaining plugin references to be updated:
I don't know the best way to test this module though. @takayuki5168 -san, what is the easiest way to test this module? |
@xmfcx Just running a test of the package is fine. Thank you. |
@takayuki5168 -san meant this for the static_centerline_generator pr. After talking with him, I learned that if I just update: these to true, then I can test it with the planning sim. I've enabled the modules with that, then, the planning sim failed to switch to "Auto" mode. Then I've updated these plugin references:
Then ran the planning simulator, it then worked. @esteve these files need to be updated too. |
@xmfcx I've applied your changes, could you have another look when you have a moment? Thanks. |
…age and namespace with autoware_ Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
…tests Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
…clang-tidy issues Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
…ve unused code Signed-off-by: Esteve Fernandez <[email protected]>
…clang-tidy issues Signed-off-by: Esteve Fernandez <[email protected]>
…vert clang-tidy fixes for naming functions Signed-off-by: Esteve Fernandez <[email protected]>
…vert performance-related clang-tidy fixes Signed-off-by: Esteve Fernandez <[email protected]>
… rename base folder Signed-off-by: Esteve Fernandez <[email protected]>
… rename base folder Signed-off-by: Esteve Fernandez <[email protected]>
…e headers files to src Signed-off-by: Esteve Fernandez <[email protected]>
… fix include paths Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
5bfccb2
to
a436772
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6636 +/- ##
==========================================
+ Coverage 15.82% 15.84% +0.02%
==========================================
Files 1900 1905 +5
Lines 132416 132419 +3
Branches 43848 43888 +40
==========================================
+ Hits 20953 20984 +31
+ Misses 88362 88294 -68
- Partials 23101 23141 +40
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Enabled it from the launch file and ran the planning simulator. Successfully loaded the modules and was able to drive, thanks for all the effort!
…age and namespace with autoware_ (autowarefoundation#6636) Signed-off-by: Esteve Fernandez <[email protected]>
…age and namespace with autoware_ (autowarefoundation#6636) Signed-off-by: Esteve Fernandez <[email protected]>
…age and namespace with autoware_ (#6636) Signed-off-by: Esteve Fernandez <[email protected]>
Description
This PR adds the
autoware_
prefix to the package and puts headers in theautoware
namespace.Part of:
Tests performed
Not applicable.
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.
After all checkboxes are checked, anyone who has write access can merge the PR.