refactor(costmap_generator): rework parameters #4695

yuntianyi-chen · 2023-08-22T04:39:22Z

Description

Refactor the ROS Node configuration layout for the costmap_generator package according to the guideline.

Create the schema and parameter files.
Remove the default values from the source code to avoid duplication.
Update the README.md file as to how to launch the node.

Tests performed

The refactored node was successfully built and tested by launching this node individually.

The running result is shown below:

tier4 internal scenarios
https://evaluation.tier4.jp/evaluation/reports/9477bdda-3c75-5953-9554-e9191df8f9d7?project_id=prd_jt
1680/1683

Notes for reviewers

It seems like the planning/costmap_generator/config/costmap_generator.param.yaml file cannot be automatically copied to the autoware/install/costmap_generator/share/config directory after building the package. Since we use find-pkg-share in the costmap_generator.launch.xml file to link the costmap_generator.param.yaml file, I have to manually copy the .param.yaml file to the share/config directory to make it work. I think we need to modify the CMakeLists.txt file to implement the copying, which I discussed here.

Interface changes

Create two configuration files planning/costmap_generator/config/costmap_generator.param.yaml and planning/costmap_generator/schema/costmap_generator.schema.json
Parameter values are now required to be passed to the node when launched.

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

…ing to the new ROS node config guideline Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

…m:yuntianyi-chen/autoware.universe into 3239-refactor-ros-cofig-costmap_generator

…en building the package. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

ambroise-arm · 2023-08-29T08:57:02Z

planning/costmap_generator/config/costmap_generator.param.yaml

+    vehicle_frame: "base_link"
+    map_frame: "map"
+    update_rate: 10.0
+    activate_by_scenario: true


I was not finding activate_by_scenario in the parameters removed from the launch file, so I was wondering how a declare_parameter on this option could work without a default value. I suppose this launch file was getting used exclusively through the parking pipeline at

autoware.universe/launch/tier4_planning_launch/launch/scenario_planning/parking.launch.py

Line 65 in 66c6e15

"activate_by_scenario": False,

.

I expect that way of setting the parameters won't have any effect anymore, right? In which case the parking.launch.py file should probably also be updated as part of this PR.

Additionally, because this was the only place where activate_by_scenario was set, should false be kept as the default?

According to the description ("if true, activate by scenario = parking. Otherwise, activate if vehicle is inside parking lot."), I think the default value should be set by the current scenario the vehicle is in.

I set it to True according to the default values of the previous .cpp file, which was already deleted.

Ah, I see. Makes sense.

What about updating autoware.universe/launch/tier4_planning_launch/launch/scenario_planning/parking.launch.py?

Yes, we need to update the autoware.universe/launch/tier4_planning_launch/launch/scenario_planning/parking.launch.py as well.

I've modified that .py file. Shall we still keep the activate_by_scenario = true in the launch.xml/schema.json/param.yaml of the planning/costmap_generator directory?

The default in the schema is an annotation. It will be used eventually to render the documentation from it, but it doesn't enforce anything in software. So it doesn't matter so much, true is fine.

Actually, for the modification in parking.launch.py I had in mind of replacing the list of parameters with a reference to the config file of this package instead, in the same way you do for this package's launch file. Not just replacing the value of activate_by_scenario.

On second thought, I don't know if the tier4 package will want to use the defaults from another package that it doesn't control. Not sure what the best solution is.

@satoshi-ota I think it relates to the point you raise at #4699 (comment). Except here it (probably) doesn't break the launch of anything if we leave it as it currently is.

I think we'd better keep only one configuration file that controls the node. Having the same set of parameters in two different places would cause some biases or inconsistencies.

The configuration file in the package's source directory is just a default one, it doesn't have to be used by other packages. They can define their own. Which is what happens with the tier4 package here (except the values are in their launch file, not in a dedicated file). So before changing it, I'd like to know what tier4 code owners think of it.

currently the default value of activate_by_scenario was removed and set only in laucher side autowarefoundation/autoware_launch#679.

And we need to set it false for pull over of lane driving scenario.

#2879

so could you please set false?

planning/costmap_generator/schema/costmap_generator.schema.json

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

launch/tier4_planning_launch/launch/scenario_planning/parking.launch.py

mitsudome-r · 2023-09-21T03:29:33Z

planning/costmap_generator/launch/costmap_generator.launch.xml

@@ -1,3 +1,15 @@
+<!--
+  Copyright 2021-2023 UCI SORA Lab, the Autoware Foundation


Thanks for adding the copyright notice to the launch file.
However, since some parts of the code comes from different author (prehaps TIER IV), we might want to have the original author mentioned as well in the notice.

