chore(map_loader): rework parameters of map_loader

[anhnv3991]

Description

In the map_loader module

• Added lanelet2_map_loader.schema.json and pointcloud_map_loader.schema.json that describe parameters of map_loader in the json format
• Moved the parameter lanelet2_map_path from lanelet2_map_loader.launch.xml to lanelet2_map_loader.param.yaml
• Moved the parameters pcd_paths_or_directory and pcd_metadata_path from pointcloud_map_loader.launch.xml to pointcloud_map_loader.param.yaml
• Modified README.md to refer to json schema files in the Parameters sections

Tests performed

Parameters of pointcloud map loader

Parameters of lanelet2 map loader

Effects on system behavior

None

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.

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

[SakodaShintaro]

Thank you for the pull request.
Please wait a little longer while we decide what to do with parameters such as pcd_paths_or_directory and pcd_metadata_path.

[SakodaShintaro]

For arguments like file paths, we decided to use the following notation to maintain the previous behavior.
It should be given from the argument.
autowarefoundation/autoware-github-actions#232 (comment)

[YamatoAndo]

@anhnv3991 Could you please submit this commit as a separate pull request?

[anhnv3991]

@YamatoAndo If I does not add that commit , one of the CLI checks will fail.

[anhnv3991]

@anhnv3991 Since we need to change the autoware_launch parameter at the same time, could you please create a pull request for that repository as well?

autoware_launch/config/map/lanelet2_map_loader.param.yaml and autoware_launch/config/map/pointcloud_map_loader.param.yaml must be changed.

https://github.com/autowarefoundation/autoware_launch/blob/main/autoware_launch/config/map/lanelet2_map_loader.param.yaml https://github.com/autowarefoundation/autoware_launch/blob/main/autoware_launch/config/map/pointcloud_map_loader.param.yaml

@SakodaShintaro How can I test those files after I made changes?

[SakodaShintaro]

@anhnv3991

• set autoware.universe to this branch (anhnv3991:chore/rework_param_map_loader)
• set autoware_launch to a branch where you changed

Then, run autoware to check if it works properly. (logging_simulator in sample rosbag is often used)

If you submit a pull request to autoware_launch and paste the URL (draft is ok), I will check it. 😃

[SakodaShintaro]

@anhnv3991
I apologize for pointing this out in detail, but we have now decided to stop setting default values in declare_parameter. (This is because it becomes difficult to understand where the actual value is set.)

https://github.com/autowarefoundation/autoware.universe/blob/e2a5f4aed880b746b76c4c5d51927fb441ce4b3e/map/map_loader/src/lanelet2_map_loader/lanelet2_map_loader_node.cpp
I would appreciate it if you could fix this.

from

  declare_parameter("lanelet2_map_path", "");
  declare_parameter("center_line_resolution", 5.0);

to

  declare_parameter<std::string>("lanelet2_map_path");
  declare_parameter<double>("center_line_resolution");

And there is a parameter in lanelet2_map_visualization_node.

autoware.universe/map/map_loader/src/lanelet2_map_loader/lanelet2_map_visualization_node.cpp

Line 74 in e2a5f4a

viz_lanelets_centerline_ = this->declare_parameter("viz_lanelets_centerline", true);

However, it is not a good idea to add a new param file and json file for this one parameter, so why not try writing true directly instead of using this value as a parameter?

from

  viz_lanelets_centerline_ = this->declare_parameter("viz_lanelets_centerline", true);

to

  viz_lanelets_centerline_ = true;

@YamatoAndo
What about the above changes? Is it better to create config and json properly?

[YamatoAndo]

However, it is not a good idea to add a new param file and json file for this one parameter, so why not try writing true directly instead of using this value as a parameter?

from

viz_lanelets_centerline_ = this->declare_parameter("viz_lanelets_centerline", true);
to

viz_lanelets_centerline_ = true;

I agree with this proposal!

[SakodaShintaro]

Thank you for the correction.
It has been confirmed that the logging_simulator runs with the same accuracy as before on AWSIM data with GT.
Looks Good To Me

When merging, please press the merge button at the same time as below
autowarefoundation/autoware_launch#843
(Strictly speaking, there is no problem even if PR at autoware_launch is merged first, so I think it would be a good idea to press the merge button of the PR at autoware_launch first.)

[SakodaShintaro]

@anhnv3991
Sorry, I forgot to check the map_loader test.

build-and-test-differential is failed. https://github.com/autowarefoundation/autoware.universe/actions/runs/7809902259/job/21302469469?pr=6199

I think it is good to fix a test code.

https://github.com/autowarefoundation/autoware.universe/blob/main/map/map_loader/test/lanelet2_map_loader_launch.test.py#L37

from

        parameters=[{"lanelet2_map_path": lanelet2_map_path}],

to

        parameters=[{
            "lanelet2_map_path": lanelet2_map_path,
            "center_line_resolution": 5.0,
        }],

The test can be confirmed by following commands.

cd ~/autoware
colcon build --symlink-install \
  --cmake-args \
    -DCMAKE_BUILD_TYPE=Release \
    -DBUILD_TESTING=ON \
    -DCMAKE_CXX_FLAGS='-fprofile-arcs -ftest-coverage' \
    -DCMAKE_C_FLAGS='-fprofile-arcs -ftest-coverage' \
  --packages-select map_loader
colcon test --return-code-on-test-failure --packages-select map_loader
colcon test-result --all --verbose | grep map_loader

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (7e53f30) 14.86% compared to head (c0f23f7) 14.87%.
Report is 3 commits behind head on main.

Files	Patch %	Lines
...c/lanelet2_map_loader/lanelet2_map_loader_node.cpp	0.00%	0 Missing and 2 partials ⚠️
...et2_map_loader/lanelet2_map_visualization_node.cpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6199   +/-   ##
=======================================
  Coverage   14.86%   14.87%           
=======================================
  Files        1845     1845           
  Lines      126638   126613   -25     
  Branches    37878    37860   -18     
=======================================
  Hits        18828    18828           
+ Misses      86647    86624   -23     
+ Partials    21163    21161    -2     

Flag	Coverage Δ		*Carryforward flag
differential	26.60% <0.00%> (?)
total	14.87% <ø> (+<0.01%)	⬆️	Carriedforward from 7e53f30

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