build(behavior_velocity_planner): prefix package and namespace with autoware_

[esteve]

Description

This PR adds the autoware_ prefix to the package and puts headers in the autoware namespace.

Part of:

• https://github.com/autowarefoundation/autoware/issues/4569

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.

• [ ] I've confirmed the contribution guidelines.
• [ ] The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

• [x] The PR follows the pull request guidelines.

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.

[esteve]

@xmfcx @mitsudome-r @kosuke55 @kyoichi-sugahara @mkuri @soblin @maxime-clem @satoshi-ota @shmpwk @taikitanaka3 @takayuki5168 @TomohitoAndo @tkimura4 @rej55 @TakaHoribe @zulfaqar-azmi-t4

This PR is now ready for review. clang-tidy reports a warning (a class forward-declared in the behavior_velocity_planner namespace), I didn't fix it because it belongs in the behavior_velocity_planner_common package, and updating that package was outside the scope of this PR and it'd also require fixing clang-tidy issues in that package. When I get to work on behavior_velocity_planner_common I'll fix the clang-tidy warning.

[xmfcx]

Thanks @esteve !

I didn't check in detail but should there be updates made on the https://github.com/autowarefoundation/autoware_launch too?

• https://github.com/autowarefoundation/autoware_launch/tree/55009f3cc06ac254c1e2093b8fc1f95e86690922/autoware_launch/config/planning/scenario_planning/lane_driving/behavior_planning/behavior_velocity_planner
• https://github.com/autowarefoundation/autoware_launch/blob/55009f3cc06ac254c1e2093b8fc1f95e86690922/autoware_launch/launch/components/tier4_planning_component.launch.xml#L38

Also, does planning simulator demo work fine with the current changes?

[esteve]

I didn't check in detail but should there be updates made on the https://github.com/autowarefoundation/autoware_launch too?

I've checked and there doesn't seem to be any find-pkg-share calls, so the changes should be ok.

Also, does planning simulator demo work fine with the current changes?

Unfortunately I can't test the simulator because of my hardware, but I can try.

[satoshi-ota]

Hi @esteve :hand: Thank you for your work.

I'll review this but could you remove function name changes at first...? I heard that we forcus on package rename for now. https://github.com/autowarefoundation/autoware.universe/pull/6817#issuecomment-2062966623

[esteve]

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 contains the prefix changes.

[esteve]

Moved back to draft to investigate the test failure.

[esteve]

Back to draft because I'm going to rename folders and move headers to a separate folder (see https://github.com/autowarefoundation/autoware/issues/4569#issuecomment-2097690750)

[esteve]

@xmfcx @mitsudome-r I've renamed the base folder for this package. This package doesn't export public headers, so no need to update any include folders.

[xmfcx]

https://github.com/autowarefoundation/autoware.universe/assets/10751153/22ed426c-28e5-4c82-9394-2906aa000ad1

Tested this with the planning simulator and it works without problems :+1:

Now will check the changes manually.

[xmfcx]

@takayuki5168 if there are no other concerns, I would like to merge this by tomorrow.

[esteve]

@maxime-clem fixed in https://github.com/autowarefoundation/autoware.universe/pull/6693/commits/8a08e4b136b368e3bbeefae84c50cca4338d0154, thanks for spotting it.

[esteve]

@xmfcx thanks for approving this PR. GitHub requires a code owner approval so this could be merged, can you merge it anyway?

[github-actions[bot]]

Documentation URL: https://autowarefoundation.github.io/autoware.universe/pr-6693/ Modified URLs:

• https://autowarefoundation.github.io/autoware.universe/pr-6693/planning/autoware_behavior_velocity_planner/

[esteve]

@xmfcx I've applied your changes, thanks. Can you approve this PR? Thanks.

[xmfcx]

Thank you for your patience. I have approved the PR. Once the required CIs pass successfully, I will proceed with the merge manually.

Since I am not a code owner but a maintainer, I need to manually merge it, auto merge only works if code owners approve it.

[xmfcx]

I'm sorry that I keep finding these in separate reviews but this needs to be updated too:

https://github.com/autowarefoundation/autoware.universe/blob/d47430cdab35b3447b4ff4be3ca6e1e6da027f02/launch/tier4_planning_launch/launch/scenario_planning/lane_driving/behavior_planning/behavior_planning.launch.xml#L223

to: <composable_node pkg="autoware_behavior_velocity_planner" plugin="autoware::behavior_velocity_planner::BehaviorVelocityPlannerNode" name="behavior_velocity_planner" namespace="">

I've found this by searching for behavior_velocity_planner::BehaviorVelocityPlannerNode in the autoware directory.

But then how did the planning sim test work with this pr?

It was working with the planning sim without this because I was compiling without cleaning the install/build folders, so the remnants of old behavior_velocity_planner was being used.

