feat(autoware_pointcloud_preprocessor): templatize Filter into FilterBase<NodeT> - #13331
Draft
Koichi98 wants to merge 1 commit into
Draft
feat(autoware_pointcloud_preprocessor): templatize Filter into FilterBase<NodeT>#13331Koichi98 wants to merge 1 commit into
Koichi98 wants to merge 1 commit into
Conversation
…Base<NodeT> Signed-off-by: Koichi Imai <koichi.imai.2@tier4.jp>
|
Thank you for contributing to the Autoware project! 🚧 If your pull request is in progress, switch it to draft mode. Please ensure:
|
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
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
autoware_pointcloud_preprocessor::Filterwas hard-wired torclcpp::Node, so a node that wants to run on Agnocast had to bypass the base class.PolarVoxelOutlierFilterComponentdoes exactly that today: it resetsFilter::sub_input_and hand-rolls its own Agnocast subscription, with aTODOwaiting for base-class support.This turns
Filterintotemplate <typename NodeT = rclcpp::Node> class FilterBase : public NodeT, with the publisher, subscription,message_filtersandautoware_utilsmember types derived fromNodeT.Filterstays as a real class deriving fromFilterBase<>, so all 26 existing subclasses (17 here, 3 inautoware_ground_segmentation, 6 inautoware_compare_map_segmentation) are unchanged.No node is instantiated on
FilterBase<autoware::agnocast_wrapper::Node>yet — that instantiation is explicit infilter.cppso both are compile-checked, and moving the first node onto it is a follow-up PR. The TF backend is deliberately untouched here:ManagedTransformBufferbuilds an internalrclcpp::Node, which anAgnocastOnlyexecutable cannot construct, and swapping it belongs with the first node that actually needs it.Depends on autowarefoundation/autoware_core#1433:
FilterBase<agnocast_wrapper::Node>::subscribe()registersinput_indices_callback— which takesPointCloud2::ConstSharedPtr— on the wrapper's synchronizer, and that callback shape is what #1433 adds.Related links
ConstSharedPtrsynchronizer callbacks inautoware_agnocast_wrapper(required to build this PR withENABLE_AGNOCAST=1).How was this PR tested?
Built
autoware_pointcloud_preprocessor,autoware_ground_segmentationandautoware_compare_map_segmentationwithENABLE_AGNOCAST=0andENABLE_AGNOCAST=1; no errors and no new warnings in either.Ran
colcon testforautoware_pointcloud_preprocessoratENABLE_AGNOCAST=0: 117 gtest cases across 14 suites, 0 failures.No tests are added. The change introduces no new behavior on the
rclcpp::Nodeinstantiation, which the existing suites already cover, and the Agnocast instantiation has no node behind it yet.Notes for reviewers
ALLOCATE_OUTPUT_MESSAGE_UNIQUE(pub_output_)inallocate_output_message()has to stay a macro and stay behindif constexpr (kIsAgnocastNode). WithoutUSE_AGNOCAST_ENABLEDthe macro expands tostd::make_unique<...>(), becauseagnocast_wrapper::Node::create_publisherreturns a plainrclcpp::Publisher::SharedPtrthere, which has noallocate_output_message_unique().Filteris a class rather thanusing Filter = FilterBase<>;because subclasses in other namespaces write a bare: Filter(...)in their mem-initializer list, which relies on the injected class name that an alias does not provide.Interface changes
None.
Effects on system behavior
None. Every node still runs on
FilterBase<rclcpp::Node>.