refactor(multi_object_tracker): rework parameters #7333

tby-udel · 2024-06-06T16:49:47Z

Description

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

Remove the default value from the source code in order to ensure all parameter values are passed from the parameter files.
Use types that match the ones returned from declare_parameter() in order to prevent confusion and potential coding errors.
Move the default parameter file to a standardized location. Use the JSON Schema for that.
Updated readme file.

Tests performed

Package built and launched locally.

Notes for reviewers

Nothing inside the src code part was changed, only taking the parameters out.

Interface changes

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.

yukkysaito

LGTM

mitsudome-r · 2024-06-20T14:17:38Z

@tby-udel Thanks for the PR.

However, the PR seems to be failing in the CI for DCO check. When you push commits, could you push with signed-off commits?

For example by running the following command:
git rebase -i --signoff origin/main

tby-udel · 2024-06-21T20:28:56Z

Thank you for your reminding, I have pushed the signed-off version.

technolojin

LGTM
Thank you for your contribution.

codecov · 2024-06-24T00:10:01Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 28.95%. Comparing base (a8ea14e) to head (34b8a24).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7333      +/-   ##
==========================================
- Coverage   28.95%   28.95%   -0.01%     
==========================================
  Files        1606     1606              
  Lines      117597   117608      +11     
  Branches    50606    50609       +3     
==========================================
  Hits        34050    34050              
- Misses      74341    74352      +11     
  Partials     9206     9206

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

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

perception/lidar_centerpoint/schema/centerpoint_base.schemal.json

github-actions · 2024-06-28T15:15:49Z

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

You've checked our contribution guidelines.
Your PR follows our pull request guidelines.
All required CI checks pass before marking the PR ready for review.

github-actions · 2024-07-04T04:48:06Z

Documentation URL: https://autowarefoundation.github.io/autoware.universe/pr-7333/
Modified URLs:

https://autowarefoundation.github.io/autoware.universe/pr-7333/perception/multi_object_tracker/

technolojin · 2024-07-05T01:38:08Z

Let me update the branch and merge to the main, since the author @tby-udel did not responded for a long time.

technolojin · 2024-07-05T02:00:42Z

@kminoda @knzo25 can you check the 'lidar_centerpoint' part? I do not know about *_base.param.yaml configs.

tby-udel · 2024-07-06T15:21:34Z

I have pushed the signoff version, what should I do next?

technolojin · 2024-07-10T11:05:28Z

I have pushed the signoff version, what should I do next?

@tby-udel
The checker says there are mismatch between /config/centerpoint_base.param.yaml 🆚 schema/centerpoint_base.schema.json
https://github.com/autowarefoundation/autoware.universe/actions/runs/9820269981/job/27264059291?pr=7333

tby-udel · 2024-07-11T18:04:48Z

I've reverted the changes in the lidar_centerpoint section to match the current version in autoware.universe.

technolojin · 2024-07-12T00:23:18Z

@tby-udel
There are conflicts in multi_object_tracker.

tby-udel · 2024-07-14T17:58:58Z

I solved the conflicts in schema files, and I didn't find conflicts in these src/tracker/model/*.cpp files.

technolojin · 2024-07-16T02:31:44Z

@tby-udel
This package had some PRs after this PR was opened and one of them was over many files.
#7863

Let me try to update this PR and resolve the conflict.

… file. Signed-off-by: Boyang <tby@udel.edu>

technolojin · 2024-07-16T04:04:45Z

@tby-udel Merge is ready! Please go on your timing.

tby-udel · 2024-07-16T14:39:29Z

@technolojin Thank you so much for your help. I have merged branch 'main' into multi-object-tracker-branch, and it seems this PR is still waiting for some check before it can be merged. Working on OpenAD Kit is my first time experience on github, could you please tell me if this is the case? Or are there any other things that I should do on this PR?

technolojin · 2024-07-17T00:15:11Z

@tby-udel

You may not need to merge the main branch unless there is a big difference. At the moment I checked yesterday was fine (no need to merge the main). The last thing to finish the work is to press Squash and Merge button.
Now, I approved to run the tests. It need to be tested after the branch is updated. You can press Enable auto-merge (squash) now, so that the final work can be done as soon as the test is passed.

technolojin · 2024-07-17T07:25:29Z

@tby-udel
Now all the conditions are clear.

You can press Squash and merge button on the left bottom side, and then it will be merged!

tby-udel · 2024-07-17T13:57:27Z

@technolojin Thank you very much for your instructions! However, I checked everywhere on this PR and several other PR that I am also working on, I don't seem to have write access to this repository, so I was never able to merge branches into autoware.universe. I will not update main branch into current branch this time, could you please help me "squash and merge" this branch？ It's been my pleasure working with you.

technolojin · 2024-07-18T05:35:16Z

@tby-udel
I did not know that. I confirmed your PR and it is merged. Thank you for your contribution!

…#7333) * Refractored the parameters, build the schema file, updated the readme file. Signed-off-by: Boyang <tby@udel.edu> * style(pre-commit): autofix --------- Signed-off-by: Boyang <tby@udel.edu> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: Y.Hisaki <yhisaki31@gmail.com>

…#7333) * Refractored the parameters, build the schema file, updated the readme file. Signed-off-by: Boyang <tby@udel.edu> * style(pre-commit): autofix --------- Signed-off-by: Boyang <tby@udel.edu> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…#7333) * Refractored the parameters, build the schema file, updated the readme file. Signed-off-by: Boyang <tby@udel.edu> * style(pre-commit): autofix --------- Signed-off-by: Boyang <tby@udel.edu> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Signed-off-by: chtseng <chtseng@itri.org.tw>

