fix(tier4_control_component_launch): fix duplicate declaration of controller parameter paths

[xuyuan-han]

Description

see #728

Tests performed

In autoware.launch.xml:

  <arg name="lat_controller_param_path" default="<path_to_arbitrary_lateral_controller_param_file>" description="lateral controller configuration file location"/>
  <arg name="lon_controller_param_path" default="<path_to_arbitrary_longitudinal_controller_param_file>" description="longitudinal controller configuration file location"/>
  <!-- Control -->
  <group if="$(var launch_control)">
    <include file="$(find-pkg-share autoware_launch)/launch/components/tier4_control_component.launch.xml"/>
      <arg name="lat_controller_param_path" value="$(var lat_controller_param_path)"/>
      <arg name="lon_controller_param_path" value="$(var lon_controller_param_path)"/>
  </group>

Effects on system behavior

After fixing, arbitrary lateral and longitudinal controller parameter file paths can be specified in the group Control in autoware.launch.xml and can be passed to tier4_control_component.launch.xml

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.

[takayuki5168]

How about removing the following instead?

  <arg name="lat_controller_param_path" value="$(var lat_controller_param_path)"/>
   <arg name="lon_controller_param_path" value="$(var lon_controller_param_path)"/>

[takayuki5168]

Please assign the PR to me once you fixed it.

[xuyuan-han]

  <arg
    name="lat_controller_param_path"
    default="$(find-pkg-share autoware_launch)/config/control/trajectory_follower/$(var latlon_controller_param_path_dir)/lateral/$(var lateral_controller_mode).param.yaml"
  />
  <arg
    name="lon_controller_param_path"
    default="$(find-pkg-share autoware_launch)/config/control/trajectory_follower/$(var latlon_controller_param_path_dir)/longitudinal/$(var longitudinal_controller_mode).param.yaml"
  />

The purpose of moving these lines above the <include file="$(find-pkg-share tier4_control_launch)/launch/control.launch.py"> is to allow the arguments to be accessible to other launch files. This enables other launch files to pass the entire parameter file path, instead of just the vehicle_id, to tier4_control_component.launch.xml. In our practice, we prefer to store the controller parameter file in a separate folder, rather than in autoware_launch/config/control/trajectory_follower/...

If exposing these arguments does not align with your requirements, simply removing the lines you suggest is the way to go.

The changes made in this pull request are intended to fix the issue. By the way, it appears that I don't have the necessary permissions to assign the pull request.

[takayuki5168]

Yes, I know.
I proposed a better way to fix it. Do you mean my proposal will not fix the issue?

[xuyuan-han]

So, exposing these arguments in the manner I did is not permitted?

[takayuki5168]

I got it.

Let me check again.
The ideal design is exposing only the lat_controller_param_path_dir. Why does this not work in your case?

[xuyuan-han]

In our project, we have our own launch package as ROS overlay above the Autoware workspace. Our launch package contains only our_launch/launch/planning_simulator.launch.xml and our_launch/launch/autoware.launch.xml. These two launch files call the low-level ROS underlay launch files in autoware_launch/launch/components /components such as tier4_control_component.launch.xml. The idea behind this is to minimize file duplication and merge workload, and to exploit the ROS underlay-overlay feature.

So if the whole lat/lon_param_path can be exposed, we can keep our own lat/lon param files in our_launch/config and let it be passed to the component launch file.

[takayuki5168]

Then, how about moving your project's lat/long parameter files in the same way as autoware_launch.
This concealing parameter's path is also applied to other components like tier4_planning_component.launch.xml, and if the specific project wants to change this method, we should create a github discussion to discuss on a larger scale.

Or you can copy tier4_control_component.launch.xml for your project and modify it.

[xuyuan-han]

OK. Currently we copy and modify tier4_control_component.launch.xml for our project.
So I will follow your solution to remove the following instead.

   <arg name="lat_controller_param_path" value="$(var lat_controller_param_path)"/>
   <arg name="lon_controller_param_path" value="$(var lon_controller_param_path)"/>

[xuyuan-han]

It is done. Could you please review the code again and approve the PR?

[takayuki5168]

Thanks for the fix!

[xuyuan-han]

Why does the task build-and-test-differential / clang-tidy-differential (pull_request) fail?

[takayuki5168]

@xuyuan-han The required build passed. Now you can merge.

[xuyuan-han]

I don't have write access permission to merge the code. Can you help with this? @takayuki5168

[takayuki5168]

Okay, I did.