-
Notifications
You must be signed in to change notification settings - Fork 549
add content-filtered-topic interfaces #1561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
424a981
0444bd2
1a57ae5
3c94995
f2460fa
323b919
95a4428
0124396
4dc661b
771e5a0
0bf1f52
b044aa2
43c59e8
e72216e
88320c9
98e3872
307b8d4
e37c626
e965be7
556215b
7b362b5
1b232ae
50068a7
98f2f66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,11 @@ cmake_minimum_required(VERSION 3.12) | |
|
|
||
| project(rclcpp) | ||
|
|
||
| option(DEFINE_CONTENT_FILTER "Content filter feature support" ON) | ||
| if(DEFINE_CONTENT_FILTER) | ||
| add_definitions(-DCONTENT_FILTER_ENABLE) | ||
| endif() | ||
|
|
||
|
Comment on lines
+5
to
+9
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this will be always on as ROS 2 rclcpp library, but user explicitly enable this when they want to use ContentFitleredTopic, otherwise build fails. my understanding correct? @wjwwood could we have feedback on this, if this is what we want?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wjwwood friendly ping.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this doesn't really help much, particularly if we don't have equivalent flags in rcl/rmw.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As @ivanpauno mentioned, this isn't really accomplishing what we wanted, so let's just remove it and provide access to CFT uniformly for now. We'll just have to document the known issues. |
||
| find_package(Threads REQUIRED) | ||
|
|
||
| find_package(ament_cmake_ros REQUIRED) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -491,6 +491,38 @@ class SubscriptionBase : public std::enable_shared_from_this<SubscriptionBase> | |
| event_handlers_[event_type]->clear_on_ready_callback(); | ||
| } | ||
|
|
||
| #ifdef CONTENT_FILTER_ENABLE | ||
| /// Set the filter expression and expression parameters for the subscription. | ||
|
fujitatomoya marked this conversation as resolved.
|
||
| /** | ||
| * \param[in] filter_expression An filter expression to set. | ||
|
wjwwood marked this conversation as resolved.
Outdated
|
||
| * \sa SubscriptionOptionsBase::ContentFilterOptions::filter_expression | ||
| * \param[in] expression_parameters Array of expression parameters to set. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The size of the vector have a limitation which is described in Do we need to describe this limitation here ?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we do, since this would be the 1st place for user application.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. used the '\sa ContentFilterOptions::expression_parameters' for |
||
| * \sa SubscriptionOptionsBase::ContentFilterOptions::expression_parameters | ||
| * \throws RCLBadAlloc if memory cannot be allocated | ||
| * \throws RCLError if an unexpect error occurs | ||
| */ | ||
| RCLCPP_PUBLIC | ||
| virtual | ||
| void | ||
| set_content_filter( | ||
| const std::string & filter_expression, | ||
| const std::vector<std::string> & expression_parameters = {}); | ||
|
|
||
| /// Get the filter expression and expression parameters for the subscription. | ||
| /** | ||
| * \param[out] filter_expression An filter expression to get. | ||
| * \param[out] expression_parameters Array of expression parameters to get. | ||
| * \throws RCLBadAlloc if memory cannot be allocated | ||
| * \throws RCLError if an unexpect error occurs | ||
| */ | ||
| RCLCPP_PUBLIC | ||
| virtual | ||
| void | ||
| get_content_filter( | ||
| std::string & filter_expression, | ||
| std::vector<std::string> & expression_parameters) const; | ||
| #endif // CONTENT_FILTER_ENABLE | ||
|
|
||
| protected: | ||
| template<typename EventCallbackT> | ||
| void | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,6 +81,22 @@ struct SubscriptionOptionsBase | |
| TopicStatisticsOptions topic_stats_options; | ||
|
|
||
| QosOverridingOptions qos_overriding_options; | ||
|
|
||
| #ifdef CONTENT_FILTER_ENABLE | ||
| /// Options to configure content filtered topic in the subscription. | ||
| struct ContentFilterOptions | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it's better to define this outside |
||
| { | ||
| /// Filter expression is similar to the WHERE part of an SQL clause | ||
| std::string filter_expression; | ||
| /** | ||
| * Expression parameters is the tokens placeholder ‘parameters’ (i.e., "%n" tokens begin from 0) | ||
| * in the filter_expression | ||
| */ | ||
| std::vector<std::string> expression_parameters; | ||
| }; | ||
|
|
||
| ContentFilterOptions content_filter_options; | ||
| #endif // CONTENT_FILTER_ENABLE | ||
| }; | ||
|
|
||
| /// Structure containing optional configuration for Subscriptions. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -354,3 +354,27 @@ SubscriptionBase::set_on_new_message_callback( | |||||||||||||
| throw_from_rcl_error(ret, "failed to set the on new message callback for subscription"); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| #ifdef CONTENT_FILTER_ENABLE | ||||||||||||||
| void | ||||||||||||||
| SubscriptionBase::set_content_filter( | ||||||||||||||
| const std::string & filter_expression, | ||||||||||||||
| const std::vector<std::string> & expression_parameters) | ||||||||||||||
| { | ||||||||||||||
| static_cast<void>(filter_expression); | ||||||||||||||
| static_cast<void>(expression_parameters); | ||||||||||||||
|
|
||||||||||||||
| rclcpp::exceptions::throw_from_rcl_error(RMW_RET_UNSUPPORTED, "content filter unsupported"); | ||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this method is stub? we will enable CFT for user application in |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| void | ||||||||||||||
| SubscriptionBase::get_content_filter( | ||||||||||||||
| std::string & filter_expression, | ||||||||||||||
| std::vector<std::string> & expression_parameters) const | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of using arguments to get the output, I would rather use
Suggested change
|
||||||||||||||
| { | ||||||||||||||
| static_cast<void>(filter_expression); | ||||||||||||||
| static_cast<void>(expression_parameters); | ||||||||||||||
|
|
||||||||||||||
| rclcpp::exceptions::throw_from_rcl_error(RMW_RET_UNSUPPORTED, "content filter unsupported"); | ||||||||||||||
| } | ||||||||||||||
| #endif // CONTENT_FILTER_ENABLE | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // Copyright 2021 Open Source Robotics Foundation, Inc. | ||
|
wjwwood marked this conversation as resolved.
Outdated
|
||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| #include <memory> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| #include "rclcpp/exceptions.hpp" | ||
| #include "rclcpp/rclcpp.hpp" | ||
|
|
||
| #include "test_msgs/msg/basic_types.hpp" | ||
|
|
||
| #ifdef RMW_IMPLEMENTATION | ||
| # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX | ||
| # define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX) | ||
| #else | ||
| # define CLASSNAME(NAME, SUFFIX) NAME | ||
| #endif | ||
|
|
||
| class CLASSNAME (TestContentFilterSubscription, RMW_IMPLEMENTATION) : public ::testing::Test | ||
| { | ||
| public: | ||
| static void SetUpTestCase() | ||
| { | ||
| rclcpp::init(0, nullptr); | ||
| } | ||
|
|
||
| static void TearDownTestCase() | ||
| { | ||
| rclcpp::shutdown(); | ||
| } | ||
|
|
||
| void SetUp() | ||
| { | ||
| node = std::make_shared<rclcpp::Node>("test_content_filter_node", "/ns"); | ||
|
|
||
| auto options = rclcpp::SubscriptionOptions(); | ||
| auto callback = [](std::shared_ptr<const test_msgs::msg::BasicTypes>) {}; | ||
| sub = node->create_subscription<test_msgs::msg::BasicTypes>( | ||
| "content_filter_topic", qos.reliable().transient_local(), callback, options); | ||
| } | ||
|
|
||
| void TearDown() | ||
| { | ||
| sub.reset(); | ||
| node.reset(); | ||
| } | ||
|
|
||
| protected: | ||
| rclcpp::Node::SharedPtr node; | ||
| rclcpp::QoS qos{rclcpp::KeepLast{10}}; | ||
| rclcpp::Subscription<test_msgs::msg::BasicTypes>::SharedPtr sub; | ||
| }; | ||
|
|
||
| TEST_F(CLASSNAME(TestContentFilterSubscription, RMW_IMPLEMENTATION), get_content_filter_error) { | ||
| std::string filter_expression; | ||
| std::vector<std::string> expression_parameters; | ||
| EXPECT_THROW( | ||
| sub->get_content_filter(filter_expression, expression_parameters), | ||
| rclcpp::exceptions::RCLError); | ||
| } | ||
|
|
||
| TEST_F(CLASSNAME(TestContentFilterSubscription, RMW_IMPLEMENTATION), set_content_filter_error) { | ||
| std::string filter_expression = "int32_value = %0"; | ||
| std::string expression_parameter = "100"; | ||
| EXPECT_THROW( | ||
| sub->set_content_filter(filter_expression, {expression_parameter}), | ||
| rclcpp::exceptions::RCLError); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.