From 5d1962f3be6e9b8d6325af42ba9f0bd40b3db7ab Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Jul 2020 13:43:58 -0300 Subject: [PATCH 1/6] Add function rcl_event_is_valid This functions checks if a passed event is a valid already init instance of a publisher/subscriber event. Signed-off-by: Jorge Perez --- rcl/include/rcl/event.h | 22 ++++++++++++++++++++++ rcl/src/rcl/event.c | 14 ++++++++++++++ rcl/test/rcl/test_events.cpp | 21 +++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/rcl/include/rcl/event.h b/rcl/include/rcl/event.h index 10a279e6a..8d791ebbf 100644 --- a/rcl/include/rcl/event.h +++ b/rcl/include/rcl/event.h @@ -170,6 +170,28 @@ RCL_WARN_UNUSED rmw_event_t * rcl_event_get_rmw_handle(const rcl_event_t * event); +/// Check that the event is valid. +/** + * The bool returned is `false` if `event` is invalid. + * The bool returned is `true` otherwise. + * In the case where `false` is to be returned, an error message is set. + * This function cannot fail. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | No + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] event pointer to the rcl event + * \return `true` if `event` is valid, otherwise `false` + */ +RCL_PUBLIC +bool +rcl_event_is_valid(const rcl_event_t * event); + #ifdef __cplusplus } #endif diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 8cc719470..f202f320e 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -190,6 +190,20 @@ rcl_event_get_rmw_handle(const rcl_event_t * event) } } +bool +rcl_event_is_valid(const rcl_event_t * event) +{ + RCL_CHECK_FOR_NULL_WITH_MSG(event, "event pointer is invalid", return false); + RCL_CHECK_FOR_NULL_WITH_MSG(event->impl, "event's implementation is invalid", return false); + /* + if (event->impl->rmw_handle.event_type == RMW_EVENT_INVALID) { + RCUTILS_SET_ERROR_MSG("event's implementation not init"); + return false; + } + */ + return true; +} + #ifdef __cplusplus } #endif diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index d645b4bbb..4b7fe4864 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -730,6 +730,27 @@ TEST_F(TestEventFixture, test_bad_get_handle) EXPECT_EQ(NULL, rcl_event_get_rmw_handle(NULL)); } +/* + * Test cases for the event_is_valid function + */ +TEST_F(TestEventFixture, test_event_is_valid) +{ + EXPECT_FALSE(rcl_event_is_valid(nullptr)); + + setup_publisher_subscriber(default_qos_profile, default_qos_profile); + rcl_event_t publisher_event_test = rcl_get_zero_initialized_event(); + EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + + rcl_ret_t ret = rcl_publisher_event_init( + &publisher_event_test, &publisher, RCL_PUBLISHER_OFFERED_DEADLINE_MISSED); + ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + EXPECT_TRUE(rcl_event_is_valid(&publisher_event_test)); + + ret = rcl_event_fini(&publisher_event_test); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + tear_down_publisher_subscriber(); +} + static std::array get_test_pubsub_incompatible_qos_inputs() From 7854bc5313145bb19cf7f739dc2ed072f56f074b Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Jul 2020 18:48:35 -0300 Subject: [PATCH 2/6] Move impl header to its own file Signed-off-by: Jorge Perez --- rcl/src/rcl/event.c | 9 +-------- rcl/src/rcl/event_impl.h | 28 ++++++++++++++++++++++++++++ rcl/test/CMakeLists.txt | 1 + rcl/test/rcl/test_events.cpp | 8 ++++++++ 4 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 rcl/src/rcl/event_impl.h diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index f202f320e..32d47903d 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -32,12 +32,7 @@ extern "C" #include "./common.h" #include "./publisher_impl.h" #include "./subscription_impl.h" - -typedef struct rcl_event_impl_t -{ - rmw_event_t rmw_handle; - rcl_allocator_t allocator; -} rcl_event_impl_t; +#include "./event_impl.h" rcl_event_t rcl_get_zero_initialized_event() @@ -195,12 +190,10 @@ rcl_event_is_valid(const rcl_event_t * event) { RCL_CHECK_FOR_NULL_WITH_MSG(event, "event pointer is invalid", return false); RCL_CHECK_FOR_NULL_WITH_MSG(event->impl, "event's implementation is invalid", return false); - /* if (event->impl->rmw_handle.event_type == RMW_EVENT_INVALID) { RCUTILS_SET_ERROR_MSG("event's implementation not init"); return false; } - */ return true; } diff --git a/rcl/src/rcl/event_impl.h b/rcl/src/rcl/event_impl.h new file mode 100644 index 000000000..1c6b8422b --- /dev/null +++ b/rcl/src/rcl/event_impl.h @@ -0,0 +1,28 @@ +// 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. + +#ifndef RCL__EVENT_IMPL_H_ +#define RCL__EVENT_IMPL_H_ + +#include "rmw/rmw.h" + +#include "rcl/event.h" + +typedef struct rcl_event_impl_t +{ + rmw_event_t rmw_handle; + rcl_allocator_t allocator; +} rcl_event_impl_t; + +#endif // RCL__EVENT_IMPL_H_ diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 9bca4c8d1..c6f823c11 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -252,6 +252,7 @@ function(test_target_function) SRCS rcl/test_events.cpp ENV ${rmw_implementation_env_var} APPEND_LIBRARY_DIRS ${extra_lib_dirs} + INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/../src/rcl/ LIBRARIES ${PROJECT_NAME} AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs" ) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 4b7fe4864..e0afcd5e9 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -25,12 +25,15 @@ #include "rcl/subscription.h" #include "rcl/error_handling.h" #include "rmw/incompatible_qos_events_statuses.h" +#include "rmw/event.h" #include "test_msgs/msg/strings.h" #include "rosidl_runtime_c/string_functions.h" #include "osrf_testing_tools_cpp/scope_exit.hpp" +#include "./event_impl.h" + using namespace std::chrono_literals; using std::chrono::seconds; using std::chrono::duration_cast; @@ -746,6 +749,11 @@ TEST_F(TestEventFixture, test_event_is_valid) ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_TRUE(rcl_event_is_valid(&publisher_event_test)); + rmw_event_type_t saved_event_type = publisher_event_test.impl->rmw_handle.event_type; + publisher_event_test.impl->rmw_handle.event_type = RMW_EVENT_INVALID; + EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + publisher_event_test.impl->rmw_handle.event_type = saved_event_type; + ret = rcl_event_fini(&publisher_event_test); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; tear_down_publisher_subscriber(); From 5a1e26ef8ca058c9784562075ece46ef63cd7b7d Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 20 Jul 2020 18:50:41 -0300 Subject: [PATCH 3/6] Reset error status Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index e0afcd5e9..6736938bd 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -739,10 +739,12 @@ TEST_F(TestEventFixture, test_bad_get_handle) TEST_F(TestEventFixture, test_event_is_valid) { EXPECT_FALSE(rcl_event_is_valid(nullptr)); + rcl_reset_error(); setup_publisher_subscriber(default_qos_profile, default_qos_profile); rcl_event_t publisher_event_test = rcl_get_zero_initialized_event(); EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + rcl_reset_error(); rcl_ret_t ret = rcl_publisher_event_init( &publisher_event_test, &publisher, RCL_PUBLISHER_OFFERED_DEADLINE_MISSED); @@ -752,6 +754,7 @@ TEST_F(TestEventFixture, test_event_is_valid) rmw_event_type_t saved_event_type = publisher_event_test.impl->rmw_handle.event_type; publisher_event_test.impl->rmw_handle.event_type = RMW_EVENT_INVALID; EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + rcl_reset_error(); publisher_event_test.impl->rmw_handle.event_type = saved_event_type; ret = rcl_event_fini(&publisher_event_test); From a291823dba2e7c271de656f24b05661e477acd71 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 21 Jul 2020 19:06:51 -0300 Subject: [PATCH 4/6] Fix alpha order Signed-off-by: Jorge Perez --- rcl/src/rcl/event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 32d47903d..8135debf0 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -30,9 +30,9 @@ extern "C" #include "rmw/event.h" #include "./common.h" +#include "./event_impl.h" #include "./publisher_impl.h" #include "./subscription_impl.h" -#include "./event_impl.h" rcl_event_t rcl_get_zero_initialized_event() From f945eb5ebd3b5f30f38c9031584f1bed9f5d1cd7 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 21 Jul 2020 19:35:41 -0300 Subject: [PATCH 5/6] Add check for bad allocator Signed-off-by: Jorge Perez --- rcl/src/rcl/event.c | 3 +++ rcl/test/rcl/test_events.cpp | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 8135debf0..acba993a2 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -24,6 +24,7 @@ extern "C" #include "rcl/error_handling.h" #include "rcl/expand_topic_name.h" #include "rcl/remap.h" +#include "rcutils/allocator.h" #include "rcutils/logging_macros.h" #include "rmw/error_handling.h" #include "rmw/validate_full_topic_name.h" @@ -194,6 +195,8 @@ rcl_event_is_valid(const rcl_event_t * event) RCUTILS_SET_ERROR_MSG("event's implementation not init"); return false; } + RCUTILS_CHECK_ALLOCATOR_WITH_MSG( + &event->impl->allocator, "not valid allocator", return false); return true; } diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index 6736938bd..ded7b1215 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -757,6 +757,13 @@ TEST_F(TestEventFixture, test_event_is_valid) rcl_reset_error(); publisher_event_test.impl->rmw_handle.event_type = saved_event_type; + rcl_allocator_t saved_alloc = publisher_event_test.impl->allocator; + rcl_allocator_t bad_alloc = rcutils_get_zero_initialized_allocator(); + publisher_event_test.impl->allocator = bad_alloc; + EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + rcl_reset_error(); + publisher_event_test.impl->allocator = saved_alloc; + ret = rcl_event_fini(&publisher_event_test); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; tear_down_publisher_subscriber(); From a3f853495cc789df3e8e7e8ab017ce8c149e9db2 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 21 Jul 2020 19:40:06 -0300 Subject: [PATCH 6/6] Add tests to check if error set Signed-off-by: Jorge Perez --- rcl/test/rcl/test_events.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rcl/test/rcl/test_events.cpp b/rcl/test/rcl/test_events.cpp index ded7b1215..351356c2b 100644 --- a/rcl/test/rcl/test_events.cpp +++ b/rcl/test/rcl/test_events.cpp @@ -739,11 +739,13 @@ TEST_F(TestEventFixture, test_bad_get_handle) TEST_F(TestEventFixture, test_event_is_valid) { EXPECT_FALSE(rcl_event_is_valid(nullptr)); + EXPECT_TRUE(rcl_error_is_set()); rcl_reset_error(); setup_publisher_subscriber(default_qos_profile, default_qos_profile); rcl_event_t publisher_event_test = rcl_get_zero_initialized_event(); EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + EXPECT_TRUE(rcl_error_is_set()); rcl_reset_error(); rcl_ret_t ret = rcl_publisher_event_init( @@ -754,6 +756,7 @@ TEST_F(TestEventFixture, test_event_is_valid) rmw_event_type_t saved_event_type = publisher_event_test.impl->rmw_handle.event_type; publisher_event_test.impl->rmw_handle.event_type = RMW_EVENT_INVALID; EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + EXPECT_TRUE(rcl_error_is_set()); rcl_reset_error(); publisher_event_test.impl->rmw_handle.event_type = saved_event_type; @@ -761,6 +764,7 @@ TEST_F(TestEventFixture, test_event_is_valid) rcl_allocator_t bad_alloc = rcutils_get_zero_initialized_allocator(); publisher_event_test.impl->allocator = bad_alloc; EXPECT_FALSE(rcl_event_is_valid(&publisher_event_test)); + EXPECT_TRUE(rcl_error_is_set()); rcl_reset_error(); publisher_event_test.impl->allocator = saved_alloc;