feat(map_loader): make some functions static

[takayuki5168]

Signed-off-by: Takayuki Murooka takayuki5168@gmail.com

Description

In order to make a static path planner with optimization as follows,
#1942
I made some functions of map_loader required in the static planner static.

The usage in the static planner is like this

autoware.universe/planning/static_centerline_optimizer/src/functions.cpp

Lines 78 to 87 in 7296a42

	// load map
	lanelet::LaneletMapPtr map_ptr;
	map_ptr = Lanelet2MapLoaderNode::load_map(node, lanelet2_file_name, "MGRS");
	if (!map_ptr) {
	return nullptr;
	}
	// create map bin msg
	const auto map_bin_msg =
	Lanelet2MapLoaderNode::create_map_bin_msg(map_ptr, lanelet2_file_name, current_time);

Related links

Tests performed

Notes for reviewers

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.
• The PR has been properly tested.
• The PR has been reviewed by the code owners.

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.
• The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

[takayuki5168]

@TakaHoribe Could you review this PR.

[TakaHoribe]

@takayuki5168 How about having two functions with different arguments? (It seems a little odd to me that all methods are static.)

HADMapBin create_map_bin_msg(
  const lanelet::LaneletMapPtr map, const std::string & lanelet2_filename) {
  return create_map_bin_msg(map, lanelet2_filename, now());
}

static HADMapBin create_map_bin_msg(
  const lanelet::LaneletMapPtr map, const std::string & lanelet2_filename,
  const rclcpp::Time & now);

[codecov]

Codecov Report

Base: 10.38% // Head: 10.38% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (aa384ab) compared to base (ee4f381).
Patch coverage: 24.77% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2014      +/-   ##
==========================================
- Coverage   10.38%   10.38%   -0.01%     
==========================================
  Files        1163     1163              
  Lines       82840    82849       +9     
  Branches    19287    19287              
==========================================
  Hits         8604     8604              
- Misses      64968    64977       +9     
  Partials     9268     9268              

Flag	Coverage Δ		*Carryforward flag
differential	0.00% <0.00%> (?)
total	10.36% <25.00%> (ø)		Carriedforward from ee4f381

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
...c/lanelet2_map_loader/lanelet2_map_loader_node.cpp	0.00% <0.00%> (ø)
common/motion_utils/src/resample/resample.cpp	30.97% <12.12%> (ø)
...n/motion_utils/test/src/resample/test_resample.cpp	25.93% <26.75%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[takayuki5168]

@TakaHoribe
To me, your idea looks redundant since there is no reason to define two types of functions.

@kenji-miyake What do you think.

(It seems a little odd to me that all methods are static.)

I made the functions only required in the static path planner static. (If I do my best, I can split the static and non-static part more specifically)

[kenji-miyake]

To me, your idea looks redundant since there is no reason to define two types of functions.
@kenji-miyake What do you think.

I also think it's redundant in this case.

But since it seems class-independent, I feel it might be better to make these functions normal functions (like libraries), not class functions.
Are there any reasons to define these in the class?

[takayuki5168]

Are there any reasons to define these in the class?

Not a big reason, but I just wanted to apply the minimum changes to map_loader and there are only two small functions which I feel exaggerated to make them a library.