niklas/rclcpp
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix returning invalid namespace if sub_namespace is empty (#1658) (#1811)

Browse source 
* Create valid effective namespace when sub-namespace is empty

Fix #1656.

Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>

* Add regression test for effective namespace and empty sub-namespace

Adds regression test for #1656.

Signed-off-by: Markus Hofstaetter <markus.hofstaetter@ait.ac.at>
(cherry picked from commit 3cddb4edab317758dc8a8cac94b90794641c7488)

Co-authored-by: M. Hofstätter <markus.hofstaetter@gmx.net>
This commit is contained in:
mergify[bot] 2021-11-11 09:22:24 -05:00 • committed by GitHub
parent d7804e1b3f
commit a58f8c1de4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

12
rclcpp/src/rclcpp/node.cpp
View file

@ -54,6 +54,12 @@ extend_sub_namespace(const std::string & existing_sub_namespace, const std::stri
extension.c_str(), extension.c_str(),
"a sub-namespace should not have a leading /", "a sub-namespace should not have a leading /",
0); 0);
} else if (existing_sub_namespace.empty() && extension.empty()) {
throw rclcpp::exceptions::NameValidationError(
"sub_namespace",
extension.c_str(),
"sub-nodes should not extend nodes by an empty sub-namespace",
0);
} }
std::string new_sub_namespace; std::string new_sub_namespace;
@ -79,7 +85,11 @@ create_effective_namespace(const std::string & node_namespace, const std::string
// and do not need trimming of `/` and other things, as they were validated // and do not need trimming of `/` and other things, as they were validated
// in other functions already. // in other functions already.
if (node_namespace.back() == '/') { // A node may not have a sub_namespace if it is no sub_node. In this case,
// just return the original namespace
if (sub_namespace.empty()) {
return node_namespace;
} else if (node_namespace.back() == '/') {
// this is the special case where node_namespace is just `/` // this is the special case where node_namespace is just `/`
return node_namespace + sub_namespace; return node_namespace + sub_namespace;
} else { } else {

13
rclcpp/test/rclcpp/test_node.cpp
View file

@ -94,6 +94,7 @@ TEST_F(TestNode, get_name_and_namespace) {
auto node = std::make_shared<rclcpp::Node>("my_node", "/ns"); auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
EXPECT_STREQ("my_node", node->get_name()); EXPECT_STREQ("my_node", node->get_name());
EXPECT_STREQ("/ns", node->get_namespace()); EXPECT_STREQ("/ns", node->get_namespace());
EXPECT_STREQ("/ns", node->get_effective_namespace().c_str());
EXPECT_STREQ("/ns/my_node", node->get_fully_qualified_name()); EXPECT_STREQ("/ns/my_node", node->get_fully_qualified_name());
} }
{ {
@ -108,30 +109,35 @@ TEST_F(TestNode, get_name_and_namespace) {
auto node = std::make_shared<rclcpp::Node>("my_node", "ns"); auto node = std::make_shared<rclcpp::Node>("my_node", "ns");
EXPECT_STREQ("my_node", node->get_name()); EXPECT_STREQ("my_node", node->get_name());
EXPECT_STREQ("/ns", node->get_namespace()); EXPECT_STREQ("/ns", node->get_namespace());
EXPECT_STREQ("/ns", node->get_effective_namespace().c_str());
EXPECT_STREQ("/ns/my_node", node->get_fully_qualified_name()); EXPECT_STREQ("/ns/my_node", node->get_fully_qualified_name());
} }
{ {
auto node = std::make_shared<rclcpp::Node>("my_node"); auto node = std::make_shared<rclcpp::Node>("my_node");
EXPECT_STREQ("my_node", node->get_name()); EXPECT_STREQ("my_node", node->get_name());
EXPECT_STREQ("/", node->get_namespace()); EXPECT_STREQ("/", node->get_namespace());
EXPECT_STREQ("/", node->get_effective_namespace().c_str());
EXPECT_STREQ("/my_node", node->get_fully_qualified_name()); EXPECT_STREQ("/my_node", node->get_fully_qualified_name());
} }
{ {
auto node = std::make_shared<rclcpp::Node>("my_node", ""); auto node = std::make_shared<rclcpp::Node>("my_node", "");
EXPECT_STREQ("my_node", node->get_name()); EXPECT_STREQ("my_node", node->get_name());
EXPECT_STREQ("/", node->get_namespace()); EXPECT_STREQ("/", node->get_namespace());
EXPECT_STREQ("/", node->get_effective_namespace().c_str());
EXPECT_STREQ("/my_node", node->get_fully_qualified_name()); EXPECT_STREQ("/my_node", node->get_fully_qualified_name());
} }
{ {
auto node = std::make_shared<rclcpp::Node>("my_node", "/my/ns"); auto node = std::make_shared<rclcpp::Node>("my_node", "/my/ns");
EXPECT_STREQ("my_node", node->get_name()); EXPECT_STREQ("my_node", node->get_name());
EXPECT_STREQ("/my/ns", node->get_namespace()); EXPECT_STREQ("/my/ns", node->get_namespace());
EXPECT_STREQ("/my/ns", node->get_effective_namespace().c_str());
EXPECT_STREQ("/my/ns/my_node", node->get_fully_qualified_name()); EXPECT_STREQ("/my/ns/my_node", node->get_fully_qualified_name());
} }
{ {
auto node = std::make_shared<rclcpp::Node>("my_node", "my/ns"); auto node = std::make_shared<rclcpp::Node>("my_node", "my/ns");
EXPECT_STREQ("my_node", node->get_name()); EXPECT_STREQ("my_node", node->get_name());
EXPECT_STREQ("/my/ns", node->get_namespace()); EXPECT_STREQ("/my/ns", node->get_namespace());
EXPECT_STREQ("/my/ns", node->get_effective_namespace().c_str());
EXPECT_STREQ("/my/ns/my_node", node->get_fully_qualified_name()); EXPECT_STREQ("/my/ns/my_node", node->get_fully_qualified_name());
} }
{ {
@ -270,6 +276,13 @@ TEST_F(TestNode, subnode_construction_and_destruction) {
auto subnode = node->create_sub_node("~sub_ns"); auto subnode = node->create_sub_node("~sub_ns");
}, rclcpp::exceptions::InvalidNamespaceError); }, rclcpp::exceptions::InvalidNamespaceError);
} }
{
ASSERT_THROW(
{
auto node = std::make_shared<rclcpp::Node>("my_node", "/ns");
auto subnode = node->create_sub_node("");
}, rclcpp::exceptions::NameValidationError);
}
} }
TEST_F(TestNode, get_logger) { TEST_F(TestNode, get_logger) {