When I removed behavior_velocity_planner folder from install and rebuilt, planning sim won't work without this update.

So if we merged this without this update and someone compiled autoware from scratch, they couldn't run the planning sim demo.

[esteve]

@xmfcx nice catch! I've applied this change as well. Thanks.

[xmfcx]

When I run the planning simulator with this PR, none of the plugins load.

Error logs:

🖱️Click to expand

[component_container_mt-30] [ERROR 1715861934.567430534] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::CrosswalkModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567489575] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::WalkwayModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567507095] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::TrafficLightModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567518435] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::IntersectionModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567527705] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::MergeFromPrivateModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567537515] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::BlindSpotModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567604035] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::DetectionAreaModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567620735] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::NoStoppingAreaModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567631615] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::StopLineModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567641965] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::RunOutModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567651525] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::OutOfLaneModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)
[component_container_mt-30] [ERROR 1715861934.567662115] [planning.scenario_planning.lane_driving.behavior_planning.behavior_velocity_planner]: The scene plugin 'behavior_velocity_planner::DynamicObstacleStopModulePlugin' is not available. (launchScenePlugin() at /home/mfc/projects/autoware/src/universe/autoware.universe/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp:77)

You might need to treat behavior_velocity_planner_common package and behavior_velocity_planner packages at the same time.

https://github.com/autowarefoundation/autoware.universe/blob/a233cd9a1c85696e4f9a791b5c660e6519b00bfb/planning/behavior_velocity_planner_common/include/behavior_velocity_planner_common/plugin_interface.hpp#L25-L29

It is using symbols from here.

https://github.com/autowarefoundation/autoware.universe/blob/a233cd9a1c85696e4f9a791b5c660e6519b00bfb/planning/behavior_velocity_planner/src/planner_manager.cpp#L52-L53

I also tried renaming it back to this but it still failed to load those.

I'm not an expert on pluginlib but I read the following a bit:

• https://docs.ros.org/en/humble/Tutorials/Beginner-Client-Libraries/Pluginlib.html

And here it says:

pluginlib::ClassLoader<polygon_base::RegularPolygon> poly_loader("polygon_base", "polygon_base::RegularPolygon");

It is templated with the base class, i.e. polygon_base::RegularPolygon.

The first argument is a string for the package name of the base class, i.e. polygon_base.

The second argument is a string with the fully qualified base class type for the plugin, i.e. polygon_base::RegularPolygon.

Also please test these with the https://autowarefoundation.github.io/autoware-documentation/main/tutorials/ad-hoc-simulation/planning-simulation/ (it only uses minimal cpu only resources) or if they get merged by oversight, it will break the functionality of the autoware.

Edit: For clarification, the vehicle is able to go from A to B. But plugins are not loaded on the start.

[esteve]

@xmfcx thanks for the thorough review, I'm looking into these issues. I'm moving this PR back to draft as it needs more changes.

[esteve]

@xmfcx thanks for the thorough review. After doing investigations, I think this is caused by a mismatched between:

https://github.com/autowarefoundation/autoware.universe/blob/a3e776d4ee9105ce9c1349fd1ebbaba011f9614b/planning/autoware_behavior_velocity_planner/src/planner_manager.cpp#L52-L56

and, for example:

https://github.com/autowarefoundation/autoware.universe/blob/a3e776d4ee9105ce9c1349fd1ebbaba011f9614b/planning/behavior_velocity_dynamic_obstacle_stop_module/src/manager.cpp#L73-L76

I'm going to update these files so they match and run the planning demo again.

[esteve]

I'm still debugging this, I prefixed code in behavior_velocity_planner_common, but it still does not work.

[esteve]

@xmfcx this is now ready for review, I've run the planning demo and it works. Also, all the plugins are loaded. The issue was in:

https://github.com/autowarefoundation/autoware.universe/blob/578d135db30f875ef6c515b2870899981ad11a17/planning/behavior_velocity_crosswalk_module/CMakeLists.txt#L6

Plugins couldn't find the manager class because it was in a different package.

[xmfcx]

https://github.com/autowarefoundation/autoware.universe/blob/4ffa82b896aac7383c4605489b4a38f2c27fb1f5/planning/autoware_static_centerline_generator/test/test_static_centerline_generator.test.py#L62

This needs to be updated to pass the failing test.

[esteve]

@xmfcx thanks, I've pushed the fix in f8b4cb6f37fd20084b55310eb56cc2e15f485b52

[xmfcx]

@esteve I will rename the PR title to refactor() instead of build() since this is a heavy refactoring task. But other than that, I've tested it and it works well. Thanks for the efforts.

[esteve]

@xmfcx thanks for your feedback and the patience 🙂