From 665faa5a1333dfb0a9b36597f0fae89eed3598d3 Mon Sep 17 00:00:00 2001 From: Prajakta Gokhale Date: Thu, 9 Apr 2020 18:14:23 -0700 Subject: [PATCH 1/4] Add SubscriptionOptions for topic statistics Signed-off-by: Prajakta Gokhale --- rclcpp/include/rclcpp/subscription_options.hpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rclcpp/include/rclcpp/subscription_options.hpp b/rclcpp/include/rclcpp/subscription_options.hpp index 2e27de6209..39ff8a964d 100644 --- a/rclcpp/include/rclcpp/subscription_options.hpp +++ b/rclcpp/include/rclcpp/subscription_options.hpp @@ -54,6 +54,18 @@ struct SubscriptionOptionsBase /// Optional RMW implementation specific payload to be used during creation of the subscription. std::shared_ptr rmw_implementation_payload = nullptr; + + // Options to configure topic statistics collector in the subscription + struct TopicStatisticsOptions + { + // Represent the state of topic statistics collector + enum class TopicStatisticsState {ENABLED, DISABLED}; + + // Enable and disable topic statistics calculation and publication. Defaults to disabled. + TopicStatisticsState state = TopicStatisticsState::DISABLED; + }; + + TopicStatisticsOptions topic_stats_options; }; /// Structure containing optional configuration for Subscriptions. @@ -104,6 +116,7 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase }; using SubscriptionOptions = SubscriptionOptionsWithAllocator>; +using TopicStatisticsState = SubscriptionOptionsBase::TopicStatisticsOptions::TopicStatisticsState; } // namespace rclcpp From 4e042c33fe856d5ebc537a208e7f7111de0f85dd Mon Sep 17 00:00:00 2001 From: Prajakta Gokhale Date: Mon, 13 Apr 2020 10:59:07 -0700 Subject: [PATCH 2/4] Add more options and unit test Signed-off-by: Prajakta Gokhale --- rclcpp/CMakeLists.txt | 6 +++ .../include/rclcpp/subscription_options.hpp | 7 +++ rclcpp/test/test_subscription_options.cpp | 44 +++++++++++++++++++ 3 files changed, 57 insertions(+) create mode 100644 rclcpp/test/test_subscription_options.cpp diff --git a/rclcpp/CMakeLists.txt b/rclcpp/CMakeLists.txt index 1389ad8aeb..781a06cf3b 100644 --- a/rclcpp/CMakeLists.txt +++ b/rclcpp/CMakeLists.txt @@ -535,6 +535,12 @@ if(BUILD_TESTING) target_link_libraries(test_wait_set ${PROJECT_NAME}) endif() + ament_add_gtest(test_subscription_options test/test_subscription_options.cpp) + if(TARGET test_subscription_options) + ament_target_dependencies(test_subscription_options "rcl") + target_link_libraries(test_subscription_options ${PROJECT_NAME}) + endif() + # Install test resources install( DIRECTORY test/resources diff --git a/rclcpp/include/rclcpp/subscription_options.hpp b/rclcpp/include/rclcpp/subscription_options.hpp index 39ff8a964d..85857f6cb1 100644 --- a/rclcpp/include/rclcpp/subscription_options.hpp +++ b/rclcpp/include/rclcpp/subscription_options.hpp @@ -15,6 +15,7 @@ #ifndef RCLCPP__SUBSCRIPTION_OPTIONS_HPP_ #define RCLCPP__SUBSCRIPTION_OPTIONS_HPP_ +#include #include #include #include @@ -63,6 +64,12 @@ struct SubscriptionOptionsBase // Enable and disable topic statistics calculation and publication. Defaults to disabled. TopicStatisticsState state = TopicStatisticsState::DISABLED; + + // Topic to which topic statistics get published when enabled. Defaults to /system_metrics. + std::string publish_topic = "system_metrics"; + + // Topic statistics publication period in ms. Defaults to one minute. + std::chrono::milliseconds publish_period{60000}; }; TopicStatisticsOptions topic_stats_options; diff --git a/rclcpp/test/test_subscription_options.cpp b/rclcpp/test/test_subscription_options.cpp new file mode 100644 index 0000000000..d7d110dfd8 --- /dev/null +++ b/rclcpp/test/test_subscription_options.cpp @@ -0,0 +1,44 @@ +// Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// +// 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 + +#include +#include +#include + +#include "rclcpp/subscription_options.hpp" + +using namespace std::chrono_literals; + +namespace +{ +constexpr const char defaultPublishTopic[] = "system_metrics"; +} + +TEST(TestSubscriptionOptions, topic_statistics_options) { + auto options = rclcpp::SubscriptionOptions(); + + EXPECT_EQ(options.topic_stats_options.state, rclcpp::TopicStatisticsState::DISABLED); + EXPECT_EQ(options.topic_stats_options.publish_topic, defaultPublishTopic); + EXPECT_EQ(options.topic_stats_options.publish_period, 1min); + + options.topic_stats_options.state = rclcpp::TopicStatisticsState::ENABLED; + options.topic_stats_options.publish_topic = "topic_statistics"; + options.topic_stats_options.publish_period = 5min; + + EXPECT_EQ(options.topic_stats_options.state, rclcpp::TopicStatisticsState::ENABLED); + EXPECT_EQ(options.topic_stats_options.publish_topic, "topic_statistics"); + EXPECT_EQ(options.topic_stats_options.publish_period, 5min); +} From bdeedf4fa43c59800d3d2ab1bafbd900bcad57f4 Mon Sep 17 00:00:00 2001 From: Prajakta Gokhale Date: Mon, 13 Apr 2020 11:18:41 -0700 Subject: [PATCH 3/4] Address review comments Signed-off-by: Prajakta Gokhale --- rclcpp/include/rclcpp/subscription_options.hpp | 10 +++++----- rclcpp/test/test_subscription_options.cpp | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rclcpp/include/rclcpp/subscription_options.hpp b/rclcpp/include/rclcpp/subscription_options.hpp index 85857f6cb1..f65a9a54ce 100644 --- a/rclcpp/include/rclcpp/subscription_options.hpp +++ b/rclcpp/include/rclcpp/subscription_options.hpp @@ -56,20 +56,20 @@ struct SubscriptionOptionsBase std::shared_ptr rmw_implementation_payload = nullptr; - // Options to configure topic statistics collector in the subscription + // Options to configure topic statistics collector in the subscription. struct TopicStatisticsOptions { - // Represent the state of topic statistics collector + // Represent the state of topic statistics collector. enum class TopicStatisticsState {ENABLED, DISABLED}; // Enable and disable topic statistics calculation and publication. Defaults to disabled. TopicStatisticsState state = TopicStatisticsState::DISABLED; - // Topic to which topic statistics get published when enabled. Defaults to /system_metrics. - std::string publish_topic = "system_metrics"; + // Topic to which topic statistics get published when enabled. Defaults to /statistics. + std::string publish_topic = "/statistics"; // Topic statistics publication period in ms. Defaults to one minute. - std::chrono::milliseconds publish_period{60000}; + std::chrono::milliseconds publish_period{std::chrono::minutes(1)}; }; TopicStatisticsOptions topic_stats_options; diff --git a/rclcpp/test/test_subscription_options.cpp b/rclcpp/test/test_subscription_options.cpp index d7d110dfd8..e78f735107 100644 --- a/rclcpp/test/test_subscription_options.cpp +++ b/rclcpp/test/test_subscription_options.cpp @@ -24,7 +24,7 @@ using namespace std::chrono_literals; namespace { -constexpr const char defaultPublishTopic[] = "system_metrics"; +constexpr const char defaultPublishTopic[] = "/statistics"; } TEST(TestSubscriptionOptions, topic_statistics_options) { From 7a6ba9bbce341c77c5a686f8c08151daf310986d Mon Sep 17 00:00:00 2001 From: Prajakta Gokhale Date: Sun, 19 Apr 2020 17:12:48 -0700 Subject: [PATCH 4/4] Make default publish period 1sec Signed-off-by: Prajakta Gokhale --- rclcpp/include/rclcpp/subscription_options.hpp | 2 +- rclcpp/test/test_subscription_options.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rclcpp/include/rclcpp/subscription_options.hpp b/rclcpp/include/rclcpp/subscription_options.hpp index f65a9a54ce..72e1f4b3ed 100644 --- a/rclcpp/include/rclcpp/subscription_options.hpp +++ b/rclcpp/include/rclcpp/subscription_options.hpp @@ -69,7 +69,7 @@ struct SubscriptionOptionsBase std::string publish_topic = "/statistics"; // Topic statistics publication period in ms. Defaults to one minute. - std::chrono::milliseconds publish_period{std::chrono::minutes(1)}; + std::chrono::milliseconds publish_period{std::chrono::seconds(1)}; }; TopicStatisticsOptions topic_stats_options; diff --git a/rclcpp/test/test_subscription_options.cpp b/rclcpp/test/test_subscription_options.cpp index e78f735107..e884b03a98 100644 --- a/rclcpp/test/test_subscription_options.cpp +++ b/rclcpp/test/test_subscription_options.cpp @@ -32,7 +32,7 @@ TEST(TestSubscriptionOptions, topic_statistics_options) { EXPECT_EQ(options.topic_stats_options.state, rclcpp::TopicStatisticsState::DISABLED); EXPECT_EQ(options.topic_stats_options.publish_topic, defaultPublishTopic); - EXPECT_EQ(options.topic_stats_options.publish_period, 1min); + EXPECT_EQ(options.topic_stats_options.publish_period, 1s); options.topic_stats_options.state = rclcpp::TopicStatisticsState::ENABLED; options.topic_stats_options.publish_topic = "topic_statistics";