From 1f27a3a00e3672c0d1d527fe4cc7b5195d5f9f69 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Thu, 29 Aug 2019 17:40:18 -0300 Subject: [PATCH 1/5] Add specific return type for non existent node Signed-off-by: ivanpauno --- rcl/include/rcl/graph.h | 4 ++++ rcl/include/rcl/types.h | 2 ++ rcl/src/rcl/common.c | 2 ++ 3 files changed, 8 insertions(+) diff --git a/rcl/include/rcl/graph.h b/rcl/include/rcl/graph.h index 5053d5d3a..e5a0b61b7 100644 --- a/rcl/include/rcl/graph.h +++ b/rcl/include/rcl/graph.h @@ -77,6 +77,7 @@ typedef rmw_names_and_types_t rcl_names_and_types_t; * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or + * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC @@ -126,6 +127,7 @@ rcl_get_publisher_names_and_types_by_node( * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or + * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC @@ -174,6 +176,7 @@ rcl_get_subscriber_names_and_types_by_node( * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or + * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC @@ -221,6 +224,7 @@ rcl_get_service_names_and_types_by_node( * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or + * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC diff --git a/rcl/include/rcl/types.h b/rcl/include/rcl/types.h index 86f988823..b6cd80775 100644 --- a/rcl/include/rcl/types.h +++ b/rcl/include/rcl/types.h @@ -52,6 +52,8 @@ typedef rmw_ret_t rcl_ret_t; #define RCL_RET_NODE_INVALID 200 #define RCL_RET_NODE_INVALID_NAME 201 #define RCL_RET_NODE_INVALID_NAMESPACE 202 +/// Failed to find node name +#define RCL_RET_NON_EXISTENT_NODE_NAME 203 // rcl publisher specific ret codes in 3XX /// Invalid rcl_publisher_t given return code. diff --git a/rcl/src/rcl/common.c b/rcl/src/rcl/common.c index 9c1d6d96c..55ca18219 100644 --- a/rcl/src/rcl/common.c +++ b/rcl/src/rcl/common.c @@ -65,6 +65,8 @@ rcl_convert_rmw_ret_to_rcl_ret(rmw_ret_t rmw_ret) return RCL_RET_BAD_ALLOC; case RMW_RET_UNSUPPORTED: return RCL_RET_UNSUPPORTED; + case RMW_RET_NON_EXISTENT_NODE_NAME: + RCL_RET_NON_EXISTENT_NODE_NAME; default: return RCL_RET_ERROR; } From 7aae62854b0ee82543d43b46d70d8e11ee7c1d82 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Fri, 30 Aug 2019 09:22:13 -0300 Subject: [PATCH 2/5] Correct error Signed-off-by: ivanpauno --- rcl/src/rcl/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/src/rcl/common.c b/rcl/src/rcl/common.c index 55ca18219..01973e743 100644 --- a/rcl/src/rcl/common.c +++ b/rcl/src/rcl/common.c @@ -66,7 +66,7 @@ rcl_convert_rmw_ret_to_rcl_ret(rmw_ret_t rmw_ret) case RMW_RET_UNSUPPORTED: return RCL_RET_UNSUPPORTED; case RMW_RET_NON_EXISTENT_NODE_NAME: - RCL_RET_NON_EXISTENT_NODE_NAME; + return RCL_RET_NON_EXISTENT_NODE_NAME; default: return RCL_RET_ERROR; } From b75b605999715368bdb3147a782cbaf0e46f18e1 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Fri, 30 Aug 2019 12:04:39 -0300 Subject: [PATCH 3/5] Address per review comments Signed-off-by: ivanpauno --- rcl/include/rcl/graph.h | 8 ++++---- rcl/include/rcl/types.h | 2 +- rcl/src/rcl/common.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rcl/include/rcl/graph.h b/rcl/include/rcl/graph.h index e5a0b61b7..28b13a4f7 100644 --- a/rcl/include/rcl/graph.h +++ b/rcl/include/rcl/graph.h @@ -77,7 +77,7 @@ typedef rmw_names_and_types_t rcl_names_and_types_t; * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or - * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or + * \return `RCL_RET_NODE_NAME_NON_EXISTENT` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC @@ -127,7 +127,7 @@ rcl_get_publisher_names_and_types_by_node( * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or - * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or + * \return `RCL_RET_NODE_NAME_NON_EXISTENT` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC @@ -176,7 +176,7 @@ rcl_get_subscriber_names_and_types_by_node( * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or - * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or + * \return `RCL_RET_NODE_NAME_NON_EXISTENT` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC @@ -224,7 +224,7 @@ rcl_get_service_names_and_types_by_node( * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_NODE_INVALID_NAME` if the node name is invalid, or * \return `RCL_RET_NODE_INVALID_NAMESPACE` if the node namespace is invalid, or - * \return `RCL_RET_NON_EXISTENT_NODE_NAME` if the node name wasn't found, or + * \return `RCL_RET_NODE_NAME_NON_EXISTENT` if the node name wasn't found, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC diff --git a/rcl/include/rcl/types.h b/rcl/include/rcl/types.h index b6cd80775..f837a9c2a 100644 --- a/rcl/include/rcl/types.h +++ b/rcl/include/rcl/types.h @@ -53,7 +53,7 @@ typedef rmw_ret_t rcl_ret_t; #define RCL_RET_NODE_INVALID_NAME 201 #define RCL_RET_NODE_INVALID_NAMESPACE 202 /// Failed to find node name -#define RCL_RET_NON_EXISTENT_NODE_NAME 203 +#define RCL_RET_NODE_NAME_NON_EXISTENT 203 // rcl publisher specific ret codes in 3XX /// Invalid rcl_publisher_t given return code. diff --git a/rcl/src/rcl/common.c b/rcl/src/rcl/common.c index 01973e743..50307aea0 100644 --- a/rcl/src/rcl/common.c +++ b/rcl/src/rcl/common.c @@ -65,8 +65,8 @@ rcl_convert_rmw_ret_to_rcl_ret(rmw_ret_t rmw_ret) return RCL_RET_BAD_ALLOC; case RMW_RET_UNSUPPORTED: return RCL_RET_UNSUPPORTED; - case RMW_RET_NON_EXISTENT_NODE_NAME: - return RCL_RET_NON_EXISTENT_NODE_NAME; + case RMW_RET_NODE_NAME_NON_EXISTENT: + return RCL_RET_NODE_NAME_NON_EXISTENT; default: return RCL_RET_ERROR; } From 6d537aeef606c26d2f707bb4e506c9edf993e96a Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Fri, 30 Aug 2019 16:58:38 -0300 Subject: [PATCH 4/5] Added name and namespace validation. Corrected tests Signed-off-by: ivanpauno --- rcl/src/rcl/graph.c | 1 + rcl/test/rcl/test_graph.cpp | 15 ++++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/rcl/src/rcl/graph.c b/rcl/src/rcl/graph.c index 331d78027..bdb267394 100644 --- a/rcl/src/rcl/graph.c +++ b/rcl/src/rcl/graph.c @@ -27,6 +27,7 @@ extern "C" #include "rmw/get_service_names_and_types.h" #include "rmw/get_topic_names_and_types.h" #include "rmw/names_and_types.h" +#include "rmw/error_handling.h" #include "rmw/rmw.h" #include "rmw/validate_namespace.h" #include "rmw/validate_node_name.h" diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index f24ec6e7c..edfa38b4e 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -329,12 +329,12 @@ TEST_F( // unknown node name ret = rcl_get_publisher_names_and_types_by_node( this->node_ptr, &allocator, false, unknown_node_name, "", &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // unknown node namespace ret = rcl_get_publisher_names_and_types_by_node( this->node_ptr, &allocator, false, this->test_graph_node_name, unknown_node_ns, &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // valid call ret = rcl_get_publisher_names_and_types_by_node( @@ -414,12 +414,12 @@ TEST_F( // unknown node name ret = rcl_get_subscriber_names_and_types_by_node( this->node_ptr, &allocator, false, unknown_node_name, "", &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // unknown node namespace ret = rcl_get_subscriber_names_and_types_by_node( this->node_ptr, &allocator, false, this->test_graph_node_name, unknown_node_ns, &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // valid call ret = rcl_get_subscriber_names_and_types_by_node( @@ -496,12 +496,12 @@ TEST_F( // unknown node name ret = rcl_get_service_names_and_types_by_node( this->node_ptr, &allocator, unknown_node_name, "", &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // unknown node namespace ret = rcl_get_service_names_and_types_by_node( this->node_ptr, &allocator, this->test_graph_node_name, unknown_node_ns, &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // valid call ret = rcl_get_service_names_and_types_by_node( @@ -525,6 +525,7 @@ TEST_F( rcl_node_t zero_node = rcl_get_zero_initialized_node(); const char * unknown_node_name = "test_rcl_get_client_names_and_types_by_node"; const char * unknown_node_ns = "/test/namespace"; + rcl_names_and_types_t nat = rcl_get_zero_initialized_names_and_types(); // invalid node ret = rcl_get_client_names_and_types_by_node( @@ -578,7 +579,7 @@ TEST_F( // unknown node name ret = rcl_get_client_names_and_types_by_node( this->node_ptr, &allocator, unknown_node_name, "", &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // unknown node namespace ret = rcl_get_client_names_and_types_by_node( From 4ac4ccffc9c8bc8107b9778ee54fa8707efd1661 Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Mon, 16 Sep 2019 18:13:57 -0300 Subject: [PATCH 5/5] Correct failures Signed-off-by: ivanpauno --- rcl/src/rcl/graph.c | 1 - rcl/test/rcl/test_graph.cpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/rcl/src/rcl/graph.c b/rcl/src/rcl/graph.c index bdb267394..331d78027 100644 --- a/rcl/src/rcl/graph.c +++ b/rcl/src/rcl/graph.c @@ -27,7 +27,6 @@ extern "C" #include "rmw/get_service_names_and_types.h" #include "rmw/get_topic_names_and_types.h" #include "rmw/names_and_types.h" -#include "rmw/error_handling.h" #include "rmw/rmw.h" #include "rmw/validate_namespace.h" #include "rmw/validate_node_name.h" diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index edfa38b4e..28f7cdae7 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -584,7 +584,7 @@ TEST_F( // unknown node namespace ret = rcl_get_client_names_and_types_by_node( this->node_ptr, &allocator, this->test_graph_node_name, unknown_node_ns, &nat); - EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str; rcl_reset_error(); // valid call ret = rcl_get_client_names_and_types_by_node(