feat(trajectory_selector): combine validator and concatenator#12532
Conversation
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
e373eec to
9765fcb
Compare
There was a problem hiding this comment.
Pull request overview
This PR reduces planning pipeline node count by integrating the trajectory concatenation stage into the TrajectoryValidator node. The standalone concatenator ROS node is removed and replaced with a pure C++ autoware::trajectory_concatenator::TrajectoryConcatenator library that the validator drives via a fixed-rate timer.
Changes:
- Refactors the trajectory concatenator into a standalone C++ library and removes the ROS node/launch/config artifacts.
- Updates
TrajectoryValidatorto subscribe to generative + backup candidate trajectory inputs, buffer them, and run validation on a 30ms timer flush. - Adjusts tests/launch/package dependencies to reflect the new in-process concatenation stage and topic interface.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| planning/autoware_trajectory_validator/src/trajectory_validator_node.cpp | Adds concatenation buffering + 30ms timer-driven validation pipeline. |
| planning/autoware_trajectory_validator/include/autoware/trajectory_validator/trajectory_validator_node.hpp | Adds concatenator state, new subscriptions, and timer declarations. |
| planning/autoware_trajectory_validator/include/autoware/trajectory_validator/parameters.hpp | Introduces combined validator+concatenator parameter handling. |
| planning/autoware_trajectory_validator/launch/trajectory_validator.launch.xml | Switches input remaps to generative/backup trajectory topics. |
| planning/autoware_trajectory_validator/package.xml | Adds dependency on autoware_trajectory_concatenator. |
| planning/autoware_trajectory_validator/test/node/test_trajectory_validator_node.cpp | Updates integration test to publish on new input topic and set trajectory stamps. |
| planning/autoware_trajectory_concatenator/include/autoware/trajectory_concatenator/trajectory_concatenator.hpp | New pure C++ concatenator API. |
| planning/autoware_trajectory_concatenator/src/trajectory_concatenator.cpp | Implements buffering + time-based pruning + concatenation. |
| planning/autoware_trajectory_concatenator/CMakeLists.txt | Builds/install library instead of registering a component node executable. |
| planning/autoware_trajectory_concatenator/src/trajectory_concatenator_node.* | Removes standalone ROS node implementation. |
| planning/autoware_trajectory_concatenator/launch/trajectory_concatenator.launch.xml | Removes concatenator launch file. |
| planning/autoware_trajectory_concatenator/config/concatenator.param.yaml | Removes concatenator node parameter YAML. |
| planning/autoware_trajectory_concatenator/src/trajectory_concatenator_structs.hpp | Removes now-unused node-only parameter struct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
danielsanchezaran
left a comment
There was a problem hiding this comment.
I dont see how the inclusion of paralelism and mutex is worth it over the removal of a node that is quite simple (concatenator), can you please illustrate the advantages of this new layout? I dont think it is worth it, especially when it is not obvious that the validator should be the one concatenating trajectories
The mutex is not new in this PR. It's already present in the original standalone concatenator node, and I ported that logic directly into the validator without modification.
There are three things to highlight
|
The points have been addressed in a private conversation, I am ok with this PR changes given the proposed future changes. @zulfaqar-azmi-t4 please take a look at the copilot review comments too |
danielsanchezaran
left a comment
There was a problem hiding this comment.
@zulfaqar-azmi-t4 please also update the READMEs
99e3534 to
e1d585d
Compare
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
…ustified mutable on_timer() computed the validated result but never called publish(), making the node a no-op at the output. Both integration tests were silently timing out because of this. Also removed sub_trajectories_ which was declared but never assigned in subscribers(), and dropped the unjustified `mutable` qualifier from time_keeper_ (no const method ever writes to it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
validate_trajectories() was constructing a new TrajectoryValidator on every call (copying the plugins_ vector each time) instead of using the validator_ptr_ member that is initialized in the constructor for exactly this purpose. validator_ptr_ was live memory that was never called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
…uplicate DebugPublisher Two cleanups in validate_trajectories / publishers(): 1. The first diagnostics_interface_ptr_->clear() was dead work: the diagnostics are cleared again five lines later, just before the add_key_value loop, so the first call never had observable effect. 2. pub_validation_reports_ and pub_debug_ were both initialized to a DebugPublisher with the identical prefix "~/debug". A single DebugPublisher handles multiple sub-topics; the duplicate object added confusion without benefit. Removed pub_validation_reports_ and routed its one call-site through pub_debug_. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp>
e42c5cc to
70a4b91
Compare
fb73b91
into
autowarefoundation:main
…refoundation/autoware_universe#12532) * feat(concatenator): add concatenator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: combine concatenator with validator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove explicit find package, and pre-commit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: failing test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: create public interface for concatenator, and move concatenator to detail folder Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: separate validator to validator interface and initialize selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix loading parameters Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(node): publish validated trajectories; remove dead member and unjustified mutable on_timer() computed the validated result but never called publish(), making the node a no-op at the output. Both integration tests were silently timing out because of this. Also removed sub_trajectories_ which was declared but never assigned in subscribers(), and dropped the unjustified `mutable` qualifier from time_keeper_ (no const method ever writes to it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): use validator_ptr_ in validate_trajectories validate_trajectories() was constructing a new TrajectoryValidator on every call (copying the plugins_ vector each time) instead of using the validator_ptr_ member that is initialized in the constructor for exactly this purpose. validator_ptr_ was live memory that was never called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): remove redundant diagnostics clear; merge duplicate DebugPublisher Two cleanups in validate_trajectories / publishers(): 1. The first diagnostics_interface_ptr_->clear() was dead work: the diagnostics are cleared again five lines later, just before the add_key_value loop, so the first call never had observable effect. 2. pub_validation_reports_ and pub_debug_ were both initialized to a DebugPublisher with the identical prefix "~/debug". A single DebugPublisher handles multiple sub-topics; the duplicate object added confusion without benefit. Removed pub_validation_reports_ and routed its one call-site through pub_debug_. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: add test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: rename context Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: return if concatenated is empty Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * doc: docstring Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove failed spellcheck Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix initial processing time value Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: rename interface to wrapper Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * remove processing time and add unit test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix * separate trajectory selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: precommit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * readme Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: addresses copilot comments Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: address minor copilot comment Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> --------- Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…refoundation#12532) * feat(concatenator): add concatenator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: combine concatenator with validator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove explicit find package, and pre-commit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: failing test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: create public interface for concatenator, and move concatenator to detail folder Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: separate validator to validator interface and initialize selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix loading parameters Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(node): publish validated trajectories; remove dead member and unjustified mutable on_timer() computed the validated result but never called publish(), making the node a no-op at the output. Both integration tests were silently timing out because of this. Also removed sub_trajectories_ which was declared but never assigned in subscribers(), and dropped the unjustified `mutable` qualifier from time_keeper_ (no const method ever writes to it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): use validator_ptr_ in validate_trajectories validate_trajectories() was constructing a new TrajectoryValidator on every call (copying the plugins_ vector each time) instead of using the validator_ptr_ member that is initialized in the constructor for exactly this purpose. validator_ptr_ was live memory that was never called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): remove redundant diagnostics clear; merge duplicate DebugPublisher Two cleanups in validate_trajectories / publishers(): 1. The first diagnostics_interface_ptr_->clear() was dead work: the diagnostics are cleared again five lines later, just before the add_key_value loop, so the first call never had observable effect. 2. pub_validation_reports_ and pub_debug_ were both initialized to a DebugPublisher with the identical prefix "~/debug". A single DebugPublisher handles multiple sub-topics; the duplicate object added confusion without benefit. Removed pub_validation_reports_ and routed its one call-site through pub_debug_. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: add test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: rename context Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: return if concatenated is empty Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * doc: docstring Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove failed spellcheck Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix initial processing time value Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: rename interface to wrapper Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * remove processing time and add unit test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix * separate trajectory selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: precommit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * readme Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: addresses copilot comments Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: address minor copilot comment Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> --------- Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Arjun Jagdish Ram <arjun.ram@tier4.jp>
…refoundation#12532) * feat(concatenator): add concatenator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: combine concatenator with validator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove explicit find package, and pre-commit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: failing test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: create public interface for concatenator, and move concatenator to detail folder Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: separate validator to validator interface and initialize selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix loading parameters Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(node): publish validated trajectories; remove dead member and unjustified mutable on_timer() computed the validated result but never called publish(), making the node a no-op at the output. Both integration tests were silently timing out because of this. Also removed sub_trajectories_ which was declared but never assigned in subscribers(), and dropped the unjustified `mutable` qualifier from time_keeper_ (no const method ever writes to it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): use validator_ptr_ in validate_trajectories validate_trajectories() was constructing a new TrajectoryValidator on every call (copying the plugins_ vector each time) instead of using the validator_ptr_ member that is initialized in the constructor for exactly this purpose. validator_ptr_ was live memory that was never called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): remove redundant diagnostics clear; merge duplicate DebugPublisher Two cleanups in validate_trajectories / publishers(): 1. The first diagnostics_interface_ptr_->clear() was dead work: the diagnostics are cleared again five lines later, just before the add_key_value loop, so the first call never had observable effect. 2. pub_validation_reports_ and pub_debug_ were both initialized to a DebugPublisher with the identical prefix "~/debug". A single DebugPublisher handles multiple sub-topics; the duplicate object added confusion without benefit. Removed pub_validation_reports_ and routed its one call-site through pub_debug_. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: add test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: rename context Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: return if concatenated is empty Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * doc: docstring Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove failed spellcheck Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix initial processing time value Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: rename interface to wrapper Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * remove processing time and add unit test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix * separate trajectory selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: precommit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * readme Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: addresses copilot comments Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: address minor copilot comment Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> --------- Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…refoundation#12532) * feat(concatenator): add concatenator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: combine concatenator with validator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove explicit find package, and pre-commit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: failing test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: create public interface for concatenator, and move concatenator to detail folder Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: separate validator to validator interface and initialize selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix loading parameters Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(node): publish validated trajectories; remove dead member and unjustified mutable on_timer() computed the validated result but never called publish(), making the node a no-op at the output. Both integration tests were silently timing out because of this. Also removed sub_trajectories_ which was declared but never assigned in subscribers(), and dropped the unjustified `mutable` qualifier from time_keeper_ (no const method ever writes to it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): use validator_ptr_ in validate_trajectories validate_trajectories() was constructing a new TrajectoryValidator on every call (copying the plugins_ vector each time) instead of using the validator_ptr_ member that is initialized in the constructor for exactly this purpose. validator_ptr_ was live memory that was never called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): remove redundant diagnostics clear; merge duplicate DebugPublisher Two cleanups in validate_trajectories / publishers(): 1. The first diagnostics_interface_ptr_->clear() was dead work: the diagnostics are cleared again five lines later, just before the add_key_value loop, so the first call never had observable effect. 2. pub_validation_reports_ and pub_debug_ were both initialized to a DebugPublisher with the identical prefix "~/debug". A single DebugPublisher handles multiple sub-topics; the duplicate object added confusion without benefit. Removed pub_validation_reports_ and routed its one call-site through pub_debug_. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: add test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: rename context Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: return if concatenated is empty Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * doc: docstring Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove failed spellcheck Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix initial processing time value Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: rename interface to wrapper Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * remove processing time and add unit test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix * separate trajectory selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: precommit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * readme Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: addresses copilot comments Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: address minor copilot comment Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> --------- Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…utowarefoundation#12532) (#3031) * feat(trajectory_selector): combine validator and concatenator (autowarefoundation#12532) * feat(concatenator): add concatenator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: combine concatenator with validator Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove explicit find package, and pre-commit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: failing test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: create public interface for concatenator, and move concatenator to detail folder Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * feat: separate validator to validator interface and initialize selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix loading parameters Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(node): publish validated trajectories; remove dead member and unjustified mutable on_timer() computed the validated result but never called publish(), making the node a no-op at the output. Both integration tests were silently timing out because of this. Also removed sub_trajectories_ which was declared but never assigned in subscribers(), and dropped the unjustified `mutable` qualifier from time_keeper_ (no const method ever writes to it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): use validator_ptr_ in validate_trajectories validate_trajectories() was constructing a new TrajectoryValidator on every call (copying the plugins_ vector each time) instead of using the validator_ptr_ member that is initialized in the constructor for exactly this purpose. validator_ptr_ was live memory that was never called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix(validator_interface): remove redundant diagnostics clear; merge duplicate DebugPublisher Two cleanups in validate_trajectories / publishers(): 1. The first diagnostics_interface_ptr_->clear() was dead work: the diagnostics are cleared again five lines later, just before the add_key_value loop, so the first call never had observable effect. 2. pub_validation_reports_ and pub_debug_ were both initialized to a DebugPublisher with the identical prefix "~/debug". A single DebugPublisher handles multiple sub-topics; the duplicate object added confusion without benefit. Removed pub_validation_reports_ and routed its one call-site through pub_debug_. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: add test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: rename context Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: return if concatenated is empty Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * doc: docstring Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: remove failed spellcheck Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix initial processing time value Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: rename interface to wrapper Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * remove processing time and add unit test Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * style(pre-commit): autofix * separate trajectory selector Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: precommit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * readme Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: addresses copilot comments Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: address minor copilot comment Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> --------- Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: pre-commit Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: add validator to debug namespace Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: PDDP-201 restore route msgs Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: json schema for collision check Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> * fix: update validation reports in planning_evaluator launcher Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> --------- Signed-off-by: Zulfaqar Azmi <zulfaqar.azmi@tier4.jp> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Description
Merges the previously separate
autoware_trajectory_concatenatornode andautoware_trajectory_validatorlibrary into a singleTrajectorySelectorNode. The node collects candidate trajectories from multiple upstream planners, concatenates them into a single set, runs the configured validator plugins, and publishes the surviving trajectories.Clean public wrappers are introduced for both subsystems (
TrajectoryConcatenatorWrapper,TrajectoryValidatorWrapper) so each remains independently readable and testable. The oldValidationStageclass is removed. Internal implementation types (TrajectoryValidator,ValidatorContext,TrajectoryValidatorReport) are moved underdetail/.Related links
Parent Issue:
How was this PR tested?
colcon build— successcolcon test— all tests pass, including:test_vehicle_constraint_filter,test_traffic_light_filter)FiltersTrajectoriesViaPlugin,HandlesPluginRejection)take_validator_data()(missing odometry, acceleration, predicted objects)Notes for reviewers
The
trajectory_validator_nodeROS node name and its associated topic namespace (~/input/trajectories) are replaced by the ROS node nametrajectory_selector_node(executable:autoware_trajectory_selector_node) and two separate input topics (see Interface changes below). Downstream launch files that remap~/input/trajectorieswill need to be updated to remap~/input/trajectories_generativeand/or~/input/trajectories_backup.Interface changes
Topic changes
Additions and removals
~/input/trajectories_generativeautoware_internal_planning_msgs/msg/CandidateTrajectories~/input/trajectories_backupautoware_internal_planning_msgs/msg/CandidateTrajectories~/output/trajectoriesautoware_internal_planning_msgs/msg/CandidateTrajectories~/debug/processing_time_detail_ms/trajectory_selectorautoware_internal_debug_msgs/msg/ProcessingTimeTree~/input/trajectoriesautoware_internal_planning_msgs/msg/CandidateTrajectoriesModifications
/trajectory_validator_node/...trajectory_validator_node/trajectory_selector_node/...trajectory_selector_nodeEffects on system behavior
The node now concatenates trajectories from multiple sources before validation, rather than validating a single pre-concatenated input. Trajectories that arrive on
trajectories_generativeortrajectories_backupare merged into one candidate set each timer tick before being passed through the validator plugins. Behavior of the validator plugins themselves is unchanged.