technolojin · 2024-07-24T06:07:47Z

https://autowarefoundation.github.io/autoware.universe/main/perception/autoware_multi_object_tracker/ shows error

@tby-udel Can you find the reason of error?

tby-udel · 2024-07-24T14:10:19Z

https://autowarefoundation.github.io/autoware.universe/main/perception/autoware_multi_object_tracker/ shows error

@tby-udel Can you find the reason of error?

I think this is caused by some parameters in schema files are lack of certain default values, I will update this multi-object-tracker branch today. Can I still use push and update on this branch?

…#7333) * Refractored the parameters, build the schema file, updated the readme file. Signed-off-by: Boyang <tby@udel.edu> * style(pre-commit): autofix --------- Signed-off-by: Boyang <tby@udel.edu> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

tby-udel requested review from technolojin, YoshiRi and yukkysaito as code owners June 6, 2024 16:49

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

yukkysaito approved these changes Jun 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 Jun 20, 2024

tby-udel force-pushed the multi-object-tracker-branch branch from 76ade60 to 69f7196 Compare June 21, 2024 20:26

tby-udel requested review from knzo25 and kminoda as code owners June 21, 2024 20:26

tby-udel force-pushed the multi-object-tracker-branch branch from bb57ea5 to 69f7196 Compare June 21, 2024 20:48

technolojin approved these changes Jun 24, 2024

View reviewed changes

mitsudome-r reviewed Jun 27, 2024

View reviewed changes

perception/lidar_centerpoint/schema/centerpoint_base.schemal.json Outdated Show resolved Hide resolved

mitsudome-r added the run:deploy-docs Mark for deploy-docs action generation. (used-by-ci) label Jul 4, 2024

technolojin force-pushed the multi-object-tracker-branch branch from 31e9da5 to d2514d8 Compare July 5, 2024 01:35

technolojin enabled auto-merge (squash) July 5, 2024 01:36

technolojin disabled auto-merge July 5, 2024 02:02

technolojin self-assigned this Jul 5, 2024

Refractored the parameters, build the schema file, updated the readme…

ae7107b

… file. Signed-off-by: Boyang <tby@udel.edu>

technolojin force-pushed the multi-object-tracker-branch branch from 5864214 to ae7107b Compare July 16, 2024 03:28

style(pre-commit): autofix

e49db87

tby-udel added 2 commits July 16, 2024 09:56

Merge branch 'main' into multi-object-tracker-branch

f954b09

Merge branch 'main' into multi-object-tracker-branch

34b8a24

tby-udel requested a review from mitsudome-r July 16, 2024 14:36

technolojin merged commit d5c95ae into autowarefoundation:main Jul 18, 2024
28 of 30 checks passed

technolojin mentioned this pull request Jul 19, 2024

refactor(multi_object_tracker): bring parameter schema to new package folder #8105

Merged

tby-udel mentioned this pull request Jul 24, 2024

docs(autoware_multi_object_tracker): add default values on the schema json #8179

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(multi_object_tracker): rework parameters #7333

refactor(multi_object_tracker): rework parameters #7333

tby-udel commented Jun 6, 2024

yukkysaito left a comment

mitsudome-r commented Jun 20, 2024

tby-udel commented Jun 21, 2024

technolojin left a comment

codecov bot commented Jun 24, 2024 •

edited

Loading

github-actions bot commented Jun 28, 2024 •

edited

Loading

github-actions bot commented Jul 4, 2024

technolojin commented Jul 5, 2024

technolojin commented Jul 5, 2024

tby-udel commented Jul 6, 2024

technolojin commented Jul 10, 2024 •

edited

Loading

tby-udel commented Jul 11, 2024

technolojin commented Jul 12, 2024

tby-udel commented Jul 14, 2024

technolojin commented Jul 16, 2024 •

edited

Loading

technolojin commented Jul 16, 2024

tby-udel commented Jul 16, 2024

technolojin commented Jul 17, 2024

technolojin commented Jul 17, 2024

tby-udel commented Jul 17, 2024

technolojin commented Jul 18, 2024

technolojin commented Jul 24, 2024

tby-udel commented Jul 24, 2024

refactor(multi_object_tracker): rework parameters #7333

refactor(multi_object_tracker): rework parameters #7333

Conversation

tby-udel commented Jun 6, 2024

Description

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

yukkysaito left a comment

Choose a reason for hiding this comment

mitsudome-r commented Jun 20, 2024

tby-udel commented Jun 21, 2024

technolojin left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 24, 2024 • edited Loading

Codecov Report

github-actions bot commented Jun 28, 2024 • edited Loading

github-actions bot commented Jul 4, 2024

technolojin commented Jul 5, 2024

technolojin commented Jul 5, 2024

tby-udel commented Jul 6, 2024

technolojin commented Jul 10, 2024 • edited Loading

tby-udel commented Jul 11, 2024

technolojin commented Jul 12, 2024

tby-udel commented Jul 14, 2024

technolojin commented Jul 16, 2024 • edited Loading

technolojin commented Jul 16, 2024

tby-udel commented Jul 16, 2024

technolojin commented Jul 17, 2024

technolojin commented Jul 17, 2024

tby-udel commented Jul 17, 2024

technolojin commented Jul 18, 2024

technolojin commented Jul 24, 2024

tby-udel commented Jul 24, 2024

codecov bot commented Jun 24, 2024 •

edited

Loading

github-actions bot commented Jun 28, 2024 •

edited

Loading

technolojin commented Jul 10, 2024 •

edited

Loading

technolojin commented Jul 16, 2024 •

edited

Loading