refactor(radar_tracks_msgs_converter): rework parameters #6533

kaspermeck-arm · 2024-03-02T00:23:59Z

Description

Implement the ROS Node configuration layout described in https://github.com/orgs/autowarefoundation/discussions/3371 for the radar_tracks_msgs_converter package.

Remove the default value from the source code in order to ensure all parameter values are passed from the parameter files.
Create the schema

Original PR

refactor(radar_tracks_msgs_converter): rework parameters #4911

Tests performed

Package built and launch locally.
colcon build --symlink-install --cmake-args -DCMAKE_BUILD_TYPE=Release --packages-up-to radar_tracks_msgs_converter

Effects on system behavior

More reliable and faster parameter configuration file creation.

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.

Rework parameters Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: If28e9c97bc6db3b522c680f215b3f8cb95ec58b7

…impler Signed-off-by: Ryohsuke Mitsudome <[email protected]>

fix: restore parameter description and make json schema description simpler

scepter914 · 2024-03-04T02:49:44Z

perception/radar_tracks_msgs_converter/launch/radar_tracks_msgs_converter.launch.xml

@@ -3,13 +3,13 @@
  <arg name="input/odometry" default="/localization/kinematic_state"/>
  <arg name="output/radar_detected_objects" default="output/radar_detected_objects"/>
  <arg name="output/radar_tracked_objects" default="output/radar_tracked_objects"/>
-  <arg name="param_path" default="$(find-pkg-share radar_tracks_msgs_converter)/config/radar_tracks_msgs_converter.param.yaml"/>
+  <arg name="config_file" default="$(find-pkg-share radar_tracks_msgs_converter)/config/radar_tracks_msgs_converter.param.yaml"/>


[MUST]
This unnecessary interface changes affect many Autoware projects.
Please revert this change.

@kaspermeck-arm
Could you provide the reasons for this modification?

Like @scepter914 says, this would affect users who include this radar_tracks_msgs_converter.launch.xml to their own project.
Since we don't want to disturb developers, let's keep this as and focus on node configuration, not launch configuration for now. If we need to unify the launch arguments as well, I think we should discuss this under #3386 first.

@mitsudome-r

To align with the original PR

https://github.com/autowarefoundation/autoware.universe/pull/4911/files#diff-9328a941a658e3bda840cef66db3784d382a5e82e94560b6b31f6aef57091e81

and because in

c53b81f

it wasn't addressed.

I'll update the launch file and we can discuss naming convention when we start the ROS Launch dojo.

scepter914 · 2024-03-04T02:49:54Z

perception/radar_tracks_msgs_converter/launch/radar_tracks_msgs_converter.launch.xml


  <node pkg="radar_tracks_msgs_converter" exec="radar_tracks_msgs_converter_node" name="radar_tracks_msgs_converter" output="screen">
    <remap from="~/input/radar_objects" to="$(var input/radar_objects)"/>
    <remap from="~/input/odometry" to="$(var input/odometry)"/>
    <remap from="~/output/radar_detected_objects" to="$(var output/radar_detected_objects)"/>
    <remap from="~/output/radar_tracked_objects" to="$(var output/radar_tracked_objects)"/>
-    <param from="$(var param_path)"/>
+    <param from="$(var config_file)"/>


[MUST]
This unnecessary interface changes affect many Autoware projects.
Please revert this change.

Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: Ic6b6fd98ba2c0e2a5510ff08cf563877d1205bb9

codecov · 2024-03-07T13:13:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 14.79%. Comparing base (c09c176) to head (78671ea).
Report is 28 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6533   +/-   ##
=======================================
  Coverage   14.79%   14.79%           
=======================================
  Files        1917     1917           
  Lines      131984   131984           
  Branches    39228    39228           
=======================================
  Hits        19524    19524           
  Misses      90671    90671           
  Partials    21789    21789

Flag	Coverage Δ		*Carryforward flag
differential	`0.00% <ø> (?)`
total	`14.79% <ø> (ø)`		Carriedforward from c09c176

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

