fix(crosswalk_traffic_light_estimator): move crosswalk after fusion #4734

tzhong518 · 2023-08-24T07:20:29Z

Description

`🤖 Generated by Copilot at b5458c5`

This pull request refactors the traffic light recognition and estimation modules to use the new TrafficSignal messages and improve modularity. It simplifies the traffic_light_node_container.launch.py file, updates the topic names and launch arguments in the traffic_light.launch.xml file, and adds a crosswalk traffic signal estimation feature. It also updates the crosswalk_traffic_light_estimator node and package to use the autoware_perception_msgs package and the traffic_signal_id field.

This pull request improves the traffic light recognition system by simplifying the launch files, updating the topic names and arguments, and replacing the deprecated tier4_perception_msgs package with the autoware_perception_msgs package. It also adds a new optional node for crosswalk traffic light estimation.

Tests performed

https://tier4.atlassian.net/wiki/spaces/~459732634/pages/2865332600/traffic+light+bug+fix#Result

Notes for reviewers

Interface changes

Effects on system behavior

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.

Signed-off-by: tzhong518 <sworgun@gmail.com>

codecov · 2023-08-24T07:35:47Z

Codecov Report

Patch coverage has no change and project coverage change: +0.06% 🎉

Comparison is base (4ed36d2) 15.11% compared to head (b5458c5) 15.17%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4734      +/-   ##
==========================================
+ Coverage   15.11%   15.17%   +0.06%     
==========================================
  Files        1568     1563       -5     
  Lines      108137   107327     -810     
  Branches    33188    32698     -490     
==========================================
- Hits        16344    16291      -53     
+ Misses      73991    73561     -430     
+ Partials    17802    17475     -327

Flag	Coverage Δ		*Carryforward flag
differential	`0.00% <0.00%> (?)`
total	`15.18% <ø> (+0.06%)`	⬆️	Carriedforward from 4586ced

*This pull request uses carry forward flags. Click here to find out more.

Files Changed	Coverage Δ
...ion/crosswalk_traffic_light_estimator/src/node.cpp	`0.00% <0.00%> (ø)`
...ath_planner/src/scene_module/avoidance/manager.cpp	`10.06% <ø> (+3.53%)`	⬆️
...h_planner/src/scene_module/lane_change/manager.cpp	`10.63% <ø> (+4.00%)`	⬆️
...ocity_virtual_traffic_light_module/src/manager.cpp	`0.00% <ø> (ø)`
...g/behavior_velocity_walkway_module/src/manager.cpp	`24.39% <ø> (+1.13%)`	⬆️

... and 53 files with indirect coverage changes

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

miursh

LGTM
jsut one minor comment

launch/tier4_perception_launch/launch/traffic_light_recognition/traffic_light.launch.xml

…n/traffic_light.launch.xml Co-authored-by: Shunsuke Miura <37187849+miursh@users.noreply.github.com>

perception/crosswalk_traffic_light_estimator/src/node.cpp

Signed-off-by: Shunsuke Miura <shunsuke.miura@tier4.jp>

miursh · 2023-08-28T23:17:49Z

Tested with internal odaiba data.

TomohitoAndo · 2023-08-29T01:55:17Z

@miursh
Is it expected that there are duplicated results?

miursh · 2023-08-29T02:11:41Z

@TomohitoAndo Should they be merged? I'm not sure about the planning desired behavior.
This occurs because of the another crosswalk lanelet (or maybe crosswalk traffic light) is connected the the same regulatory element id.

TomohitoAndo · 2023-08-29T03:04:10Z

@miursh
Planning module maybe works well even if there are duplicated data.
But I think duplicated data in the message is confusing and might cause unexpected behavior in the future, so it would be better to be merged. (I think it is okay to fix this in another PR)
What do you think? 🙏

miursh · 2023-08-29T04:34:15Z

@TomohitoAndo Thanks! As we talked, could you make a separate pull request to fix this?

miursh · 2023-08-29T04:35:48Z

@satoshi-ota Could you check this PR since you are the only codeowner.

…utowarefoundation#4734) * fix: move crosswalk after fusion Signed-off-by: tzhong518 <sworgun@gmail.com> * Update launch/tier4_perception_launch/launch/traffic_light_recognition/traffic_light.launch.xml Co-authored-by: Shunsuke Miura <37187849+miursh@users.noreply.github.com> * Rename TrafficLight to TrafficSignal Signed-off-by: Shunsuke Miura <shunsuke.miura@tier4.jp> * change input to be considered as the regulatory-element Signed-off-by: Shunsuke Miura <shunsuke.miura@tier4.jp> --------- Signed-off-by: tzhong518 <sworgun@gmail.com> Signed-off-by: Shunsuke Miura <shunsuke.miura@tier4.jp> Co-authored-by: Shunsuke Miura <37187849+miursh@users.noreply.github.com> Co-authored-by: Shunsuke Miura <shunsuke.miura@tier4.jp>

