refactor(autoware_traffic_light_visualization): introduce Bulb struct and tidy internal naming#12553
Open
takam5f2 wants to merge 16 commits intoautowarefoundation:mainfrom
Conversation
Hoist TrafficLightElement using-declaration to the anonymous namespace scope and pass the small Time stamp by value in helper functions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ion from marker generation Introduce a Bulb domain struct and extract_bulbs free function that walk lanelet primitives once. TrafficLightVisualizer now receives a prebuilt BulbsByGroupId map and is independent of lanelet attribute schema, isolating map-format knowledge to a single seam. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…aded" debug log Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p to lanelet_map Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ment to traffic_light The variable holds AutowareTrafficLightConstPtr produced by autowareTrafficLights query, so name it for what it is rather than its abstract base concept in lanelet2. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… to map_traffic_lights in extract_bulbs Disambiguate from detected_traffic_lights used on the perception side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… and shorten message types Add namespace-scope using-aliases for TrafficLightGroupArray and LaneletMapBin in the internal node header so member declarations and definitions stay readable, and rename the callback parameter to detected_traffic_lights to mirror the map_traffic_lights naming on the lanelet side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ublisher members for specificity Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…o stamp for consistency Match the parameter name used by generate_markers(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 2020-2026 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…color_attribute Match the codebase convention of expanding abbreviations (regulatory_element, traffic_light, etc.). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lor and document BulbsByGroupId
…iases inside the node class Move the TrafficLightGroupArray and LaneletMapBin using-declarations from the autoware::traffic_light namespace into the private section of TrafficLightMapVisualizerNode. The aliases are an internal convenience for the node's callback signatures and members; exposing them at namespace scope leaks them to every translation unit including this header.
github-actions
Bot
added
the
component:perception
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
takam5f2
changed the title
takam5f2
added
the
run:build-and-test-differential
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12553 +/- ##
===========================================
- Coverage 18.64% 18.58% -0.06%
===========================================
Files 1918 7 -1911
Lines 131362 269 -131093
Branches 44504 42 -44462
===========================================
- Hits 24493 50 -24443
+ Misses 86754 187 -86567
+ Partials 20115 32 -20083
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…tate callback and subscriber names Rename traffic_lights_callback to detected_traffic_lights_callback and traffic_light_state_sub_ to detected_traffic_lights_sub_ so the callback, its parameter, and the subscription member all share the detected_traffic_lights vocabulary. This mirrors the map side (map_traffic_lights) and the visualizer interface (generate_markers's detected_traffic_lights argument).
…ck to lanelet_map_callback Rename bin_map_callback to lanelet_map_callback and its parameter to lanelet_map_msg so the callback name aligns with the subscriber member (lanelet_map_sub_) and the local variable (lanelet_map). The "bin" adjective referred to binary encoding, a transport-level detail that does not belong in the callback's identifier.
Contributor
Author
|
Sorry to bother you, but could you please review this PR when you have a chance? This PR moves the lanelet-specific extraction behind a free function This is part of a planned series following #12511 (merged); follow-ups will cover the test layer. Behavior is preserved across all PRs and is anchored by the characterization test introduced in #12487. |
takam5f2
requested review from
MasatoSaeki,
a-maumau and
technolojin
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Decouple
TrafficLightVisualizerfrom lanelet types by introducing a small ROS-friendlyBulb { id, position, color }struct, and tidy internal naming. This is the second of a planned series of incremental PRs; #12511 was the first.Changes:
BulbandBulbsByGroupIdtypes, and extractextract_bulbs(map_traffic_lights) → BulbsByGroupIdas a free function.TrafficLightVisualizernow receives a prebuiltBulbsByGroupIdand no longer holdslanelet::ConstPoint3d. Lanelet API knowledge is confined to the conversion layer (extract_bulbs).parse_bulb_colorhelper to flatten the red/green/yellow lookup.Timeby value, hoistusingto namespace scope).viz_lanelet_map→lanelet_map,regulatory_element→traffic_light,traffic_lights→map_traffic_lights,current_time→stamp, plus subscription/publisher member name updates.TrafficLightGroupArray/LaneletMapBinusing-declarations to the node class private section so they are not leaked at namespace scope."Map is loaded\n"debug log.Behavior is preserved end-to-end; the characterization test from #12487 acts as the safety net.
Related links
Parent Issue:
TrafficLightVisualizerclass)How was this PR tested?
colcon build --packages-select autoware_traffic_light_visualization \ --cmake-args -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON colcon test --packages-select autoware_traffic_light_visualization \ --event-handlers console_direct+All 5 characterization test cases pass.
Notes for reviewers
This PR is the second of a planned sequence to incrementally refactor
traffic_light_map_visualizer:TrafficLightVisualizerclass + readability cleanupBulbstruct + decouple from lanelet + internal tidyTrafficLightVisualizerandextract_bulbs, a Node-level smoke test, and test layout reorganizationThe plugin name (
autoware::traffic_light::TrafficLightMapVisualizerNode) and executable name are preserved.Interface changes
None.
Effects on system behavior
None.