From cef91930e795b5b45cad022249a5bbb3a12ea377 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Thu, 27 Apr 2023 15:34:03 -0700 Subject: [PATCH 1/4] Decouple rosout publisher init from node init. Signed-off-by: Tomoya Fujita --- rcl/src/rcl/node.c | 10 ---------- rcl/test/rcl/test_graph.cpp | 10 ++++++++++ rcl/test/rcl/test_logging_rosout.cpp | 4 ++++ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index fa09fe77e..fbff5839a 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -282,16 +282,6 @@ rcl_node_init( goto fail; } - // The initialization for the rosout publisher requires the node to be initialized to a point - // that it can create new topic publishers - if (rcl_logging_rosout_enabled() && node->impl->options.enable_rosout) { - ret = rcl_logging_rosout_init_publisher_for_node(node); - if (ret != RCL_RET_OK) { - // error message already set - goto fail; - } - } - RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Node initialized"); TRACETOOLS_TRACEPOINT( rcl_node_init, diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 4ac5e7a9a..d58725a2d 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -31,6 +31,7 @@ #include "rcl/error_handling.h" #include "rcl/graph.h" #include "rcl/logging.h" +#include "rcl/logging_rosout.h" #include "rcl/rcl.h" #include "rcutils/logging_macros.h" @@ -99,6 +100,10 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test const char * name = "test_graph_node"; ret = rcl_node_init(this->node_ptr, name, "", this->context_ptr, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (rcl_logging_rosout_enabled() && node_options.enable_rosout) { + ret = rcl_logging_rosout_init_publisher_for_node(this->node_ptr); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + } this->wait_set_ptr = new rcl_wait_set_t; *this->wait_set_ptr = rcl_get_zero_initialized_wait_set(); @@ -949,6 +954,11 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME remote_node_ptr, remote_node_name, "", this->remote_context_ptr, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (rcl_logging_rosout_enabled() && node_options.enable_rosout) { + ret = rcl_logging_rosout_init_publisher_for_node(remote_node_ptr); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + } + sub_func = std::bind( rcl_get_subscriber_names_and_types_by_node, std::placeholders::_1, diff --git a/rcl/test/rcl/test_logging_rosout.cpp b/rcl/test/rcl/test_logging_rosout.cpp index e14f11fe7..b5b7e7871 100644 --- a/rcl/test/rcl/test_logging_rosout.cpp +++ b/rcl/test/rcl/test_logging_rosout.cpp @@ -101,6 +101,10 @@ class TestLoggingRosout : public ::testing::Test ret = rcl_node_init( this->node_ptr, name, namespace_, this->context_ptr, &node_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (rcl_logging_rosout_enabled() && node_options.enable_rosout) { + ret = rcl_logging_rosout_init_publisher_for_node(this->node_ptr); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + } // create rosout subscription const rosidl_message_type_support_t * ts = From ce0471e8387c2d4f1d957b490c101c0fc42446a8 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Mon, 11 Sep 2023 13:11:21 -0700 Subject: [PATCH 2/4] Decouple rosout publisher fini from node fini. Signed-off-by: Tomoya Fujita --- rcl/src/rcl/node.c | 7 ------- rcl/test/rcl/test_graph.cpp | 8 ++++++++ rcl/test/rcl/test_logging_rosout.cpp | 4 ++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index fbff5839a..35f995e42 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -353,13 +353,6 @@ rcl_node_fini(rcl_node_t * node) rcl_allocator_t allocator = node->impl->options.allocator; rcl_ret_t result = RCL_RET_OK; rcl_ret_t rcl_ret = RCL_RET_OK; - if (rcl_logging_rosout_enabled() && node->impl->options.enable_rosout) { - rcl_ret = rcl_logging_rosout_fini_publisher_for_node(node); - if (rcl_ret != RCL_RET_OK && rcl_ret != RCL_RET_NOT_INIT) { - RCL_SET_ERROR_MSG("Unable to fini publisher for node."); - result = RCL_RET_ERROR; - } - } rcl_ret = rcl_node_type_cache_fini(node); if (rcl_ret != RCL_RET_OK) { RCL_SET_ERROR_MSG("Unable to fini type cache for node."); diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index d58725a2d..f77d9b81e 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -123,6 +123,10 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test delete this->wait_set_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (rcl_logging_rosout_enabled()) { + ret = rcl_logging_rosout_fini_publisher_for_node(this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + } ret = rcl_node_fini(this->node_ptr); delete this->node_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -996,6 +1000,10 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME { rcl_ret_t ret; CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION) ::TearDown(); + if (rcl_logging_rosout_enabled()) { + ret = rcl_logging_rosout_fini_publisher_for_node(this->remote_node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + } ret = rcl_node_fini(this->remote_node_ptr); delete this->remote_node_ptr; diff --git a/rcl/test/rcl/test_logging_rosout.cpp b/rcl/test/rcl/test_logging_rosout.cpp index b5b7e7871..6dc6ef65d 100644 --- a/rcl/test/rcl/test_logging_rosout.cpp +++ b/rcl/test/rcl/test_logging_rosout.cpp @@ -123,6 +123,10 @@ class TestLoggingRosout : public ::testing::Test rcl_ret_t ret = rcl_subscription_fini(this->subscription_ptr, this->node_ptr); delete this->subscription_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + if (rcl_logging_rosout_enabled()) { + ret = rcl_logging_rosout_fini_publisher_for_node(this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + } ret = rcl_node_fini(this->node_ptr); delete this->node_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; From 5abc429578a39a23666b38ff3e78e85a8c53779e Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Tue, 19 Sep 2023 11:09:47 -0700 Subject: [PATCH 3/4] address review comment. Signed-off-by: Tomoya Fujita --- rcl/test/rcl/test_logging_rosout.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_logging_rosout.cpp b/rcl/test/rcl/test_logging_rosout.cpp index 6dc6ef65d..59ed82a6f 100644 --- a/rcl/test/rcl/test_logging_rosout.cpp +++ b/rcl/test/rcl/test_logging_rosout.cpp @@ -123,7 +123,7 @@ class TestLoggingRosout : public ::testing::Test rcl_ret_t ret = rcl_subscription_fini(this->subscription_ptr, this->node_ptr); delete this->subscription_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - if (rcl_logging_rosout_enabled()) { + if (rcl_logging_rosout_enabled() && node_options.enable_rosout) { ret = rcl_logging_rosout_fini_publisher_for_node(this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } From cefce8a9077cc920f86b111af4a15b94d43ef1f7 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Wed, 20 Sep 2023 11:11:37 -0700 Subject: [PATCH 4/4] add node_option check. Signed-off-by: Tomoya Fujita --- rcl/test/rcl/test_graph.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index f77d9b81e..742dba992 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -123,7 +123,8 @@ class CLASSNAME (TestGraphFixture, RMW_IMPLEMENTATION) : public ::testing::Test delete this->wait_set_ptr; EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - if (rcl_logging_rosout_enabled()) { + const rcl_node_options_t * node_ops = rcl_node_get_options(this->node_ptr); + if (rcl_logging_rosout_enabled() && node_ops->enable_rosout) { ret = rcl_logging_rosout_fini_publisher_for_node(this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } @@ -1000,7 +1001,8 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME { rcl_ret_t ret; CLASSNAME(TestGraphFixture, RMW_IMPLEMENTATION) ::TearDown(); - if (rcl_logging_rosout_enabled()) { + const rcl_node_options_t * node_ops = rcl_node_get_options(this->remote_node_ptr); + if (rcl_logging_rosout_enabled() && node_ops->enable_rosout) { ret = rcl_logging_rosout_fini_publisher_for_node(this->remote_node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; }