fix: move crosswalk after fusion

071edae

Signed-off-by: tzhong518 <sworgun@gmail.com>

tzhong518 requested review from satoshi-ota, miursh, YoshiRi and yukkysaito as code owners August 24, 2023 07:20

github-actions bot added component:perception Advanced sensor data processing and environment understanding. (auto-assigned) component:launch Launch files, scripts and initialization tools. (auto-assigned) labels Aug 24, 2023

yukkysaito added run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) openai-pr-reviewer labels Aug 24, 2023

miursh approved these changes Aug 26, 2023

View reviewed changes

launch/tier4_perception_launch/launch/traffic_light_recognition/traffic_light.launch.xml Outdated Show resolved Hide resolved

miursh added run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) and removed run:build-and-test-differential Mark to enable build-and-test-differential workflow. (used-by-ci) labels Aug 26, 2023

Update launch/tier4_perception_launch/launch/traffic_light_recognitio…

4586ced

…n/traffic_light.launch.xml Co-authored-by: Shunsuke Miura <37187849+miursh@users.noreply.github.com>

TomohitoAndo reviewed Aug 28, 2023

View reviewed changes

perception/crosswalk_traffic_light_estimator/src/node.cpp Outdated Show resolved Hide resolved

miursh added 3 commits August 29, 2023 07:51

Merge branch 'main' into feature/change_crosswalk_position

563ff8d

Rename TrafficLight to TrafficSignal

cfa78de

Signed-off-by: Shunsuke Miura <shunsuke.miura@tier4.jp>

change input to be considered as the regulatory-element

b5458c5

Signed-off-by: Shunsuke Miura <shunsuke.miura@tier4.jp>

miursh changed the title ~~fix(change_crosswalk_position): move crosswalk after fusion~~ fix(crosswalk_traffic_light_estimator): move crosswalk after fusion Aug 28, 2023

miursh mentioned this pull request Aug 29, 2023

Change traffic signal interface to support V2X/V2I #2567

Closed

8 tasks

TomohitoAndo self-requested a review August 29, 2023 05:21

TomohitoAndo approved these changes Aug 29, 2023

View reviewed changes

satoshi-ota approved these changes Aug 30, 2023

View reviewed changes

miursh merged commit 56c4585 into autowarefoundation:main Aug 30, 2023

tier4-autoware-public-bot bot mentioned this pull request Aug 30, 2023

chore: sync upstream tier4/autoware.universe#779

Merged

ktro2828 mentioned this pull request Nov 22, 2023

fix(crosswalk_traffic_light_estimator): add operation to remove traffic signals with duplicated ids #5653

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(crosswalk_traffic_light_estimator): move crosswalk after fusion #4734

fix(crosswalk_traffic_light_estimator): move crosswalk after fusion #4734

tzhong518 commented Aug 24, 2023 •

edited by miursh

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading

miursh left a comment •

edited

Loading

miursh commented Aug 28, 2023 •

edited

Loading

TomohitoAndo commented Aug 29, 2023

miursh commented Aug 29, 2023 •

edited

Loading

TomohitoAndo commented Aug 29, 2023

miursh commented Aug 29, 2023

miursh commented Aug 29, 2023

fix(crosswalk_traffic_light_estimator): move crosswalk after fusion #4734

fix(crosswalk_traffic_light_estimator): move crosswalk after fusion #4734

Conversation

tzhong518 commented Aug 24, 2023 • edited by miursh Loading

Description

🤖 Generated by Copilot at b5458c5

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

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

miursh left a comment • edited Loading

Choose a reason for hiding this comment

miursh commented Aug 28, 2023 • edited Loading

TomohitoAndo commented Aug 29, 2023

miursh commented Aug 29, 2023 • edited Loading

TomohitoAndo commented Aug 29, 2023

miursh commented Aug 29, 2023

miursh commented Aug 29, 2023

tzhong518 commented Aug 24, 2023 •

edited by miursh

Loading

`🤖 Generated by Copilot at b5458c5`

codecov bot commented Aug 24, 2023 •

edited

Loading

miursh left a comment •

edited

Loading

miursh commented Aug 28, 2023 •

edited

Loading

miursh commented Aug 29, 2023 •

edited

Loading