feat(autoware_carla_interface): publish CARLA traffic-light states, matched to the map by position - #13327
Conversation
github-actions
Bot
added
type:documentation
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
9d538e5 to
706b02c
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d538e50b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
hakuturu583
added
the
run:build-and-test-differential
…atched to the map by position Bridge the CARLA server's traffic-light states into Autoware's perception output so a CARLA closed loop can run without camera-based recognition. The key problem is associating a CARLA traffic light with an Autoware `traffic_light_group_id` (a `traffic_light` regulatory-element id in the lanelet2 map). Instead of assuming the CARLA OpenDRIVE signal id equals the regulatory-element id (true only for maps auto-generated from the same OpenDRIVE) or hand-writing an id table, the bridge discovers the mapping geometrically: each CARLA light head is matched to the nearest lanelet2 light head and its state is published under every regulatory element that references that head. This works for hand-authored / Vector Map Builder maps too. - New `modules/traffic_light_matcher.py`: parses the lanelet2 `.osm` directly (reads `local_x`/`local_y`, i.e. the map frame; no lanelet2/projector dependency), keys physical heads by their `refers` way, and matches CARLA heads conservatively (distance threshold + a disjoint-group ambiguity ratio), dropping and logging ambiguous / too-far lights rather than guessing. - `carla_ros.py`: publishes `TrafficLightGroupArray` on /perception/traffic_light_recognition/traffic_signals, aggregated per group. - `carla_autoware.py`: `force_green` freezes all lights green for camera-less runs. - Parameters grouped under the `traffic_light.` namespace; resolution order is id-map override -> position match -> OpenDRIVE-id fallback. - Unit tests for the matcher; README documents the feature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
…matching
- Use the resolved map origin (`_current_map_origin()`) instead of the raw
`map_origin_x/y` parameters when placing CARLA light heads in the map frame,
so georeferenced maps (origin derived from the OpenDRIVE geoReference in
`on_world_ready`, parameters left at zero) match correctly instead of falling
outside the distance threshold and publishing nothing. (P1)
- Treat a candidate head as a genuine alternative for the ambiguity test unless
its group set is exactly equal to the winner's, replacing the `isdisjoint`
check. Overlapping-but-unequal sets (e.g. {500, 501} vs {501}) would otherwise
be accepted by arbitrary ranking and publish a missing or spurious group. Adds
a regression test. (P2)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
46c8693 to
da0463e
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13327 +/- ##
=======================================
Coverage 20.49% 20.49%
=======================================
Files 1948 1948
Lines 137202 137170 -32
Branches 48933 48932 -1
=======================================
- Hits 28113 28109 -4
+ Misses 86314 86283 -31
- Partials 22775 22778 +3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da0463e0ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@hakuturu583 Hi, thanks for the PR. Could please help solve the failed CodeScene Check? |
…cene
Address the second review round:
- traffic_light.id_map now accepts several group ids per OpenDRIVE signal id,
formatted `opendrive_id:group_id[|group_id...],...` (repeated keys merge). A
physical head shared by multiple regulatory elements can therefore be pinned
to all of them via one override, matching the position matcher's shared-head
behaviour, instead of only ever recovering a single group. (Codex P2)
- Reduce complexity flagged by the CodeScene gate by extracting helpers so the
hot methods stay flat and short:
- traffic_light_matcher: split `load_map_traffic_lights` into node/way/relation
parsers (Complex Method / Bumpy Road) and pull the per-head decision out of
`match_traffic_lights` into `_classify_head` (Large Method).
- carla_ros: split `_resolve_traffic_light_groups` into `_apply_id_map_override`
/ `_match_actors_to_map` / `_fallback_opendrive_groups`; the override parser
is now the pure, unit-tested `parse_id_map_override`.
- carla_autoware: fold the enable check into `_force_green_traffic_lights` so
`load_world` keeps a single unconditional call and gains no branch.
- Extend unit tests for the multi-id override parsing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
|
@Max-Bin thanks for the review! I've addressed the failing CodeScene gate in c52ece1 by refactoring the methods it flagged (rather than suppressing them):
The same commit also addresses the Codex P2 comment (id_map now supports multiple group ids per signal). Tests still pass and black is clean; CI should re-run the CodeScene check on the new commit. |
Follow-up to the previous refactor, targeting the two rules still flagged on the traffic-light matcher: - Extract `_add_relation_heads` so `load_map_traffic_lights` is a single flat loop with no nested conditional block (clears Bumpy Road Ahead). - Pass the per-run matching state as one `_MatchContext` namedtuple instead of a seven-parameter list, so `_classify_head(head, ctx)` takes two arguments (clears Excess Number of Function Arguments). No behavioural change; tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
…own table) On top of the bot autofix, finish the parts it could not: - Capitalize the first word of the parser-helper docstrings (flake8 / pydocstyle D403). - Order the traffic_light_matcher imports as isort (profile=black) expects. - Remove the literal `|` from the traffic_light.id_map row of the README parameter table (it split the cell and broke the column count; the bot's --fix left it mangled). The `|`-separated multi-id syntax stays documented in prose. markdownlint and prettier now pass on the table. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Masaya Kataoka <cld-masaya.kataoka@tier4.jp>
| item = item.strip() | ||
| if not item: | ||
| continue | ||
| opendrive_str, groups_str = item.split(":") |
There was a problem hiding this comment.
Could we validate malformed traffic_light.id_map entries instead of letting split()/int() raise? A typo like 12 terminatesn the bridge on the first simulation tick, while 12: silently overrides the actor with an empty group list. A clear warning for the offending entry would make this safer to configure.
| element = TrafficLightElement() | ||
| element.color = color | ||
| element.shape = TrafficLightElement.CIRCLE | ||
| element.status = TrafficLightElement.SOLID_ON |
There was a problem hiding this comment.
Could we preserve the CARLA state here? TrafficLightState.Off is converted to UNKNOWN, but the message is still published as SOLID_ON. That produces an “unknown-color, on” signal for a known-off lamp. Please map Off to SOLID_OFF and truly unknown states to UNKNOWN.
| None, | ||
| ) | ||
| entry["second"] = second_dist | ||
| if second_dist is not None and nearest_dist > ctx.ambiguity_ratio * second_dist: |
There was a problem hiding this comment.
if two different group sets have exactly the same centroid, both distances are 0, so 0 > ratio * 0 is false and the result depends on XML order. Could we treat equal-distance candidates, especially second_dist == 0, as ambiguous?
| <arg name="sensor_mapping_file" default="$(var sensor_mapping_file_default)" description="Path to sensor mapping YAML"/> | ||
| <arg name="flatten_steering_curve" default="false" description="Replace the ego vehicle's speed-based steering curve with an identity curve (CARLA 0.10 corrupt-curve workaround)"/> | ||
| <arg name="wake_sleeping_physics" default="false" description="Nudge the ego physics body awake when launching from standstill (CARLA 0.10 sleeping-body workaround; not needed on 0.9.15)"/> | ||
| <arg name="traffic_light.publish" default="false" description="Publish CARLA traffic-light states on /perception/traffic_light_recognition/traffic_signals"/> |
There was a problem hiding this comment.
The launch argument names in the PR description (publish_traffic_lights, traffic_light_map_path, etc.) do not match the arguments actually declared here (traffic_light.publish, traffic_light.map_path, etc.).
2 participants
Description
Bridges the CARLA server's traffic-light states into Autoware's perception output, so a CARLA
closed loop can run without camera-based traffic-light recognition.
This reworks the approach explored in #13309. The core problem is how a CARLA traffic light is
associated with an Autoware
traffic_light_group_id(the id of atraffic_lightregulatoryelement in the lanelet2 map). Rather than assume the CARLA OpenDRIVE signal id equals the
regulatory-element id — which only holds for maps auto-generated straight from the same OpenDRIVE —
or require a hand-written id table, the bridge discovers the mapping geometrically: each CARLA
light head is matched to the nearest lanelet2 light head and its state is published under every
regulatory element that references that head. This makes the feature work for hand-authored / TIER
IV Vector Map Builder maps, whose regulatory-element ids do not correspond to the OpenDRIVE signal
ids.
What's added
Launch arguments (all off / empty by default, so existing behavior is unchanged):
publish_traffic_lightstraffic_light.publishautoware_perception_msgs/TrafficLightGroupArrayon/perception/traffic_light_recognition/traffic_signalsevery tick.force_green_traffic_lightstraffic_light.force_greentraffic_light_map_pathtraffic_light.map_path.osmused for position matching.traffic_light_match_distancetraffic_light.match_distance5.0m).traffic_light_match_ratiotraffic_light.match_ratio0.6).traffic_light_id_maptraffic_light.id_mapopendrive_id:group_id,...override that pins a light, taking precedence over matching.The node parameters are grouped under a
traffic_light.namespace so they stay together inros2 param list.How the matching works
modules/traffic_light_matcher.py(ROS-free, unit-tested):.osmdirectly as XML, reading each node'slocal_x/local_ytags — theAutoware map-frame coordinates (for an MGRS map, exactly the easting/northing the map loader
produces). This avoids depending on the lanelet2 C++ regulatory-element registration and
sidesteps any projector mismatch between this process and the map loader.
refersway, commonly shared by several regulatory elements (one perapproaching lane / stop line), so each head carries the set of group ids that reference it; a
matched CARLA light publishes its state under all of them.
get_light_boxes()centresvia the existing
carla_location_to_ros_point(map_origin_x, map_origin_y)transform.resolving to a different signal is nearly as close (
nearest > ratio * second, the "lightacross the intersection" case) or nothing is within
match_distance, the light is leftunpublished and logged as ambiguous / too-far — never guessed. The startup log prints a match
report (
N matched, M ambiguous, K too far); reported lights can be pinned viatraffic_light_id_map.Resolution precedence per light:
traffic_light.id_mapoverride → position match (map_path) →OpenDRIVE-id-as-group-id fallback (no map path).
How was this PR tested?
test/test_traffic_light_matcher.py— unit tests covering shared-head group expansion, isolatedmatches, the ambiguity guard (both the disjoint-signal midpoint that must be dropped and the
close same-group / clear-winner neighbours that must not), too-far, and
.osmparsing.realistic alignment (~0.3 m) ~97 % of heads match, and genuinely-ambiguous sub-metre distinct
signals are reported rather than mis-assigned.
black(line-length 100) clean; dotted parameter names verified to declare / resolve underrclpy.
publish_traffic_lights(and, for matching,traffic_light_map_path)is set.