From b02e501756d61e93de5c4762abe8736e320c8978 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sun, 14 Jan 2024 10:33:19 +0100 Subject: [PATCH 01/33] restructured interface in oder to allow replacing the existing RealtimeBox, enable/disable certain methods for pointer types, Box is now usable for pointer types --- CMakeLists.txt | 1 + .../realtime_tools/realtime_box_best_effort.h | 87 ++++++++++++++++--- test/realtime_box_best_effort_tests.cpp | 37 ++++++-- 3 files changed, 103 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 63bd9b67..454f9616 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,6 +9,7 @@ set(THIS_PACKAGE_INCLUDE_DEPENDS rclcpp rclcpp_action Threads + rcpputils ) find_package(ament_cmake REQUIRED) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 07995c51..7b7ed0f9 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -1,18 +1,22 @@ #pragma once -#include - #include +#include #include +#include namespace realtime_tools { +template +constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; + /*! A Box that ensures thread safe access to the boxed contents. Access is best effort. If it can not lock it will return. - Note: Be carefull with pointers as this implementation will actually just copy the pointer - See the tests for an example on how to work with pointers + NOTE about pointers: + You can use pointers with this box but the access will be different. + Only use the get/set methods that take function pointer for accessing the internal value. */ template class RealtimeBoxBestEffort @@ -42,7 +46,8 @@ class RealtimeBoxBestEffort * @brief set a new content with best effort * @return false if mutex could not be locked */ - bool set(const T & value) + template + typename std::enable_if_t, bool> trySet(const T & value) { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { @@ -51,11 +56,23 @@ class RealtimeBoxBestEffort value_ = value; return true; } + + bool trySet(const std::function & func) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + + func(value_); + return true; + } /** * @brief get the content with best effort * @return std::nullopt if content could not be access, otherwise the content is returned */ - [[nodiscard]] std::optional get() const + template + [[nodiscard]] typename std::enable_if_t, std::optional> tryGet() const { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { @@ -64,36 +81,80 @@ class RealtimeBoxBestEffort return value_; } + bool tryGet(const std::is_function & func) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + + func(value_); + return true; + } + /** * @brief set the content and wait until the mutex could be locked (RealtimeBox behaviour) * @return true */ - bool setNonRT(const T & value) + template + typename std::enable_if_t, void> set(const T & value) { std::lock_guard guard(lock_); value_ = value; - //Also return a bool in order to mimic the behaviour from 'set' - return true; } + + void set(const std::function & func) + { + std::lock_guard guard(lock_); + func(value_); + } + /** * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) * @return copy of the value */ - [[nodiscard]] T getNonRT() const + template + [[nodiscard]] typename std::enable_if_t, U> get() const + { + std::lock_guard guard(lock_); + return value_; + } + /** + * @brief get the content and wait until the mutex could be locked + * @note same signature as in the existing RealtimeBox + */ + template + typename std::enable_if_t, void> get(T & in) const { std::lock_guard guard(lock_); return value_; } + + void get(const std::function & func) + { + std::lock_guard guard(lock_); + func(value_); + } + //Make the usage easier by providing a custom assignment operator and a custom conversion operator //Only to be used from non-RT! - void operator=(const T & value) { set(value); } + template + typename std::enable_if_t, void> operator=(const T & value) + { + set(value); + } + template >> [[nodiscard]] operator T() const { //Only makes sense with the getNonRT method otherwise we would return an std::optional - return getNonRT(); + return get(); + } + template >> + [[nodiscard]] operator std::optional() const + { + return tryGet(); } - [[nodiscard]] operator std::optional() const { return get(); } //In case one wants to actually use a pointer in this implementation we allow accessing the lock directly. //Note: Be carefull with lock.unlock(). It may only be called from the thread that locked the mutext! diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index 06820f07..b2797f78 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -29,7 +29,7 @@ TEST(RealtimeBoxBestEffort, empty_construct) { RealtimeBoxBestEffort box; - auto value = box.getNonRT(); + auto value = box.get(); EXPECT_EQ(value.a, 10); EXPECT_EQ(value.str, "hallo"); } @@ -41,7 +41,7 @@ TEST(RealtimeBoxBestEffort, default_construct) RealtimeBoxBestEffort box(data); - auto value = box.getNonRT(); + auto value = box.get(); EXPECT_EQ(value.a, 100); EXPECT_EQ(value.str, "hallo"); } @@ -50,7 +50,7 @@ TEST(RealtimeBoxBestEffort, non_default_constructable) { RealtimeBoxBestEffort box(NonDefaultConstructable(-10, "hello")); - auto value = box.getNonRT(); + auto value = box.get(); EXPECT_EQ(value.a, -10); EXPECT_EQ(value.str, "hello"); } @@ -59,7 +59,7 @@ TEST(RealtimeBoxBestEffort, initializer_list) { RealtimeBoxBestEffort box({1, 2, 3}); - auto value = box.getNonRT(); + auto value = box.get(); EXPECT_EQ(value.data[0], 1); EXPECT_EQ(value.data[1], 2); EXPECT_EQ(value.data[2], 3); @@ -73,7 +73,7 @@ TEST(RealtimeBoxBestEffort, assignment_operator) //Assignement operator is always non RT! box = data; - auto value = box.getNonRT(); + auto value = box.get(); EXPECT_EQ(value.a, 1000); } TEST(RealtimeBoxBestEffort, typecast_operator) @@ -99,10 +99,29 @@ TEST(RealtimeBoxBestEffort, pointer_type) int * ptr = &a; RealtimeBoxBestEffort box(ptr); + //This does not and should not compile! + //auto value = box.get(); - auto value = box.getNonRT(); + //Instead access it via a passed function. This assues that we access the data within the scope of the lock + box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); - //Correctly working with pointer types is nasty... - std::lock_guard guard(box.getMutex()); - EXPECT_EQ(*value, 100); + box.set([](auto & i) { *i = 200; }); + + box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); +} + +TEST(RealtimeBoxBestEffort, smart_ptr_type) +{ + std::shared_ptr ptr = std::make_shared(100); + + RealtimeBoxBestEffort box(ptr); + //This does not and should not compile! + //auto value = box.get(); + + //Instead access it via a passed function. This assues that we access the data within the scope of the lock + box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); + + box.set([](auto & i) { *i = 200; }); + + box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); } \ No newline at end of file From 613c55b487b91bbe2609b19dbd2ea3cfdc803c90 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sun, 14 Jan 2024 10:43:31 +0100 Subject: [PATCH 02/33] added some additional comments --- .../realtime_tools/realtime_box_best_effort.h | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 7b7ed0f9..0fc3fc17 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -29,7 +29,6 @@ class RealtimeBoxBestEffort using mutex_t = mutex_type; using type = T; //Provide various constructors - constexpr explicit RealtimeBoxBestEffort(const T & init = T{}) : value_(init) {} constexpr explicit RealtimeBoxBestEffort(const T && init) : value_(std::move(init)) {} @@ -45,6 +44,7 @@ class RealtimeBoxBestEffort /** * @brief set a new content with best effort * @return false if mutex could not be locked + * @note disabled for pointer types */ template typename std::enable_if_t, bool> trySet(const T & value) @@ -56,7 +56,11 @@ class RealtimeBoxBestEffort value_ = value; return true; } - + /** + * @brief access the content readable with best effort + * @return false if the mutex could not be locked + * @note only safe way to access pointer type content (rw) + */ bool trySet(const std::function & func) { std::unique_lock guard(lock_, std::defer_lock); @@ -80,7 +84,11 @@ class RealtimeBoxBestEffort } return value_; } - + /** + * @brief access the content (r) with best effort + * @return false if the mutex could not be locked + * @note only safe way to access pointer type content (r) + */ bool tryGet(const std::is_function & func) { std::unique_lock guard(lock_, std::defer_lock); @@ -102,7 +110,9 @@ class RealtimeBoxBestEffort std::lock_guard guard(lock_); value_ = value; } - + /** + * @brief access the content (rw) and wait until the mutex could locked + */ void set(const std::function & func) { std::lock_guard guard(lock_); @@ -129,7 +139,11 @@ class RealtimeBoxBestEffort std::lock_guard guard(lock_); return value_; } - + /** + * @brief access the content (r) and wait until the mutex could be locked + * @note only safe way to access pointer type content (r) + * @note same signature as in the existing RealtimeBox + */ void get(const std::function & func) { std::lock_guard guard(lock_); From 93e14012946f607f1c6a38816cb34eb6b7fec58b Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sun, 14 Jan 2024 12:31:48 +0100 Subject: [PATCH 03/33] fix bug --- include/realtime_tools/realtime_box_best_effort.h | 2 +- test/realtime_box_best_effort_tests.cpp | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 0fc3fc17..6db417e5 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -89,7 +89,7 @@ class RealtimeBoxBestEffort * @return false if the mutex could not be locked * @note only safe way to access pointer type content (r) */ - bool tryGet(const std::is_function & func) + bool tryGet(const std::function & func) { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index b2797f78..9e66014f 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -108,6 +108,8 @@ TEST(RealtimeBoxBestEffort, pointer_type) box.set([](auto & i) { *i = 200; }); box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); + + box.tryGet([](const auto &i){EXPECT_EQ(*i,200);}); } TEST(RealtimeBoxBestEffort, smart_ptr_type) From fc2e4a2595484dbafbc156a0f0851e69aa35f5be Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sun, 14 Jan 2024 12:40:00 +0100 Subject: [PATCH 04/33] fixed wrong return type in templated method --- include/realtime_tools/realtime_box_best_effort.h | 2 +- test/realtime_box_best_effort_tests.cpp | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 6db417e5..f81762f9 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -137,7 +137,7 @@ class RealtimeBoxBestEffort typename std::enable_if_t, void> get(T & in) const { std::lock_guard guard(lock_); - return value_; + in = value_; } /** * @brief access the content (r) and wait until the mutex could be locked diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index 9e66014f..1e0642c0 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -54,6 +54,21 @@ TEST(RealtimeBoxBestEffort, non_default_constructable) EXPECT_EQ(value.a, -10); EXPECT_EQ(value.str, "hello"); } +TEST(RealtimeBoxBestEffort, standard_get) +{ + RealtimeBoxBestEffort box(DefaultConstructable{.a=1000}); + + DefaultConstructable data; + box.get(data); + EXPECT_EQ(data.a,1000); + data.a = 10000; + + + box.set(data); + + auto value = box.get(); + EXPECT_EQ(value.a, 10000); +} TEST(RealtimeBoxBestEffort, initializer_list) { From dce1458bac2e6d7e1ad4d5430ee2e1fbdd925170 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Tue, 16 Jan 2024 11:03:53 +0100 Subject: [PATCH 05/33] added copyright notice --- .../realtime_tools/realtime_box_best_effort.h | 32 ++++++++++++++- test/realtime_box_best_effort_tests.cpp | 41 ++++++++++++++++--- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index f81762f9..414d2426 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -1,3 +1,33 @@ +// Copyright (c) 2024, Lennart Nachtigall +// +// 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 HOLDER 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. + +// Author: Lennart Nachtigall + #pragma once #include @@ -179,4 +209,4 @@ class RealtimeBoxBestEffort T value_; mutable mutex_t lock_; }; -} // namespace realtime_tools \ No newline at end of file +} // namespace realtime_tools diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index 1e0642c0..c08b1665 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -1,3 +1,33 @@ +// Copyright (c) 2024, Lennart Nachtigall +// +// 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 HOLDER 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. + +// Author: Lennart Nachtigall + #include #include @@ -56,18 +86,17 @@ TEST(RealtimeBoxBestEffort, non_default_constructable) } TEST(RealtimeBoxBestEffort, standard_get) { - RealtimeBoxBestEffort box(DefaultConstructable{.a=1000}); + RealtimeBoxBestEffort box(DefaultConstructable{.a = 1000}); DefaultConstructable data; box.get(data); - EXPECT_EQ(data.a,1000); + EXPECT_EQ(data.a, 1000); data.a = 10000; - box.set(data); auto value = box.get(); - EXPECT_EQ(value.a, 10000); + EXPECT_EQ(value.a, 10000); } TEST(RealtimeBoxBestEffort, initializer_list) @@ -124,7 +153,7 @@ TEST(RealtimeBoxBestEffort, pointer_type) box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); - box.tryGet([](const auto &i){EXPECT_EQ(*i,200);}); + box.tryGet([](const auto & i) { EXPECT_EQ(*i, 200); }); } TEST(RealtimeBoxBestEffort, smart_ptr_type) @@ -141,4 +170,4 @@ TEST(RealtimeBoxBestEffort, smart_ptr_type) box.set([](auto & i) { *i = 200; }); box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); -} \ No newline at end of file +} From eaedafe027879e9f5e74e81eb519a07586dd9f29 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Tue, 23 Jan 2024 07:32:08 +0100 Subject: [PATCH 06/33] Fixed issues due to merge with master --- CMakeLists.txt | 3 +++ include/realtime_tools/realtime_box_best_effort.h | 6 +++--- test/realtime_box_best_effort_tests.cpp | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index de507fbe..21f2e96e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,6 +46,9 @@ if(BUILD_TESTING) ament_add_gmock(realtime_box_tests test/realtime_box_tests.cpp) target_link_libraries(realtime_box_tests realtime_tools) + ament_add_gmock(realtime_box_best_effort_tests test/realtime_box_best_effort_tests.cpp) + target_link_libraries(realtime_box_best_effort_tests realtime_tools) + ament_add_gmock(realtime_buffer_tests test/realtime_buffer_tests.cpp) target_link_libraries(realtime_buffer_tests realtime_tools) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 414d2426..7cd3c79e 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -41,11 +41,11 @@ template constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; /*! - A Box that ensures thread safe access to the boxed contents. + A Box that ensures thread safe access to the boxed contents. Access is best effort. If it can not lock it will return. NOTE about pointers: - You can use pointers with this box but the access will be different. + You can use pointers with this box but the access will be different. Only use the get/set methods that take function pointer for accessing the internal value. */ template @@ -115,7 +115,7 @@ class RealtimeBoxBestEffort return value_; } /** - * @brief access the content (r) with best effort + * @brief access the content (r) with best effort * @return false if the mutex could not be locked * @note only safe way to access pointer type content (r) */ diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index c08b1665..887644d7 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -114,7 +114,7 @@ TEST(RealtimeBoxBestEffort, assignment_operator) DefaultConstructable data; data.a = 1000; RealtimeBoxBestEffort box; - //Assignement operator is always non RT! + //Assignment operator is always non RT! box = data; auto value = box.get(); From a22d150703383e9c7249c40684691d9677879c09 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 25 Jan 2024 07:33:49 +0100 Subject: [PATCH 07/33] fixed spelling mistake --- include/realtime_tools/realtime_box_best_effort.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 7cd3c79e..7f057557 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -201,7 +201,7 @@ class RealtimeBoxBestEffort } //In case one wants to actually use a pointer in this implementation we allow accessing the lock directly. - //Note: Be carefull with lock.unlock(). It may only be called from the thread that locked the mutext! + //Note: Be careful with lock.unlock(). It may only be called from the thread that locked the mutext! [[nodiscard]] const mutex_t & getMutex() const { return lock_; } [[nodiscard]] mutex_t & getMutex() { return lock_; } From 65bca5d06aa5abcb55e33d8795aeca6129bfb0d4 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 25 Jan 2024 07:45:45 +0100 Subject: [PATCH 08/33] Replaced existing RealtimeBox with RealtimeBoxBestEffort, merged the tests into a single file and adapted the naming of the existing tests for the RealtimeBox --- CMakeLists.txt | 3 - include/realtime_tools/realtime_box.h | 195 +++++++++++++--- .../realtime_tools/realtime_box_best_effort.h | 212 ------------------ test/realtime_box_best_effort_tests.cpp | 173 -------------- test/realtime_box_tests.cpp | 166 +++++++++++++- 5 files changed, 320 insertions(+), 429 deletions(-) delete mode 100644 include/realtime_tools/realtime_box_best_effort.h delete mode 100644 test/realtime_box_best_effort_tests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 21f2e96e..de507fbe 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,9 +46,6 @@ if(BUILD_TESTING) ament_add_gmock(realtime_box_tests test/realtime_box_tests.cpp) target_link_libraries(realtime_box_tests realtime_tools) - ament_add_gmock(realtime_box_best_effort_tests test/realtime_box_best_effort_tests.cpp) - target_link_libraries(realtime_box_best_effort_tests realtime_tools) - ament_add_gmock(realtime_buffer_tests test/realtime_buffer_tests.cpp) target_link_libraries(realtime_buffer_tests realtime_tools) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index c5fc3002..f0b5f18e 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -1,4 +1,4 @@ -// Copyright (c) 2009, Willow Garage, Inc. +// Copyright (c) 2024, Lennart Nachtigall // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are met: @@ -26,52 +26,187 @@ // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE // POSSIBILITY OF SUCH DAMAGE. -// Author: Stuart Glaser +// Author: Lennart Nachtigall -#ifndef REALTIME_TOOLS__REALTIME_BOX_H__ -#define REALTIME_TOOLS__REALTIME_BOX_H__ +#pragma once +#include #include -#include - +#include +#include namespace realtime_tools { -/*! - Strongly suggested that you use an std::shared_ptr in this box to - guarantee realtime safety. +template +constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; - */ -template +/*! + A Box that ensures thread safe access to the boxed contents. + Access is best effort. If it can not lock it will return. + + NOTE about pointers: + You can use pointers with this box but the access will be different. + Only use the get/set methods that take function pointer for accessing the internal value. +*/ +template class RealtimeBox { + static_assert( + std::is_same_v || std::is_same_v); + static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); + public: - explicit RealtimeBox(const T & initial = T()) : thing_(initial) {} + using mutex_t = mutex_type; + using type = T; + //Provide various constructors + constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {} + constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {} + + //Only enabled for types that can be constructed from an initializer list + template + constexpr RealtimeBox( + const std::initializer_list & init, + std::enable_if_t>) + : value_(init) + { + } + + /** + * @brief set a new content with best effort + * @return false if mutex could not be locked + * @note disabled for pointer types + */ + template + typename std::enable_if_t, bool> trySet(const T & value) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + value_ = value; + return true; + } + /** + * @brief access the content readable with best effort + * @return false if the mutex could not be locked + * @note only safe way to access pointer type content (rw) + */ + bool trySet(const std::function & func) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + + func(value_); + return true; + } + /** + * @brief get the content with best effort + * @return std::nullopt if content could not be access, otherwise the content is returned + */ + template + [[nodiscard]] typename std::enable_if_t, std::optional> tryGet() const + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return std::nullopt; + } + return value_; + } + /** + * @brief access the content (r) with best effort + * @return false if the mutex could not be locked + * @note only safe way to access pointer type content (r) + */ + bool tryGet(const std::function & func) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + + func(value_); + return true; + } + + /** + * @brief set the content and wait until the mutex could be locked (RealtimeBox behaviour) + * @return true + */ + template + typename std::enable_if_t, void> set(const T & value) + { + std::lock_guard guard(lock_); + value_ = value; + } + /** + * @brief access the content (rw) and wait until the mutex could locked + */ + void set(const std::function & func) + { + std::lock_guard guard(lock_); + func(value_); + } + + /** + * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) + * @return copy of the value + */ + template + [[nodiscard]] typename std::enable_if_t, U> get() const + { + std::lock_guard guard(lock_); + return value_; + } + /** + * @brief get the content and wait until the mutex could be locked + * @note same signature as in the existing RealtimeBox + */ + template + typename std::enable_if_t, void> get(T & in) const + { + std::lock_guard guard(lock_); + in = value_; + } + /** + * @brief access the content (r) and wait until the mutex could be locked + * @note only safe way to access pointer type content (r) + * @note same signature as in the existing RealtimeBox + */ + void get(const std::function & func) + { + std::lock_guard guard(lock_); + func(value_); + } - void set(const T & value) + //Make the usage easier by providing a custom assignment operator and a custom conversion operator + //Only to be used from non-RT! + template + typename std::enable_if_t, void> operator=(const T & value) { - std::lock_guard guard(thing_lock_RT_); - thing_ = value; + set(value); } - void get(T & ref) + template >> + [[nodiscard]] operator T() const { - std::lock_guard guard(thing_lock_RT_); - ref = thing_; + //Only makes sense with the getNonRT method otherwise we would return an std::optional + return get(); } + template >> + [[nodiscard]] operator std::optional() const + { + return tryGet(); + } + + //In case one wants to actually use a pointer in this implementation we allow accessing the lock directly. + //Note: Be careful with lock.unlock(). It may only be called from the thread that locked the mutext! + [[nodiscard]] const mutex_t & getMutex() const { return lock_; } + [[nodiscard]] mutex_t & getMutex() { return lock_; } private: - // The thing that's in the box. - T thing_; - - // Protects access to the thing in the box. This mutex is - // guaranteed to be locked for no longer than the duration of the - // copy, so as long as the copy is realtime safe and the OS has - // priority inheritance for mutexes, this lock can be safely locked - // from within realtime. - std::mutex thing_lock_RT_; + T value_; + mutable mutex_t lock_; }; - } // namespace realtime_tools - -#endif // REALTIME_TOOLS__REALTIME_BOX_H_ diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h deleted file mode 100644 index 7f057557..00000000 --- a/include/realtime_tools/realtime_box_best_effort.h +++ /dev/null @@ -1,212 +0,0 @@ -// Copyright (c) 2024, Lennart Nachtigall -// -// 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 HOLDER 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. - -// Author: Lennart Nachtigall - -#pragma once - -#include -#include -#include -#include -namespace realtime_tools -{ - -template -constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; - -/*! - A Box that ensures thread safe access to the boxed contents. - Access is best effort. If it can not lock it will return. - - NOTE about pointers: - You can use pointers with this box but the access will be different. - Only use the get/set methods that take function pointer for accessing the internal value. -*/ -template -class RealtimeBoxBestEffort -{ - static_assert( - std::is_same_v || std::is_same_v); - static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); - -public: - using mutex_t = mutex_type; - using type = T; - //Provide various constructors - constexpr explicit RealtimeBoxBestEffort(const T & init = T{}) : value_(init) {} - constexpr explicit RealtimeBoxBestEffort(const T && init) : value_(std::move(init)) {} - - //Only enabled for types that can be constructed from an initializer list - template - constexpr RealtimeBoxBestEffort( - const std::initializer_list & init, - std::enable_if_t>) - : value_(init) - { - } - - /** - * @brief set a new content with best effort - * @return false if mutex could not be locked - * @note disabled for pointer types - */ - template - typename std::enable_if_t, bool> trySet(const T & value) - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return false; - } - value_ = value; - return true; - } - /** - * @brief access the content readable with best effort - * @return false if the mutex could not be locked - * @note only safe way to access pointer type content (rw) - */ - bool trySet(const std::function & func) - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return false; - } - - func(value_); - return true; - } - /** - * @brief get the content with best effort - * @return std::nullopt if content could not be access, otherwise the content is returned - */ - template - [[nodiscard]] typename std::enable_if_t, std::optional> tryGet() const - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return std::nullopt; - } - return value_; - } - /** - * @brief access the content (r) with best effort - * @return false if the mutex could not be locked - * @note only safe way to access pointer type content (r) - */ - bool tryGet(const std::function & func) - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return false; - } - - func(value_); - return true; - } - - /** - * @brief set the content and wait until the mutex could be locked (RealtimeBox behaviour) - * @return true - */ - template - typename std::enable_if_t, void> set(const T & value) - { - std::lock_guard guard(lock_); - value_ = value; - } - /** - * @brief access the content (rw) and wait until the mutex could locked - */ - void set(const std::function & func) - { - std::lock_guard guard(lock_); - func(value_); - } - - /** - * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) - * @return copy of the value - */ - template - [[nodiscard]] typename std::enable_if_t, U> get() const - { - std::lock_guard guard(lock_); - return value_; - } - /** - * @brief get the content and wait until the mutex could be locked - * @note same signature as in the existing RealtimeBox - */ - template - typename std::enable_if_t, void> get(T & in) const - { - std::lock_guard guard(lock_); - in = value_; - } - /** - * @brief access the content (r) and wait until the mutex could be locked - * @note only safe way to access pointer type content (r) - * @note same signature as in the existing RealtimeBox - */ - void get(const std::function & func) - { - std::lock_guard guard(lock_); - func(value_); - } - - //Make the usage easier by providing a custom assignment operator and a custom conversion operator - //Only to be used from non-RT! - template - typename std::enable_if_t, void> operator=(const T & value) - { - set(value); - } - - template >> - [[nodiscard]] operator T() const - { - //Only makes sense with the getNonRT method otherwise we would return an std::optional - return get(); - } - template >> - [[nodiscard]] operator std::optional() const - { - return tryGet(); - } - - //In case one wants to actually use a pointer in this implementation we allow accessing the lock directly. - //Note: Be careful with lock.unlock(). It may only be called from the thread that locked the mutext! - [[nodiscard]] const mutex_t & getMutex() const { return lock_; } - [[nodiscard]] mutex_t & getMutex() { return lock_; } - -private: - T value_; - mutable mutex_t lock_; -}; -} // namespace realtime_tools diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp deleted file mode 100644 index 887644d7..00000000 --- a/test/realtime_box_best_effort_tests.cpp +++ /dev/null @@ -1,173 +0,0 @@ -// Copyright (c) 2024, Lennart Nachtigall -// -// 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 HOLDER 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. - -// Author: Lennart Nachtigall - -#include -#include - -struct DefaultConstructable -{ - int a = 10; - std::string str = "hallo"; -}; - -struct NonDefaultConstructable -{ - NonDefaultConstructable(int a_, const std::string & str_) : a(a_), str(str_) {} - int a; - std::string str; -}; - -struct FromInitializerList -{ - FromInitializerList(std::initializer_list list) - { - std::copy(list.begin(), list.end(), data.begin()); - } - std::array data; -}; - -using namespace realtime_tools; - -TEST(RealtimeBoxBestEffort, empty_construct) -{ - RealtimeBoxBestEffort box; - - auto value = box.get(); - EXPECT_EQ(value.a, 10); - EXPECT_EQ(value.str, "hallo"); -} - -TEST(RealtimeBoxBestEffort, default_construct) -{ - DefaultConstructable data; - data.a = 100; - - RealtimeBoxBestEffort box(data); - - auto value = box.get(); - EXPECT_EQ(value.a, 100); - EXPECT_EQ(value.str, "hallo"); -} - -TEST(RealtimeBoxBestEffort, non_default_constructable) -{ - RealtimeBoxBestEffort box(NonDefaultConstructable(-10, "hello")); - - auto value = box.get(); - EXPECT_EQ(value.a, -10); - EXPECT_EQ(value.str, "hello"); -} -TEST(RealtimeBoxBestEffort, standard_get) -{ - RealtimeBoxBestEffort box(DefaultConstructable{.a = 1000}); - - DefaultConstructable data; - box.get(data); - EXPECT_EQ(data.a, 1000); - data.a = 10000; - - box.set(data); - - auto value = box.get(); - EXPECT_EQ(value.a, 10000); -} - -TEST(RealtimeBoxBestEffort, initializer_list) -{ - RealtimeBoxBestEffort box({1, 2, 3}); - - auto value = box.get(); - EXPECT_EQ(value.data[0], 1); - EXPECT_EQ(value.data[1], 2); - EXPECT_EQ(value.data[2], 3); -} - -TEST(RealtimeBoxBestEffort, assignment_operator) -{ - DefaultConstructable data; - data.a = 1000; - RealtimeBoxBestEffort box; - //Assignment operator is always non RT! - box = data; - - auto value = box.get(); - EXPECT_EQ(value.a, 1000); -} -TEST(RealtimeBoxBestEffort, typecast_operator) -{ - RealtimeBoxBestEffort box(DefaultConstructable{.a = 100, .str = ""}); - - //Use non RT access - DefaultConstructable data = box; - - EXPECT_EQ(data.a, 100); - - //Use RT access -> returns std::nullopt if the mutex could not be locked - std::optional rt_data_access = box; - - if (rt_data_access) { - EXPECT_EQ(rt_data_access->a, 100); - } -} - -TEST(RealtimeBoxBestEffort, pointer_type) -{ - int a = 100; - int * ptr = &a; - - RealtimeBoxBestEffort box(ptr); - //This does not and should not compile! - //auto value = box.get(); - - //Instead access it via a passed function. This assues that we access the data within the scope of the lock - box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); - - box.set([](auto & i) { *i = 200; }); - - box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); - - box.tryGet([](const auto & i) { EXPECT_EQ(*i, 200); }); -} - -TEST(RealtimeBoxBestEffort, smart_ptr_type) -{ - std::shared_ptr ptr = std::make_shared(100); - - RealtimeBoxBestEffort box(ptr); - //This does not and should not compile! - //auto value = box.get(); - - //Instead access it via a passed function. This assues that we access the data within the scope of the lock - box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); - - box.set([](auto & i) { *i = 200; }); - - box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); -} diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index 1c443f60..adf64fa3 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2019, Open Source Robotics Foundation, Inc. +// Copyright (c) 2024, Lennart Nachtigall // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are met: @@ -10,7 +10,7 @@ // 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 Open Source Robotics Foundation, Inc. nor the names of its +// * 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. // @@ -26,31 +26,175 @@ // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE // POSSIBILITY OF SUCH DAMAGE. +// Author: Lennart Nachtigall + #include #include -using realtime_tools::RealtimeBox; +struct DefaultConstructable +{ + int a = 10; + std::string str = "hallo"; +}; + +struct NonDefaultConstructable +{ + NonDefaultConstructable(int a_, const std::string & str_) : a(a_), str(str_) {} + int a; + std::string str; +}; + +struct FromInitializerList +{ + FromInitializerList(std::initializer_list list) + { + std::copy(list.begin(), list.end(), data.begin()); + } + std::array data; +}; + +using namespace realtime_tools; + +TEST(RealtimeBox, empty_construct) +{ + RealtimeBox box; + + auto value = box.get(); + EXPECT_EQ(value.a, 10); + EXPECT_EQ(value.str, "hallo"); +} + +TEST(RealtimeBox, default_construct) +{ + DefaultConstructable data; + data.a = 100; + + RealtimeBox box(data); + + auto value = box.get(); + EXPECT_EQ(value.a, 100); + EXPECT_EQ(value.str, "hallo"); +} + +TEST(RealtimeBox, non_default_constructable) +{ + RealtimeBox box(NonDefaultConstructable(-10, "hello")); + + auto value = box.get(); + EXPECT_EQ(value.a, -10); + EXPECT_EQ(value.str, "hello"); +} +TEST(RealtimeBox, standard_get) +{ + RealtimeBox box(DefaultConstructable{.a = 1000}); + + DefaultConstructable data; + box.get(data); + EXPECT_EQ(data.a, 1000); + data.a = 10000; + + box.set(data); + + auto value = box.get(); + EXPECT_EQ(value.a, 10000); +} + +TEST(RealtimeBox, initializer_list) +{ + RealtimeBox box({1, 2, 3}); + + auto value = box.get(); + EXPECT_EQ(value.data[0], 1); + EXPECT_EQ(value.data[1], 2); + EXPECT_EQ(value.data[2], 3); +} -class DefaultConstructable +TEST(RealtimeBox, assignment_operator) +{ + DefaultConstructable data; + data.a = 1000; + RealtimeBox box; + //Assignment operator is always non RT! + box = data; + + auto value = box.get(); + EXPECT_EQ(value.a, 1000); +} +TEST(RealtimeBox, typecast_operator) +{ + RealtimeBox box(DefaultConstructable{.a = 100, .str = ""}); + + //Use non RT access + DefaultConstructable data = box; + + EXPECT_EQ(data.a, 100); + + //Use RT access -> returns std::nullopt if the mutex could not be locked + std::optional rt_data_access = box; + + if (rt_data_access) { + EXPECT_EQ(rt_data_access->a, 100); + } +} + +TEST(RealtimeBox, pointer_type) +{ + int a = 100; + int * ptr = &a; + + RealtimeBox box(ptr); + //This does not and should not compile! + //auto value = box.get(); + + //Instead access it via a passed function. This assues that we access the data within the scope of the lock + box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); + + box.set([](auto & i) { *i = 200; }); + + box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); + + box.tryGet([](const auto & i) { EXPECT_EQ(*i, 200); }); +} + +TEST(RealtimeBox, smart_ptr_type) +{ + std::shared_ptr ptr = std::make_shared(100); + + RealtimeBox box(ptr); + //This does not and should not compile! + //auto value = box.get(); + + //Instead access it via a passed function. This assues that we access the data within the scope of the lock + box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); + + box.set([](auto & i) { *i = 200; }); + + box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); +} + +//These are the tests from the old RealtimeBox implementation +//They are therefore suffixed with _existing + +class DefaultConstructable_existing { public: - DefaultConstructable() : number_(42) {} - ~DefaultConstructable() {} + DefaultConstructable_existing() : number_(42) {} + ~DefaultConstructable_existing() {} int number_; }; -TEST(RealtimeBox, default_construct) +TEST(RealtimeBox, default_construct_existing) { - DefaultConstructable thing; + DefaultConstructable_existing thing; thing.number_ = 5; - RealtimeBox box; + RealtimeBox box; box.get(thing); EXPECT_EQ(42, thing.number_); } -TEST(RealtimeBox, initial_value) +TEST(RealtimeBox, initial_value_existing) { RealtimeBox box(3.14); double num = 0.0; @@ -58,7 +202,7 @@ TEST(RealtimeBox, initial_value) EXPECT_DOUBLE_EQ(3.14, num); } -TEST(RealtimeBox, set_and_get) +TEST(RealtimeBox, set_and_get_existing) { RealtimeBox box('a'); From 186b4aaf78cc3f3b4bd24d9278456b3b1f4278c8 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 25 Jan 2024 10:54:49 +0100 Subject: [PATCH 09/33] fixed ament_cpplint issues --- .../realtime_tools/realtime_box_best_effort.h | 18 +++++++++------ test/realtime_box_best_effort_tests.cpp | 22 ++++++++++--------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 7f057557..7934bb62 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -34,6 +34,7 @@ #include #include #include +#include namespace realtime_tools { @@ -58,11 +59,11 @@ class RealtimeBoxBestEffort public: using mutex_t = mutex_type; using type = T; - //Provide various constructors + // Provide various constructors constexpr explicit RealtimeBoxBestEffort(const T & init = T{}) : value_(init) {} constexpr explicit RealtimeBoxBestEffort(const T && init) : value_(std::move(init)) {} - //Only enabled for types that can be constructed from an initializer list + // Only enabled for types that can be constructed from an initializer list template constexpr RealtimeBoxBestEffort( const std::initializer_list & init, @@ -180,8 +181,9 @@ class RealtimeBoxBestEffort func(value_); } - //Make the usage easier by providing a custom assignment operator and a custom conversion operator - //Only to be used from non-RT! + // Make the usage easier by providing a custom assignment operator + // and a custom conversion operator + // Only to be used from non-RT! template typename std::enable_if_t, void> operator=(const T & value) { @@ -191,7 +193,7 @@ class RealtimeBoxBestEffort template >> [[nodiscard]] operator T() const { - //Only makes sense with the getNonRT method otherwise we would return an std::optional + // Only makes sense with the getNonRT method otherwise we would return an std::optional return get(); } template >> @@ -200,8 +202,10 @@ class RealtimeBoxBestEffort return tryGet(); } - //In case one wants to actually use a pointer in this implementation we allow accessing the lock directly. - //Note: Be careful with lock.unlock(). It may only be called from the thread that locked the mutext! + // In case one wants to actually use a pointer + // in this implementation we allow accessing the lock directly. + // Note: Be careful with lock.unlock(). + // It may only be called from the thread that locked the mutext! [[nodiscard]] const mutex_t & getMutex() const { return lock_; } [[nodiscard]] mutex_t & getMutex() { return lock_; } diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index 887644d7..16b869cc 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -53,7 +53,7 @@ struct FromInitializerList std::array data; }; -using namespace realtime_tools; +using realtime_tools::RealtimeBoxBestEffort; TEST(RealtimeBoxBestEffort, empty_construct) { @@ -114,7 +114,7 @@ TEST(RealtimeBoxBestEffort, assignment_operator) DefaultConstructable data; data.a = 1000; RealtimeBoxBestEffort box; - //Assignment operator is always non RT! + // Assignment operator is always non RT! box = data; auto value = box.get(); @@ -124,12 +124,12 @@ TEST(RealtimeBoxBestEffort, typecast_operator) { RealtimeBoxBestEffort box(DefaultConstructable{.a = 100, .str = ""}); - //Use non RT access + // Use non RT access DefaultConstructable data = box; EXPECT_EQ(data.a, 100); - //Use RT access -> returns std::nullopt if the mutex could not be locked + // Use RT access -> returns std::nullopt if the mutex could not be locked std::optional rt_data_access = box; if (rt_data_access) { @@ -143,10 +143,11 @@ TEST(RealtimeBoxBestEffort, pointer_type) int * ptr = &a; RealtimeBoxBestEffort box(ptr); - //This does not and should not compile! - //auto value = box.get(); + // This does not and should not compile! + // auto value = box.get(); - //Instead access it via a passed function. This assues that we access the data within the scope of the lock + // Instead access it via a passed function. + // This assures that we access the data within the scope of the lock box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); box.set([](auto & i) { *i = 200; }); @@ -161,10 +162,11 @@ TEST(RealtimeBoxBestEffort, smart_ptr_type) std::shared_ptr ptr = std::make_shared(100); RealtimeBoxBestEffort box(ptr); - //This does not and should not compile! - //auto value = box.get(); + // This does not and should not compile! + // auto value = box.get(); - //Instead access it via a passed function. This assues that we access the data within the scope of the lock + // Instead access it via a passed function. + // This assures that we access the data within the scope of the lock box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); box.set([](auto & i) { *i = 200; }); From a8b3f8a276b0ffe1c55076300a8d25e11bcaff95 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 25 Jan 2024 11:58:09 +0100 Subject: [PATCH 10/33] fixed last remaining cpplint issue --- include/realtime_tools/realtime_box_best_effort.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 7934bb62..8254c583 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -33,8 +33,10 @@ #include #include #include -#include #include + +#include + namespace realtime_tools { From 69b224c91c5adeba910e5d894fcc265e65361acf Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 25 Jan 2024 14:32:41 +0100 Subject: [PATCH 11/33] suppress cppcheck missingReturn --- include/realtime_tools/realtime_box_best_effort.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 8254c583..5e91c80d 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -141,6 +141,7 @@ class RealtimeBoxBestEffort typename std::enable_if_t, void> set(const T & value) { std::lock_guard guard(lock_); + // cppcheck-suppress missingReturn value_ = value; } /** @@ -170,6 +171,7 @@ class RealtimeBoxBestEffort typename std::enable_if_t, void> get(T & in) const { std::lock_guard guard(lock_); + // cppcheck-suppress missingReturn in = value_; } /** From 220111a4155e7327fcb353bccf5ad570add29a90 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Wed, 28 Feb 2024 10:45:17 +0100 Subject: [PATCH 12/33] try to resolve comments wherever possible --- .../realtime_tools/realtime_box_best_effort.h | 58 +++++++++++-------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 5e91c80d..5a8c7c29 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -28,7 +28,8 @@ // Author: Lennart Nachtigall -#pragma once +#ifndef REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ +#define REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ #include #include @@ -75,10 +76,10 @@ class RealtimeBoxBestEffort } /** - * @brief set a new content with best effort - * @return false if mutex could not be locked - * @note disabled for pointer types - */ + * @brief set a new content with best effort + * @return false if mutex could not be locked + * @note disabled for pointer types + */ template typename std::enable_if_t, bool> trySet(const T & value) { @@ -93,7 +94,7 @@ class RealtimeBoxBestEffort * @brief access the content readable with best effort * @return false if the mutex could not be locked * @note only safe way to access pointer type content (rw) - */ + */ bool trySet(const std::function & func) { std::unique_lock guard(lock_, std::defer_lock); @@ -105,9 +106,9 @@ class RealtimeBoxBestEffort return true; } /** - * @brief get the content with best effort - * @return std::nullopt if content could not be access, otherwise the content is returned - */ + * @brief get the content with best effort + * @return std::nullopt if content could not be access, otherwise the content is returned + */ template [[nodiscard]] typename std::enable_if_t, std::optional> tryGet() const { @@ -121,7 +122,7 @@ class RealtimeBoxBestEffort * @brief access the content (r) with best effort * @return false if the mutex could not be locked * @note only safe way to access pointer type content (r) - */ + */ bool tryGet(const std::function & func) { std::unique_lock guard(lock_, std::defer_lock); @@ -134,9 +135,9 @@ class RealtimeBoxBestEffort } /** - * @brief set the content and wait until the mutex could be locked (RealtimeBox behaviour) - * @return true - */ + * @brief set the content and wait until the mutex could be locked (RealtimeBox behavior) + * @return true + */ template typename std::enable_if_t, void> set(const T & value) { @@ -146,7 +147,7 @@ class RealtimeBoxBestEffort } /** * @brief access the content (rw) and wait until the mutex could locked - */ + */ void set(const std::function & func) { std::lock_guard guard(lock_); @@ -154,9 +155,9 @@ class RealtimeBoxBestEffort } /** - * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) - * @return copy of the value - */ + * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) + * @return copy of the value + */ template [[nodiscard]] typename std::enable_if_t, U> get() const { @@ -166,7 +167,7 @@ class RealtimeBoxBestEffort /** * @brief get the content and wait until the mutex could be locked * @note same signature as in the existing RealtimeBox - */ + */ template typename std::enable_if_t, void> get(T & in) const { @@ -178,28 +179,37 @@ class RealtimeBoxBestEffort * @brief access the content (r) and wait until the mutex could be locked * @note only safe way to access pointer type content (r) * @note same signature as in the existing RealtimeBox - */ + */ void get(const std::function & func) { std::lock_guard guard(lock_); func(value_); } - // Make the usage easier by providing a custom assignment operator - // and a custom conversion operator - // Only to be used from non-RT! + /** + * @brief provide a custom assignment operator for easier usage + * @note only to be used from non-RT! + */ template typename std::enable_if_t, void> operator=(const T & value) { set(value); } + /** + * @brief provide a custom conversion operator + * @note Can only be used from non-RT! + */ template >> [[nodiscard]] operator T() const { // Only makes sense with the getNonRT method otherwise we would return an std::optional return get(); } + /** + * @brief provide a custom conversion operator + * @note Can be used from non-RT and RT contexts + */ template >> [[nodiscard]] operator std::optional() const { @@ -209,7 +219,7 @@ class RealtimeBoxBestEffort // In case one wants to actually use a pointer // in this implementation we allow accessing the lock directly. // Note: Be careful with lock.unlock(). - // It may only be called from the thread that locked the mutext! + // It may only be called from the thread that locked the mutex! [[nodiscard]] const mutex_t & getMutex() const { return lock_; } [[nodiscard]] mutex_t & getMutex() { return lock_; } @@ -218,3 +228,5 @@ class RealtimeBoxBestEffort mutable mutex_t lock_; }; } // namespace realtime_tools + +#endif //REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ From c2900a3651d212e25ec1f2a4811f1eb9b12bdcb3 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Fri, 1 Mar 2024 10:32:37 +0100 Subject: [PATCH 13/33] Should fix pre-commit formatting warning Should fix pre-commit formatting warning --- include/realtime_tools/realtime_box_best_effort.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index 5a8c7c29..fae1c3c9 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -229,4 +229,4 @@ class RealtimeBoxBestEffort }; } // namespace realtime_tools -#endif //REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ +#endif // REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ From 4fce1842977971ae5a7400f48692c51ed4d42743 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Fri, 22 Mar 2024 08:01:23 +0100 Subject: [PATCH 14/33] Implemented suggestions --- .../realtime_tools/realtime_box_best_effort.h | 18 +++++++++--------- test/realtime_box_best_effort_tests.cpp | 4 ++++ 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/include/realtime_tools/realtime_box_best_effort.h b/include/realtime_tools/realtime_box_best_effort.h index fae1c3c9..4763c5a0 100644 --- a/include/realtime_tools/realtime_box_best_effort.h +++ b/include/realtime_tools/realtime_box_best_effort.h @@ -83,7 +83,7 @@ class RealtimeBoxBestEffort template typename std::enable_if_t, bool> trySet(const T & value) { - std::unique_lock guard(lock_, std::defer_lock); + std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { return false; } @@ -97,7 +97,7 @@ class RealtimeBoxBestEffort */ bool trySet(const std::function & func) { - std::unique_lock guard(lock_, std::defer_lock); + std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { return false; } @@ -112,7 +112,7 @@ class RealtimeBoxBestEffort template [[nodiscard]] typename std::enable_if_t, std::optional> tryGet() const { - std::unique_lock guard(lock_, std::defer_lock); + std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { return std::nullopt; } @@ -125,7 +125,7 @@ class RealtimeBoxBestEffort */ bool tryGet(const std::function & func) { - std::unique_lock guard(lock_, std::defer_lock); + std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { return false; } @@ -141,7 +141,7 @@ class RealtimeBoxBestEffort template typename std::enable_if_t, void> set(const T & value) { - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); // cppcheck-suppress missingReturn value_ = value; } @@ -150,7 +150,7 @@ class RealtimeBoxBestEffort */ void set(const std::function & func) { - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); func(value_); } @@ -161,7 +161,7 @@ class RealtimeBoxBestEffort template [[nodiscard]] typename std::enable_if_t, U> get() const { - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); return value_; } /** @@ -171,7 +171,7 @@ class RealtimeBoxBestEffort template typename std::enable_if_t, void> get(T & in) const { - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); // cppcheck-suppress missingReturn in = value_; } @@ -182,7 +182,7 @@ class RealtimeBoxBestEffort */ void get(const std::function & func) { - std::lock_guard guard(lock_); + std::lock_guard guard(lock_); func(value_); } diff --git a/test/realtime_box_best_effort_tests.cpp b/test/realtime_box_best_effort_tests.cpp index 16b869cc..1cda8186 100644 --- a/test/realtime_box_best_effort_tests.cpp +++ b/test/realtime_box_best_effort_tests.cpp @@ -172,4 +172,8 @@ TEST(RealtimeBoxBestEffort, smart_ptr_type) box.set([](auto & i) { *i = 200; }); box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); + + box.trySet([](const auto & p) { *p = 10; }); + + box.tryGet([](const auto & p) { EXPECT_EQ(*p, 10); }); } From ccd13a09d18c1e5ab03f0574e413dff060603df2 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Fri, 5 Apr 2024 09:03:31 +0200 Subject: [PATCH 15/33] run precommit --- test/realtime_box_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index 5be458f0..5a87918e 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -171,7 +171,7 @@ TEST(RealtimeBox, smart_ptr_type) box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); - box.trySet([](const auto & p) { *p = 10; }); + box.trySet([](const auto & p) { *p = 10; }); box.tryGet([](const auto & p) { EXPECT_EQ(*p, 10); }); } From 9e0ce28fb1c0bc970b35dc3caaf8271fb15f14d4 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Fri, 5 Apr 2024 09:17:46 +0200 Subject: [PATCH 16/33] precommit should be satisfied now --- include/realtime_tools/realtime_box.h | 6 +++--- test/realtime_box_tests.cpp | 26 ++++++++++++++------------ 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 5642ea32..a44899e1 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -28,8 +28,8 @@ // Author: Lennart Nachtigall -#ifndef REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ -#define REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ +#ifndef REALTIME_TOOLS__REALTIME_BOX_H_ +#define REALTIME_TOOLS__REALTIME_BOX_H_ #include #include @@ -229,4 +229,4 @@ class RealtimeBox }; } // namespace realtime_tools -#endif // REALTIME_TOOLS__REALTIME_BOX_BEST_EFFORT_H_ +#endif // REALTIME_TOOLS__REALTIME_BOX_H_ diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index 5a87918e..fa145bfa 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -53,7 +53,7 @@ struct FromInitializerList std::array data; }; -using namespace realtime_tools; +using realtime_tools::RealtimeBox; TEST(RealtimeBox, empty_construct) { @@ -114,7 +114,7 @@ TEST(RealtimeBox, assignment_operator) DefaultConstructable data; data.a = 1000; RealtimeBox box; - //Assignment operator is always non RT! + // Assignment operator is always non RT! box = data; auto value = box.get(); @@ -124,12 +124,12 @@ TEST(RealtimeBox, typecast_operator) { RealtimeBox box(DefaultConstructable{.a = 100, .str = ""}); - //Use non RT access + // Use non RT access DefaultConstructable data = box; EXPECT_EQ(data.a, 100); - //Use RT access -> returns std::nullopt if the mutex could not be locked + // Use RT access -> returns std::nullopt if the mutex could not be locked std::optional rt_data_access = box; if (rt_data_access) { @@ -143,10 +143,11 @@ TEST(RealtimeBox, pointer_type) int * ptr = &a; RealtimeBox box(ptr); - //This does not and should not compile! - //auto value = box.get(); + // This does not and should not compile! + // auto value = box.get(); - //Instead access it via a passed function. This assues that we access the data within the scope of the lock + // Instead access it via a passed function. + // This assures that we access the data within the scope of the lock box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); box.set([](auto & i) { *i = 200; }); @@ -161,10 +162,11 @@ TEST(RealtimeBox, smart_ptr_type) std::shared_ptr ptr = std::make_shared(100); RealtimeBox box(ptr); - //This does not and should not compile! - //auto value = box.get(); + // This does not and should not compile! + // auto value = box.get(); - //Instead access it via a passed function. This assues that we access the data within the scope of the lock + // Instead access it via a passed function. + // This assures that we access the data within the scope of the lock box.get([](const auto & i) { EXPECT_EQ(*i, 100); }); box.set([](auto & i) { *i = 200; }); @@ -176,8 +178,8 @@ TEST(RealtimeBox, smart_ptr_type) box.tryGet([](const auto & p) { EXPECT_EQ(*p, 10); }); } -//These are the tests from the old RealtimeBox implementation -//They are therefore suffixed with _existing +// These are the tests from the old RealtimeBox implementation +// They are therefore suffixed with _existing class DefaultConstructable_existing { From e3c0d8abc5ce511ce2bb4cb7e3a91560bb9a3a0e Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Fri, 5 Apr 2024 13:53:56 +0200 Subject: [PATCH 17/33] provide specialisation for pointer with deprecation notice --- include/realtime_tools/realtime_box.h | 201 +++++++++++++++++++++++++- test/realtime_box_tests.cpp | 26 +++- 2 files changed, 222 insertions(+), 5 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index a44899e1..79308df8 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -52,9 +52,18 @@ constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; You can use pointers with this box but the access will be different. Only use the get/set methods that take function pointer for accessing the internal value. */ -template -class RealtimeBox + +// Provide a base template for the class +template > +class RealtimeBox; + +// Provide a specialisation for non pointer types +// NOTE: When migrating to a safe access only version just remove the specialisation for pointer +// and let this be the only version! +template +class RealtimeBox { + static_assert(!is_ptr_or_smart_ptr); static_assert( std::is_same_v || std::is_same_v); static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); @@ -227,6 +236,194 @@ class RealtimeBox T value_; mutable mutex_t lock_; }; + +/** + * @brief Specialisation for pointer types. + * WHY is this specialised. We do not want to break compatibility but show a deprecation note + * for get/set etc. methods if used with a pointer type + * They are unsafe to use and should therefore be replace with their correspondents that take an std::function for accessing + * the value behind the pointer +*/ +template +class RealtimeBox +{ + static_assert(is_ptr_or_smart_ptr); + static_assert( + std::is_same_v || std::is_same_v); + static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); + +public: + using mutex_t = mutex_type; + using type = T; + // Provide various constructors + constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {} + constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {} + + // Only enabled for types that can be constructed from an initializer list + template + constexpr RealtimeBox( + const std::initializer_list & init, + std::enable_if_t>) + : value_(init) + { + } + + /** + * @brief set a new content with best effort + * @return false if mutex could not be locked + * @note disabled for pointer types + */ + [[deprecated("trySet is not safe for pointer types - use trySet(std::function...) instead")]] + bool trySet(const T & value) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + value_ = value; + return true; + } + /** + * @brief access the content readable with best effort + * @return false if the mutex could not be locked + * @note only safe way to access pointer type content (rw) + */ + bool trySet(const std::function & func) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + + func(value_); + return true; + } + /** + * @brief get the content with best effort + * @return std::nullopt if content could not be access, otherwise the content is returned + */ + [[deprecated( + "tryGet is not safe for pointer types - use tryGet(std::function...) " + "instead")]] [[nodiscard]] std::optional + tryGet() const + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return std::nullopt; + } + return value_; + } + /** + * @brief access the content (r) with best effort + * @return false if the mutex could not be locked + * @note only safe way to access pointer type content (r) + */ + bool tryGet(const std::function & func) + { + std::unique_lock guard(lock_, std::defer_lock); + if (!guard.try_lock()) { + return false; + } + + func(value_); + return true; + } + + /** + * @brief set the content and wait until the mutex could be locked (RealtimeBox behavior) + * @return true + */ + [[deprecated("set is not safe for pointer types - use set(std::function...) instead")]] + void set(const T & value) + { + std::lock_guard guard(lock_); + // cppcheck-suppress missingReturn + value_ = value; + } + /** + * @brief access the content (rw) and wait until the mutex could locked + */ + void set(const std::function & func) + { + std::lock_guard guard(lock_); + func(value_); + } + + /** + * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) + * @return copy of the value + */ + [[deprecated( + "get is not safe for pointer types - use get(std::function...) instead")]] [[nodiscard]] T + get() const + { + std::lock_guard guard(lock_); + return value_; + } + /** + * @brief get the content and wait until the mutex could be locked + * @note same signature as in the existing RealtimeBox + */ + [[deprecated("get is not safe for pointer types - use get(std::function...) instead")]] + void get(T & in) const + { + std::lock_guard guard(lock_); + // cppcheck-suppress missingReturn + in = value_; + } + /** + * @brief access the content (r) and wait until the mutex could be locked + * @note only safe way to access pointer type content (r) + * @note same signature as in the existing RealtimeBox + */ + void get(const std::function & func) + { + std::lock_guard guard(lock_); + func(value_); + } + + /** + * @brief provide a custom assignment operator for easier usage + * @note only to be used from non-RT! + */ + template + typename std::enable_if_t, void> operator=(const T & value) + { + set(value); + } + + /** + * @brief provide a custom conversion operator + * @note Can only be used from non-RT! + */ + template >> + [[nodiscard]] operator T() const + { + // Only makes sense with the getNonRT method otherwise we would return an std::optional + return get(); + } + /** + * @brief provide a custom conversion operator + * @note Can be used from non-RT and RT contexts + */ + template >> + [[nodiscard]] operator std::optional() const + { + return tryGet(); + } + + // In case one wants to actually use a pointer + // in this implementation we allow accessing the lock directly. + // Note: Be careful with lock.unlock(). + // It may only be called from the thread that locked the mutex! + [[nodiscard]] const mutex_t & getMutex() const { return lock_; } + [[nodiscard]] mutex_t & getMutex() { return lock_; } + +private: + T value_; + mutable mutex_t lock_; +}; + } // namespace realtime_tools #endif // REALTIME_TOOLS__REALTIME_BOX_H_ diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index fa145bfa..5265b6df 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -122,7 +122,7 @@ TEST(RealtimeBox, assignment_operator) } TEST(RealtimeBox, typecast_operator) { - RealtimeBox box(DefaultConstructable{.a = 100, .str = ""}); + RealtimeBox box(DefaultConstructable{.a = 100, .str = ""}); // Use non RT access DefaultConstructable data = box; @@ -142,7 +142,7 @@ TEST(RealtimeBox, pointer_type) int a = 100; int * ptr = &a; - RealtimeBox box(ptr); + RealtimeBox box(ptr); // This does not and should not compile! // auto value = box.get(); @@ -161,7 +161,7 @@ TEST(RealtimeBox, smart_ptr_type) { std::shared_ptr ptr = std::make_shared(100); - RealtimeBox box(ptr); + RealtimeBox> box(ptr); // This does not and should not compile! // auto value = box.get(); @@ -178,6 +178,26 @@ TEST(RealtimeBox, smart_ptr_type) box.tryGet([](const auto & p) { EXPECT_EQ(*p, 10); }); } +TEST(RealtimeBox, deprecated_note) +{ + int a = 100; + int * ptr = &a; + + RealtimeBox box(ptr); + + int * res; + box.get(res); + EXPECT_EQ(*res, 100); + + EXPECT_EQ(*box.get(), 100); + + std::shared_ptr sptr = std::make_shared(10); + + RealtimeBox> sbox(sptr); + + EXPECT_EQ(*sbox.get(), 10); +} + // These are the tests from the old RealtimeBox implementation // They are therefore suffixed with _existing From d3869d254d302332cad1d591631d52d5896cf8de Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sat, 6 Apr 2024 09:30:07 +0200 Subject: [PATCH 18/33] Adapt copyright notice --- test/realtime_box_tests.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index 5265b6df..f36a4cf3 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -1,4 +1,5 @@ // Copyright (c) 2024, Lennart Nachtigall +// Copyright (c) 2019, Open Source Robotics Foundation, Inc. // // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are met: @@ -26,8 +27,6 @@ // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE // POSSIBILITY OF SUCH DAMAGE. -// Author: Lennart Nachtigall - #include #include From 85a421ec04515efb0e415a30446b601a7444ad3e Mon Sep 17 00:00:00 2001 From: "Dr. Denis" Date: Wed, 9 Oct 2024 19:12:25 +0200 Subject: [PATCH 19/33] Update test/realtime_box_tests.cpp Co-authored-by: Sai Kishor Kothakota --- test/realtime_box_tests.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index f36a4cf3..21d41789 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -161,8 +161,6 @@ TEST(RealtimeBox, smart_ptr_type) std::shared_ptr ptr = std::make_shared(100); RealtimeBox> box(ptr); - // This does not and should not compile! - // auto value = box.get(); // Instead access it via a passed function. // This assures that we access the data within the scope of the lock From 02df04af08d8892958d1bdaadf44e1bef1c2547b Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Wed, 30 Oct 2024 20:22:26 +0000 Subject: [PATCH 20/33] Add missing include --- include/realtime_tools/realtime_box.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 79308df8..a84fa032 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -31,6 +31,7 @@ #ifndef REALTIME_TOOLS__REALTIME_BOX_H_ #define REALTIME_TOOLS__REALTIME_BOX_H_ +#include #include #include #include From dcf97cd606ada1baf7585e796d7ffcbef9f30100 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sun, 10 Nov 2024 09:25:22 +0100 Subject: [PATCH 21/33] removed deprecation overload for pointer types and addressed snake case recommendation --- include/realtime_tools/realtime_box.h | 210 ++------------------------ test/realtime_box_tests.cpp | 28 +--- 2 files changed, 13 insertions(+), 225 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index a84fa032..8b87711f 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -54,17 +54,12 @@ constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; Only use the get/set methods that take function pointer for accessing the internal value. */ -// Provide a base template for the class -template > -class RealtimeBox; - // Provide a specialisation for non pointer types // NOTE: When migrating to a safe access only version just remove the specialisation for pointer // and let this be the only version! -template -class RealtimeBox +template +class RealtimeBox { - static_assert(!is_ptr_or_smart_ptr); static_assert( std::is_same_v || std::is_same_v); static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); @@ -91,7 +86,7 @@ class RealtimeBox * @note disabled for pointer types */ template - typename std::enable_if_t, bool> trySet(const T & value) + typename std::enable_if_t, bool> try_set(const T & value) { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { @@ -105,7 +100,7 @@ class RealtimeBox * @return false if the mutex could not be locked * @note only safe way to access pointer type content (rw) */ - bool trySet(const std::function & func) + bool try_set(const std::function & func) { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { @@ -120,7 +115,7 @@ class RealtimeBox * @return std::nullopt if content could not be access, otherwise the content is returned */ template - [[nodiscard]] typename std::enable_if_t, std::optional> tryGet() const + [[nodiscard]] typename std::enable_if_t, std::optional> try_get() const { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { @@ -133,7 +128,7 @@ class RealtimeBox * @return false if the mutex could not be locked * @note only safe way to access pointer type content (r) */ - bool tryGet(const std::function & func) + bool try_get(const std::function & func) { std::unique_lock guard(lock_, std::defer_lock); if (!guard.try_lock()) { @@ -223,202 +218,15 @@ class RealtimeBox template >> [[nodiscard]] operator std::optional() const { - return tryGet(); - } - - // In case one wants to actually use a pointer - // in this implementation we allow accessing the lock directly. - // Note: Be careful with lock.unlock(). - // It may only be called from the thread that locked the mutex! - [[nodiscard]] const mutex_t & getMutex() const { return lock_; } - [[nodiscard]] mutex_t & getMutex() { return lock_; } - -private: - T value_; - mutable mutex_t lock_; -}; - -/** - * @brief Specialisation for pointer types. - * WHY is this specialised. We do not want to break compatibility but show a deprecation note - * for get/set etc. methods if used with a pointer type - * They are unsafe to use and should therefore be replace with their correspondents that take an std::function for accessing - * the value behind the pointer -*/ -template -class RealtimeBox -{ - static_assert(is_ptr_or_smart_ptr); - static_assert( - std::is_same_v || std::is_same_v); - static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); - -public: - using mutex_t = mutex_type; - using type = T; - // Provide various constructors - constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {} - constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {} - - // Only enabled for types that can be constructed from an initializer list - template - constexpr RealtimeBox( - const std::initializer_list & init, - std::enable_if_t>) - : value_(init) - { - } - - /** - * @brief set a new content with best effort - * @return false if mutex could not be locked - * @note disabled for pointer types - */ - [[deprecated("trySet is not safe for pointer types - use trySet(std::function...) instead")]] - bool trySet(const T & value) - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return false; - } - value_ = value; - return true; - } - /** - * @brief access the content readable with best effort - * @return false if the mutex could not be locked - * @note only safe way to access pointer type content (rw) - */ - bool trySet(const std::function & func) - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return false; - } - - func(value_); - return true; - } - /** - * @brief get the content with best effort - * @return std::nullopt if content could not be access, otherwise the content is returned - */ - [[deprecated( - "tryGet is not safe for pointer types - use tryGet(std::function...) " - "instead")]] [[nodiscard]] std::optional - tryGet() const - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return std::nullopt; - } - return value_; - } - /** - * @brief access the content (r) with best effort - * @return false if the mutex could not be locked - * @note only safe way to access pointer type content (r) - */ - bool tryGet(const std::function & func) - { - std::unique_lock guard(lock_, std::defer_lock); - if (!guard.try_lock()) { - return false; - } - - func(value_); - return true; - } - - /** - * @brief set the content and wait until the mutex could be locked (RealtimeBox behavior) - * @return true - */ - [[deprecated("set is not safe for pointer types - use set(std::function...) instead")]] - void set(const T & value) - { - std::lock_guard guard(lock_); - // cppcheck-suppress missingReturn - value_ = value; - } - /** - * @brief access the content (rw) and wait until the mutex could locked - */ - void set(const std::function & func) - { - std::lock_guard guard(lock_); - func(value_); - } - - /** - * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour) - * @return copy of the value - */ - [[deprecated( - "get is not safe for pointer types - use get(std::function...) instead")]] [[nodiscard]] T - get() const - { - std::lock_guard guard(lock_); - return value_; - } - /** - * @brief get the content and wait until the mutex could be locked - * @note same signature as in the existing RealtimeBox - */ - [[deprecated("get is not safe for pointer types - use get(std::function...) instead")]] - void get(T & in) const - { - std::lock_guard guard(lock_); - // cppcheck-suppress missingReturn - in = value_; - } - /** - * @brief access the content (r) and wait until the mutex could be locked - * @note only safe way to access pointer type content (r) - * @note same signature as in the existing RealtimeBox - */ - void get(const std::function & func) - { - std::lock_guard guard(lock_); - func(value_); - } - - /** - * @brief provide a custom assignment operator for easier usage - * @note only to be used from non-RT! - */ - template - typename std::enable_if_t, void> operator=(const T & value) - { - set(value); - } - - /** - * @brief provide a custom conversion operator - * @note Can only be used from non-RT! - */ - template >> - [[nodiscard]] operator T() const - { - // Only makes sense with the getNonRT method otherwise we would return an std::optional - return get(); - } - /** - * @brief provide a custom conversion operator - * @note Can be used from non-RT and RT contexts - */ - template >> - [[nodiscard]] operator std::optional() const - { - return tryGet(); + return try_set(); } // In case one wants to actually use a pointer // in this implementation we allow accessing the lock directly. // Note: Be careful with lock.unlock(). // It may only be called from the thread that locked the mutex! - [[nodiscard]] const mutex_t & getMutex() const { return lock_; } - [[nodiscard]] mutex_t & getMutex() { return lock_; } + [[nodiscard]] const mutex_t & get_mutex() const { return lock_; } + [[nodiscard]] mutex_t & get_mutex() { return lock_; } private: T value_; diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index e3a3db77..32924cfa 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -129,7 +129,7 @@ TEST(RealtimeBox, typecast_operator) EXPECT_EQ(data.a, 100); // Use RT access -> returns std::nullopt if the mutex could not be locked - std::optional rt_data_access = box.tryGet(); + std::optional rt_data_access = box.try_get(); if (rt_data_access) { EXPECT_EQ(rt_data_access->a, 100); @@ -153,7 +153,7 @@ TEST(RealtimeBox, pointer_type) box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); - box.tryGet([](const auto & i) { EXPECT_EQ(*i, 200); }); + box.try_get([](const auto & i) { EXPECT_EQ(*i, 200); }); } TEST(RealtimeBox, smart_ptr_type) @@ -170,29 +170,9 @@ TEST(RealtimeBox, smart_ptr_type) box.get([](const auto & i) { EXPECT_EQ(*i, 200); }); - box.trySet([](const auto & p) { *p = 10; }); + box.try_set([](const auto & p) { *p = 10; }); - box.tryGet([](const auto & p) { EXPECT_EQ(*p, 10); }); -} - -TEST(RealtimeBox, deprecated_note) -{ - int a = 100; - int * ptr = &a; - - RealtimeBox box(ptr); - - int * res; - box.get(res); - EXPECT_EQ(*res, 100); - - EXPECT_EQ(*box.get(), 100); - - std::shared_ptr sptr = std::make_shared(10); - - RealtimeBox> sbox(sptr); - - EXPECT_EQ(*sbox.get(), 10); + box.try_get([](const auto & p) { EXPECT_EQ(*p, 10); }); } // These are the tests from the old RealtimeBox implementation From e22b575a97bc8688695214f40727d8b4430bae08 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Sun, 10 Nov 2024 09:51:29 +0100 Subject: [PATCH 22/33] added copy/move constructors --- include/realtime_tools/realtime_box.h | 40 +++++++++++++++++++++++++++ test/realtime_box_tests.cpp | 36 ++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 8b87711f..c8d2a268 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -71,6 +71,34 @@ class RealtimeBox constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {} constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {} + //Copy constructor + constexpr RealtimeBox(const RealtimeBox & o) + { + //Lock the other box mutex + std::unique_lock lock(o.lock_); + //We do not need to lock our own mutex because we are currently in the process of being created + value_ = o.value_; + } + //Copy assignment constructor + constexpr RealtimeBox & operator=(const RealtimeBox & o) + { + //Check for self assignment (and a potential deadlock) + if (&o != this) { + //Lock the other box mutex + std::unique_lock lock_other(o.lock_); + std::unique_lock lock_self(lock_); + + value_ = o.value_; + } + return *this; + } + constexpr RealtimeBox(RealtimeBox && o) + { + //Lock the other box mutex + std::unique_lock lock(o.lock_); + //We do not need to lock our own mutex because we are currently in the process of being created + value_ = std::move(o.value_); + } // Only enabled for types that can be constructed from an initializer list template constexpr RealtimeBox( @@ -79,6 +107,18 @@ class RealtimeBox : value_(init) { } + constexpr RealtimeBox & operator=(RealtimeBox && o) + { + //Check for self assignment (and a potential deadlock) + if (&o != this) { + //Lock the other box mutex + std::unique_lock lock_other(o.lock_); + std::unique_lock lock_self(lock_); + + value_ = std::move(o.value_); + } + return *this; + } /** * @brief set a new content with best effort diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index 32924cfa..c7fbacce 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -218,3 +218,39 @@ TEST(RealtimeBox, set_and_get_existing) box.get(output); EXPECT_EQ('z', output); } + +TEST(RealtimeBox, copy_assign) +{ + RealtimeBox box_a('a'); + RealtimeBox box_b('b'); + + //Assign b to a -> a should now contain b + box_a = box_b; + + EXPECT_EQ('b', box_a.try_get().value()); +} +TEST(RealtimeBox, copy) +{ + RealtimeBox box_b('b'); + RealtimeBox box_a(box_b); + + EXPECT_EQ('b', box_a.try_get().value()); +} + +TEST(RealtimeBox, move_assign) +{ + RealtimeBox box_a('a'); + RealtimeBox box_b('b'); + + //Move b to a -> a should now contain b + box_a = std::move(box_b); + + EXPECT_EQ('b', box_a.try_get().value()); +} +TEST(RealtimeBox, move) +{ + RealtimeBox box_b('b'); + RealtimeBox box_a(std::move(box_b)); + + EXPECT_EQ('b', box_a.try_get().value()); +} From 6ece6a48a289dcbdf0af8e842538f322328c853f Mon Sep 17 00:00:00 2001 From: Christoph Froehlich Date: Sun, 10 Nov 2024 19:28:30 +0000 Subject: [PATCH 23/33] Satisfy pre-commit --- include/realtime_tools/realtime_box.h | 20 ++++++++++---------- test/realtime_box_tests.cpp | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index c8d2a268..50c097f5 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -71,20 +71,20 @@ class RealtimeBox constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {} constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {} - //Copy constructor + // Copy constructor constexpr RealtimeBox(const RealtimeBox & o) { - //Lock the other box mutex + // Lock the other box mutex std::unique_lock lock(o.lock_); - //We do not need to lock our own mutex because we are currently in the process of being created + // We do not need to lock our own mutex because we are currently in the process of being created value_ = o.value_; } - //Copy assignment constructor + // Copy assignment constructor constexpr RealtimeBox & operator=(const RealtimeBox & o) { - //Check for self assignment (and a potential deadlock) + // Check for self assignment (and a potential deadlock) if (&o != this) { - //Lock the other box mutex + // Lock the other box mutex std::unique_lock lock_other(o.lock_); std::unique_lock lock_self(lock_); @@ -94,9 +94,9 @@ class RealtimeBox } constexpr RealtimeBox(RealtimeBox && o) { - //Lock the other box mutex + // Lock the other box mutex std::unique_lock lock(o.lock_); - //We do not need to lock our own mutex because we are currently in the process of being created + // We do not need to lock our own mutex because we are currently in the process of being created value_ = std::move(o.value_); } // Only enabled for types that can be constructed from an initializer list @@ -109,9 +109,9 @@ class RealtimeBox } constexpr RealtimeBox & operator=(RealtimeBox && o) { - //Check for self assignment (and a potential deadlock) + // Check for self assignment (and a potential deadlock) if (&o != this) { - //Lock the other box mutex + // Lock the other box mutex std::unique_lock lock_other(o.lock_); std::unique_lock lock_self(lock_); diff --git a/test/realtime_box_tests.cpp b/test/realtime_box_tests.cpp index c7fbacce..5d13dee7 100644 --- a/test/realtime_box_tests.cpp +++ b/test/realtime_box_tests.cpp @@ -224,7 +224,7 @@ TEST(RealtimeBox, copy_assign) RealtimeBox box_a('a'); RealtimeBox box_b('b'); - //Assign b to a -> a should now contain b + // Assign b to a -> a should now contain b box_a = box_b; EXPECT_EQ('b', box_a.try_get().value()); @@ -242,7 +242,7 @@ TEST(RealtimeBox, move_assign) RealtimeBox box_a('a'); RealtimeBox box_b('b'); - //Move b to a -> a should now contain b + // Move b to a -> a should now contain b box_a = std::move(box_b); EXPECT_EQ('b', box_a.try_get().value()); From 450e3d5dc0ab51accab448aac6872e14507a7866 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 10:13:16 +0100 Subject: [PATCH 24/33] Update include/realtime_tools/realtime_box.h conversion operator should use try_get not try_set Co-authored-by: Sai Kishor Kothakota --- include/realtime_tools/realtime_box.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 50c097f5..2dd28a40 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -258,7 +258,7 @@ class RealtimeBox template >> [[nodiscard]] operator std::optional() const { - return try_set(); + return try_get(); } // In case one wants to actually use a pointer From 3a31a02417ceb10caa47f3c0ad8d7d6b607af14c Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 10:14:52 +0100 Subject: [PATCH 25/33] Update realtime_box.h Fixed copyright and comments --- include/realtime_tools/realtime_box.h | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 2dd28a40..9dd75fea 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -1,3 +1,4 @@ +// Copyright (c) 2009, Willow Garage, Inc. // Copyright (c) 2024, Lennart Nachtigall // // Redistribution and use in source and binary forms, with or without @@ -53,10 +54,6 @@ constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; You can use pointers with this box but the access will be different. Only use the get/set methods that take function pointer for accessing the internal value. */ - -// Provide a specialisation for non pointer types -// NOTE: When migrating to a safe access only version just remove the specialisation for pointer -// and let this be the only version! template class RealtimeBox { From 0b61023db7c1116a1bc2ccb67310a73b62609226 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 10:28:17 +0100 Subject: [PATCH 26/33] Re add comment about mutex --- include/realtime_tools/realtime_box.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 9dd75fea..b6a2fdc7 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -267,6 +267,12 @@ class RealtimeBox private: T value_; + + // Protects access to the thing in the box. This mutex is + // guaranteed to be locked for no longer than the duration of the + // copy, so as long as the copy is realtime safe and the OS has + // priority inheritance for mutexes, this lock can be safely locked + // from within realtime. mutable mutex_t lock_; }; From 9b9444db75ac3361dea0912bc55bb19700d0b537 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 13:41:37 +0100 Subject: [PATCH 27/33] Provide different specialisations for more convenience - remove striction to std::mutex and std::recursive_mutex --- include/realtime_tools/realtime_box.h | 35 +++++++++++++++++++-------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index b6a2fdc7..cda42b27 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -55,21 +55,19 @@ constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer::value; Only use the get/set methods that take function pointer for accessing the internal value. */ template -class RealtimeBox +class RealtimeBoxBase { - static_assert( - std::is_same_v || std::is_same_v); static_assert(std::is_copy_constructible_v, "Passed type must be copy constructible"); public: using mutex_t = mutex_type; using type = T; // Provide various constructors - constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {} - constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {} + constexpr explicit RealtimeBoxBase(const T & init = T{}) : value_(init) {} + constexpr explicit RealtimeBoxBase(const T && init) : value_(std::move(init)) {} // Copy constructor - constexpr RealtimeBox(const RealtimeBox & o) + constexpr RealtimeBoxBase(const RealtimeBoxBase & o) { // Lock the other box mutex std::unique_lock lock(o.lock_); @@ -77,7 +75,7 @@ class RealtimeBox value_ = o.value_; } // Copy assignment constructor - constexpr RealtimeBox & operator=(const RealtimeBox & o) + constexpr RealtimeBoxBase & operator=(const RealtimeBoxBase & o) { // Check for self assignment (and a potential deadlock) if (&o != this) { @@ -89,7 +87,7 @@ class RealtimeBox } return *this; } - constexpr RealtimeBox(RealtimeBox && o) + constexpr RealtimeBox(RealtimeBoxBase && o) { // Lock the other box mutex std::unique_lock lock(o.lock_); @@ -98,13 +96,13 @@ class RealtimeBox } // Only enabled for types that can be constructed from an initializer list template - constexpr RealtimeBox( + constexpr RealtimeBoxBase( const std::initializer_list & init, std::enable_if_t>) : value_(init) { } - constexpr RealtimeBox & operator=(RealtimeBox && o) + constexpr RealtimeBoxBase & operator=(RealtimeBoxBase && o) { // Check for self assignment (and a potential deadlock) if (&o != this) { @@ -276,6 +274,23 @@ class RealtimeBox mutable mutex_t lock_; }; +//Introduce some easier to use names + +// Only kept for compatibility reasons +template +using RealtimeBoxBestEffort [[deprecated]] = RealtimeBox; + +// Provide specialisations for different mutex types +template +using RealtimeBoxStandard = RealtimeBox; + +template +using RealtimeBoxRecursive = RealtimeBox; + +// This is the specialisation we recommend to use in the end +template +using RealtimeBox = RealtimeBoxDefault; + } // namespace realtime_tools #endif // REALTIME_TOOLS__REALTIME_BOX_H_ From 16bd2ba9eefb0f5dbf7f661f7a3940f5084680b1 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 13:43:49 +0100 Subject: [PATCH 28/33] fix pre-commit --- include/realtime_tools/realtime_box.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index cda42b27..7a23fad8 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -274,7 +274,7 @@ class RealtimeBoxBase mutable mutex_t lock_; }; -//Introduce some easier to use names +// Introduce some easier to use names // Only kept for compatibility reasons template From 4138280738c2363a44334bc74f0fea2db7912975 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 13:53:33 +0100 Subject: [PATCH 29/33] Fixed some happy little mistakes --- include/realtime_tools/realtime_box.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 7a23fad8..5243a62c 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -87,7 +87,7 @@ class RealtimeBoxBase } return *this; } - constexpr RealtimeBox(RealtimeBoxBase && o) + constexpr RealtimeBoxBase(RealtimeBoxBase && o) { // Lock the other box mutex std::unique_lock lock(o.lock_); @@ -278,18 +278,18 @@ class RealtimeBoxBase // Only kept for compatibility reasons template -using RealtimeBoxBestEffort [[deprecated]] = RealtimeBox; +using RealtimeBoxBestEffort [[deprecated]] = RealtimeBoxBase; // Provide specialisations for different mutex types template -using RealtimeBoxStandard = RealtimeBox; +using RealtimeBoxStandard = RealtimeBoxBase; template -using RealtimeBoxRecursive = RealtimeBox; +using RealtimeBoxRecursive = RealtimeBoxBase; // This is the specialisation we recommend to use in the end template -using RealtimeBox = RealtimeBoxDefault; +using RealtimeBox = RealtimeBoxStandard; } // namespace realtime_tools From d128cc3e1dcb437f6e80402fc45f2d438845a4d4 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 15:09:15 +0100 Subject: [PATCH 30/33] Useful deprecation message Co-authored-by: Sai Kishor Kothakota --- include/realtime_tools/realtime_box.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 5243a62c..fd74c898 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -278,7 +278,7 @@ class RealtimeBoxBase // Only kept for compatibility reasons template -using RealtimeBoxBestEffort [[deprecated]] = RealtimeBoxBase; +using RealtimeBoxBestEffort [[deprecated ("Use RealtimeBox instead")]] = RealtimeBoxBase; // Provide specialisations for different mutex types template From 0cb679d2aa24db621a0d8fa8615922d2013e83d5 Mon Sep 17 00:00:00 2001 From: Lennart Nachtigall Date: Thu, 21 Nov 2024 15:25:30 +0100 Subject: [PATCH 31/33] precommit --- include/realtime_tools/realtime_box.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index fd74c898..77132a75 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -278,7 +278,8 @@ class RealtimeBoxBase // Only kept for compatibility reasons template -using RealtimeBoxBestEffort [[deprecated ("Use RealtimeBox instead")]] = RealtimeBoxBase; +using RealtimeBoxBestEffort [[deprecated("Use RealtimeBox instead")]] = + RealtimeBoxBase; // Provide specialisations for different mutex types template From bbe227a1bfe56b7a92dfa8b99911f50eb0576a6d Mon Sep 17 00:00:00 2001 From: Sai Kishor Kothakota Date: Thu, 21 Nov 2024 20:49:11 +0100 Subject: [PATCH 32/33] Readd the old realtime_box author --- include/realtime_tools/realtime_box.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 77132a75..48b50db4 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -27,6 +27,7 @@ // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE // POSSIBILITY OF SUCH DAMAGE. +// Author: Stuart Glaser // Author: Lennart Nachtigall #ifndef REALTIME_TOOLS__REALTIME_BOX_H_ From 3f57b786f5e4eafbdb00ba11427631b63bb4d9cf Mon Sep 17 00:00:00 2001 From: Bence Magyar Date: Sat, 23 Nov 2024 09:41:21 +0000 Subject: [PATCH 33/33] Update include/realtime_tools/realtime_box.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Christoph Fröhlich --- include/realtime_tools/realtime_box.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/realtime_tools/realtime_box.h b/include/realtime_tools/realtime_box.h index 48b50db4..975a38d5 100644 --- a/include/realtime_tools/realtime_box.h +++ b/include/realtime_tools/realtime_box.h @@ -99,7 +99,7 @@ class RealtimeBoxBase template constexpr RealtimeBoxBase( const std::initializer_list & init, - std::enable_if_t>) + std::enable_if_t>>) : value_(init) { }