-
Notifications
You must be signed in to change notification settings - Fork 640
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(crosswalk_traffic_light_estimator): rework parameters #4699
refactor(crosswalk_traffic_light_estimator): rework parameters #4699
Conversation
…estimator according to the new ROS node config guideline. update the parameter information in the README.md Signed-off-by: yuntianyi-chen <[email protected]>
Signed-off-by: yuntianyi-chen <[email protected]>
…of github.com:yuntianyi-chen/autoware.universe into refactor-ros-config-crosswalk_traffic_light_estimator
Signed-off-by: yuntianyi-chen <[email protected]>
…en building the package. Signed-off-by: yuntianyi-chen <[email protected]>
...ption/crosswalk_traffic_light_estimator/schema/crosswalk_traffic_light_estimator.schema.json
Show resolved
Hide resolved
Signed-off-by: yuntianyi-chen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ambroise-arm LGTM. Thanks 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. Sorry. I think it needs to add parameter config path in launcher file since default parameters are removed.
I think it's already added in the launch.xml file. |
Sorry I missed it. LGTM. |
hmm... It seems that this node called in tier4_perception_launch.launch.xml directly. (not via the one of this package.) Lines 140 to 147 in b09f309
|
@yuntianyi-chen |
@miursh Sure! Please do it. |
…nch.xml Signed-off-by: Shunsuke Miura <[email protected]>
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #4699 +/- ##
==========================================
- Coverage 15.91% 15.91% -0.01%
==========================================
Files 1580 1580
Lines 109068 109016 -52
Branches 33634 33616 -18
==========================================
- Hits 17363 17347 -16
+ Misses 73079 73051 -28
+ Partials 18626 18618 -8
*This pull request uses carry forward flags. Click here to find out more.
☔ View full report in Codecov by Sentry. |
@yuntianyi-chen Done. 5cb0f3f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Refactor the ROS Node configuration layout for the crosswalk_traffic_light_estimator package according to the guideline.
last_detect_color_hold_time
in the README.md file.Related links
None
Tests performed
The refactored node was successfully built and tested by launching this node individually.
The running result is shown below:
Notes for reviewers
None
Interface changes
perception/crosswalk_traffic_light_estimator/config/crosswalk_traffic_light_estimator.param.yaml
andperception/crosswalk_traffic_light_estimator/schema/crosswalk_traffic_light_estimator.schema.json
Effects on system behavior
None
Pre-review checklist for the PR author
The PR author must check the checkboxes below when creating the PR.
In-review checklist for the PR reviewers
The PR reviewers must check the checkboxes below before approval.
Post-review checklist for the PR author
The PR author must check the checkboxes below before merging.
After all checkboxes are checked, anyone who has write access can merge the PR.