For the policy on copyright notice, let's discuss in today's Open AD Kit meeting.

Conclusion from the WG discussion is posted in this issue
#2652

The copyright notice has been modified.

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

stale · 2023-11-21T07:35:58Z

This pull request has been automatically marked as stale because it has not had recent activity.

….universe into refactor-ros-config-costmap_generator Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

satoshi-ota · 2023-11-29T00:39:34Z

@yuntianyi-chen parking.launch.py was removed. So please rebase or update your branch? Thank you for your cooperation.

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

yuntianyi-chen · 2023-11-29T17:18:01Z

@yuntianyi-chen parking.launch.py was removed. So please rebase or update your branch? Thank you for your cooperation.

Thanks for noticing! I just removed this file.

kosuke55 · 2023-11-30T03:14:48Z

planning/costmap_generator/config/costmap_generator.param.yaml

+    vehicle_frame: "base_link"
+    map_frame: "map"
+    update_rate: 10.0
+    activate_by_scenario: true


currently the default value of activate_by_scenario was removed and set only in laucher side autowarefoundation/autoware_launch#679.

And we need to set it false for pull over of lane driving scenario.

#2879

so could you please set false?

planning/costmap_generator/schema/costmap_generator.schema.json

planning/costmap_generator/config/costmap_generator.param.yaml

Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>

kosuke55

thanks! LGTM!

codecov · 2023-12-02T04:15:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (765a596) 15.32% compared to head (e3438fc) 31.57%.
Report is 134 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4695       +/-   ##
===========================================
+ Coverage   15.32%   31.57%   +16.25%     
===========================================
  Files        1721        6     -1715     
  Lines      118559      532   -118027     
  Branches    37995      137    -37858     
===========================================
- Hits        18169      168    -18001     
+ Misses      79657      285    -79372     
+ Partials    20733       79    -20654

Flag	Coverage Δ
differential	`31.57% <ø> (?)`
total	`?`

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* refactor the configuration files of the node costmap_generator according to the new ROS node config guideline Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * style(pre-commit): autofix * update the organization name in the copyright license Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Modify the CMakeLists.txt file to enalbe /config directory sharing when building the package. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Update the schema file according to the review. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Update a parameter in the parking.launch.py file. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * revert copyright info Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * style(pre-commit): autofix * delete parking.launch.py file Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Update planning/costmap_generator/config/costmap_generator.param.yaml Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com> * Update planning/costmap_generator/schema/costmap_generator.schema.json Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com> --------- Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>

* refactor the configuration files of the node costmap_generator according to the new ROS node config guideline Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * style(pre-commit): autofix * update the organization name in the copyright license Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Modify the CMakeLists.txt file to enalbe /config directory sharing when building the package. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Update the schema file according to the review. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Update a parameter in the parking.launch.py file. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * revert copyright info Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * style(pre-commit): autofix * delete parking.launch.py file Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> * Update planning/costmap_generator/config/costmap_generator.param.yaml Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com> * Update planning/costmap_generator/schema/costmap_generator.schema.json Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com> --------- Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com> Signed-off-by: karishma <karishma@interpl.ai>

yuntianyi-chen and others added 5 commits August 21, 2023 01:24

refactor the configuration files of the node costmap_generator accord…

2f7b8f5

…ing to the new ROS node config guideline Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

Merge branch 'main' into 3239-refactor-ros-cofig-costmap_generator

5673798

style(pre-commit): autofix

f69f35d

update the organization name in the copyright license

cca645f

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

Merge branch '3239-refactor-ros-cofig-costmap_generator' of github.co…

7e31574

…m:yuntianyi-chen/autoware.universe into 3239-refactor-ros-cofig-costmap_generator

yuntianyi-chen requested review from kosuke55, TakaHoribe, takayuki5168 and purewater0901 as code owners August 22, 2023 04:39

github-actions bot added type:documentation Creating or refining documentation. (auto-assigned) component:planning Route planning, decision-making, and navigation. (auto-assigned) labels Aug 22, 2023

kaspermeck-arm requested a review from ambroise-arm August 22, 2023 22:50

Modify the CMakeLists.txt file to enalbe /config directory sharing wh…

12d7cbb

…en building the package. Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

ambroise-arm reviewed Aug 29, 2023

View reviewed changes

yuntianyi-chen added 2 commits August 29, 2023 10:53

Update the schema file according to the review.

ed00f54

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

Update a parameter in the parking.launch.py file.

9ec86bd

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

yuntianyi-chen requested review from rej55, kyoichi-sugahara, mkuri, soblin, satoshi-ota, shmpwk, taikitanaka3, TomohitoAndo, tkimura4 and zulfaqar-azmi-t4 as code owners August 30, 2023 17:28

