fix(behavior_velocity_planner): fixed virtual wall marker id duplication

[maxime-clem]

Description

For a MarkerArray topic, Rviz expects markers to have a unique id for each namespace.
In the current implementation, some modules (e.g., crosswalk and intersection) will create multiple markers with the same id and namespace, causing warnings in Rviz and preventing some virtual walls from being displayed.

To solve the issue, this PR adds an optional namespace_prefix argument to the function used to create the virtual wall markers.

Tests performed

Tested with Psim.

Effects on system behavior

Not applicable.

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.

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.

After all checkboxes are checked, anyone who has write access can merge the PR.

[codecov]

Codecov Report

Patch coverage: 12.50% and project coverage change: +0.65 🎉

Comparison is base (7024f65) 13.80% compared to head (cc9e1cc) 14.45%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3654      +/-   ##
==========================================
+ Coverage   13.80%   14.45%   +0.65%     
==========================================
  Files        1407     1409       +2     
  Lines       99175   101206    +2031     
  Branches    29304    30688    +1384     
==========================================
+ Hits        13688    14627     +939     
- Misses      70717    70812      +95     
- Partials    14770    15767     +997     

Flag	Coverage Δ		*Carryforward flag
differential	19.26% <12.50%> (?)
total	13.80% <ø> (+<0.01%)	⬆️	Carriedforward from 3ae0c03

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

Impacted Files	Coverage Δ
...ocity_planner/src/scene_module/crosswalk/debug.cpp	0.00% <0.00%> (ø)
...ty_planner/src/scene_module/intersection/debug.cpp	0.00% <0.00%> (ø)
common/motion_utils/src/marker/marker_helper.cpp	41.37% <18.18%> (+13.60%)	⬆️

... and 62 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[shmpwk]

https://github.com/autowarefoundation/autoware.universe/pull/3654/files#diff-25c5c8d3c739cc801f1200bc12d97093adc4cdc342e0047272b0e39f08144735R229

If this PR solves the marker id duplication problem, does it mean we don't need this modifications?
https://github.com/autowarefoundation/autoware.universe/pull/3603/files

cc. @soblin

[soblin]

LGTM

[soblin]

The main purpose of using VirtualWallMarkerCreator is that:

1. the client side does not need to send delete action manually
2. the client side does not handle the id of the rviz marker

To properly use this feature the client side need to care if the ns_prefix is unique in some modules, especially the submodules of behavior_velocity_planner. That is because multiple instances of a specific module can run in parallel depending on the path/map(like crosswalk, intersection, stopline, blind_spot, speed_bump.

As far as I know, the subinstances of intersection and blind_spot modules can be distinguishes by its lane_id_.

Several stopline and speed_bump may exist on a specific lane, so their subinstances should be distinguished by module_id_ (which comes from the RegulatoryElement ID), not by lane_id_ (if they lie on a same lanelet, this value conflicts).

I had a look at the usage of virtual wall marker creator of other planning modules as well

• behavior_velocity_planner
  • blind_spot: this module need to set ns_prefix to lane_id_ == module_id_ additionally
  • detection_area: this module need to set ns_prefix to module_id_ additionally
  • no_stopping_area: this module need to set ns_prefix to module_id_ additionally
  • occlusion_spot: there is only one occlusion_spot module subinstance running, so there is no chance of marker id conflict
  • speed_bump: this module need to set ns_prefix to module_id_ additionally. And the client does not need to explicitly set id unless it sends delete action manually as well.
  • stopline: this module need to set ns_prefix to module_id_ additionally
  • traffic_light: this module is setting lane_id_ == module_id_, but this may be fixed in case multiple traffic lights exist for a specific lane. And this module need to set ns_prefix to module_id_ additionally.
  • virtual_traffic_light: this module need to set ns_prefix to module_id_ additionally

I'll try fixing these possible issues later.

[soblin]

@kyoichi-sugahara Can you approve this PR ?