From f6cd4d20a10f0a7770bd392605b69eadb3c043e3 Mon Sep 17 00:00:00 2001 From: Martin Pecka Date: Sun, 9 Apr 2023 20:42:30 +0200 Subject: [PATCH] roscpp: Fixed remapping of private parameters in anonymous nodes. --- clients/roscpp/src/libros/this_node.cpp | 16 ++++ test/test_roscpp/test/CMakeLists.txt | 2 + .../launch/anonymous_node_remap_private.xml | 23 +++++ test/test_roscpp/test/src/CMakeLists.txt | 5 ++ .../test/src/anonymous_node_remap_private.cpp | 88 +++++++++++++++++++ 5 files changed, 134 insertions(+) create mode 100644 test/test_roscpp/test/launch/anonymous_node_remap_private.xml create mode 100644 test/test_roscpp/test/src/anonymous_node_remap_private.cpp diff --git a/clients/roscpp/src/libros/this_node.cpp b/clients/roscpp/src/libros/this_node.cpp index 0a2497672a..b03e350672 100644 --- a/clients/roscpp/src/libros/this_node.cpp +++ b/clients/roscpp/src/libros/this_node.cpp @@ -167,6 +167,22 @@ void ThisNode::init(const std::string& name, const M_string& remappings, uint32_ char buf[200]; std::snprintf(buf, sizeof(buf), "_%llu", (unsigned long long)WallTime::now().toNSec()); name_ += buf; + + // names::init() above did not yet know the full name of the node, so the + // remappings for private names it set are wrong. This second call fixes + // that. However, the old remappings (without the anonymizing part) still + // remain set. There is no easy way to remove them, so we keep them there. + M_string privateRemappings; + for (const auto& leftRight : remappings) + { + const auto& left = leftRight.first; + if (!left.empty() && left[0] == '~') + { + privateRemappings[left] = leftRight.second; + } + } + + names::init(privateRemappings); } ros::console::setFixedFilterToken("node", name_); diff --git a/test/test_roscpp/test/CMakeLists.txt b/test/test_roscpp/test/CMakeLists.txt index 122f9d5b6b..b5679aca5d 100644 --- a/test/test_roscpp/test/CMakeLists.txt +++ b/test/test_roscpp/test/CMakeLists.txt @@ -156,3 +156,5 @@ add_rostest(launch/multiple_latched_publishers.xml) # Test spinners add_rostest(launch/spinners.xml) + +add_rostest(launch/anonymous_node_remap_private.xml DEPENDENCIES ${PROJECT_NAME}-anonymous_node_remap_private) diff --git a/test/test_roscpp/test/launch/anonymous_node_remap_private.xml b/test/test_roscpp/test/launch/anonymous_node_remap_private.xml new file mode 100644 index 0000000000..3ccfa3f3d7 --- /dev/null +++ b/test/test_roscpp/test/launch/anonymous_node_remap_private.xml @@ -0,0 +1,23 @@ + + + + + + + + + + + + + + + + + + + diff --git a/test/test_roscpp/test/src/CMakeLists.txt b/test/test_roscpp/test/src/CMakeLists.txt index 7d5c324314..fe51f2efe9 100644 --- a/test/test_roscpp/test/src/CMakeLists.txt +++ b/test/test_roscpp/test/src/CMakeLists.txt @@ -227,6 +227,9 @@ add_executable(${PROJECT_NAME}-subscriber EXCLUDE_FROM_ALL subscriber.cpp) target_link_libraries(${PROJECT_NAME}-subscriber ${catkin_LIBRARIES}) add_dependencies(${PROJECT_NAME}-subscriber ${std_msgs_EXPORTED_TARGETS}) +add_executable(${PROJECT_NAME}-anonymous_node_remap_private EXCLUDE_FROM_ALL anonymous_node_remap_private.cpp) +target_link_libraries(${PROJECT_NAME}-anonymous_node_remap_private ${GTEST_LIBRARIES} ${catkin_LIBRARIES}) + if(TARGET tests) add_dependencies(tests ${PROJECT_NAME}-handles @@ -294,6 +297,7 @@ if(TARGET tests) ${PROJECT_NAME}-stamped_topic_statistics_empty_timestamp ${PROJECT_NAME}-topic_statistic_frequency ${PROJECT_NAME}-multiple_latched_publishers + ${PROJECT_NAME}-anonymous_node_remap_private ) endif() @@ -361,3 +365,4 @@ add_dependencies(${PROJECT_NAME}-stamped_topic_statistics_empty_timestamp ${${PROJECT_NAME}_EXPORTED_TARGETS}) add_dependencies(${PROJECT_NAME}-topic_statistic_frequency ${${PROJECT_NAME}_EXPORTED_TARGETS}) add_dependencies(${PROJECT_NAME}-multiple_latched_publishers ${${PROJECT_NAME}_EXPORTED_TARGETS}) +add_dependencies(${PROJECT_NAME}-anonymous_node_remap_private ${${PROJECT_NAME}_EXPORTED_TARGETS}) diff --git a/test/test_roscpp/test/src/anonymous_node_remap_private.cpp b/test/test_roscpp/test/src/anonymous_node_remap_private.cpp new file mode 100644 index 0000000000..2822fbc73a --- /dev/null +++ b/test/test_roscpp/test/src/anonymous_node_remap_private.cpp @@ -0,0 +1,88 @@ +/* + * SPDX-License-Identifier: BSD-3-Clause + * SPDX-FileCopyrightText: Open Source Robotics Foundation + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the Willow Garage, Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include + +#include +#include + +std::string name_; +bool removeName_ {false}; + +/** + * \brief This test tests that even anonymous nodes launched directly via rosrun + * (without an explicit __name:= arg) will get the private names remapped + * correctly. + */ +TEST(AnonymousNodeRemapPrivate, test) +{ + ros::NodeHandle pnh("~"); + + if (!removeName_) + { + EXPECT_EQ(name_, ros::this_node::getName()); + EXPECT_EQ(name_, pnh.getNamespace()); + } + + const auto remappedName = "/remapped"; + EXPECT_EQ(remappedName, pnh.resolveName("topic")); + EXPECT_EQ(remappedName, ros::names::resolve("~topic")); +} + +int main(int argc, char** argv) +{ + testing::InitGoogleTest(&argc, argv); + + // the first arg of this test has to be the name of the test node, or "noname" + // "noname" means that no __name:= will be passed to node::init(). + + name_ = ros::names::resolve(argv[1], false); + removeName_ = name_ == "/noname"; + + // remove __name:= so that ros::init() does not use it + if (removeName_) + { + for (int i = 0; i < argc; ++i) + { + if (std::string(argv[i]).find("__name:=") == 0) + { + std::swap(argv[i], argv[argc - 1]); + --argc; + break; + } + } + } + + ros::init(argc, argv, "anonymous_node_remap_private", + ros::InitOption::AnonymousName); + return RUN_ALL_TESTS(); +}