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

build(behavior_path_external_request_lane_change_module): prefix package and namespace with autoware_ #6636

Merged
merged 17 commits into from
May 23, 2024

Conversation

esteve
Copy link
Contributor

@esteve esteve commented Mar 15, 2024

Description

This PR adds the autoware_ prefix to the package and puts headers in the autoware 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.

  • 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 Mar 15, 2024
@esteve esteve added the tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 15, 2024
@esteve esteve force-pushed the prefix-behavior_path_external_request_lane_change_module branch from 9b080aa to fb85a73 Compare March 27, 2024 14:58
@esteve esteve marked this pull request as draft March 27, 2024 16:39
@esteve esteve force-pushed the prefix-behavior_path_external_request_lane_change_module branch from fb85a73 to dc486f9 Compare April 2, 2024 13:33
@esteve esteve force-pushed the prefix-behavior_path_external_request_lane_change_module branch from 2bce890 to a2c712d Compare April 10, 2024 11:07
@esteve esteve marked this pull request as ready for review April 12, 2024 15:43
@esteve
Copy link
Contributor Author

esteve commented Apr 12, 2024

@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.

@esteve esteve enabled auto-merge (squash) April 12, 2024 15:44
@esteve esteve force-pushed the prefix-behavior_path_external_request_lane_change_module branch from 90eac3d to b2a6bc8 Compare April 18, 2024 12:51
@esteve
Copy link
Contributor Author

esteve commented Apr 18, 2024

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.

@esteve esteve marked this pull request as draft May 7, 2024 12:47
auto-merge was automatically disabled May 7, 2024 12:47

Pull request was converted to draft

@esteve
Copy link
Contributor Author

esteve commented May 7, 2024

Back to draft because I'm going to rename folders and move headers to a separate folder (see autowarefoundation/autoware#4569 (comment))

@github-actions github-actions bot added the type:ci Continuous Integration (CI) processes and testing. (auto-assigned) label May 13, 2024
@esteve esteve marked this pull request as ready for review May 13, 2024 14:46
@esteve esteve enabled auto-merge (squash) May 13, 2024 14:46
@esteve
Copy link
Contributor Author

esteve commented May 13, 2024

@xmfcx this is now ready for review again.

@xmfcx
Copy link
Contributor

xmfcx commented May 14, 2024

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.

https://github.com/autowarefoundation/autoware_launch/blob/0aea8a35b9831e207827504297c6ac1a2e6cd9e1/autoware_launch/config/planning/preset/default_preset.yaml#L24-L26

The default is false.

Then I looked for the references of this plugin, since it should be called from somewhere else. (searched for behavior_path_planner::ExternalRequestLaneChangeLeftModuleManager in the entire autoware folder)

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?

@takayuki5168
Copy link
Contributor

@xmfcx Just running a test of the package is fine. Thank you.

@xmfcx
Copy link
Contributor

xmfcx commented May 14, 2024

@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 xmfcx self-requested a review May 14, 2024 14:55
@esteve
Copy link
Contributor Author

esteve commented May 15, 2024

@xmfcx I've applied your changes, could you have another look when you have a moment? Thanks.

@github-actions github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label May 15, 2024
esteve and others added 17 commits May 23, 2024 14:48
…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]>
Signed-off-by: Esteve Fernandez <[email protected]>
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]>
…e headers files to src

Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
Signed-off-by: Esteve Fernandez <[email protected]>
@esteve esteve force-pushed the prefix-behavior_path_external_request_lane_change_module branch from 5bfccb2 to a436772 Compare May 23, 2024 12:49
Copy link

codecov bot commented May 23, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 15.84%. Comparing base (5145bb5) to head (a436772).
Report is 3 commits behind head on main.

Files Patch % Lines
...test/test_behavior_path_planner_node_interface.cpp 0.00% 0 Missing and 2 partials ⚠️
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     
Flag Coverage Δ *Carryforward flag
differential 7.74% <0.00%> (?)
total 15.83% <ø> (+<0.01%) ⬆️ Carriedforward from 5145bb5

*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.

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.

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!

@xmfcx xmfcx disabled auto-merge May 23, 2024 14:55
@xmfcx xmfcx merged commit 4ffa82b into main May 23, 2024
27 of 29 checks passed
@xmfcx xmfcx deleted the prefix-behavior_path_external_request_lane_change_module branch May 23, 2024 14:55
karishma1911 pushed a commit to Interplai/autoware.universe that referenced this pull request Jun 3, 2024
a-maumau pushed a commit to a-maumau/autoware.universe that referenced this pull request Jun 7, 2024
KhalilSelyan pushed a commit that referenced this pull request Jul 22, 2024
…age and namespace with autoware_ (#6636)

Signed-off-by: Esteve Fernandez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:launch Launch files, scripts and initialization tools. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) tag:run-build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) type:ci Continuous Integration (CI) processes and testing. (auto-assigned)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants