From 5029e4fdecf7c3e0aed1761caba85a49feeebcba Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 26 May 2020 13:37:01 -0300 Subject: [PATCH 1/4] Change namespacing-specific API visibility. Signed-off-by: Michel Hidalgo --- .../include/rmw_fastrtps_shared_cpp/namespace_prefix.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/namespace_prefix.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/namespace_prefix.hpp index 08bb3a787..a2063a0ab 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/namespace_prefix.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/namespace_prefix.hpp @@ -36,18 +36,22 @@ RMW_FASTRTPS_SHARED_CPP_PUBLIC extern const std::vector _ros_prefix * \return name stripped of prefix, or * \return "" if name doesn't start with prefix */ +RMW_FASTRTPS_SHARED_CPP_PUBLIC std::string _resolve_prefix(const std::string & name, const std::string & prefix); /// Return the ROS specific prefix if it exists, otherwise "". +RMW_FASTRTPS_SHARED_CPP_PUBLIC std::string _get_ros_prefix_if_exists(const std::string & topic_name); /// Returns the topic name stripped of and ROS specific prefix if exists. +RMW_FASTRTPS_SHARED_CPP_PUBLIC std::string _strip_ros_prefix_if_exists(const std::string & topic_name); /// Returns the list of ros prefixes +RMW_FASTRTPS_SHARED_CPP_PUBLIC const std::vector & _get_all_ros_prefixes(); #endif // RMW_FASTRTPS_SHARED_CPP__NAMESPACE_PREFIX_HPP_ From 2a0acb3e83cded5bcd4261cf8db3a9902a5e0822 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 18 May 2020 15:46:21 -0300 Subject: [PATCH 2/4] Add test coverage for name mangling and namespacing-specific API. Signed-off-by: Michel Hidalgo --- .../include/rmw_fastrtps_shared_cpp/names.hpp | 2 + .../src/namespace_prefix.cpp | 18 ++-- rmw_fastrtps_shared_cpp/test/CMakeLists.txt | 6 ++ rmw_fastrtps_shared_cpp/test/test_names.cpp | 90 +++++++++++++++++++ 4 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 rmw_fastrtps_shared_cpp/test/test_names.cpp diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/names.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/names.hpp index f4aff1c16..b19e95706 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/names.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/names.hpp @@ -60,6 +60,8 @@ _create_topic_name( const char * base, const char * suffix = nullptr) { + assert(qos_profile); + assert(base); if (qos_profile->avoid_ros_namespace_conventions) { prefix = nullptr; } diff --git a/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp b/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp index f42b689e1..4945e8518 100644 --- a/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp +++ b/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp @@ -31,8 +31,10 @@ const std::vector _ros_prefixes = std::string _resolve_prefix(const std::string & name, const std::string & prefix) { - if (name.rfind(prefix, 0) == 0 && name.at(prefix.length()) == '/') { - return name.substr(prefix.length()); + if (name.length() > prefix.length()) { + if (name.rfind(prefix, 0) == 0 && name.at(prefix.length()) == '/') { + return name.substr(prefix.length()); + } } return ""; } @@ -42,8 +44,10 @@ std::string _get_ros_prefix_if_exists(const std::string & topic_name) { for (const auto & prefix : _ros_prefixes) { - if (topic_name.rfind(prefix, 0) == 0 && topic_name.at(prefix.length()) == '/') { - return prefix; + if (topic_name.length() > prefix.length()) { + if (topic_name.rfind(prefix, 0) == 0 && topic_name.at(prefix.length()) == '/') { + return prefix; + } } } return ""; @@ -54,8 +58,10 @@ std::string _strip_ros_prefix_if_exists(const std::string & topic_name) { for (const auto & prefix : _ros_prefixes) { - if (topic_name.rfind(prefix, 0) == 0 && topic_name.at(prefix.length()) == '/') { - return topic_name.substr(prefix.length()); + if (topic_name.length() > prefix.length()) { + if (topic_name.rfind(prefix, 0) == 0 && topic_name.at(prefix.length()) == '/') { + return topic_name.substr(prefix.length()); + } } } return topic_name; diff --git a/rmw_fastrtps_shared_cpp/test/CMakeLists.txt b/rmw_fastrtps_shared_cpp/test/CMakeLists.txt index d434ea6e4..3db9b12e6 100644 --- a/rmw_fastrtps_shared_cpp/test/CMakeLists.txt +++ b/rmw_fastrtps_shared_cpp/test/CMakeLists.txt @@ -23,3 +23,9 @@ ament_add_gtest(test_guid_utils test_guid_utils.cpp) if(TARGET test_guid_utils) target_link_libraries(test_guid_utils ${PROJECT_NAME}) endif() + +ament_add_gtest(test_names test_names.cpp) +if(TARGET test_names) + ament_target_dependencies(test_names rmw) + target_link_libraries(test_names ${PROJECT_NAME}) +endif() diff --git a/rmw_fastrtps_shared_cpp/test/test_names.cpp b/rmw_fastrtps_shared_cpp/test/test_names.cpp new file mode 100644 index 000000000..6be353ee6 --- /dev/null +++ b/rmw_fastrtps_shared_cpp/test/test_names.cpp @@ -0,0 +1,90 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// 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 "rmw/qos_profiles.h" + +#include "rmw_fastrtps_shared_cpp/names.hpp" +#include "rmw_fastrtps_shared_cpp/namespace_prefix.hpp" + +TEST(NamespaceTest, get_prefix) { + EXPECT_EQ("", _get_ros_prefix_if_exists("")); + EXPECT_EQ("", _get_ros_prefix_if_exists("not/a_ros_prefix")); + for (const auto & prefix : _get_all_ros_prefixes()) { + EXPECT_EQ("", _get_ros_prefix_if_exists(prefix)); + EXPECT_EQ("", _get_ros_prefix_if_exists(prefix + "_should_not_match")); + EXPECT_EQ("", _get_ros_prefix_if_exists("th/is_should_not_match/" + prefix)); + EXPECT_EQ(prefix, _get_ros_prefix_if_exists(prefix + "/")); + EXPECT_EQ(prefix, _get_ros_prefix_if_exists(prefix + "/should_match")); + } +} + +TEST(NamespaceTest, strip_prefix) { + EXPECT_EQ("", _strip_ros_prefix_if_exists("")); + EXPECT_EQ("/no_ros_prefix/test", _strip_ros_prefix_if_exists("/no_ros_prefix/test")); + for (const auto & prefix : _get_all_ros_prefixes()) { + EXPECT_EQ(prefix, _strip_ros_prefix_if_exists(prefix)); + EXPECT_EQ("/", _strip_ros_prefix_if_exists(prefix + "/")); + EXPECT_EQ( + prefix + "should_not_be_stripped", + _strip_ros_prefix_if_exists(prefix + "should_not_be_stripped")); + EXPECT_EQ( + "th/is_should_not_be_stripped/" + prefix, + _strip_ros_prefix_if_exists("th/is_should_not_be_stripped/" + prefix)); + EXPECT_EQ("/should_be_stripped", _strip_ros_prefix_if_exists(prefix + "/should_be_stripped")); + } +} + +TEST(NamespaceTest, resolve_prefix) { + EXPECT_EQ("", _resolve_prefix("", "")); + EXPECT_EQ("", _resolve_prefix("", "some_ros_prefix")); + EXPECT_EQ("", _resolve_prefix("/test", "some_ros_prefix")); + EXPECT_EQ("/test", _resolve_prefix("/test", "")); + EXPECT_EQ("", _resolve_prefix("some_ros_prefix", "some_ros_prefix")); + EXPECT_EQ("/", _resolve_prefix("some_ros_prefix/", "some_ros_prefix")); + EXPECT_EQ( + "/test_some_ros_prefix", + _resolve_prefix("some_ros_prefix/test_some_ros_prefix", "some_ros_prefix")); + EXPECT_EQ( + "", _resolve_prefix("some_ros_prefix_test", "some_ros_prefix")); + EXPECT_EQ( + "", _resolve_prefix("this_ros_prefix/test/some_ros_prefix", "some_ros_prefix")); +} + +TEST(NamespaceTest, name_mangling) { + rmw_qos_profile_t qos_profile = rmw_qos_profile_unknown; + qos_profile.avoid_ros_namespace_conventions = false; + + EXPECT_DEATH(_create_topic_name(nullptr, "", "", ""), "qos_profile"); + + EXPECT_DEATH(_create_topic_name(&qos_profile, "", nullptr, ""), "base"); + + EXPECT_STREQ( + "some_ros_prefix/test__suffix", _create_topic_name( + &qos_profile, "some_ros_prefix", "/test", "__suffix").c_str()); + + EXPECT_STREQ( + "/test__suffix", _create_topic_name( + &qos_profile, nullptr, "/test", "__suffix").c_str()); + + EXPECT_STREQ( + "some_ros_prefix/test", _create_topic_name( + &qos_profile, "some_ros_prefix", "/test", nullptr).c_str()); + + qos_profile.avoid_ros_namespace_conventions = true; + EXPECT_STREQ( + "/test__suffix", _create_topic_name( + &qos_profile, "some_ros_prefix", "/test", "__suffix").c_str()); +} From e0fe9ca02c9d9287f990f7855f87b957e6470f48 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 26 May 2020 16:56:05 -0300 Subject: [PATCH 3/4] Simplify namespacing utilities implementation. Signed-off-by: Michel Hidalgo --- .../src/namespace_prefix.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp b/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp index 4945e8518..d02fd0d51 100644 --- a/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp +++ b/rmw_fastrtps_shared_cpp/src/namespace_prefix.cpp @@ -31,10 +31,8 @@ const std::vector _ros_prefixes = std::string _resolve_prefix(const std::string & name, const std::string & prefix) { - if (name.length() > prefix.length()) { - if (name.rfind(prefix, 0) == 0 && name.at(prefix.length()) == '/') { - return name.substr(prefix.length()); - } + if (name.rfind(prefix + "/", 0) == 0) { + return name.substr(prefix.length()); } return ""; } @@ -44,10 +42,8 @@ std::string _get_ros_prefix_if_exists(const std::string & topic_name) { for (const auto & prefix : _ros_prefixes) { - if (topic_name.length() > prefix.length()) { - if (topic_name.rfind(prefix, 0) == 0 && topic_name.at(prefix.length()) == '/') { - return prefix; - } + if (topic_name.rfind(prefix + "/", 0) == 0) { + return prefix; } } return ""; @@ -58,10 +54,8 @@ std::string _strip_ros_prefix_if_exists(const std::string & topic_name) { for (const auto & prefix : _ros_prefixes) { - if (topic_name.length() > prefix.length()) { - if (topic_name.rfind(prefix, 0) == 0 && topic_name.at(prefix.length()) == '/') { - return topic_name.substr(prefix.length()); - } + if (topic_name.rfind(prefix + "/", 0) == 0) { + return topic_name.substr(prefix.length()); } } return topic_name; From bb21fc458862579a40d8bdc61858b2ab1e972fd6 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 26 May 2020 16:57:45 -0300 Subject: [PATCH 4/4] Drop death tests matchers. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_shared_cpp/test/test_names.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/test/test_names.cpp b/rmw_fastrtps_shared_cpp/test/test_names.cpp index 6be353ee6..3d9a40bcc 100644 --- a/rmw_fastrtps_shared_cpp/test/test_names.cpp +++ b/rmw_fastrtps_shared_cpp/test/test_names.cpp @@ -67,9 +67,9 @@ TEST(NamespaceTest, name_mangling) { rmw_qos_profile_t qos_profile = rmw_qos_profile_unknown; qos_profile.avoid_ros_namespace_conventions = false; - EXPECT_DEATH(_create_topic_name(nullptr, "", "", ""), "qos_profile"); + EXPECT_DEATH(_create_topic_name(nullptr, "", "", ""), ""); - EXPECT_DEATH(_create_topic_name(&qos_profile, "", nullptr, ""), "base"); + EXPECT_DEATH(_create_topic_name(&qos_profile, "", nullptr, ""), ""); EXPECT_STREQ( "some_ros_prefix/test__suffix", _create_topic_name(