-
Notifications
You must be signed in to change notification settings - Fork 682
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
feat(traffic_light_category_merger): add new traffic_light_category_merger package #9748
feat(traffic_light_category_merger): add new traffic_light_category_merger package #9748
Conversation
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
6eb9e76
to
c3240d9
Compare
This reverts commit 9999d28.
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9748 +/- ##
==========================================
- Coverage 28.37% 28.28% -0.09%
==========================================
Files 1485 1485
Lines 111087 111078 -9
Branches 43148 43112 -36
==========================================
- Hits 31521 31423 -98
+ Misses 76539 76519 -20
- Partials 3027 3136 +109
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Thank you for PR.
I have the discussion item below.
autoware_traffic_light_category_merger
is better thanautoware_traffic_light_signal_merger
. Because the wordsignal
have disappear in traffic light pipeline (ref refactor(tier4_perception_msgs): rename traffic_signal to traffic_light #6375, refactor(tier4_perception_msgs): rename traffic_signal to traffic_light tier4/tier4_autoware_msgs#112). So that name is good to avoid to confuse it.input/expect_rois
andtf
are not needed, because they are not used in this package.
| -------------------------- | ---------------------------------------------- | ----------------------------- | | ||
| `input/car_signals` | tier4_perception_msgs::msg::TrafficLightArray | Car TLs classification | | ||
| `input/pedestrian_signals` | tier4_perception_msgs::msg::TrafficLightArray | Pedestrian TLs classification | | ||
| `input/expect_rois` | autoware_perception_msgs::TrafficLightRoiArray | expected TL ROIs | |
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.
| `input/expect_rois` | autoware_perception_msgs::TrafficLightRoiArray | expected TL ROIs | | |
| `input/expect_rois` | tier4_perception_msgs::msg::TrafficLightRoiArray | expected TL ROIs | |
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.
@MasatoSaeki
Thank you for your review!
input/expect_rois and tf are not needed, because they are not used in this package.
Yes, it was removed at the latest version. I update docs here 2930d08
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.
@badai-nguyen
Thank you for fixing!
btw, tf
is not needed I think, what do you think?
https://github.com/badai-nguyen/autoware.universe/blob/feat/traffic_signal_merger/perception/autoware_traffic_light_category_merger/src/traffic_light_category_merger_node.hpp#L43-L44
https://github.com/badai-nguyen/autoware.universe/blob/feat/traffic_signal_merger/perception/autoware_traffic_light_category_merger/src/traffic_light_category_merger_node.cpp#L27-L28
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Signed-off-by: badai-nguyen <[email protected]>
Description
autoware_traffic_light_occlusion_predictor
when new TLs detector can handle occlusion and it becomes unnessary.Related links
Parent Issue:
How was this PR tested?
Notes for reviewers
None.
Interface changes
None.
Effects on system behavior
None.