github-actions bot added the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Aug 30, 2023

satoshi-ota reviewed Sep 21, 2023

View reviewed changes

launch/tier4_planning_launch/launch/scenario_planning/parking.launch.py Outdated Show resolved Hide resolved

mitsudome-r reviewed Sep 21, 2023

View reviewed changes

revert copyright info

d9d745a

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

This was referenced Oct 16, 2023

refactor(radar_tracks_msgs_converter): rework parameters #4911

Closed

refactor(front_vehicle_velocity_estimator): rework parameters #5241

Closed

chore(radar_static_pointcloud_filter): fix copyright #5316

Merged

stale bot added the status:stale Inactive or outdated issues. (auto-assigned) label Nov 21, 2023

Merge branch 'main' of https://github.com/autowarefoundation/autoware…

8fa2e27

….universe into refactor-ros-config-costmap_generator Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

stale bot removed the status:stale Inactive or outdated issues. (auto-assigned) label Nov 24, 2023

style(pre-commit): autofix

b9fc50b

delete parking.launch.py file

c8c3929

Signed-off-by: yuntianyi-chen <yuntianyichen@outlook.com>

github-actions bot removed the component:launch Launch files, scripts and initialization tools. (auto-assigned) label Nov 29, 2023

Merge branch 'main' into refactor-ros-config-costmap_generator

1a4e5ce

kosuke55 reviewed Nov 30, 2023

View reviewed changes

planning/costmap_generator/config/costmap_generator.param.yaml Show resolved Hide resolved

yuntianyi-chen and others added 3 commits November 29, 2023 23:17

Update planning/costmap_generator/config/costmap_generator.param.yaml

5e509f1

Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>

Update planning/costmap_generator/schema/costmap_generator.schema.json

707cba1

Co-authored-by: Kosuke Takeuchi <kosuke.tnp@gmail.com>

Merge branch 'main' into refactor-ros-config-costmap_generator

e3438fc

kosuke55 approved these changes Dec 1, 2023

View reviewed changes

satoshi-ota added the run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) label Dec 2, 2023

satoshi-ota enabled auto-merge (squash) December 2, 2023 04:07

satoshi-ota merged commit c9ceaab into autowarefoundation:main Dec 2, 2023

tier4-autoware-public-bot bot mentioned this pull request Dec 6, 2023

chore: sync upstream tier4/autoware.universe#1059

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(costmap_generator): rework parameters #4695

refactor(costmap_generator): rework parameters #4695

yuntianyi-chen commented Aug 22, 2023 •

edited by kosuke55

Loading

ambroise-arm Aug 29, 2023

yuntianyi-chen Aug 29, 2023

ambroise-arm Aug 30, 2023

yuntianyi-chen Aug 30, 2023

ambroise-arm Aug 31, 2023

ambroise-arm Aug 31, 2023 •

edited

Loading

yuntianyi-chen Sep 1, 2023

ambroise-arm Sep 4, 2023

kosuke55 Nov 30, 2023 •

edited

Loading

mitsudome-r Sep 21, 2023

mitsudome-r Sep 21, 2023

mitsudome-r Sep 21, 2023

yuntianyi-chen Sep 22, 2023

stale bot commented Nov 21, 2023

satoshi-ota commented Nov 29, 2023

yuntianyi-chen commented Nov 29, 2023

kosuke55 Nov 30, 2023 •

edited

Loading

kosuke55 left a comment

codecov bot commented Dec 2, 2023 •

edited

Loading

		@@ -1,3 +1,15 @@
		<!--
		Copyright 2021-2023 UCI SORA Lab, the Autoware Foundation

refactor(costmap_generator): rework parameters #4695

refactor(costmap_generator): rework parameters #4695

Conversation

yuntianyi-chen commented Aug 22, 2023 • edited by kosuke55 Loading

Description

Related links

Tests performed

Notes for reviewers

Interface changes

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ambroise-arm Aug 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kosuke55 Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Nov 21, 2023

satoshi-ota commented Nov 29, 2023

yuntianyi-chen commented Nov 29, 2023

kosuke55 Nov 30, 2023 • edited Loading

Choose a reason for hiding this comment

kosuke55 left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 2, 2023 • edited Loading

Codecov Report

yuntianyi-chen commented Aug 22, 2023 •

edited by kosuke55

Loading

ambroise-arm Aug 31, 2023 •

edited

Loading

kosuke55 Nov 30, 2023 •

edited

Loading

kosuke55 Nov 30, 2023 •

edited

Loading

codecov bot commented Dec 2, 2023 •

edited

Loading