* refactor(radar_tracks_msgs_converter) Rework parameters Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: If28e9c97bc6db3b522c680f215b3f8cb95ec58b7 * fix: restore parameter description and make json schema description simpler Signed-off-by: Ryohsuke Mitsudome <[email protected]> * style(pre-commit): autofix * Updated param file path Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: Ic6b6fd98ba2c0e2a5510ff08cf563877d1205bb9 --------- Signed-off-by: kaspermeck-arm <[email protected]> Signed-off-by: Ryohsuke Mitsudome <[email protected]> Co-authored-by: Ryohsuke Mitsudome <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: Kotaro Yoshimoto <[email protected]>

…ndation#6533) * refactor(radar_tracks_msgs_converter) Rework parameters Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: If28e9c97bc6db3b522c680f215b3f8cb95ec58b7 * fix: restore parameter description and make json schema description simpler Signed-off-by: Ryohsuke Mitsudome <[email protected]> * style(pre-commit): autofix * Updated param file path Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: Ic6b6fd98ba2c0e2a5510ff08cf563877d1205bb9 --------- Signed-off-by: kaspermeck-arm <[email protected]> Signed-off-by: Ryohsuke Mitsudome <[email protected]> Co-authored-by: Ryohsuke Mitsudome <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: kaigohirao <[email protected]>

…ndation#6533) * refactor(radar_tracks_msgs_converter) Rework parameters Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: If28e9c97bc6db3b522c680f215b3f8cb95ec58b7 * fix: restore parameter description and make json schema description simpler Signed-off-by: Ryohsuke Mitsudome <[email protected]> * style(pre-commit): autofix * Updated param file path Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: Ic6b6fd98ba2c0e2a5510ff08cf563877d1205bb9 --------- Signed-off-by: kaspermeck-arm <[email protected]> Signed-off-by: Ryohsuke Mitsudome <[email protected]> Co-authored-by: Ryohsuke Mitsudome <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

kaspermeck-arm and others added 3 commits February 29, 2024 19:24

refactor(radar_tracks_msgs_converter)

5d2ac41

Rework parameters Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: If28e9c97bc6db3b522c680f215b3f8cb95ec58b7

fix: restore parameter description and make json schema description s…

8203640

…impler Signed-off-by: Ryohsuke Mitsudome <[email protected]>

Merge pull request #1 from mitsudome-r/fix/radar_tracks_msgs_converter

c53b81f

fix: restore parameter description and make json schema description simpler

kaspermeck-arm added the DevOps Dojo: ROS Node Conf Related to Open AD Kit WG label Mar 2, 2024

kaspermeck-arm requested review from mitsudome-r and ambroise-arm March 2, 2024 00:23

kaspermeck-arm self-assigned this Mar 2, 2024

kaspermeck-arm requested review from scepter914, miursh, technolojin and YoshiRi as code owners March 2, 2024 00:24

github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:perception Advanced sensor data processing and environment understanding. (auto-assigned) labels Mar 2, 2024

style(pre-commit): autofix

dc26b46

scepter914 requested changes Mar 4, 2024

View reviewed changes

Updated param file path

78671ea

Signed-off-by: kaspermeck-arm <[email protected]> Change-Id: Ic6b6fd98ba2c0e2a5510ff08cf563877d1205bb9

scepter914 approved these changes Mar 7, 2024

View reviewed changes

mitsudome-r added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Mar 7, 2024

mitsudome-r merged commit baa923e into autowarefoundation:main Mar 7, 2024
33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(radar_tracks_msgs_converter): rework parameters #6533

refactor(radar_tracks_msgs_converter): rework parameters #6533

kaspermeck-arm commented Mar 2, 2024

scepter914 Mar 4, 2024

mitsudome-r Mar 4, 2024 •

edited

Loading

kaspermeck-arm Mar 4, 2024

scepter914 Mar 4, 2024

codecov bot commented Mar 7, 2024

refactor(radar_tracks_msgs_converter): rework parameters #6533

refactor(radar_tracks_msgs_converter): rework parameters #6533

Conversation

kaspermeck-arm commented Mar 2, 2024

Description

Tests performed

Effects on system behavior

Pre-review checklist for the PR author

In-review checklist for the PR reviewers

Post-review checklist for the PR author

scepter914 Mar 4, 2024

Choose a reason for hiding this comment

mitsudome-r Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

kaspermeck-arm Mar 4, 2024

Choose a reason for hiding this comment

scepter914 Mar 4, 2024

Choose a reason for hiding this comment

codecov bot commented Mar 7, 2024

Codecov Report

mitsudome-r Mar 4, 2024 •

edited

Loading