From 577ff94ea1f4ceaa53edae0714c2c673fad4d0cb Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 17 Oct 2024 21:42:43 +0200 Subject: [PATCH 01/22] [#490] Implement slice loaning in iceoryx2_cxx --- iceoryx2-ffi/cxx/include/iox/slice.hpp | 95 +++++++--- .../include/iox2/port_factory_publisher.hpp | 21 ++- iceoryx2-ffi/cxx/include/iox2/publisher.hpp | 47 ++++- iceoryx2-ffi/cxx/include/iox2/sample.hpp | 27 ++- iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 40 +++- .../cxx/include/iox2/sample_mut_uninit.hpp | 22 +++ .../service_builder_publish_subscribe.hpp | 44 +++-- .../src/service_publish_subscribe_tests.cpp | 172 ++++++++++++++++++ .../src/api/port_factory_publisher_builder.rs | 26 +++ iceoryx2-ffi/ffi/src/api/publisher.rs | 43 +++-- iceoryx2-ffi/ffi/src/api/sample.rs | 21 ++- iceoryx2-ffi/ffi/src/api/sample_mut.rs | 48 +++-- .../ffi/src/api/service_builder_pub_sub.rs | 35 ++++ iceoryx2/src/port/publisher.rs | 5 + 14 files changed, 553 insertions(+), 93 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index c0fcae70a..4b7cc4e04 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -16,6 +16,7 @@ #include "iox/assertions_addendum.hpp" #include +#include namespace iox { template @@ -25,29 +26,81 @@ class Slice { using ConstIterator = const T*; using ValueType = T; - auto size() const -> uint64_t { - IOX_TODO(); - } - auto operator[](const uint64_t n) const -> const T& { - IOX_TODO(); - } - auto operator[](const uint64_t n) -> T& { - IOX_TODO(); - } - auto begin() -> Iterator { - IOX_TODO(); - } - auto begin() const -> ConstIterator { - IOX_TODO(); - } - auto end() -> Iterator { - IOX_TODO(); - } - auto end() const -> ConstIterator { - IOX_TODO(); - } + Slice(const T* data, uint64_t number_of_elements); + + auto number_of_elements() const -> uint64_t; + auto operator[](uint64_t n) const -> const T&; + auto operator[](uint64_t n) -> T&; + + auto begin() -> Iterator; + auto begin() const -> ConstIterator; + auto end() -> Iterator; + auto end() const -> ConstIterator; + + auto data() -> T*; + auto data() const -> const T*; + + private: + T* m_data; + uint64_t m_number_of_elements; }; +template +Slice::Slice(const T* data, uint64_t number_of_elements) + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) constness protected by const class specification + : m_data { const_cast(data) } + , m_number_of_elements { number_of_elements } { +} + +template +auto Slice::number_of_elements() const -> uint64_t { + return m_number_of_elements; +} + +template +auto Slice::operator[](const uint64_t n) const -> const T& { + IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); + return *(m_data + n); +} + +template +auto Slice::operator[](const uint64_t n) -> T& { + IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); + return *(m_data + n); +} + +template +auto Slice::begin() -> Iterator { + return m_data; +} + +template +auto Slice::begin() const -> ConstIterator { + return m_data; +} + +template +auto Slice::end() -> Iterator { + static_assert(!std::is_same_v, "Slice is not allowed"); + return m_data + m_number_of_elements; +} + +template +auto Slice::end() const -> ConstIterator { + static_assert(!std::is_same_v, "Slice is not allowed"); + return m_data + m_number_of_elements; +} + +template +auto Slice::data() -> T* { + return m_data; +} + +template +auto Slice::data() const -> const T* { + return m_data; +} + template struct IsSlice { static constexpr bool VALUE = false; diff --git a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp index c5b784368..164f5a7ee 100644 --- a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp @@ -71,7 +71,26 @@ PortFactoryPublisher::create() && -> iox::expected(iox::into(value))); }); - m_max_slice_len.and_then([](auto) { IOX_TODO(); }); + m_max_slice_len + .and_then([&](auto value) { + // The payload type used by the C API is always a [u8]. + // Thus need to convert from N to N * sizeof(payload). + // TODO: Consider alignment... not aligning each element properly will impact performance + if constexpr (iox::IsSlice::VALUE) { + iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, + value * sizeof(typename Payload::ValueType)); + } else { + iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, value * sizeof(Payload)); + } + }) + .or_else([&]() { + // Assume only one element if not otherwise specified + if constexpr (iox::IsSlice::VALUE) { + iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, sizeof(typename Payload::ValueType)); + } else { + iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, sizeof(Payload)); + } + }); m_max_loaned_samples.and_then( [&](auto value) { iox2_port_factory_publisher_builder_set_max_loaned_samples(&m_handle, value); }); diff --git a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp index bec3e6bf6..f678c3343 100644 --- a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp @@ -47,6 +47,9 @@ class Publisher { /// since the [`Subscriber`]s buffer is full. auto unable_to_deliver_strategy() const -> UnableToDeliverStrategy; + /// Returns the maximum number of elements that can be loaned in a slice. + auto max_slice_len() const -> uint64_t; + /// Copies the input `value` into a [`SampleMut`] and delivers it. /// On success it returns the number of [`Subscriber`]s that received /// the data, otherwise a [`PublisherSendError`] describing the failure. @@ -56,6 +59,7 @@ class Publisher { /// The user has to initialize the payload before it can be sent. /// /// On failure it returns [`PublisherLoanError`] describing the failure. + template ::VALUE, void>> auto loan_uninit() -> iox::expected, PublisherLoanError>; /// Loans/allocates a [`SampleMut`] from the underlying data segment of the [`Publisher`] @@ -63,6 +67,7 @@ class Publisher { /// can be used to loan an uninitalized [`SampleMut`]. /// /// On failure it returns [`PublisherLoanError`] describing the failure. + template ::VALUE, void>> auto loan() -> iox::expected, PublisherLoanError>; /// Loans/allocates a [`SampleMut`] from the underlying data segment of the [`Publisher`] @@ -139,6 +144,20 @@ inline auto Publisher::unable_to_deliver_strategy() cons return iox::into(static_cast(iox2_publisher_unable_to_deliver_strategy(&m_handle))); } + +template +inline auto Publisher::max_slice_len() const -> uint64_t { + // NOTE: The C API always uses a [u8] payload, therefore the max length returned is the number of bytes. + // Dividing by the size gives the number of slice elements available to the C++ API. + // + // NOTE: For non-slice types, the number of slice elements is always 1. + if constexpr (iox::IsSlice::VALUE) { + return iox2_publisher_max_slice_len(&m_handle) / sizeof(typename Payload::ValueType); + } else { + return iox2_publisher_max_slice_len(&m_handle) / sizeof(Payload); + } +} + template inline auto Publisher::id() const -> UniquePublisherId { iox2_unique_publisher_id_h id_handle = nullptr; @@ -164,11 +183,12 @@ inline auto Publisher::send_copy(const Payload& payload) } template +template inline auto Publisher::loan_uninit() -> iox::expected, PublisherLoanError> { SampleMutUninit sample; - auto result = iox2_publisher_loan(&m_handle, &sample.m_sample.m_sample, &sample.m_sample.m_handle); + auto result = iox2_publisher_loan_slice_uninit(&m_handle, &sample.m_sample.m_sample, &sample.m_sample.m_handle, 1); if (result == IOX2_OK) { return iox::ok(std::move(sample)); @@ -178,6 +198,7 @@ inline auto Publisher::loan_uninit() } template +template inline auto Publisher::loan() -> iox::expected, PublisherLoanError> { auto sample = loan_uninit(); @@ -195,14 +216,34 @@ template template inline auto Publisher::loan_slice(const uint64_t number_of_elements) -> iox::expected, PublisherLoanError> { - IOX_TODO(); + auto sample_uninit = loan_slice_uninit(number_of_elements); + + if (sample_uninit.has_error()) { + return iox::err(sample_uninit.error()); + } + auto sample_init = std::move(sample_uninit.value()); + + for (auto& item : sample_init.payload_slice()) { + new (&item) typename T::ValueType(); + } + + return iox::ok(assume_init(std::move(sample_init))); } template template inline auto Publisher::loan_slice_uninit(const uint64_t number_of_elements) -> iox::expected, PublisherLoanError> { - IOX_TODO(); + SampleMutUninit sample; + + auto result = iox2_publisher_loan_slice_uninit( + &m_handle, &sample.m_sample.m_sample, &sample.m_sample.m_handle, number_of_elements); + + if (result == IOX2_OK) { + return iox::ok(std::move(sample)); + } + + return iox::err(iox::into(result)); } template diff --git a/iceoryx2-ffi/cxx/include/iox2/sample.hpp b/iceoryx2-ffi/cxx/include/iox2/sample.hpp index bc95d9c15..ee7d8590a 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample.hpp @@ -14,6 +14,7 @@ #define IOX2_SAMPLE_HPP #include "iox/assertions_addendum.hpp" +#include "iox/slice.hpp" #include "iox2/header_publish_subscribe.hpp" #include "iox2/internal/iceoryx2.hpp" #include "iox2/service_type.hpp" @@ -49,8 +50,13 @@ class Sample { auto operator->() const -> const Payload*; /// Returns a reference to the payload of the [`Sample`] + template ::VALUE, void>> auto payload() const -> const Payload&; + /// Returns a slice to navigate the payload of the [`Sample`] + template ::VALUE, void>> + auto payload_slice() const -> Payload; + /// Returns a reference to the user_header of the [`Sample`] template , T>> auto user_header() const -> const T&; @@ -121,14 +127,24 @@ inline auto Sample::operator->() const -> const Payload* } template +template inline auto Sample::payload() const -> const Payload& { - const void* payload_ptr = nullptr; - size_t payload_len = 0; + const void* ptr = nullptr; - iox2_sample_payload(&m_handle, &payload_ptr, &payload_len); - IOX_ASSERT(sizeof(Payload) <= payload_len, ""); + iox2_sample_payload(&m_handle, &ptr, nullptr); - return *static_cast(payload_ptr); + return *static_cast(ptr); +} + +template +template +inline auto Sample::payload_slice() const -> Payload { + const void* ptr = nullptr; + size_t number_of_elements = 0; + + iox2_sample_payload(&m_handle, &ptr, &number_of_elements); + + return Payload(static_cast(ptr), number_of_elements); } template @@ -154,7 +170,6 @@ inline auto Sample::origin() const -> UniquePublisherId return header().publisher_id(); } - } // namespace iox2 #endif diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index dd9d04400..7219eb245 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -76,11 +76,19 @@ class SampleMut { auto user_header_mut() -> T&; /// Returns a reference to the const payload of the sample. + template ::VALUE, void>> auto payload() const -> const Payload&; /// Returns a reference to the payload of the sample. + template ::VALUE, void>> auto payload_mut() -> Payload&; + template ::VALUE, void>> + auto payload_slice() const -> const Payload; + + template ::VALUE, void>> + auto payload_slice_mut() -> Payload; + private: template friend class Publisher; @@ -186,27 +194,47 @@ inline auto SampleMut::user_header_mut() -> T& { } template +template inline auto SampleMut::payload() const -> const Payload& { const void* ptr = nullptr; - size_t payload_len = 0; - iox2_sample_mut_payload(&m_handle, &ptr, &payload_len); - IOX_ASSERT(sizeof(Payload) <= payload_len, ""); + iox2_sample_mut_payload(&m_handle, &ptr, nullptr); return *static_cast(ptr); } template +template inline auto SampleMut::payload_mut() -> Payload& { void* ptr = nullptr; - size_t payload_len = 0; - iox2_sample_mut_payload_mut(&m_handle, &ptr, &payload_len); - IOX_ASSERT(sizeof(Payload) <= payload_len, ""); + iox2_sample_mut_payload_mut(&m_handle, &ptr, nullptr); return *static_cast(ptr); } +template +template +inline auto SampleMut::payload_slice() const -> const Payload { + const void* ptr = nullptr; + size_t number_of_elements = 0; + + iox2_sample_mut_payload(&m_handle, &ptr, &number_of_elements); + + return Payload(static_cast(const_cast(ptr)), number_of_elements); +} + +template +template +inline auto SampleMut::payload_slice_mut() -> Payload { + void* ptr = nullptr; + size_t number_of_elements = 0; + + iox2_sample_mut_payload_mut(&m_handle, &ptr, &number_of_elements); + + return Payload(static_cast(const_cast(ptr)), number_of_elements); +} + template inline auto send(SampleMut&& sample) -> iox::expected { size_t number_of_recipients = 0; diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp index 886f25ba8..ec4020063 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp @@ -56,11 +56,19 @@ class SampleMutUninit { auto user_header_mut() -> T&; /// Returns a reference to the const payload of the sample. + template ::VALUE, void>> auto payload() const -> const Payload&; /// Returns a reference to the payload of the sample. + template ::VALUE, void>> auto payload_mut() -> Payload&; + template ::VALUE, void>> + auto payload_slice() const -> Payload; + + template ::VALUE, void>> + auto payload_slice_mut() -> Payload; + /// Writes the payload to the sample template ::VALUE, T>> void write_payload(T&& value); @@ -132,15 +140,29 @@ inline auto SampleMutUninit::user_header_mut() -> T& { } template +template inline auto SampleMutUninit::payload() const -> const Payload& { return m_sample.payload(); } template +template inline auto SampleMutUninit::payload_mut() -> Payload& { return m_sample.payload_mut(); } +template +template +inline auto SampleMutUninit::payload_slice() const -> Payload { + return m_sample.payload_slice(); +} + +template +template +inline auto SampleMutUninit::payload_slice_mut() -> Payload { + return m_sample.payload_slice_mut(); +} + template template inline void SampleMutUninit::write_payload(T&& value) { diff --git a/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp b/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp index 9db74ce67..4cf9a70bf 100644 --- a/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp @@ -26,6 +26,17 @@ #include namespace iox2 { + +template +struct PayloadValueType { + using TYPE = T; +}; + +template +struct PayloadValueType> { + using TYPE = typename iox::Slice::ValueType; +}; + /// Builder to create new [`MessagingPattern::PublishSubscribe`] based [`Service`]s template class ServiceBuilderPublishSubscribe { @@ -125,7 +136,6 @@ inline ServiceBuilderPublishSubscribe::ServiceBuilderPub template inline void ServiceBuilderPublishSubscribe::set_parameters() { - m_payload_alignment.and_then([](auto) { IOX_TODO(); }); m_enable_safe_overflow.and_then( [&](auto value) { iox2_service_builder_pub_sub_set_enable_safe_overflow(&m_handle, value); }); m_subscriber_max_borrowed_samples.and_then( @@ -135,20 +145,21 @@ inline void ServiceBuilderPublishSubscribe::set_paramete [&](auto value) { iox2_service_builder_pub_sub_set_subscriber_max_buffer_size(&m_handle, value); }); m_max_subscribers.and_then([&](auto value) { iox2_service_builder_pub_sub_set_max_subscribers(&m_handle, value); }); m_max_publishers.and_then([&](auto value) { iox2_service_builder_pub_sub_set_max_publishers(&m_handle, value); }); + m_payload_alignment.and_then( + [&](auto value) { iox2_service_builder_pub_sub_set_payload_alignment(&m_handle, value); }); m_max_nodes.and_then([&](auto value) { iox2_service_builder_pub_sub_set_max_nodes(&m_handle, value); }); + using ValueType = typename PayloadValueType::TYPE; + auto type_variant = iox::IsSlice::VALUE ? iox2_type_variant_e_DYNAMIC : iox2_type_variant_e_FIXED_SIZE; + // payload type details - const auto* payload_type_name = typeid(Payload).name(); + const auto* payload_type_name = typeid(ValueType).name(); const auto payload_type_name_len = strlen(payload_type_name); - const auto payload_type_size = sizeof(Payload); - const auto payload_type_align = alignof(Payload); + const auto payload_type_size = sizeof(ValueType); + const auto payload_type_align = alignof(ValueType); - const auto payload_result = iox2_service_builder_pub_sub_set_payload_type_details(&m_handle, - iox2_type_variant_e_FIXED_SIZE, - payload_type_name, - payload_type_name_len, - payload_type_size, - payload_type_align); + const auto payload_result = iox2_service_builder_pub_sub_set_payload_type_details( + &m_handle, type_variant, payload_type_name, payload_type_name_len, payload_type_size, payload_type_align); if (payload_result != IOX2_OK) { IOX_PANIC("This should never happen! Implementation failure while setting the Payload-Type."); @@ -161,13 +172,12 @@ inline void ServiceBuilderPublishSubscribe::set_paramete const auto user_header_type_size = header_layout.size(); const auto user_header_type_align = header_layout.alignment(); - const auto user_header_result = - iox2_service_builder_pub_sub_set_user_header_type_details(&m_handle, - iox2_type_variant_e_FIXED_SIZE, - user_header_type_name, - user_header_type_name_len, - user_header_type_size, - user_header_type_align); + const auto user_header_result = iox2_service_builder_pub_sub_set_user_header_type_details(&m_handle, + type_variant, + user_header_type_name, + user_header_type_name_len, + user_header_type_size, + user_header_type_align); if (user_header_result != IOX2_OK) { IOX_PANIC("This should never happen! Implementation failure while setting the User-Header-Type."); diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index 3e4be6f07..70de3070c 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -199,6 +199,160 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_uninit_send_receive_works) { ASSERT_THAT(**recv_sample, Eq(payload)); } +TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr uint64_t PAYLOAD_ALIGNMENT = 8; + constexpr auto CXX_SLICE_MAX_LENGTH = 10; + + constexpr uint64_t DEFAULT_VALUE_A = 42; + constexpr uint64_t DEFAULT_VALUE_B = 777; + constexpr uint64_t DEFAULT_VALUE_Z = 21; + struct MyNestedStruct { + uint64_t a { DEFAULT_VALUE_A }; + uint64_t b { DEFAULT_VALUE_B }; + }; + struct MyStruct { + uint64_t z { DEFAULT_VALUE_Z }; + MyNestedStruct data; + }; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = node.service_builder(service_name) + .template publish_subscribe>() + .payload_alignment(PAYLOAD_ALIGNMENT) + .create() + .expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(CXX_SLICE_MAX_LENGTH).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + auto send_sample = sut_publisher.loan_slice(CXX_SLICE_MAX_LENGTH).expect(""); + + send(std::move(send_sample)).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + auto iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z)); + ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A)); + ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(CXX_SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(CXX_SLICE_MAX_LENGTH)); +} + +TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr uint64_t PAYLOAD_ALIGNMENT = 8; + constexpr auto CXX_SLICE_MAX_LENGTH = 10; + + constexpr uint64_t DEFAULT_VALUE_A = 42; + constexpr uint64_t DEFAULT_VALUE_B = 777; + constexpr uint64_t DEFAULT_VALUE_Z = 21; + struct MyNestedStruct { + uint64_t a { DEFAULT_VALUE_A }; + uint64_t b { DEFAULT_VALUE_B }; + }; + struct MyStruct { + uint64_t z { DEFAULT_VALUE_Z }; + MyNestedStruct data; + }; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = node.service_builder(service_name) + .template publish_subscribe>() + .payload_alignment(PAYLOAD_ALIGNMENT) + .create() + .expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(CXX_SLICE_MAX_LENGTH).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + auto send_sample = sut_publisher.loan_slice_uninit(CXX_SLICE_MAX_LENGTH).expect(""); + + auto iterations = 0; + for (auto& item : send_sample.payload_slice()) { + new (&item) MyStruct { DEFAULT_VALUE_Z + iterations, + MyNestedStruct { DEFAULT_VALUE_A + iterations, DEFAULT_VALUE_B + iterations } }; + ++iterations; + } + + send(assume_init(std::move(send_sample))).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z + iterations)); + ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A + iterations)); + ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B + iterations)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(CXX_SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(CXX_SLICE_MAX_LENGTH)); +} + +TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr uint64_t PAYLOAD_ALIGNMENT = 8; + constexpr auto CXX_SLICE_MAX_LENGTH = 10; + + constexpr uint64_t DEFAULT_VALUE_A = 42; + constexpr uint64_t DEFAULT_VALUE_B = 777; + constexpr uint64_t DEFAULT_VALUE_Z = 21; + struct MyNestedStruct { + uint64_t a { DEFAULT_VALUE_A }; + uint64_t b { DEFAULT_VALUE_B }; + }; + struct MyStruct { + uint64_t z { DEFAULT_VALUE_Z }; + MyNestedStruct data; + }; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = node.service_builder(service_name) + .template publish_subscribe>() + .payload_alignment(PAYLOAD_ALIGNMENT) + .create() + .expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(sizeof(MyStruct)).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + auto send_sample = sut_publisher.loan_slice_uninit(sizeof(MyStruct)).expect(""); + + new (send_sample.payload_slice().data()) + MyStruct { DEFAULT_VALUE_Z, MyNestedStruct { DEFAULT_VALUE_A, DEFAULT_VALUE_B } }; + + send(assume_init(std::move(send_sample))).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + + auto recv_sample = std::move(recv_result.value()); + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(sizeof(MyStruct))); + + auto recv_data = reinterpret_cast(recv_sample.payload_slice().data()); + + ASSERT_THAT(recv_data->z, Eq(DEFAULT_VALUE_Z)); + ASSERT_THAT(recv_data->data.a, Eq(DEFAULT_VALUE_A)); + ASSERT_THAT(recv_data->data.b, Eq(DEFAULT_VALUE_B)); +} + TYPED_TEST(ServicePublishSubscribeTest, loan_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; @@ -252,6 +406,7 @@ TYPED_TEST(ServicePublishSubscribeTest, setting_service_properties_works) { constexpr uint64_t HISTORY_SIZE = 13; constexpr uint64_t SUBSCRIBER_MAX_BUFFER_SIZE = 14; constexpr uint64_t SUBSCRIBER_MAX_BORROWED_SAMPLES = 15; + constexpr uint64_t PAYLOAD_ALIGNMENT = 4; const auto service_name = iox2_testing::generate_service_name(); @@ -264,6 +419,7 @@ TYPED_TEST(ServicePublishSubscribeTest, setting_service_properties_works) { .history_size(HISTORY_SIZE) .subscriber_max_buffer_size(SUBSCRIBER_MAX_BUFFER_SIZE) .subscriber_max_borrowed_samples(SUBSCRIBER_MAX_BORROWED_SAMPLES) + .payload_alignment(PAYLOAD_ALIGNMENT) .create() .expect(""); @@ -363,6 +519,22 @@ TYPED_TEST(ServicePublishSubscribeTest, publisher_applies_unable_to_deliver_stra ASSERT_THAT(sut_pub_2.unable_to_deliver_strategy(), Eq(UnableToDeliverStrategy::DiscardSample)); } +TYPED_TEST(ServicePublishSubscribeTest, publisher_applies_max_slice_len) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr uint64_t DESIRED_MAX_SLICE_LEN = 10; + using ValueType = uint8_t; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = + node.service_builder(service_name).template publish_subscribe>().create().expect(""); + + auto sut = service.publisher_builder().max_slice_len(DESIRED_MAX_SLICE_LEN).create().expect(""); + + ASSERT_THAT(sut.max_slice_len(), Eq(DESIRED_MAX_SLICE_LEN)); +} + TYPED_TEST(ServicePublishSubscribeTest, send_receive_with_user_header_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; diff --git a/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs b/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs index 20750c57b..98f12e1ae 100644 --- a/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs +++ b/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs @@ -173,6 +173,32 @@ impl HandleToType for iox2_port_factory_publisher_builder_h_ref { // BEGIN C API +#[no_mangle] +pub unsafe extern "C" fn iox2_port_factory_publisher_builder_set_max_slice_len( + port_factory_handle: iox2_port_factory_publisher_builder_h_ref, + value: c_size_t, +) { + port_factory_handle.assert_non_null(); + + let port_factory_struct = unsafe { &mut *port_factory_handle.as_type() }; + match port_factory_struct.service_type { + iox2_service_type_e::IPC => { + let port_factory = ManuallyDrop::take(&mut port_factory_struct.value.as_mut().ipc); + + port_factory_struct.set(PortFactoryPublisherBuilderUnion::new_ipc( + port_factory.max_slice_len(value), + )); + } + iox2_service_type_e::LOCAL => { + let port_factory = ManuallyDrop::take(&mut port_factory_struct.value.as_mut().local); + + port_factory_struct.set(PortFactoryPublisherBuilderUnion::new_local( + port_factory.max_slice_len(value), + )); + } + } +} + /// Sets the max loaned samples for the publisher /// /// # Arguments diff --git a/iceoryx2-ffi/ffi/src/api/publisher.rs b/iceoryx2-ffi/ffi/src/api/publisher.rs index 142b68e45..d5d60cb40 100644 --- a/iceoryx2-ffi/ffi/src/api/publisher.rs +++ b/iceoryx2-ffi/ffi/src/api/publisher.rs @@ -72,18 +72,14 @@ impl IntoCInt for PublisherSendError { impl IntoCInt for PublisherLoanError { fn into_c_int(self) -> c_int { (match self { - PublisherLoanError::OutOfMemory => { - iox2_publisher_send_error_e::LOAN_ERROR_OUT_OF_MEMORY - } + PublisherLoanError::OutOfMemory => iox2_publisher_loan_error_e::OUT_OF_MEMORY, PublisherLoanError::ExceedsMaxLoanedSamples => { - iox2_publisher_send_error_e::LOAN_ERROR_EXCEEDS_MAX_LOANED_SAMPLES + iox2_publisher_loan_error_e::EXCEEDS_MAX_LOANED_SAMPLES } PublisherLoanError::ExceedsMaxLoanSize => { - iox2_publisher_send_error_e::LOAN_ERROR_EXCEEDS_MAX_LOAN_SIZE - } - PublisherLoanError::InternalFailure => { - iox2_publisher_send_error_e::LOAN_ERROR_INTERNAL_FAILURE + iox2_publisher_loan_error_e::EXCEEDS_MAX_LOAN_SIZE } + PublisherLoanError::InternalFailure => iox2_publisher_loan_error_e::INTERNAL_FAILURE, }) as c_int } } @@ -253,6 +249,19 @@ pub unsafe extern "C" fn iox2_publisher_unable_to_deliver_strategy( } } +#[no_mangle] +pub unsafe extern "C" fn iox2_publisher_max_slice_len( + publisher_handle: iox2_publisher_h_ref, +) -> c_int { + publisher_handle.assert_non_null(); + + let publisher = &mut *publisher_handle.as_type(); + match publisher.service_type { + iox2_service_type_e::IPC => publisher.value.as_mut().ipc.max_slice_len() as c_int, + iox2_service_type_e::LOCAL => publisher.value.as_mut().local.max_slice_len() as c_int, + } +} + /// Returns the unique port id of the publisher. /// /// # Arguments @@ -349,6 +358,7 @@ pub unsafe extern "C" fn iox2_publisher_send_copy( /// * `sample_struct_ptr` - Must be either a NULL pointer or a pointer to a valid [`iox2_sample_mut_t`]. /// If it is a NULL pointer, the storage will be allocated on the heap. /// * `sample_handle_ptr` - An uninitialized or dangling [`iox2_sample_mut_h`] handle which will be initialized by this function call if a sample is obtained, otherwise it will be set to NULL. +/// * `number_of_bytes` - The number of bytes to loan from the publisher's payload segment /// /// Return [`IOX2_OK`] on success, otherwise [`iox2_publisher_loan_error_e`]. /// @@ -357,10 +367,11 @@ pub unsafe extern "C" fn iox2_publisher_send_copy( /// * `publisher_handle` is valid and non-null /// * The `sample_handle_ptr` is pointing to a valid [`iox2_sample_mut_h`]. #[no_mangle] -pub unsafe extern "C" fn iox2_publisher_loan( +pub unsafe extern "C" fn iox2_publisher_loan_slice_uninit( publisher_handle: iox2_publisher_h_ref, sample_struct_ptr: *mut iox2_sample_mut_t, sample_handle_ptr: *mut iox2_sample_mut_h, + number_of_elements: usize, ) -> c_int { publisher_handle.assert_non_null(); debug_assert!(!sample_handle_ptr.is_null()); @@ -383,7 +394,12 @@ pub unsafe extern "C" fn iox2_publisher_loan( let publisher = &mut *publisher_handle.as_type(); match publisher.service_type { - iox2_service_type_e::IPC => match publisher.value.as_ref().ipc.loan_custom_payload(1) { + iox2_service_type_e::IPC => match publisher + .value + .as_ref() + .ipc + .loan_custom_payload(number_of_elements) + { Ok(sample) => { let (sample_struct_ptr, deleter) = init_sample_struct_ptr(sample_struct_ptr); (*sample_struct_ptr).init( @@ -396,7 +412,12 @@ pub unsafe extern "C" fn iox2_publisher_loan( } Err(error) => error.into_c_int(), }, - iox2_service_type_e::LOCAL => match publisher.value.as_ref().local.loan_custom_payload(1) { + iox2_service_type_e::LOCAL => match publisher + .value + .as_ref() + .local + .loan_custom_payload(number_of_elements) + { Ok(sample) => { let (sample_struct_ptr, deleter) = init_sample_struct_ptr(sample_struct_ptr); (*sample_struct_ptr).init( diff --git a/iceoryx2-ffi/ffi/src/api/sample.rs b/iceoryx2-ffi/ffi/src/api/sample.rs index b45757f48..e2ba8ff84 100644 --- a/iceoryx2-ffi/ffi/src/api/sample.rs +++ b/iceoryx2-ffi/ffi/src/api/sample.rs @@ -214,26 +214,31 @@ pub unsafe extern "C" fn iox2_sample_user_header( /// /// * `handle` obtained by [`iox2_subscriber_receive()`](crate::iox2_subscriber_receive()) /// * `payload_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer. -/// * `payload_len` (optional) either a null poitner or a valid pointer pointing to a [`c_size_t`]. +/// * `number_of_elements` (optional) either a null poitner or a valid pointer pointing to a [`c_size_t`] with +/// the number of elements of the underlying type #[no_mangle] pub unsafe extern "C" fn iox2_sample_payload( handle: iox2_sample_h_ref, payload_ptr: *mut *const c_void, - payload_len: *mut c_size_t, + number_of_elements: *mut c_size_t, ) { handle.assert_non_null(); debug_assert!(!payload_ptr.is_null()); let sample = &mut *handle.as_type(); + let payload = sample.value.as_mut().local.payload(); - let payload = match sample.service_type { - iox2_service_type_e::IPC => sample.value.as_mut().ipc.payload(), - iox2_service_type_e::LOCAL => sample.value.as_mut().local.payload(), + match sample.service_type { + iox2_service_type_e::IPC => { + *payload_ptr = payload.as_ptr().cast(); + } + iox2_service_type_e::LOCAL => { + *payload_ptr = payload.as_ptr().cast(); + } }; - *payload_ptr = payload.as_ptr().cast(); - if !payload_len.is_null() { - *payload_len = payload.len(); + if !number_of_elements.is_null() { + *number_of_elements = sample.value.as_mut().local.header().number_of_elements() as c_size_t; } } diff --git a/iceoryx2-ffi/ffi/src/api/sample_mut.rs b/iceoryx2-ffi/ffi/src/api/sample_mut.rs index ef9f8987a..ee623f913 100644 --- a/iceoryx2-ffi/ffi/src/api/sample_mut.rs +++ b/iceoryx2-ffi/ffi/src/api/sample_mut.rs @@ -157,7 +157,7 @@ pub unsafe extern "C" fn iox2_sample_mut_move( /// /// # Safety /// -/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan()) +/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit()) /// * `header_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer. #[no_mangle] pub unsafe extern "C" fn iox2_sample_mut_user_header( @@ -181,7 +181,7 @@ pub unsafe extern "C" fn iox2_sample_mut_user_header( /// /// # Safety /// -/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan()) +/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit()) /// * `header_struct_ptr` - Must be either a NULL pointer or a pointer to a valid /// [`iox2_publish_subscribe_header_t`]. If it is a NULL pointer, the storage will be allocated on the heap. /// * `header_handle_ptr` valid pointer to a [`iox2_publish_subscribe_header_h`]. @@ -218,7 +218,7 @@ pub unsafe extern "C" fn iox2_sample_mut_header( /// /// # Safety /// -/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan()) +/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit()) /// * `header_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer. #[no_mangle] pub unsafe extern "C" fn iox2_sample_mut_user_header_mut( @@ -242,28 +242,32 @@ pub unsafe extern "C" fn iox2_sample_mut_user_header_mut( /// /// # Safety /// -/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan()) +/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit()) /// * `payload_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer. /// * `payload_len` (optional) either a null poitner or a valid pointer pointing to a [`c_size_t`]. #[no_mangle] pub unsafe extern "C" fn iox2_sample_mut_payload_mut( handle: iox2_sample_mut_h_ref, payload_ptr: *mut *mut c_void, - payload_len: *mut c_size_t, + number_of_elements: *mut c_size_t, ) { handle.assert_non_null(); debug_assert!(!payload_ptr.is_null()); let sample = &mut *handle.as_type(); + let payload = sample.value.as_mut().ipc.payload_mut(); - let payload = match sample.service_type { - iox2_service_type_e::IPC => sample.value.as_mut().ipc.payload_mut(), - iox2_service_type_e::LOCAL => sample.value.as_mut().local.payload_mut(), + match sample.service_type { + iox2_service_type_e::IPC => { + *payload_ptr = payload.as_mut_ptr().cast(); + } + iox2_service_type_e::LOCAL => { + *payload_ptr = payload.as_mut_ptr().cast(); + } }; - *payload_ptr = payload.as_mut_ptr().cast(); - if !payload_len.is_null() { - *payload_len = payload.len(); + if !number_of_elements.is_null() { + *number_of_elements = sample.value.as_mut().local.header().number_of_elements() as c_size_t; } } @@ -271,28 +275,32 @@ pub unsafe extern "C" fn iox2_sample_mut_payload_mut( /// /// # Safety /// -/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan()) +/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit()) /// * `payload_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer. /// * `payload_len` (optional) either a null poitner or a valid pointer pointing to a [`c_size_t`]. #[no_mangle] pub unsafe extern "C" fn iox2_sample_mut_payload( handle: iox2_sample_mut_h_ref, payload_ptr: *mut *const c_void, - payload_len: *mut c_size_t, + number_of_elements: *mut c_size_t, ) { handle.assert_non_null(); debug_assert!(!payload_ptr.is_null()); let sample = &mut *handle.as_type(); + let payload = sample.value.as_mut().ipc.payload_mut(); - let payload = match sample.service_type { - iox2_service_type_e::IPC => sample.value.as_mut().ipc.payload(), - iox2_service_type_e::LOCAL => sample.value.as_mut().local.payload(), + match sample.service_type { + iox2_service_type_e::IPC => { + *payload_ptr = payload.as_mut_ptr().cast(); + } + iox2_service_type_e::LOCAL => { + *payload_ptr = payload.as_mut_ptr().cast(); + } }; - *payload_ptr = payload.as_ptr().cast(); - if !payload_len.is_null() { - *payload_len = payload.len(); + if !number_of_elements.is_null() { + *number_of_elements = sample.value.as_mut().local.header().number_of_elements() as c_size_t; } } @@ -300,7 +308,7 @@ pub unsafe extern "C" fn iox2_sample_mut_payload( /// /// # Safety /// -/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan()) +/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit()) /// * `number_of_recipients`, can be null or must point to a valid [`c_size_t`] to store the number /// of subscribers that received the sample #[no_mangle] diff --git a/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs b/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs index 0252cec04..e954797a3 100644 --- a/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs +++ b/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs @@ -480,6 +480,41 @@ pub unsafe extern "C" fn iox2_service_builder_pub_sub_set_max_subscribers( } } +#[no_mangle] +pub unsafe extern "C" fn iox2_service_builder_pub_sub_set_payload_alignment( + service_builder_handle: iox2_service_builder_pub_sub_h_ref, + value: c_size_t, +) { + service_builder_handle.assert_non_null(); + + let service_builder_struct = unsafe { &mut *service_builder_handle.as_type() }; + + match service_builder_struct.service_type { + iox2_service_type_e::IPC => { + let service_builder = + ManuallyDrop::take(&mut service_builder_struct.value.as_mut().ipc); + + let service_builder = ManuallyDrop::into_inner(service_builder.pub_sub); + service_builder_struct.set(ServiceBuilderUnion::new_ipc_pub_sub( + service_builder.payload_alignment( + Alignment::new(value).unwrap_or(Alignment::new_unchecked(8)), + ), + )); + } + iox2_service_type_e::LOCAL => { + let service_builder = + ManuallyDrop::take(&mut service_builder_struct.value.as_mut().local); + + let service_builder = ManuallyDrop::into_inner(service_builder.pub_sub); + service_builder_struct.set(ServiceBuilderUnion::new_local_pub_sub( + service_builder.payload_alignment( + Alignment::new(value).unwrap_or(Alignment::new_unchecked(8)), + ), + )); + } + } +} + /// Sets the history size /// /// # Arguments diff --git a/iceoryx2/src/port/publisher.rs b/iceoryx2/src/port/publisher.rs index ce4b9fb37..e7e4b7b30 100644 --- a/iceoryx2/src/port/publisher.rs +++ b/iceoryx2/src/port/publisher.rs @@ -704,6 +704,11 @@ impl self.data_segment.config.unable_to_deliver_strategy } + /// Returns the maximum slice length configured for this [`Publisher`]. + pub fn max_slice_len(&self) -> usize { + self.data_segment.config.max_slice_len + } + fn allocate(&self, layout: Layout) -> Result { let msg = "Unable to allocate Sample with"; From 7b8d7369bea53d207805d87e17ac28193dd01879 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 31 Oct 2024 16:40:37 +0100 Subject: [PATCH 02/22] [#490] Add release notes --- doc/release-notes/iceoryx2-unreleased.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/release-notes/iceoryx2-unreleased.md b/doc/release-notes/iceoryx2-unreleased.md index 8362aa404..8cd2d0547 100644 --- a/doc/release-notes/iceoryx2-unreleased.md +++ b/doc/release-notes/iceoryx2-unreleased.md @@ -15,6 +15,7 @@ * Add `PeriodicTimer` into POSIX building blocks [#425](https://github.com/eclipse-iceoryx/iceoryx2/issues/425) * Developer permissions for resources [#460](https://github.com/eclipse-iceoryx/iceoryx2/issues/460) * Add `--send-copy` flag to Benchmark to consider mem operations [#483](https://github.com/eclipse-iceoryx/iceoryx2/issues/483) +* Support for slices in the C++ bindings [#490](https://github.com/eclipse-iceoryx/iceoryx2/issues/490) ### Bugfixes @@ -56,7 +57,7 @@ conflicts when merging. --> -* Example text [#1](https://github.com/eclipse-iceoryx/iceoryx2/issues/1) +* APIs to support slices in the C++ bindings [#490](https://github.com/eclipse-iceoryx/iceoryx2/issues/490) ### API Breaking Changes From 6e86397e9a0ab09db318b536915b1e9913fbe0f3 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 31 Oct 2024 16:57:31 +0100 Subject: [PATCH 03/22] [#490] Add documentation --- .../ffi/src/api/port_factory_publisher_builder.rs | 11 +++++++++++ iceoryx2-ffi/ffi/src/api/publisher.rs | 12 ++++++++++++ iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs | 11 +++++++++++ 3 files changed, 34 insertions(+) diff --git a/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs b/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs index 98f12e1ae..c3f68d9ee 100644 --- a/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs +++ b/iceoryx2-ffi/ffi/src/api/port_factory_publisher_builder.rs @@ -173,6 +173,17 @@ impl HandleToType for iox2_port_factory_publisher_builder_h_ref { // BEGIN C API +/// Sets the max slice length for the publisher +/// +/// # Arguments +/// +/// * `port_factory_handle` - Must be a valid [`iox2_port_factory_publisher_builder_h_ref`] +/// obtained by [`iox2_port_factory_pub_sub_publisher_builder`](crate::iox2_port_factory_pub_sub_publisher_builder). +/// * `value` - The value to set max slice length to +/// +/// # Safety +/// +/// * `port_factory_handle` must be valid handles #[no_mangle] pub unsafe extern "C" fn iox2_port_factory_publisher_builder_set_max_slice_len( port_factory_handle: iox2_port_factory_publisher_builder_h_ref, diff --git a/iceoryx2-ffi/ffi/src/api/publisher.rs b/iceoryx2-ffi/ffi/src/api/publisher.rs index d5d60cb40..193667a50 100644 --- a/iceoryx2-ffi/ffi/src/api/publisher.rs +++ b/iceoryx2-ffi/ffi/src/api/publisher.rs @@ -249,6 +249,18 @@ pub unsafe extern "C" fn iox2_publisher_unable_to_deliver_strategy( } } +/// Returns the maximum `[u8]` length that can be loaned in one sample, i.e. the max number of +/// elements in the `[u8]` payload type used by the C binding. +/// +/// # Arguments +/// +/// * `publisher_handle` obtained by [`iox2_port_factory_publisher_builder_create`](crate::iox2_port_factory_publisher_builder_create) +/// +/// Returns the maximum number of elements as a [`c_int`]. +/// +/// # Safety +/// +/// * `publisher_handle` is valid and non-null #[no_mangle] pub unsafe extern "C" fn iox2_publisher_max_slice_len( publisher_handle: iox2_publisher_h_ref, diff --git a/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs b/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs index e954797a3..b87464509 100644 --- a/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs +++ b/iceoryx2-ffi/ffi/src/api/service_builder_pub_sub.rs @@ -480,6 +480,17 @@ pub unsafe extern "C" fn iox2_service_builder_pub_sub_set_max_subscribers( } } +/// Sets the payload alignment for the builder +/// +/// # Arguments +/// +/// * `service_builder_handle` - Must be a valid [`iox2_service_builder_pub_sub_h_ref`] +/// obtained by [`iox2_service_builder_pub_sub`](crate::iox2_service_builder_pub_sub). +/// * `value` - The value to set the payload alignment to +/// +/// # Safety +/// +/// * `service_builder_handle` must be valid handles #[no_mangle] pub unsafe extern "C" fn iox2_service_builder_pub_sub_set_payload_alignment( service_builder_handle: iox2_service_builder_pub_sub_h_ref, From b23d27d0c4c63eb74d732004daf8991c567c8358 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Sat, 2 Nov 2024 21:26:41 +0100 Subject: [PATCH 04/22] [#490] Implement send_copy for slices --- iceoryx2-ffi/cxx/include/iox2/publisher.hpp | 23 +++++ iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 1 + .../src/service_publish_subscribe_tests.cpp | 71 ++++++++++++--- iceoryx2-ffi/ffi/src/api/publisher.rs | 86 ++++++++++++++++++- 4 files changed, 169 insertions(+), 12 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp index f678c3343..28a49c580 100644 --- a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp @@ -53,8 +53,12 @@ class Publisher { /// Copies the input `value` into a [`SampleMut`] and delivers it. /// On success it returns the number of [`Subscriber`]s that received /// the data, otherwise a [`PublisherSendError`] describing the failure. + template ::VALUE, void>> auto send_copy(const Payload& payload) const -> iox::expected; + template ::VALUE, void>> + auto send_slice_copy(const Payload& payload) const -> iox::expected; + /// Loans/allocates a [`SampleMutUninit`] from the underlying data segment of the [`Publisher`]. /// The user has to initialize the payload before it can be sent. /// @@ -167,6 +171,7 @@ inline auto Publisher::id() const -> UniquePublisherId { } template +template inline auto Publisher::send_copy(const Payload& payload) const -> iox::expected { static_assert(std::is_trivially_copyable::value); @@ -182,6 +187,24 @@ inline auto Publisher::send_copy(const Payload& payload) return iox::err(iox::into(result)); } +template +template +inline auto Publisher::send_slice_copy(const Payload& payload) const + -> iox::expected { + size_t number_of_recipients = 0; + auto result = iox2_publisher_send_slice_copy(&m_handle, + payload.data(), + sizeof(typename Payload::ValueType), + payload.number_of_elements(), + &number_of_recipients); + + if (result == IOX2_OK) { + return iox::ok(number_of_recipients); + } + + return iox::err(iox::into(result)); +} + template template inline auto Publisher::loan_uninit() diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index 7219eb245..88b50c043 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -15,6 +15,7 @@ #include "iox/assertions.hpp" #include "iox/expected.hpp" +#include "iox/slice.hpp" #include "iox2/header_publish_subscribe.hpp" #include "iox2/iceoryx2.h" #include "iox2/internal/iceoryx2.hpp" diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index 70de3070c..6b1dacdb3 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -10,6 +10,7 @@ // // SPDX-License-Identifier: Apache-2.0 OR MIT +#include "iox/uninitialized_array.hpp" #include "iox2/node.hpp" #include "iox2/node_name.hpp" #include "iox2/service.hpp" @@ -199,10 +200,58 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_uninit_send_receive_works) { ASSERT_THAT(**recv_sample, Eq(payload)); } +TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr auto SLICE_MAX_LENGTH = 10; + + constexpr uint64_t DEFAULT_VALUE_A = 42; + constexpr uint64_t DEFAULT_VALUE_B = 777; + constexpr uint64_t DEFAULT_VALUE_Z = 21; + struct MyNestedStruct { + uint64_t a { DEFAULT_VALUE_A }; + uint64_t b { DEFAULT_VALUE_B }; + }; + struct MyStruct { + uint64_t z { DEFAULT_VALUE_Z }; + MyNestedStruct data; + }; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = + node.service_builder(service_name).template publish_subscribe>().create().expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + iox::UninitializedArray elements; + for (auto& item : elements) { + new (&item) MyStruct {}; + } + auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); + sut_publisher.send_slice_copy(payload).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + auto iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z)); + ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A)); + ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); +} + TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr uint64_t PAYLOAD_ALIGNMENT = 8; - constexpr auto CXX_SLICE_MAX_LENGTH = 10; + constexpr auto SLICE_MAX_LENGTH = 10; constexpr uint64_t DEFAULT_VALUE_A = 42; constexpr uint64_t DEFAULT_VALUE_B = 777; @@ -225,10 +274,10 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { .create() .expect(""); - auto sut_publisher = service.publisher_builder().max_slice_len(CXX_SLICE_MAX_LENGTH).create().expect(""); + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - auto send_sample = sut_publisher.loan_slice(CXX_SLICE_MAX_LENGTH).expect(""); + auto send_sample = sut_publisher.loan_slice(SLICE_MAX_LENGTH).expect(""); send(std::move(send_sample)).expect(""); @@ -244,14 +293,14 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(CXX_SLICE_MAX_LENGTH)); - ASSERT_THAT(iterations, Eq(CXX_SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr uint64_t PAYLOAD_ALIGNMENT = 8; - constexpr auto CXX_SLICE_MAX_LENGTH = 10; + constexpr auto SLICE_MAX_LENGTH = 10; constexpr uint64_t DEFAULT_VALUE_A = 42; constexpr uint64_t DEFAULT_VALUE_B = 777; @@ -274,10 +323,10 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { .create() .expect(""); - auto sut_publisher = service.publisher_builder().max_slice_len(CXX_SLICE_MAX_LENGTH).create().expect(""); + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - auto send_sample = sut_publisher.loan_slice_uninit(CXX_SLICE_MAX_LENGTH).expect(""); + auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); auto iterations = 0; for (auto& item : send_sample.payload_slice()) { @@ -300,14 +349,14 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(CXX_SLICE_MAX_LENGTH)); - ASSERT_THAT(iterations, Eq(CXX_SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr uint64_t PAYLOAD_ALIGNMENT = 8; - constexpr auto CXX_SLICE_MAX_LENGTH = 10; + constexpr auto SLICE_MAX_LENGTH = 10; constexpr uint64_t DEFAULT_VALUE_A = 42; constexpr uint64_t DEFAULT_VALUE_B = 777; diff --git a/iceoryx2-ffi/ffi/src/api/publisher.rs b/iceoryx2-ffi/ffi/src/api/publisher.rs index 193667a50..c7562eedd 100644 --- a/iceoryx2-ffi/ffi/src/api/publisher.rs +++ b/iceoryx2-ffi/ffi/src/api/publisher.rs @@ -182,7 +182,7 @@ impl HandleToType for iox2_publisher_h_ref { unsafe fn send_copy( publisher: &Publisher, data_ptr: *const c_void, - data_len: usize, + size_of_element: usize, number_of_recipients: *mut usize, ) -> c_int { // loan_slice_uninit(1) <= 1 is correct here since it defines the number of @@ -193,6 +193,37 @@ unsafe fn send_copy( Err(e) => return e.into_c_int(), }; + if sample.payload().len() < size_of_element { + return iox2_publisher_send_error_e::LOAN_ERROR_EXCEEDS_MAX_LOAN_SIZE as c_int; + } + + let sample_ptr = sample.payload_mut().as_mut_ptr(); + core::ptr::copy_nonoverlapping(data_ptr, sample_ptr.cast(), size_of_element); + match sample.assume_init().send() { + Ok(v) => { + if !number_of_recipients.is_null() { + *number_of_recipients = v; + } + } + Err(e) => return e.into_c_int(), + } + + IOX2_OK +} + +unsafe fn send_slice_copy( + publisher: &Publisher, + data_ptr: *const c_void, + size_of_element: usize, + number_of_elements: usize, + number_of_recipients: *mut usize, +) -> c_int { + let mut sample = match publisher.loan_custom_payload(number_of_elements) { + Ok(sample) => sample, + Err(e) => return e.into_c_int(), + }; + + let data_len = size_of_element * number_of_elements; if sample.payload().len() < data_len { return iox2_publisher_send_error_e::LOAN_ERROR_EXCEEDS_MAX_LOAN_SIZE as c_int; } @@ -316,6 +347,59 @@ pub unsafe extern "C" fn iox2_publisher_id( *id_handle_ptr = (*storage_ptr).as_handle(); } +/// Sends a copy of the provided slice data via the publisher. +/// +/// # Arguments +/// +/// * `publisher_handle` - Handle to the publisher obtained from `iox2_port_factory_publisher_builder_create` +/// * `data_ptr` - Pointer to the start of the slice data to be sent +/// * `size_of_element` - Size of each element in the slice in bytes +/// * `number_of_elements` - Number of elements in the slice +/// * `number_of_recipients` - Optional pointer to store the number of subscribers that received the data +/// +/// # Returns +/// +/// Returns `IOX2_OK` on success, otherwise an error code from `iox2_publisher_send_error_e` +/// +/// # Safety +/// +/// * `publisher_handle` must be valid and non-null +/// * `data_ptr` must be a valid pointer to the start of the slice data +/// * `size_of_element` must be the correct size of each element in bytes +/// * `number_of_elements` must accurately represent the number of elements in the slice +/// * `number_of_recipients` can be null, otherwise it must be a valid pointer to a `usize` +#[no_mangle] +pub unsafe extern "C" fn iox2_publisher_send_slice_copy( + publisher_handle: iox2_publisher_h_ref, + data_ptr: *const c_void, + size_of_element: usize, + number_of_elements: usize, + number_of_recipients: *mut usize, +) -> c_int { + publisher_handle.assert_non_null(); + debug_assert!(!data_ptr.is_null()); + debug_assert!(size_of_element != 0); + + let publisher = &mut *publisher_handle.as_type(); + + match publisher.service_type { + iox2_service_type_e::IPC => send_slice_copy( + &publisher.value.as_mut().ipc, + data_ptr, + size_of_element, + number_of_elements, + number_of_recipients, + ), + iox2_service_type_e::LOCAL => send_slice_copy( + &publisher.value.as_mut().local, + data_ptr, + size_of_element, + number_of_elements, + number_of_recipients, + ), + } +} + /// Sends a copy of the provided data via the publisher. The data must be copyable via `memcpy`. /// /// # Arguments From 8ffd1d9e97af6d894c36dcd95c2d15a6e7672556 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Sat, 2 Nov 2024 22:06:36 +0100 Subject: [PATCH 05/22] [#490] Add size() method to iox::Slice --- iceoryx2-ffi/cxx/include/iox/slice.hpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 4b7cc4e04..961b9ba7d 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -28,7 +28,9 @@ class Slice { Slice(const T* data, uint64_t number_of_elements); + auto size() const -> uint64_t; auto number_of_elements() const -> uint64_t; + auto operator[](uint64_t n) const -> const T&; auto operator[](uint64_t n) -> T&; @@ -52,6 +54,11 @@ Slice::Slice(const T* data, uint64_t number_of_elements) , m_number_of_elements { number_of_elements } { } +template +auto Slice::size() const -> uint64_t { + return (sizeof(ValueType) * m_number_of_elements + alignof(ValueType) - 1) & ~(alignof(ValueType) - 1); +} + template auto Slice::number_of_elements() const -> uint64_t { return m_number_of_elements; From 5c4c28a702fd13b904b228ab543f585e08007f5f Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Tue, 5 Nov 2024 18:17:09 +0100 Subject: [PATCH 06/22] [#490] Fix C/C++ examples with slices --- examples/c/domains/src/publisher.c | 2 +- examples/c/publish_subscribe/src/publisher.c | 2 +- .../src/publisher.c | 2 +- .../src/publisher.cpp | 12 +-- .../src/subscriber.cpp | 7 +- iceoryx2-ffi/cxx/include/iox/slice.hpp | 38 +++++++ iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 8 +- .../cxx/include/iox2/sample_mut_uninit.hpp | 9 +- .../src/service_publish_subscribe_tests.cpp | 99 +++++++++++++++++++ 9 files changed, 161 insertions(+), 18 deletions(-) diff --git a/examples/c/domains/src/publisher.c b/examples/c/domains/src/publisher.c index d0e164454..0ffefc71c 100644 --- a/examples/c/domains/src/publisher.c +++ b/examples/c/domains/src/publisher.c @@ -99,7 +99,7 @@ int main(int argc, char** argv) { // loan sample iox2_sample_mut_h sample = NULL; - if (iox2_publisher_loan(&publisher, NULL, &sample) != IOX2_OK) { + if (iox2_publisher_loan_slice_uninit(&publisher, NULL, &sample, 1) != IOX2_OK) { printf("Failed to loan sample\n"); goto drop_publisher; } diff --git a/examples/c/publish_subscribe/src/publisher.c b/examples/c/publish_subscribe/src/publisher.c index e3282b2bd..ead151e30 100644 --- a/examples/c/publish_subscribe/src/publisher.c +++ b/examples/c/publish_subscribe/src/publisher.c @@ -79,7 +79,7 @@ int main(void) { // loan sample iox2_sample_mut_h sample = NULL; - if (iox2_publisher_loan(&publisher, NULL, &sample) != IOX2_OK) { + if (iox2_publisher_loan_slice_uninit(&publisher, NULL, &sample, 1) != IOX2_OK) { printf("Failed to loan sample\n"); goto drop_publisher; } diff --git a/examples/c/publish_subscribe_with_user_header/src/publisher.c b/examples/c/publish_subscribe_with_user_header/src/publisher.c index 3e13075c8..b5b7d9103 100644 --- a/examples/c/publish_subscribe_with_user_header/src/publisher.c +++ b/examples/c/publish_subscribe_with_user_header/src/publisher.c @@ -92,7 +92,7 @@ int main(void) { // loan sample iox2_sample_mut_h sample = NULL; - if (iox2_publisher_loan(&publisher, NULL, &sample) != IOX2_OK) { + if (iox2_publisher_loan_slice_uninit(&publisher, NULL, &sample, 1) != IOX2_OK) { printf("Failed to loan sample\n"); goto drop_publisher; } diff --git a/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp b/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp index bae023812..4baee32fb 100644 --- a/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp +++ b/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp @@ -32,25 +32,25 @@ auto main() -> int { .open_or_create() .expect("successful service creation/opening"); - uint64_t worst_case_memory_size = 1024; // NOLINT + const uint64_t worst_case_memory_size = 1024; // NOLINT auto publisher = service.publisher_builder() .max_slice_len(worst_case_memory_size) .create() .expect("successful publisher creation"); - auto counter = 1; + auto counter = 0; while (node.wait(CYCLE_TIME).has_value()) { - counter += 1; - - auto required_memory_size = (8 + counter) % 16; // NOLINT + const auto required_memory_size = (counter % 16) + 1; // NOLINT auto sample = publisher.loan_slice_uninit(required_memory_size).expect("acquire sample"); sample.write_from_fn([&](auto byte_idx) { return (byte_idx + counter) % 255; }); // NOLINT auto initialized_sample = assume_init(std::move(sample)); send(std::move(initialized_sample)).expect("send successful"); - std::cout << "Send sample " << counter << "..." << std::endl; + std::cout << "Send sample " << counter << " with " << required_memory_size << " bytes..." << std::endl; + + counter++; } std::cout << "exit" << std::endl; diff --git a/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp b/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp index fee340f43..593d00f35 100644 --- a/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp +++ b/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp @@ -17,6 +17,7 @@ #include "iox2/service_type.hpp" #include +#include #include constexpr iox::units::Duration CYCLE_TIME = iox::units::Duration::fromSeconds(1); @@ -35,9 +36,9 @@ auto main() -> int { while (node.wait(CYCLE_TIME).has_value()) { auto sample = subscriber.receive().expect("receive succeeds"); while (sample.has_value()) { - std::cout << "received " << sample->payload().size() << " bytes: "; - for (auto byte : sample->payload()) { - std::cout << std::hex << byte << " "; + std::cout << "received " << std::dec << static_cast(sample->payload_slice().size()) << " bytes: "; + for (auto byte : sample->payload_slice()) { + std::cout << std::setw(2) << std::setfill('0') << std::hex << static_cast(byte) << " "; } std::cout << std::endl; sample = subscriber.receive().expect("receive succeeds"); diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 961b9ba7d..5509131d4 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -26,20 +26,51 @@ class Slice { using ConstIterator = const T*; using ValueType = T; + Slice(T* data, uint64_t number_of_elements); Slice(const T* data, uint64_t number_of_elements); + /// @brief Returns the size of the slice in bytes. + /// @return The size of the slice in bytes. auto size() const -> uint64_t; + + /// @brief Returns the number of elements in the slice. + /// @return The number of elements in the slice. auto number_of_elements() const -> uint64_t; + /// @brief Accesses the element at the specified index (const version). + /// @param[in] n The index of the element to access. + /// @return A const reference to the element at the specified index. + /// @pre The index must be less than the number of elements in the slice. auto operator[](uint64_t n) const -> const T&; + + /// @brief Accesses the element at the specified index (non-const version). + /// @param[in] n The index of the element to access. + /// @return A reference to the element at the specified index. + /// @pre The index must be less than the number of elements in the slice. auto operator[](uint64_t n) -> T&; + /// @brief Returns an iterator to the beginning of the slice. + /// @return An iterator pointing to the first element of the slice. auto begin() -> Iterator; + + /// @brief Returns a const iterator to the beginning of the slice. + /// @return A const iterator pointing to the first element of the slice. auto begin() const -> ConstIterator; + + /// @brief Returns an iterator to the end of the slice. + /// @return An iterator pointing one past the last element of the slice. auto end() -> Iterator; + + /// @brief Returns a const iterator to the end of the slice. + /// @return A const iterator pointing one past the last element of the slice. auto end() const -> ConstIterator; + /// @brief Returns a pointer to the underlying data of the slice. + /// @return A pointer to the first element of the slice. auto data() -> T*; + + /// @brief Returns a const pointer to the underlying data of the slice. + /// @return A const pointer to the first element of the slice. auto data() const -> const T*; private: @@ -47,6 +78,13 @@ class Slice { uint64_t m_number_of_elements; }; +template +Slice::Slice(T* data, uint64_t number_of_elements) + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) constness protected by const class specification + : m_data { data } + , m_number_of_elements { number_of_elements } { +} + template Slice::Slice(const T* data, uint64_t number_of_elements) // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) constness protected by const class specification diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index 88b50c043..b1f3a9e88 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -85,7 +85,7 @@ class SampleMut { auto payload_mut() -> Payload&; template ::VALUE, void>> - auto payload_slice() const -> const Payload; + auto payload_slice() const -> Payload; template ::VALUE, void>> auto payload_slice_mut() -> Payload; @@ -216,13 +216,13 @@ inline auto SampleMut::payload_mut() -> Payload& { template template -inline auto SampleMut::payload_slice() const -> const Payload { +inline auto SampleMut::payload_slice() const -> Payload { const void* ptr = nullptr; size_t number_of_elements = 0; iox2_sample_mut_payload(&m_handle, &ptr, &number_of_elements); - return Payload(static_cast(const_cast(ptr)), number_of_elements); + return Payload(static_cast(ptr), number_of_elements); } template @@ -233,7 +233,7 @@ inline auto SampleMut::payload_slice_mut() -> Payload { iox2_sample_mut_payload_mut(&m_handle, &ptr, &number_of_elements); - return Payload(static_cast(const_cast(ptr)), number_of_elements); + return Payload(static_cast(ptr), number_of_elements); } template diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp index ec4020063..9d774ec13 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp @@ -173,13 +173,18 @@ template template inline void SampleMutUninit::write_from_fn( const iox::function& initializer) { - IOX_TODO(); + auto slice = payload_slice_mut(); + for (uint64_t i = 0; i < slice.number_of_elements(); ++i) { + new (&slice[i]) typename T::ValueType(initializer(i)); + } } template template inline void SampleMutUninit::write_from_slice(const T& value) { - IOX_TODO(); + auto dest = payload_slice_mut(); + IOX_ASSERT(dest.size() >= value.size(), "Destination payload size is smaller than source slice size"); + std::memcpy(dest.begin(), value.begin(), value.size()); } } // namespace iox2 diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index 6b1dacdb3..d0020d656 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -248,6 +248,105 @@ TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } +TYPED_TEST(ServicePublishSubscribeTest, write_from_fn_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr auto SLICE_MAX_LENGTH = 10; + + constexpr uint64_t DEFAULT_VALUE_A = 42; + constexpr uint64_t DEFAULT_VALUE_B = 777; + constexpr uint64_t DEFAULT_VALUE_Z = 21; + struct MyNestedStruct { + uint64_t a { DEFAULT_VALUE_A }; + uint64_t b { DEFAULT_VALUE_B }; + }; + struct MyStruct { + uint64_t z { DEFAULT_VALUE_Z }; + MyNestedStruct data; + }; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = + node.service_builder(service_name).template publish_subscribe>().create().expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); + send_sample.write_from_fn([](auto index) { + std::cout << index << std::endl; + return MyStruct { DEFAULT_VALUE_Z + index, + MyNestedStruct { DEFAULT_VALUE_A + index, DEFAULT_VALUE_B + index } }; + }); + send(assume_init(std::move(send_sample))).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + auto iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z + iterations)); + ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A + iterations)); + ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B + iterations)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); +} + +TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr auto SLICE_MAX_LENGTH = 10; + + constexpr uint64_t DEFAULT_VALUE_A = 42; + constexpr uint64_t DEFAULT_VALUE_B = 777; + constexpr uint64_t DEFAULT_VALUE_Z = 21; + struct MyNestedStruct { + uint64_t a { DEFAULT_VALUE_A }; + uint64_t b { DEFAULT_VALUE_B }; + }; + struct MyStruct { + uint64_t z { DEFAULT_VALUE_Z }; + MyNestedStruct data; + }; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = + node.service_builder(service_name).template publish_subscribe>().create().expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + iox::UninitializedArray elements; + for (auto& item : elements) { + new (&item) MyStruct {}; + } + auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); + auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); + send_sample.write_from_slice(payload); + send(assume_init(std::move(send_sample))).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + auto iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z)); + ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A)); + ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); +} + TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr uint64_t PAYLOAD_ALIGNMENT = 8; From 7d464cd612e8762c3e2a6df814eb9ee8e7a0c3fe Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Tue, 5 Nov 2024 19:41:21 +0100 Subject: [PATCH 07/22] [#490] Reduce complexity in tests --- .../src/service_publish_subscribe_tests.cpp | 280 +++++++----------- 1 file changed, 103 insertions(+), 177 deletions(-) diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index d0020d656..ef07e77e5 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -179,7 +179,7 @@ TYPED_TEST(ServicePublishSubscribeTest, send_copy_receive_works) { ASSERT_THAT(**sample, Eq(payload)); } -TYPED_TEST(ServicePublishSubscribeTest, loan_uninit_send_receive_works) { +TYPED_TEST(ServicePublishSubscribeTest, loan_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; const auto service_name = iox2_testing::generate_service_name(); @@ -190,146 +190,64 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_uninit_send_receive_works) { auto sut_publisher = service.publisher_builder().create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - auto sample = sut_publisher.loan_uninit().expect(""); - const uint64_t payload = 78123791; - sample.write_payload(payload); - send(assume_init(std::move(sample))).expect(""); + auto sample = sut_publisher.loan().expect(""); + const uint64_t payload = 781891729871; + *sample = payload; + send(std::move(sample)).expect(""); auto recv_sample = sut_subscriber.receive().expect(""); ASSERT_TRUE(recv_sample.has_value()); ASSERT_THAT(**recv_sample, Eq(payload)); } -TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { +TYPED_TEST(ServicePublishSubscribeTest, loan_uninit_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; - constexpr auto SLICE_MAX_LENGTH = 10; - - constexpr uint64_t DEFAULT_VALUE_A = 42; - constexpr uint64_t DEFAULT_VALUE_B = 777; - constexpr uint64_t DEFAULT_VALUE_Z = 21; - struct MyNestedStruct { - uint64_t a { DEFAULT_VALUE_A }; - uint64_t b { DEFAULT_VALUE_B }; - }; - struct MyStruct { - uint64_t z { DEFAULT_VALUE_Z }; - MyNestedStruct data; - }; const auto service_name = iox2_testing::generate_service_name(); auto node = NodeBuilder().create().expect(""); - auto service = - node.service_builder(service_name).template publish_subscribe>().create().expect(""); + auto service = node.service_builder(service_name).template publish_subscribe().create().expect(""); - auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); + auto sut_publisher = service.publisher_builder().create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - iox::UninitializedArray elements; - for (auto& item : elements) { - new (&item) MyStruct {}; - } - auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); - sut_publisher.send_slice_copy(payload).expect(""); - - auto recv_result = sut_subscriber.receive().expect(""); - ASSERT_TRUE(recv_result.has_value()); - auto recv_sample = std::move(recv_result.value()); - - auto iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { - ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z)); - ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A)); - ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B)); - ++iterations; - } + auto sample = sut_publisher.loan_uninit().expect(""); + const uint64_t payload = 78123791; + sample.write_payload(payload); + send(assume_init(std::move(sample))).expect(""); + auto recv_sample = sut_subscriber.receive().expect(""); - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); - ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); + ASSERT_TRUE(recv_sample.has_value()); + ASSERT_THAT(**recv_sample, Eq(payload)); } -TYPED_TEST(ServicePublishSubscribeTest, write_from_fn_send_receive_works) { - constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; - constexpr auto SLICE_MAX_LENGTH = 10; - - constexpr uint64_t DEFAULT_VALUE_A = 42; - constexpr uint64_t DEFAULT_VALUE_B = 777; - constexpr uint64_t DEFAULT_VALUE_Z = 21; - struct MyNestedStruct { - uint64_t a { DEFAULT_VALUE_A }; - uint64_t b { DEFAULT_VALUE_B }; - }; - struct MyStruct { - uint64_t z { DEFAULT_VALUE_Z }; - MyNestedStruct data; - }; - - const auto service_name = iox2_testing::generate_service_name(); - - auto node = NodeBuilder().create().expect(""); - auto service = - node.service_builder(service_name).template publish_subscribe>().create().expect(""); - - auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); - auto sut_subscriber = service.subscriber_builder().create().expect(""); - - auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); - send_sample.write_from_fn([](auto index) { - std::cout << index << std::endl; - return MyStruct { DEFAULT_VALUE_Z + index, - MyNestedStruct { DEFAULT_VALUE_A + index, DEFAULT_VALUE_B + index } }; - }); - send(assume_init(std::move(send_sample))).expect(""); - - auto recv_result = sut_subscriber.receive().expect(""); - ASSERT_TRUE(recv_result.has_value()); - auto recv_sample = std::move(recv_result.value()); - - auto iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { - ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z + iterations)); - ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A + iterations)); - ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B + iterations)); - ++iterations; - } - - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); - ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); -} +struct DummyData { + static constexpr uint64_t DEFAULT_VALUE_A = 42; + static constexpr bool DEFAULT_VALUE_Z { false }; + uint64_t a { DEFAULT_VALUE_A }; + bool z { DEFAULT_VALUE_Z }; +}; -TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { +// NOLINTBEGIN(cppcoreguidelines-cognitive-complexity) : Cognitive complexity of 26 (+1) is OK. Test case is complex. +TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr auto SLICE_MAX_LENGTH = 10; - constexpr uint64_t DEFAULT_VALUE_A = 42; - constexpr uint64_t DEFAULT_VALUE_B = 777; - constexpr uint64_t DEFAULT_VALUE_Z = 21; - struct MyNestedStruct { - uint64_t a { DEFAULT_VALUE_A }; - uint64_t b { DEFAULT_VALUE_B }; - }; - struct MyStruct { - uint64_t z { DEFAULT_VALUE_Z }; - MyNestedStruct data; - }; - const auto service_name = iox2_testing::generate_service_name(); auto node = NodeBuilder().create().expect(""); auto service = - node.service_builder(service_name).template publish_subscribe>().create().expect(""); + node.service_builder(service_name).template publish_subscribe>().create().expect(""); auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - iox::UninitializedArray elements; + iox::UninitializedArray elements; for (auto& item : elements) { - new (&item) MyStruct {}; + new (&item) DummyData {}; } auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); - auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); - send_sample.write_from_slice(payload); - send(assume_init(std::move(send_sample))).expect(""); + sut_publisher.send_slice_copy(payload).expect(""); auto recv_result = sut_subscriber.receive().expect(""); ASSERT_TRUE(recv_result.has_value()); @@ -337,38 +255,26 @@ TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { auto iterations = 0; for (const auto& item : recv_sample.payload_slice()) { - ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z)); - ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A)); - ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B)); + ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(item.z, Eq(DummyData::DEFAULT_VALUE_Z)); ++iterations; } ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } +// NOLINTEND(cppcoreguidelines-cognitive-complexity) TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr uint64_t PAYLOAD_ALIGNMENT = 8; constexpr auto SLICE_MAX_LENGTH = 10; - constexpr uint64_t DEFAULT_VALUE_A = 42; - constexpr uint64_t DEFAULT_VALUE_B = 777; - constexpr uint64_t DEFAULT_VALUE_Z = 21; - struct MyNestedStruct { - uint64_t a { DEFAULT_VALUE_A }; - uint64_t b { DEFAULT_VALUE_B }; - }; - struct MyStruct { - uint64_t z { DEFAULT_VALUE_Z }; - MyNestedStruct data; - }; - const auto service_name = iox2_testing::generate_service_name(); auto node = NodeBuilder().create().expect(""); auto service = node.service_builder(service_name) - .template publish_subscribe>() + .template publish_subscribe>() .payload_alignment(PAYLOAD_ALIGNMENT) .create() .expect(""); @@ -386,9 +292,8 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { auto iterations = 0; for (const auto& item : recv_sample.payload_slice()) { - ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z)); - ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A)); - ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B)); + ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(item.z, Eq(DummyData::DEFAULT_VALUE_Z)); ++iterations; } @@ -401,23 +306,11 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { constexpr uint64_t PAYLOAD_ALIGNMENT = 8; constexpr auto SLICE_MAX_LENGTH = 10; - constexpr uint64_t DEFAULT_VALUE_A = 42; - constexpr uint64_t DEFAULT_VALUE_B = 777; - constexpr uint64_t DEFAULT_VALUE_Z = 21; - struct MyNestedStruct { - uint64_t a { DEFAULT_VALUE_A }; - uint64_t b { DEFAULT_VALUE_B }; - }; - struct MyStruct { - uint64_t z { DEFAULT_VALUE_Z }; - MyNestedStruct data; - }; - const auto service_name = iox2_testing::generate_service_name(); auto node = NodeBuilder().create().expect(""); auto service = node.service_builder(service_name) - .template publish_subscribe>() + .template publish_subscribe>() .payload_alignment(PAYLOAD_ALIGNMENT) .create() .expect(""); @@ -429,8 +322,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { auto iterations = 0; for (auto& item : send_sample.payload_slice()) { - new (&item) MyStruct { DEFAULT_VALUE_Z + iterations, - MyNestedStruct { DEFAULT_VALUE_A + iterations, DEFAULT_VALUE_B + iterations } }; + new (&item) DummyData { DummyData::DEFAULT_VALUE_A + iterations, iterations % 2 == 0 }; ++iterations; } @@ -442,9 +334,8 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { iterations = 0; for (const auto& item : recv_sample.payload_slice()) { - ASSERT_THAT(item.z, Eq(DEFAULT_VALUE_Z + iterations)); - ASSERT_THAT(item.data.a, Eq(DEFAULT_VALUE_A + iterations)); - ASSERT_THAT(item.data.b, Eq(DEFAULT_VALUE_B + iterations)); + ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A + iterations)); + ASSERT_THAT(item.z, Eq(iterations % 2 == 0)); ++iterations; } @@ -457,18 +348,6 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv constexpr uint64_t PAYLOAD_ALIGNMENT = 8; constexpr auto SLICE_MAX_LENGTH = 10; - constexpr uint64_t DEFAULT_VALUE_A = 42; - constexpr uint64_t DEFAULT_VALUE_B = 777; - constexpr uint64_t DEFAULT_VALUE_Z = 21; - struct MyNestedStruct { - uint64_t a { DEFAULT_VALUE_A }; - uint64_t b { DEFAULT_VALUE_B }; - }; - struct MyStruct { - uint64_t z { DEFAULT_VALUE_Z }; - MyNestedStruct data; - }; - const auto service_name = iox2_testing::generate_service_name(); auto node = NodeBuilder().create().expect(""); @@ -478,13 +357,12 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv .create() .expect(""); - auto sut_publisher = service.publisher_builder().max_slice_len(sizeof(MyStruct)).create().expect(""); + auto sut_publisher = service.publisher_builder().max_slice_len(sizeof(DummyData)).create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - auto send_sample = sut_publisher.loan_slice_uninit(sizeof(MyStruct)).expect(""); + auto send_sample = sut_publisher.loan_slice_uninit(sizeof(DummyData)).expect(""); - new (send_sample.payload_slice().data()) - MyStruct { DEFAULT_VALUE_Z, MyNestedStruct { DEFAULT_VALUE_A, DEFAULT_VALUE_B } }; + new (send_sample.payload_slice().data()) DummyData {}; send(assume_init(std::move(send_sample))).expect(""); @@ -492,34 +370,82 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv ASSERT_TRUE(recv_result.has_value()); auto recv_sample = std::move(recv_result.value()); - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(sizeof(MyStruct))); + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(sizeof(DummyData))); - auto recv_data = reinterpret_cast(recv_sample.payload_slice().data()); + const auto* recv_data = static_cast(static_cast(recv_sample.payload_slice().data())); - ASSERT_THAT(recv_data->z, Eq(DEFAULT_VALUE_Z)); - ASSERT_THAT(recv_data->data.a, Eq(DEFAULT_VALUE_A)); - ASSERT_THAT(recv_data->data.b, Eq(DEFAULT_VALUE_B)); + ASSERT_THAT(recv_data->a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(recv_data->z, Eq(DummyData::DEFAULT_VALUE_Z)); } -TYPED_TEST(ServicePublishSubscribeTest, loan_send_receive_works) { +TYPED_TEST(ServicePublishSubscribeTest, write_from_fn_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr auto SLICE_MAX_LENGTH = 10; const auto service_name = iox2_testing::generate_service_name(); auto node = NodeBuilder().create().expect(""); - auto service = node.service_builder(service_name).template publish_subscribe().create().expect(""); + auto service = + node.service_builder(service_name).template publish_subscribe>().create().expect(""); - auto sut_publisher = service.publisher_builder().create().expect(""); + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); auto sut_subscriber = service.subscriber_builder().create().expect(""); - auto sample = sut_publisher.loan().expect(""); - const uint64_t payload = 781891729871; - *sample = payload; - send(std::move(sample)).expect(""); - auto recv_sample = sut_subscriber.receive().expect(""); + auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); + send_sample.write_from_fn( + [](auto index) { return DummyData { DummyData::DEFAULT_VALUE_A + index, index % 2 == 0 }; }); + send(assume_init(std::move(send_sample))).expect(""); - ASSERT_TRUE(recv_sample.has_value()); - ASSERT_THAT(**recv_sample, Eq(payload)); + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + auto iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A + iterations)); + ASSERT_THAT(item.z, Eq(iterations % 2 == 0)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); +} + +TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { + constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; + constexpr auto SLICE_MAX_LENGTH = 10; + + const auto service_name = iox2_testing::generate_service_name(); + + auto node = NodeBuilder().create().expect(""); + auto service = + node.service_builder(service_name).template publish_subscribe>().create().expect(""); + + auto sut_publisher = service.publisher_builder().max_slice_len(SLICE_MAX_LENGTH).create().expect(""); + auto sut_subscriber = service.subscriber_builder().create().expect(""); + + iox::UninitializedArray elements; + for (auto& item : elements) { + new (&item) DummyData {}; + } + auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); + auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); + send_sample.write_from_slice(payload); + send(assume_init(std::move(send_sample))).expect(""); + + auto recv_result = sut_subscriber.receive().expect(""); + ASSERT_TRUE(recv_result.has_value()); + auto recv_sample = std::move(recv_result.value()); + + auto iterations = 0; + for (const auto& item : recv_sample.payload_slice()) { + ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(item.z, Eq(DummyData::DEFAULT_VALUE_Z)); + ++iterations; + } + + ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } TYPED_TEST(ServicePublishSubscribeTest, update_connections_delivers_history) { From 299d9d882312825579d29e5cb20e8ab4eb7d74c7 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Tue, 5 Nov 2024 19:55:06 +0100 Subject: [PATCH 08/22] [#490] Add trait to determine payload value type of any payload --- .../cxx/include/iox2/payload_info.hpp | 31 +++++++++++++++++++ .../include/iox2/port_factory_publisher.hpp | 16 +++------- .../service_builder_publish_subscribe.hpp | 13 ++------ 3 files changed, 38 insertions(+), 22 deletions(-) create mode 100644 iceoryx2-ffi/cxx/include/iox2/payload_info.hpp diff --git a/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp b/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp new file mode 100644 index 000000000..30f2634b5 --- /dev/null +++ b/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp @@ -0,0 +1,31 @@ +// Copyright (c) 2024 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache Software License 2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0, or the MIT license +// which is available at https://opensource.org/licenses/MIT. +// +// SPDX-License-Identifier: Apache-2.0 OR MIT + +#ifndef IOX2_PAYLOAD_INFO_HPP +#define IOX2_PAYLOAD_INFO_HPP + +#include "iox/slice.hpp" + +namespace iox2 { + +template +struct PayloadInfo { + using TYPE = T; +}; + +template +struct PayloadInfo> { + using TYPE = typename iox::Slice::ValueType; +}; + +} // namespace iox2 +#endif diff --git a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp index 164f5a7ee..47487992f 100644 --- a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp @@ -17,6 +17,7 @@ #include "iox/builder_addendum.hpp" #include "iox/expected.hpp" #include "iox2/internal/iceoryx2.hpp" +#include "iox2/payload_info.hpp" #include "iox2/publisher.hpp" #include "iox2/service_type.hpp" #include "iox2/unable_to_deliver_strategy.hpp" @@ -76,20 +77,13 @@ PortFactoryPublisher::create() && -> iox::expected::VALUE) { - iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, - value * sizeof(typename Payload::ValueType)); - } else { - iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, value * sizeof(Payload)); - } + iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, + value * sizeof(typename PayloadInfo::TYPE)); }) .or_else([&]() { // Assume only one element if not otherwise specified - if constexpr (iox::IsSlice::VALUE) { - iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, sizeof(typename Payload::ValueType)); - } else { - iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, sizeof(Payload)); - } + iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, + sizeof(typename PayloadInfo::TYPE)); }); m_max_loaned_samples.and_then( [&](auto value) { iox2_port_factory_publisher_builder_set_max_loaned_samples(&m_handle, value); }); diff --git a/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp b/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp index 4cf9a70bf..757252a44 100644 --- a/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp @@ -19,6 +19,7 @@ #include "iox2/attribute_specifier.hpp" #include "iox2/attribute_verifier.hpp" #include "iox2/internal/iceoryx2.hpp" +#include "iox2/payload_info.hpp" #include "iox2/port_factory_publish_subscribe.hpp" #include "iox2/service_builder_publish_subscribe_error.hpp" #include "iox2/service_type.hpp" @@ -27,16 +28,6 @@ namespace iox2 { -template -struct PayloadValueType { - using TYPE = T; -}; - -template -struct PayloadValueType> { - using TYPE = typename iox::Slice::ValueType; -}; - /// Builder to create new [`MessagingPattern::PublishSubscribe`] based [`Service`]s template class ServiceBuilderPublishSubscribe { @@ -149,7 +140,7 @@ inline void ServiceBuilderPublishSubscribe::set_paramete [&](auto value) { iox2_service_builder_pub_sub_set_payload_alignment(&m_handle, value); }); m_max_nodes.and_then([&](auto value) { iox2_service_builder_pub_sub_set_max_nodes(&m_handle, value); }); - using ValueType = typename PayloadValueType::TYPE; + using ValueType = typename PayloadInfo::TYPE; auto type_variant = iox::IsSlice::VALUE ? iox2_type_variant_e_DYNAMIC : iox2_type_variant_e_FIXED_SIZE; // payload type details From 656eafcae728dfcb33eabf70f9c6fee1a356ce09 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 20:26:48 +0100 Subject: [PATCH 09/22] [#490] Maintain const correctness with slices --- iceoryx2-ffi/cxx/include/iox/slice.hpp | 65 ++++++------------- .../cxx/include/iox2/payload_info.hpp | 4 +- .../include/iox2/port_factory_publisher.hpp | 6 +- iceoryx2-ffi/cxx/include/iox2/publisher.hpp | 10 +-- iceoryx2-ffi/cxx/include/iox2/sample.hpp | 10 ++- iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 15 +++-- .../cxx/include/iox2/sample_mut_uninit.hpp | 14 ++-- .../service_builder_publish_subscribe.hpp | 2 +- .../src/service_publish_subscribe_tests.cpp | 8 +-- iceoryx2-ffi/cxx/tests/src/slice_tests.cpp | 43 ++++++++++++ 10 files changed, 102 insertions(+), 75 deletions(-) create mode 100644 iceoryx2-ffi/cxx/tests/src/slice_tests.cpp diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 5509131d4..95e825bfa 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -19,15 +19,14 @@ #include namespace iox { + template class Slice { public: - using Iterator = T*; - using ConstIterator = const T*; - using ValueType = T; + using Iterator = std::conditional_t, const T*, T*>; + using ValueType = std::remove_const_t; Slice(T* data, uint64_t number_of_elements); - Slice(const T* data, uint64_t number_of_elements); /// @brief Returns the size of the slice in bytes. /// @return The size of the slice in bytes. @@ -41,37 +40,27 @@ class Slice { /// @param[in] n The index of the element to access. /// @return A const reference to the element at the specified index. /// @pre The index must be less than the number of elements in the slice. - auto operator[](uint64_t n) const -> const T&; + template >> + auto operator[](uint64_t n) const -> const ValueType&; /// @brief Accesses the element at the specified index (non-const version). /// @param[in] n The index of the element to access. /// @return A reference to the element at the specified index. /// @pre The index must be less than the number of elements in the slice. - auto operator[](uint64_t n) -> T&; + template >> + auto operator[](uint64_t n) -> ValueType&; /// @brief Returns an iterator to the beginning of the slice. /// @return An iterator pointing to the first element of the slice. auto begin() -> Iterator; - /// @brief Returns a const iterator to the beginning of the slice. - /// @return A const iterator pointing to the first element of the slice. - auto begin() const -> ConstIterator; - /// @brief Returns an iterator to the end of the slice. /// @return An iterator pointing one past the last element of the slice. auto end() -> Iterator; - /// @brief Returns a const iterator to the end of the slice. - /// @return A const iterator pointing one past the last element of the slice. - auto end() const -> ConstIterator; - /// @brief Returns a pointer to the underlying data of the slice. /// @return A pointer to the first element of the slice. - auto data() -> T*; - - /// @brief Returns a const pointer to the underlying data of the slice. - /// @return A const pointer to the first element of the slice. - auto data() const -> const T*; + auto data() -> Iterator; private: T* m_data; @@ -79,16 +68,14 @@ class Slice { }; template -Slice::Slice(T* data, uint64_t number_of_elements) - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) constness protected by const class specification - : m_data { data } - , m_number_of_elements { number_of_elements } { -} +using MutableSlice = Slice; + +template +using ImmutableSlice = Slice; template -Slice::Slice(const T* data, uint64_t number_of_elements) - // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) constness protected by const class specification - : m_data { const_cast(data) } +Slice::Slice(T* data, uint64_t number_of_elements) + : m_data { data } , m_number_of_elements { number_of_elements } { } @@ -103,13 +90,15 @@ auto Slice::number_of_elements() const -> uint64_t { } template -auto Slice::operator[](const uint64_t n) const -> const T& { +template +auto Slice::operator[](const uint64_t n) const -> const ValueType& { IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); return *(m_data + n); } template -auto Slice::operator[](const uint64_t n) -> T& { +template +auto Slice::operator[](const uint64_t n) -> ValueType& { IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); return *(m_data + n); } @@ -119,11 +108,6 @@ auto Slice::begin() -> Iterator { return m_data; } -template -auto Slice::begin() const -> ConstIterator { - return m_data; -} - template auto Slice::end() -> Iterator { static_assert(!std::is_same_v, "Slice is not allowed"); @@ -131,18 +115,7 @@ auto Slice::end() -> Iterator { } template -auto Slice::end() const -> ConstIterator { - static_assert(!std::is_same_v, "Slice is not allowed"); - return m_data + m_number_of_elements; -} - -template -auto Slice::data() -> T* { - return m_data; -} - -template -auto Slice::data() const -> const T* { +auto Slice::data() -> Iterator { return m_data; } diff --git a/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp b/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp index 30f2634b5..15744e964 100644 --- a/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/payload_info.hpp @@ -19,12 +19,12 @@ namespace iox2 { template struct PayloadInfo { - using TYPE = T; + using ValueType = T; }; template struct PayloadInfo> { - using TYPE = typename iox::Slice::ValueType; + using ValueType = typename iox::Slice::ValueType; }; } // namespace iox2 diff --git a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp index 47487992f..866084079 100644 --- a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp @@ -77,13 +77,13 @@ PortFactoryPublisher::create() && -> iox::expected::TYPE)); + iox2_port_factory_publisher_builder_set_max_slice_len( + &m_handle, value * sizeof(typename PayloadInfo::ValueType)); }) .or_else([&]() { // Assume only one element if not otherwise specified iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, - sizeof(typename PayloadInfo::TYPE)); + sizeof(typename PayloadInfo::ValueType)); }); m_max_loaned_samples.and_then( [&](auto value) { iox2_port_factory_publisher_builder_set_max_loaned_samples(&m_handle, value); }); diff --git a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp index 28a49c580..3f2e5473f 100644 --- a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp @@ -32,6 +32,8 @@ namespace iox2 { /// Sending endpoint of a publish-subscriber based communication. template class Publisher { + using ValueType = typename PayloadInfo::ValueType; + public: Publisher(Publisher&& rhs) noexcept; auto operator=(Publisher&& rhs) noexcept -> Publisher&; @@ -57,7 +59,7 @@ class Publisher { auto send_copy(const Payload& payload) const -> iox::expected; template ::VALUE, void>> - auto send_slice_copy(const Payload& payload) const -> iox::expected; + auto send_slice_copy(iox::ImmutableSlice& payload) const -> iox::expected; /// Loans/allocates a [`SampleMutUninit`] from the underlying data segment of the [`Publisher`]. /// The user has to initialize the payload before it can be sent. @@ -189,7 +191,7 @@ inline auto Publisher::send_copy(const Payload& payload) template template -inline auto Publisher::send_slice_copy(const Payload& payload) const +inline auto Publisher::send_slice_copy(iox::ImmutableSlice& payload) const -> iox::expected { size_t number_of_recipients = 0; auto result = iox2_publisher_send_slice_copy(&m_handle, @@ -246,8 +248,8 @@ inline auto Publisher::loan_slice(const uint64_t number_ } auto sample_init = std::move(sample_uninit.value()); - for (auto& item : sample_init.payload_slice()) { - new (&item) typename T::ValueType(); + for (auto& item : sample_init.payload_slice_mut()) { + new (&item) ValueType(); } return iox::ok(assume_init(std::move(sample_init))); diff --git a/iceoryx2-ffi/cxx/include/iox2/sample.hpp b/iceoryx2-ffi/cxx/include/iox2/sample.hpp index ee7d8590a..07c06f069 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample.hpp @@ -17,10 +17,12 @@ #include "iox/slice.hpp" #include "iox2/header_publish_subscribe.hpp" #include "iox2/internal/iceoryx2.hpp" +#include "iox2/payload_info.hpp" #include "iox2/service_type.hpp" #include "iox2/unique_port_id.hpp" namespace iox2 { + /// It stores the payload and is acquired by the [`Subscriber`] whenever /// it receives new data from a [`Publisher`] via /// [`Subscriber::receive()`]. @@ -35,6 +37,8 @@ namespace iox2 { template // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init,hicpp-member-init) 'm_sample' is not used directly but only via the initialized 'm_handle'; furthermore, it will be initialized on the call site class Sample { + using ValueType = typename PayloadInfo::ValueType; + public: Sample(Sample&& rhs) noexcept; auto operator=(Sample&& rhs) noexcept -> Sample&; @@ -55,7 +59,7 @@ class Sample { /// Returns a slice to navigate the payload of the [`Sample`] template ::VALUE, void>> - auto payload_slice() const -> Payload; + auto payload_slice() const -> iox::ImmutableSlice; /// Returns a reference to the user_header of the [`Sample`] template , T>> @@ -138,13 +142,13 @@ inline auto Sample::payload() const -> const Payload& { template template -inline auto Sample::payload_slice() const -> Payload { +inline auto Sample::payload_slice() const -> iox::ImmutableSlice { const void* ptr = nullptr; size_t number_of_elements = 0; iox2_sample_payload(&m_handle, &ptr, &number_of_elements); - return Payload(static_cast(ptr), number_of_elements); + return iox::ImmutableSlice(static_cast(ptr), number_of_elements); } template diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index b1f3a9e88..4781172f8 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -19,6 +19,7 @@ #include "iox2/header_publish_subscribe.hpp" #include "iox2/iceoryx2.h" #include "iox2/internal/iceoryx2.hpp" +#include "iox2/payload_info.hpp" #include "iox2/publisher_error.hpp" #include "iox2/service_type.hpp" @@ -45,6 +46,8 @@ namespace iox2 { template // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init,hicpp-member-init) 'm_sample' is not used directly but only via the initialized 'm_handle'; furthermore, it will be initialized on the call site class SampleMut { + using ValueType = typename PayloadInfo::ValueType; + public: SampleMut(SampleMut&& rhs) noexcept; auto operator=(SampleMut&& rhs) noexcept -> SampleMut&; @@ -85,10 +88,10 @@ class SampleMut { auto payload_mut() -> Payload&; template ::VALUE, void>> - auto payload_slice() const -> Payload; + auto payload_slice() const -> iox::ImmutableSlice; template ::VALUE, void>> - auto payload_slice_mut() -> Payload; + auto payload_slice_mut() const -> iox::MutableSlice; private: template @@ -216,24 +219,24 @@ inline auto SampleMut::payload_mut() -> Payload& { template template -inline auto SampleMut::payload_slice() const -> Payload { +inline auto SampleMut::payload_slice() const -> iox::ImmutableSlice { const void* ptr = nullptr; size_t number_of_elements = 0; iox2_sample_mut_payload(&m_handle, &ptr, &number_of_elements); - return Payload(static_cast(ptr), number_of_elements); + return iox::ImmutableSlice(static_cast(ptr), number_of_elements); } template template -inline auto SampleMut::payload_slice_mut() -> Payload { +inline auto SampleMut::payload_slice_mut() const -> iox::MutableSlice { void* ptr = nullptr; size_t number_of_elements = 0; iox2_sample_mut_payload_mut(&m_handle, &ptr, &number_of_elements); - return Payload(static_cast(ptr), number_of_elements); + return iox::MutableSlice(static_cast(ptr), number_of_elements); } template diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp index 9d774ec13..49e3d9d65 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp @@ -24,6 +24,8 @@ namespace iox2 { template // NOLINTNEXTLINE(cppcoreguidelines-pro-type-member-init,hicpp-member-init) 'm_sample' is not used directly but only via the initialized 'm_handle'; furthermore, it will be initialized on the call site class SampleMutUninit { + using ValueType = typename PayloadInfo::ValueType; + public: SampleMutUninit(SampleMutUninit&& rhs) noexcept = default; auto operator=(SampleMutUninit&& rhs) noexcept -> SampleMutUninit& = default; @@ -64,10 +66,10 @@ class SampleMutUninit { auto payload_mut() -> Payload&; template ::VALUE, void>> - auto payload_slice() const -> Payload; + auto payload_slice() const -> iox::ImmutableSlice; template ::VALUE, void>> - auto payload_slice_mut() -> Payload; + auto payload_slice_mut() -> iox::MutableSlice; /// Writes the payload to the sample template ::VALUE, T>> @@ -79,7 +81,7 @@ class SampleMutUninit { /// mem copies the value to the sample template ::VALUE, T>> - void write_from_slice(const T& value); + void write_from_slice(iox::ImmutableSlice& value); private: template @@ -153,13 +155,13 @@ inline auto SampleMutUninit::payload_mut() -> Payload& { template template -inline auto SampleMutUninit::payload_slice() const -> Payload { +inline auto SampleMutUninit::payload_slice() const -> iox::ImmutableSlice { return m_sample.payload_slice(); } template template -inline auto SampleMutUninit::payload_slice_mut() -> Payload { +inline auto SampleMutUninit::payload_slice_mut() -> iox::MutableSlice { return m_sample.payload_slice_mut(); } @@ -181,7 +183,7 @@ inline void SampleMutUninit::write_from_fn( template template -inline void SampleMutUninit::write_from_slice(const T& value) { +inline void SampleMutUninit::write_from_slice(iox::ImmutableSlice& value) { auto dest = payload_slice_mut(); IOX_ASSERT(dest.size() >= value.size(), "Destination payload size is smaller than source slice size"); std::memcpy(dest.begin(), value.begin(), value.size()); diff --git a/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp b/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp index 757252a44..bcbfa00e3 100644 --- a/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/service_builder_publish_subscribe.hpp @@ -140,7 +140,7 @@ inline void ServiceBuilderPublishSubscribe::set_paramete [&](auto value) { iox2_service_builder_pub_sub_set_payload_alignment(&m_handle, value); }); m_max_nodes.and_then([&](auto value) { iox2_service_builder_pub_sub_set_max_nodes(&m_handle, value); }); - using ValueType = typename PayloadInfo::TYPE; + using ValueType = typename PayloadInfo::ValueType; auto type_variant = iox::IsSlice::VALUE ? iox2_type_variant_e_DYNAMIC : iox2_type_variant_e_FIXED_SIZE; // payload type details diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index ef07e77e5..1b262be09 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -246,7 +246,7 @@ TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { for (auto& item : elements) { new (&item) DummyData {}; } - auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); + auto payload = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); sut_publisher.send_slice_copy(payload).expect(""); auto recv_result = sut_subscriber.receive().expect(""); @@ -321,7 +321,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); auto iterations = 0; - for (auto& item : send_sample.payload_slice()) { + for (auto& item : send_sample.payload_slice_mut()) { new (&item) DummyData { DummyData::DEFAULT_VALUE_A + iterations, iterations % 2 == 0 }; ++iterations; } @@ -362,7 +362,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv auto send_sample = sut_publisher.loan_slice_uninit(sizeof(DummyData)).expect(""); - new (send_sample.payload_slice().data()) DummyData {}; + new (send_sample.payload_slice_mut().data()) DummyData {}; send(assume_init(std::move(send_sample))).expect(""); @@ -428,7 +428,7 @@ TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { for (auto& item : elements) { new (&item) DummyData {}; } - auto payload = iox::Slice(elements.begin(), SLICE_MAX_LENGTH); + auto payload = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); send_sample.write_from_slice(payload); send(assume_init(std::move(send_sample))).expect(""); diff --git a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp new file mode 100644 index 000000000..538ef13d0 --- /dev/null +++ b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp @@ -0,0 +1,43 @@ +// Copyright (c) 2024 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache Software License 2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0, or the MIT license +// which is available at https://opensource.org/licenses/MIT. +// +// SPDX-License-Identifier: Apache-2.0 OR MIT + +#include "iox/slice.hpp" +#include "test.hpp" + +namespace { + +struct DummyData { + static constexpr uint64_t DEFAULT_VALUE_A = 42; + static constexpr bool DEFAULT_VALUE_Z { false }; + uint64_t a { DEFAULT_VALUE_A }; + bool z { DEFAULT_VALUE_Z }; +}; + +TEST(SliceTest, const_correctness_is_maintained) { + constexpr uint64_t SLICE_MAX_LENGTH = 10; + + auto elements = std::array {}; + + auto slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const>::value); + + auto const_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const>::value); +} + +} // namespace From 5d71e22c27822c5e8169b81a51ce5585b40b5cbc Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 20:43:16 +0100 Subject: [PATCH 10/22] [#490] Fix constness findings in review --- iceoryx2-ffi/cxx/include/iox2/sample.hpp | 2 +- iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 4 ++-- iceoryx2-ffi/cxx/tests/src/slice_tests.cpp | 22 ++++++++++---------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox2/sample.hpp b/iceoryx2-ffi/cxx/include/iox2/sample.hpp index 07c06f069..10431a1ee 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample.hpp @@ -59,7 +59,7 @@ class Sample { /// Returns a slice to navigate the payload of the [`Sample`] template ::VALUE, void>> - auto payload_slice() const -> iox::ImmutableSlice; + auto payload_slice() const -> iox::ImmutableSlice; /// Returns a reference to the user_header of the [`Sample`] template , T>> diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index 4781172f8..47de5c4b7 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -91,7 +91,7 @@ class SampleMut { auto payload_slice() const -> iox::ImmutableSlice; template ::VALUE, void>> - auto payload_slice_mut() const -> iox::MutableSlice; + auto payload_slice_mut() -> iox::MutableSlice; private: template @@ -230,7 +230,7 @@ inline auto SampleMut::payload_slice() const -> iox::Imm template template -inline auto SampleMut::payload_slice_mut() const -> iox::MutableSlice { +inline auto SampleMut::payload_slice_mut() -> iox::MutableSlice { void* ptr = nullptr; size_t number_of_elements = 0; diff --git a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp index 538ef13d0..b773e03eb 100644 --- a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp @@ -27,17 +27,17 @@ TEST(SliceTest, const_correctness_is_maintained) { auto elements = std::array {}; - auto slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); - ASSERT_FALSE(std::is_const>::value); - ASSERT_FALSE(std::is_const>::value); - ASSERT_FALSE(std::is_const>::value); - ASSERT_FALSE(std::is_const>::value); - - auto const_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); - ASSERT_TRUE(std::is_const>::value); - ASSERT_TRUE(std::is_const>::value); - ASSERT_TRUE(std::is_const>::value); - ASSERT_TRUE(std::is_const>::value); + auto mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const>::value); + + auto immutable_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const>::value); } } // namespace From 1c06ce8fda67d4d288685d538326e99661ea7d02 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 20:49:07 +0100 Subject: [PATCH 11/22] [#490] Rename payload_slice to payload --- .../src/subscriber.cpp | 5 ++-- iceoryx2-ffi/cxx/include/iox2/publisher.hpp | 2 +- iceoryx2-ffi/cxx/include/iox2/sample.hpp | 4 +-- iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 8 +++--- .../cxx/include/iox2/sample_mut_uninit.hpp | 16 +++++------ .../src/service_publish_subscribe_tests.cpp | 28 +++++++++---------- 6 files changed, 32 insertions(+), 31 deletions(-) diff --git a/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp b/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp index 593d00f35..09c5db583 100644 --- a/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp +++ b/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp @@ -36,8 +36,9 @@ auto main() -> int { while (node.wait(CYCLE_TIME).has_value()) { auto sample = subscriber.receive().expect("receive succeeds"); while (sample.has_value()) { - std::cout << "received " << std::dec << static_cast(sample->payload_slice().size()) << " bytes: "; - for (auto byte : sample->payload_slice()) { + auto payload = sample->payload(); + std::cout << "received " << std::dec << static_cast(payload.size()) << " bytes: "; + for (auto byte : payload) { std::cout << std::setw(2) << std::setfill('0') << std::hex << static_cast(byte) << " "; } std::cout << std::endl; diff --git a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp index 3f2e5473f..6ce655f8b 100644 --- a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp @@ -248,7 +248,7 @@ inline auto Publisher::loan_slice(const uint64_t number_ } auto sample_init = std::move(sample_uninit.value()); - for (auto& item : sample_init.payload_slice_mut()) { + for (auto& item : sample_init.payload_mut()) { new (&item) ValueType(); } diff --git a/iceoryx2-ffi/cxx/include/iox2/sample.hpp b/iceoryx2-ffi/cxx/include/iox2/sample.hpp index 10431a1ee..30c850c8a 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample.hpp @@ -59,7 +59,7 @@ class Sample { /// Returns a slice to navigate the payload of the [`Sample`] template ::VALUE, void>> - auto payload_slice() const -> iox::ImmutableSlice; + auto payload() const -> iox::ImmutableSlice; /// Returns a reference to the user_header of the [`Sample`] template , T>> @@ -142,7 +142,7 @@ inline auto Sample::payload() const -> const Payload& { template template -inline auto Sample::payload_slice() const -> iox::ImmutableSlice { +inline auto Sample::payload() const -> iox::ImmutableSlice { const void* ptr = nullptr; size_t number_of_elements = 0; diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index 47de5c4b7..9264d5e34 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -88,10 +88,10 @@ class SampleMut { auto payload_mut() -> Payload&; template ::VALUE, void>> - auto payload_slice() const -> iox::ImmutableSlice; + auto payload() const -> iox::ImmutableSlice; template ::VALUE, void>> - auto payload_slice_mut() -> iox::MutableSlice; + auto payload_mut() -> iox::MutableSlice; private: template @@ -219,7 +219,7 @@ inline auto SampleMut::payload_mut() -> Payload& { template template -inline auto SampleMut::payload_slice() const -> iox::ImmutableSlice { +inline auto SampleMut::payload() const -> iox::ImmutableSlice { const void* ptr = nullptr; size_t number_of_elements = 0; @@ -230,7 +230,7 @@ inline auto SampleMut::payload_slice() const -> iox::Imm template template -inline auto SampleMut::payload_slice_mut() -> iox::MutableSlice { +inline auto SampleMut::payload_mut() -> iox::MutableSlice { void* ptr = nullptr; size_t number_of_elements = 0; diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp index 49e3d9d65..a49540409 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp @@ -66,10 +66,10 @@ class SampleMutUninit { auto payload_mut() -> Payload&; template ::VALUE, void>> - auto payload_slice() const -> iox::ImmutableSlice; + auto payload() const -> iox::ImmutableSlice; template ::VALUE, void>> - auto payload_slice_mut() -> iox::MutableSlice; + auto payload_mut() -> iox::MutableSlice; /// Writes the payload to the sample template ::VALUE, T>> @@ -155,14 +155,14 @@ inline auto SampleMutUninit::payload_mut() -> Payload& { template template -inline auto SampleMutUninit::payload_slice() const -> iox::ImmutableSlice { - return m_sample.payload_slice(); +inline auto SampleMutUninit::payload() const -> iox::ImmutableSlice { + return m_sample.payload(); } template template -inline auto SampleMutUninit::payload_slice_mut() -> iox::MutableSlice { - return m_sample.payload_slice_mut(); +inline auto SampleMutUninit::payload_mut() -> iox::MutableSlice { + return m_sample.payload_mut(); } template @@ -175,7 +175,7 @@ template template inline void SampleMutUninit::write_from_fn( const iox::function& initializer) { - auto slice = payload_slice_mut(); + auto slice = payload_mut(); for (uint64_t i = 0; i < slice.number_of_elements(); ++i) { new (&slice[i]) typename T::ValueType(initializer(i)); } @@ -184,7 +184,7 @@ inline void SampleMutUninit::write_from_fn( template template inline void SampleMutUninit::write_from_slice(iox::ImmutableSlice& value) { - auto dest = payload_slice_mut(); + auto dest = payload_mut(); IOX_ASSERT(dest.size() >= value.size(), "Destination payload size is smaller than source slice size"); std::memcpy(dest.begin(), value.begin(), value.size()); } diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index 1b262be09..c3fa88bb0 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -254,13 +254,13 @@ TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { auto recv_sample = std::move(recv_result.value()); auto iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { + for (const auto& item : recv_sample.payload()) { ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A)); ASSERT_THAT(item.z, Eq(DummyData::DEFAULT_VALUE_Z)); ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } // NOLINTEND(cppcoreguidelines-cognitive-complexity) @@ -291,13 +291,13 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { auto recv_sample = std::move(recv_result.value()); auto iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { + for (const auto& item : recv_sample.payload()) { ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A)); ASSERT_THAT(item.z, Eq(DummyData::DEFAULT_VALUE_Z)); ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } @@ -321,7 +321,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { auto send_sample = sut_publisher.loan_slice_uninit(SLICE_MAX_LENGTH).expect(""); auto iterations = 0; - for (auto& item : send_sample.payload_slice_mut()) { + for (auto& item : send_sample.payload_mut()) { new (&item) DummyData { DummyData::DEFAULT_VALUE_A + iterations, iterations % 2 == 0 }; ++iterations; } @@ -333,13 +333,13 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { auto recv_sample = std::move(recv_result.value()); iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { + for (const auto& item : recv_sample.payload()) { ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A + iterations)); ASSERT_THAT(item.z, Eq(iterations % 2 == 0)); ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } @@ -362,7 +362,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv auto send_sample = sut_publisher.loan_slice_uninit(sizeof(DummyData)).expect(""); - new (send_sample.payload_slice_mut().data()) DummyData {}; + new (send_sample.payload_mut().data()) DummyData {}; send(assume_init(std::move(send_sample))).expect(""); @@ -370,9 +370,9 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv ASSERT_TRUE(recv_result.has_value()); auto recv_sample = std::move(recv_result.value()); - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(sizeof(DummyData))); + ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(sizeof(DummyData))); - const auto* recv_data = static_cast(static_cast(recv_sample.payload_slice().data())); + const auto* recv_data = static_cast(static_cast(recv_sample.payload().data())); ASSERT_THAT(recv_data->a, Eq(DummyData::DEFAULT_VALUE_A)); ASSERT_THAT(recv_data->z, Eq(DummyData::DEFAULT_VALUE_Z)); @@ -401,13 +401,13 @@ TYPED_TEST(ServicePublishSubscribeTest, write_from_fn_send_receive_works) { auto recv_sample = std::move(recv_result.value()); auto iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { + for (const auto& item : recv_sample.payload()) { ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A + iterations)); ASSERT_THAT(item.z, Eq(iterations % 2 == 0)); ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } @@ -438,13 +438,13 @@ TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { auto recv_sample = std::move(recv_result.value()); auto iterations = 0; - for (const auto& item : recv_sample.payload_slice()) { + for (const auto& item : recv_sample.payload()) { ASSERT_THAT(item.a, Eq(DummyData::DEFAULT_VALUE_A)); ASSERT_THAT(item.z, Eq(DummyData::DEFAULT_VALUE_Z)); ++iterations; } - ASSERT_THAT(recv_sample.payload_slice().number_of_elements(), Eq(SLICE_MAX_LENGTH)); + ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } From 41227a2fe3c3588ffb618fa92263779b5de7c5d9 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 20:55:06 +0100 Subject: [PATCH 12/22] [#490] Restrict slice-only methods in C++ publisher to slice payloads --- iceoryx2-ffi/cxx/include/iox2/publisher.hpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp index 6ce655f8b..3ac562139 100644 --- a/iceoryx2-ffi/cxx/include/iox2/publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/publisher.hpp @@ -50,6 +50,7 @@ class Publisher { auto unable_to_deliver_strategy() const -> UnableToDeliverStrategy; /// Returns the maximum number of elements that can be loaned in a slice. + template ::VALUE, void>> auto max_slice_len() const -> uint64_t; /// Copies the input `value` into a [`SampleMut`] and delivers it. @@ -152,16 +153,9 @@ inline auto Publisher::unable_to_deliver_strategy() cons template +template inline auto Publisher::max_slice_len() const -> uint64_t { - // NOTE: The C API always uses a [u8] payload, therefore the max length returned is the number of bytes. - // Dividing by the size gives the number of slice elements available to the C++ API. - // - // NOTE: For non-slice types, the number of slice elements is always 1. - if constexpr (iox::IsSlice::VALUE) { - return iox2_publisher_max_slice_len(&m_handle) / sizeof(typename Payload::ValueType); - } else { - return iox2_publisher_max_slice_len(&m_handle) / sizeof(Payload); - } + return iox2_publisher_max_slice_len(&m_handle); } template From acfa6913bbaa56f4de0b7141d5192e8207055b58 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 20:59:13 +0100 Subject: [PATCH 13/22] [#490] Update release notes --- doc/release-notes/iceoryx2-unreleased.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/release-notes/iceoryx2-unreleased.md b/doc/release-notes/iceoryx2-unreleased.md index 8cd2d0547..811dfc59a 100644 --- a/doc/release-notes/iceoryx2-unreleased.md +++ b/doc/release-notes/iceoryx2-unreleased.md @@ -58,6 +58,9 @@ --> * APIs to support slices in the C++ bindings [#490](https://github.com/eclipse-iceoryx/iceoryx2/issues/490) +* Rename `iox2_publisher_loan` to `iox2_publisher_loan_slice_uninit` [#490](https://github.com/eclipse-iceoryx/iceoryx2/issues/490) + 1. C always loans slices, for a single element, specify the + `number_of_elements` to be 1 ### API Breaking Changes From b83b0b3de1e5f92ffb68466bac2371024f3dc568 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 21:42:21 +0100 Subject: [PATCH 14/22] [#490] Fix clang tidy issues --- .../src/service_publish_subscribe_tests.cpp | 11 +++++++---- iceoryx2-ffi/cxx/tests/src/slice_tests.cpp | 16 ++++++++-------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp index c3fa88bb0..1cd702f8b 100644 --- a/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/service_publish_subscribe_tests.cpp @@ -228,7 +228,7 @@ struct DummyData { bool z { DEFAULT_VALUE_Z }; }; -// NOLINTBEGIN(cppcoreguidelines-cognitive-complexity) : Cognitive complexity of 26 (+1) is OK. Test case is complex. +// NOLINTBEGIN(readability-function-cognitive-complexity) : Cognitive complexity of 26 (+1) is OK. Test case is complex. TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr auto SLICE_MAX_LENGTH = 10; @@ -263,7 +263,7 @@ TYPED_TEST(ServicePublishSubscribeTest, slice_copy_send_receive_works) { ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } -// NOLINTEND(cppcoreguidelines-cognitive-complexity) +// NOLINTEND(readability-function-cognitive-complexity) TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; @@ -301,6 +301,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_send_receive_works) { ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } +// NOLINTBEGIN(readability-function-cognitive-complexity) : Cognitive complexity of 26 (+1) is OK. Test case is complex. TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr uint64_t PAYLOAD_ALIGNMENT = 8; @@ -342,6 +343,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_send_receive_works) { ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } +// NOLINTEND(readability-function-cognitive-complexity) TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; @@ -371,8 +373,7 @@ TYPED_TEST(ServicePublishSubscribeTest, loan_slice_uninit_with_bytes_send_receiv auto recv_sample = std::move(recv_result.value()); ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(sizeof(DummyData))); - - const auto* recv_data = static_cast(static_cast(recv_sample.payload().data())); + const auto* recv_data = reinterpret_cast(recv_sample.payload().data()); // NOLINT ASSERT_THAT(recv_data->a, Eq(DummyData::DEFAULT_VALUE_A)); ASSERT_THAT(recv_data->z, Eq(DummyData::DEFAULT_VALUE_Z)); @@ -411,6 +412,7 @@ TYPED_TEST(ServicePublishSubscribeTest, write_from_fn_send_receive_works) { ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } +// NOLINTBEGIN(readability-function-cognitive-complexity) TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; constexpr auto SLICE_MAX_LENGTH = 10; @@ -447,6 +449,7 @@ TYPED_TEST(ServicePublishSubscribeTest, write_from_slice_send_receive_works) { ASSERT_THAT(recv_sample.payload().number_of_elements(), Eq(SLICE_MAX_LENGTH)); ASSERT_THAT(iterations, Eq(SLICE_MAX_LENGTH)); } +// NOLINTEND(readability-function-cognitive-complexity) TYPED_TEST(ServicePublishSubscribeTest, update_connections_delivers_history) { constexpr ServiceType SERVICE_TYPE = TestFixture::TYPE; diff --git a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp index b773e03eb..d6c37ecda 100644 --- a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp @@ -28,16 +28,16 @@ TEST(SliceTest, const_correctness_is_maintained) { auto elements = std::array {}; auto mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); - ASSERT_FALSE(std::is_const>::value); - ASSERT_FALSE(std::is_const>::value); - ASSERT_FALSE(std::is_const>::value); - ASSERT_FALSE(std::is_const>::value); + ASSERT_FALSE(std::is_const_v>); + ASSERT_FALSE(std::is_const_v>); + ASSERT_FALSE(std::is_const_v>); + ASSERT_FALSE(std::is_const_v>); auto immutable_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); - ASSERT_TRUE(std::is_const>::value); - ASSERT_TRUE(std::is_const>::value); - ASSERT_TRUE(std::is_const>::value); - ASSERT_TRUE(std::is_const>::value); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); } } // namespace From a03dfc4cc10b84ffb7f69cf70e3f30589962c35e Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 22:07:23 +0100 Subject: [PATCH 15/22] [#490] Properly support const Slices --- iceoryx2-ffi/cxx/include/iox/slice.hpp | 47 ++++++++++---- iceoryx2-ffi/cxx/tests/src/slice_tests.cpp | 72 ++++++++++++++++++++++ 2 files changed, 108 insertions(+), 11 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 95e825bfa..48220726e 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -40,25 +40,35 @@ class Slice { /// @param[in] n The index of the element to access. /// @return A const reference to the element at the specified index. /// @pre The index must be less than the number of elements in the slice. - template >> - auto operator[](uint64_t n) const -> const ValueType&; + auto operator[](uint64_t n) const -> std::conditional_t, const ValueType&, ValueType&>; /// @brief Accesses the element at the specified index (non-const version). /// @param[in] n The index of the element to access. /// @return A reference to the element at the specified index. /// @pre The index must be less than the number of elements in the slice. - template >> - auto operator[](uint64_t n) -> ValueType&; + auto operator[](uint64_t n) -> std::conditional_t, const ValueType&, ValueType&>; - /// @brief Returns an iterator to the beginning of the slice. + /// @brief Returns an iterator to the beginning of the slice (const version). + /// @return An iterator pointing to the first element of the slice. + auto begin() const -> Iterator; + + /// @brief Returns an iterator to the beginning of the slice (non-const version). /// @return An iterator pointing to the first element of the slice. auto begin() -> Iterator; - /// @brief Returns an iterator to the end of the slice. + /// @brief Returns an iterator to the end of the slice (const version). + /// @return An iterator pointing one past the last element of the slice. + auto end() const -> Iterator; + + /// @brief Returns an iterator to the end of the slice (non-const version). /// @return An iterator pointing one past the last element of the slice. auto end() -> Iterator; - /// @brief Returns a pointer to the underlying data of the slice. + /// @brief Returns a pointer to the underlying data of the slice (const version). + /// @return A pointer to the first element of the slice. + auto data() const -> Iterator; + + /// @brief Returns a pointer to the underlying data of the slice (non-const version). /// @return A pointer to the first element of the slice. auto data() -> Iterator; @@ -90,15 +100,14 @@ auto Slice::number_of_elements() const -> uint64_t { } template -template -auto Slice::operator[](const uint64_t n) const -> const ValueType& { +auto Slice::operator[](const uint64_t n) const + -> std::conditional_t, const ValueType&, ValueType&> { IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); return *(m_data + n); } template -template -auto Slice::operator[](const uint64_t n) -> ValueType& { +auto Slice::operator[](const uint64_t n) -> std::conditional_t, const ValueType&, ValueType&> { IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); return *(m_data + n); } @@ -108,17 +117,33 @@ auto Slice::begin() -> Iterator { return m_data; } +template +auto Slice::begin() const -> Iterator { + return m_data; +} + template auto Slice::end() -> Iterator { static_assert(!std::is_same_v, "Slice is not allowed"); return m_data + m_number_of_elements; } +template +auto Slice::end() const -> Iterator { + static_assert(!std::is_same_v, "Slice is not allowed"); + return m_data + m_number_of_elements; +} + template auto Slice::data() -> Iterator { return m_data; } +template +auto Slice::data() const -> Iterator { + return m_data; +} + template struct IsSlice { static constexpr bool VALUE = false; diff --git a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp index d6c37ecda..07de4373b 100644 --- a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp @@ -33,11 +33,83 @@ TEST(SliceTest, const_correctness_is_maintained) { ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); + const auto const_mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + ASSERT_FALSE(std::is_const_v>); + ASSERT_FALSE(std::is_const_v>); + ASSERT_FALSE(std::is_const_v>); + ASSERT_FALSE(std::is_const_v>); + auto immutable_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); + + const auto const_immutable_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); +} + +TEST(SliceTest, can_iterate_mutable_slice) { + constexpr uint64_t SLICE_MAX_LENGTH = 10; + + auto elements = std::array {}; + + auto mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto iterations = 0; + for (auto element : mutable_slice) { + ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(element.z, Eq(DummyData::DEFAULT_VALUE_Z)); + iterations++; + } + ASSERT_EQ(iterations, SLICE_MAX_LENGTH); +} + +TEST(SliceTest, can_iterate_const_mutable_slice) { + constexpr uint64_t SLICE_MAX_LENGTH = 10; + + auto elements = std::array {}; + + const auto slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto iterations = 0; + for (auto element : slice) { + ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(element.z, Eq(DummyData::DEFAULT_VALUE_Z)); + iterations++; + } + ASSERT_EQ(iterations, SLICE_MAX_LENGTH); +} + +TEST(SliceTest, can_iterate_immutable_slice) { + constexpr uint64_t SLICE_MAX_LENGTH = 10; + + auto elements = std::array {}; + + auto slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto iterations = 0; + for (auto element : slice) { + ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(element.z, Eq(DummyData::DEFAULT_VALUE_Z)); + iterations++; + } + ASSERT_EQ(iterations, SLICE_MAX_LENGTH); +} + +TEST(SliceTest, can_iterate_const_immutable_slice) { + constexpr uint64_t SLICE_MAX_LENGTH = 10; + + auto elements = std::array {}; + + const auto slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto iterations = 0; + for (auto element : slice) { + ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); + ASSERT_THAT(element.z, Eq(DummyData::DEFAULT_VALUE_Z)); + iterations++; + } + ASSERT_EQ(iterations, SLICE_MAX_LENGTH); } } // namespace From 103a02c4a0e4479476c15a5ace84404b13dd3a1d Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Wed, 6 Nov 2024 22:15:52 +0100 Subject: [PATCH 16/22] [#490] Properly set max slice length consdering custom type settings --- .../cxx/include/iox2/port_factory_publisher.hpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp index 866084079..9ff5d84f9 100644 --- a/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/port_factory_publisher.hpp @@ -73,18 +73,8 @@ PortFactoryPublisher::create() && -> iox::expected(iox::into(value))); }); m_max_slice_len - .and_then([&](auto value) { - // The payload type used by the C API is always a [u8]. - // Thus need to convert from N to N * sizeof(payload). - // TODO: Consider alignment... not aligning each element properly will impact performance - iox2_port_factory_publisher_builder_set_max_slice_len( - &m_handle, value * sizeof(typename PayloadInfo::ValueType)); - }) - .or_else([&]() { - // Assume only one element if not otherwise specified - iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, - sizeof(typename PayloadInfo::ValueType)); - }); + .and_then([&](auto value) { iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, value); }) + .or_else([&]() { iox2_port_factory_publisher_builder_set_max_slice_len(&m_handle, 1); }); m_max_loaned_samples.and_then( [&](auto value) { iox2_port_factory_publisher_builder_set_max_loaned_samples(&m_handle, value); }); From 47feca2eee19439499efbdf85f68a96f8f6e9d08 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 7 Nov 2024 01:24:01 +0100 Subject: [PATCH 17/22] [#490] Rename Slice::size to Size::number_of_bytes --- iceoryx2-ffi/cxx/include/iox/slice.hpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 48220726e..00ae72828 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -20,17 +20,26 @@ namespace iox { +/// @brief A class representing a slice of contiguous elements of type T. +/// +/// A Slice provides a view into a contiguous sequence of elements without owning the memory. +/// It allows for efficient access and iteration over a portion of a contiguous data structure. +/// +/// @tparam T The type of elements in the slice. Can be const-qualified for read-only slices. template class Slice { public: using Iterator = std::conditional_t, const T*, T*>; using ValueType = std::remove_const_t; + /// @brief Constructs a Slice object. + /// @param[in] data Pointer to the beginning of the data. + /// @param[in] number_of_elements The number of elements in the slice. Slice(T* data, uint64_t number_of_elements); - /// @brief Returns the size of the slice in bytes. - /// @return The size of the slice in bytes. - auto size() const -> uint64_t; + /// @brief Returns the total number of bytes occupied by the slice. + /// @return The number of bytes occupied by the slice, rounded up to the nearest alignment boundary. + auto number_of_bytes() const -> uint64_t; /// @brief Returns the number of elements in the slice. /// @return The number of elements in the slice. @@ -90,7 +99,7 @@ Slice::Slice(T* data, uint64_t number_of_elements) } template -auto Slice::size() const -> uint64_t { +auto Slice::number_of_bytes() const -> uint64_t { return (sizeof(ValueType) * m_number_of_elements + alignof(ValueType) - 1) & ~(alignof(ValueType) - 1); } From 1a8a87e149dcc0710e336b2ba846ef9de4ef14b4 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 7 Nov 2024 01:29:34 +0100 Subject: [PATCH 18/22] [#490] Rename worst_case_memory_size to maximum_elements in slice example --- .../publish_subscribe_dynamic_data/src/publisher.cpp | 10 +++++----- .../publish_subscribe_dynamic_data/src/subscriber.cpp | 2 +- iceoryx2-ffi/cxx/include/iox/slice.hpp | 3 +-- iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp | 5 +++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp b/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp index 4baee32fb..e8c34ea3d 100644 --- a/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp +++ b/examples/cxx/publish_subscribe_dynamic_data/src/publisher.cpp @@ -32,11 +32,11 @@ auto main() -> int { .open_or_create() .expect("successful service creation/opening"); - const uint64_t worst_case_memory_size = 1024; // NOLINT - auto publisher = service.publisher_builder() - .max_slice_len(worst_case_memory_size) - .create() - .expect("successful publisher creation"); + // Since the payload type is uint8_t, this number is the same as the number of bytes in the payload. + // For other types, number of bytes used by the payload will be max_slice_len * sizeof(Payload::ValueType) + const uint64_t maximum_elements = 1024; // NOLINT + auto publisher = + service.publisher_builder().max_slice_len(maximum_elements).create().expect("successful publisher creation"); auto counter = 0; diff --git a/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp b/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp index 09c5db583..4a789d4d3 100644 --- a/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp +++ b/examples/cxx/publish_subscribe_dynamic_data/src/subscriber.cpp @@ -37,7 +37,7 @@ auto main() -> int { auto sample = subscriber.receive().expect("receive succeeds"); while (sample.has_value()) { auto payload = sample->payload(); - std::cout << "received " << std::dec << static_cast(payload.size()) << " bytes: "; + std::cout << "received " << std::dec << static_cast(payload.number_of_bytes()) << " bytes: "; for (auto byte : payload) { std::cout << std::setw(2) << std::setfill('0') << std::hex << static_cast(byte) << " "; } diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 00ae72828..217d29dda 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -96,6 +96,7 @@ template Slice::Slice(T* data, uint64_t number_of_elements) : m_data { data } , m_number_of_elements { number_of_elements } { + static_assert(!std::is_same_v, "Slice is not allowed"); } template @@ -133,13 +134,11 @@ auto Slice::begin() const -> Iterator { template auto Slice::end() -> Iterator { - static_assert(!std::is_same_v, "Slice is not allowed"); return m_data + m_number_of_elements; } template auto Slice::end() const -> Iterator { - static_assert(!std::is_same_v, "Slice is not allowed"); return m_data + m_number_of_elements; } diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp index a49540409..faaa2e9c2 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp @@ -185,8 +185,9 @@ template template inline void SampleMutUninit::write_from_slice(iox::ImmutableSlice& value) { auto dest = payload_mut(); - IOX_ASSERT(dest.size() >= value.size(), "Destination payload size is smaller than source slice size"); - std::memcpy(dest.begin(), value.begin(), value.size()); + IOX_ASSERT(dest.number_of_bytes() >= value.number_of_bytes(), + "Destination payload size is smaller than source slice size"); + std::memcpy(dest.begin(), value.begin(), value.number_of_bytes()); } } // namespace iox2 From 9a25cbc2d43a775e3d2c8586d7c0ff99baaff76e Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 7 Nov 2024 11:26:29 +0100 Subject: [PATCH 19/22] [#490] Consistent use of ValueType when retrieving Payload --- iceoryx2-ffi/cxx/include/iox2/sample.hpp | 6 +++--- iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp | 12 ++++++------ iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox2/sample.hpp b/iceoryx2-ffi/cxx/include/iox2/sample.hpp index 30c850c8a..4a1f8e5f7 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample.hpp @@ -55,7 +55,7 @@ class Sample { /// Returns a reference to the payload of the [`Sample`] template ::VALUE, void>> - auto payload() const -> const Payload&; + auto payload() const -> const ValueType&; /// Returns a slice to navigate the payload of the [`Sample`] template ::VALUE, void>> @@ -132,12 +132,12 @@ inline auto Sample::operator->() const -> const Payload* template template -inline auto Sample::payload() const -> const Payload& { +inline auto Sample::payload() const -> const ValueType& { const void* ptr = nullptr; iox2_sample_payload(&m_handle, &ptr, nullptr); - return *static_cast(ptr); + return *static_cast(ptr); } template diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp index 9264d5e34..2451d3797 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp @@ -81,11 +81,11 @@ class SampleMut { /// Returns a reference to the const payload of the sample. template ::VALUE, void>> - auto payload() const -> const Payload&; + auto payload() const -> const ValueType&; /// Returns a reference to the payload of the sample. template ::VALUE, void>> - auto payload_mut() -> Payload&; + auto payload_mut() -> ValueType&; template ::VALUE, void>> auto payload() const -> iox::ImmutableSlice; @@ -199,22 +199,22 @@ inline auto SampleMut::user_header_mut() -> T& { template template -inline auto SampleMut::payload() const -> const Payload& { +inline auto SampleMut::payload() const -> const ValueType& { const void* ptr = nullptr; iox2_sample_mut_payload(&m_handle, &ptr, nullptr); - return *static_cast(ptr); + return *static_cast(ptr); } template template -inline auto SampleMut::payload_mut() -> Payload& { +inline auto SampleMut::payload_mut() -> ValueType& { void* ptr = nullptr; iox2_sample_mut_payload_mut(&m_handle, &ptr, nullptr); - return *static_cast(ptr); + return *static_cast(ptr); } template diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp index faaa2e9c2..b90dc6489 100644 --- a/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/sample_mut_uninit.hpp @@ -59,11 +59,11 @@ class SampleMutUninit { /// Returns a reference to the const payload of the sample. template ::VALUE, void>> - auto payload() const -> const Payload&; + auto payload() const -> const ValueType&; /// Returns a reference to the payload of the sample. template ::VALUE, void>> - auto payload_mut() -> Payload&; + auto payload_mut() -> ValueType&; template ::VALUE, void>> auto payload() const -> iox::ImmutableSlice; @@ -143,13 +143,13 @@ inline auto SampleMutUninit::user_header_mut() -> T& { template template -inline auto SampleMutUninit::payload() const -> const Payload& { +inline auto SampleMutUninit::payload() const -> const ValueType& { return m_sample.payload(); } template template -inline auto SampleMutUninit::payload_mut() -> Payload& { +inline auto SampleMutUninit::payload_mut() -> ValueType& { return m_sample.payload_mut(); } From b353e0a4f440e8ac984ea3e63e477b3c908900c0 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 7 Nov 2024 11:47:23 +0100 Subject: [PATCH 20/22] [#490] Fix windows build --- iceoryx2-ffi/cxx/tests/src/slice_tests.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp index 07de4373b..9c0cbf95d 100644 --- a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp @@ -13,6 +13,8 @@ #include "iox/slice.hpp" #include "test.hpp" +#include + namespace { struct DummyData { @@ -27,25 +29,25 @@ TEST(SliceTest, const_correctness_is_maintained) { auto elements = std::array {}; - auto mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto mutable_slice = iox::MutableSlice(elements.data(), SLICE_MAX_LENGTH); ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); - const auto const_mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + const auto const_mutable_slice = iox::MutableSlice(elements.data(), SLICE_MAX_LENGTH); ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); - auto immutable_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto immutable_slice = iox::ImmutableSlice(elements.data(), SLICE_MAX_LENGTH); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); - const auto const_immutable_slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + const auto const_immutable_slice = iox::ImmutableSlice(elements.data(), SLICE_MAX_LENGTH); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); ASSERT_TRUE(std::is_const_v>); @@ -57,7 +59,7 @@ TEST(SliceTest, can_iterate_mutable_slice) { auto elements = std::array {}; - auto mutable_slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto mutable_slice = iox::MutableSlice(elements.data(), SLICE_MAX_LENGTH); auto iterations = 0; for (auto element : mutable_slice) { ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); @@ -72,7 +74,7 @@ TEST(SliceTest, can_iterate_const_mutable_slice) { auto elements = std::array {}; - const auto slice = iox::MutableSlice(elements.begin(), SLICE_MAX_LENGTH); + const auto slice = iox::MutableSlice(elements.data(), SLICE_MAX_LENGTH); auto iterations = 0; for (auto element : slice) { ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); @@ -87,7 +89,7 @@ TEST(SliceTest, can_iterate_immutable_slice) { auto elements = std::array {}; - auto slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + auto slice = iox::ImmutableSlice(elements.data(), SLICE_MAX_LENGTH); auto iterations = 0; for (auto element : slice) { ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); @@ -102,7 +104,7 @@ TEST(SliceTest, can_iterate_const_immutable_slice) { auto elements = std::array {}; - const auto slice = iox::ImmutableSlice(elements.begin(), SLICE_MAX_LENGTH); + const auto slice = iox::ImmutableSlice(elements.data(), SLICE_MAX_LENGTH); auto iterations = 0; for (auto element : slice) { ASSERT_THAT(element.a, Eq(DummyData::DEFAULT_VALUE_A)); From 636e1990c72b89210fad1c323819799702d5a6f9 Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 7 Nov 2024 12:33:31 +0100 Subject: [PATCH 21/22] [#490] Make const MutableSlice also prevent mutation of elements --- iceoryx2-ffi/cxx/include/iox/slice.hpp | 20 ++++++++++---------- iceoryx2-ffi/cxx/tests/src/slice_tests.cpp | 9 +++++---- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp index 217d29dda..7edd953fe 100644 --- a/iceoryx2-ffi/cxx/include/iox/slice.hpp +++ b/iceoryx2-ffi/cxx/include/iox/slice.hpp @@ -29,7 +29,8 @@ namespace iox { template class Slice { public: - using Iterator = std::conditional_t, const T*, T*>; + using Iterator = T*; + using ConstIterator = const T*; using ValueType = std::remove_const_t; /// @brief Constructs a Slice object. @@ -49,7 +50,7 @@ class Slice { /// @param[in] n The index of the element to access. /// @return A const reference to the element at the specified index. /// @pre The index must be less than the number of elements in the slice. - auto operator[](uint64_t n) const -> std::conditional_t, const ValueType&, ValueType&>; + auto operator[](uint64_t n) const -> const ValueType&; /// @brief Accesses the element at the specified index (non-const version). /// @param[in] n The index of the element to access. @@ -59,7 +60,7 @@ class Slice { /// @brief Returns an iterator to the beginning of the slice (const version). /// @return An iterator pointing to the first element of the slice. - auto begin() const -> Iterator; + auto begin() const -> ConstIterator; /// @brief Returns an iterator to the beginning of the slice (non-const version). /// @return An iterator pointing to the first element of the slice. @@ -67,7 +68,7 @@ class Slice { /// @brief Returns an iterator to the end of the slice (const version). /// @return An iterator pointing one past the last element of the slice. - auto end() const -> Iterator; + auto end() const -> ConstIterator; /// @brief Returns an iterator to the end of the slice (non-const version). /// @return An iterator pointing one past the last element of the slice. @@ -75,7 +76,7 @@ class Slice { /// @brief Returns a pointer to the underlying data of the slice (const version). /// @return A pointer to the first element of the slice. - auto data() const -> Iterator; + auto data() const -> ConstIterator; /// @brief Returns a pointer to the underlying data of the slice (non-const version). /// @return A pointer to the first element of the slice. @@ -110,8 +111,7 @@ auto Slice::number_of_elements() const -> uint64_t { } template -auto Slice::operator[](const uint64_t n) const - -> std::conditional_t, const ValueType&, ValueType&> { +auto Slice::operator[](const uint64_t n) const -> const ValueType& { IOX_ASSERT(n < m_number_of_elements, "Index out of bounds"); return *(m_data + n); } @@ -128,7 +128,7 @@ auto Slice::begin() -> Iterator { } template -auto Slice::begin() const -> Iterator { +auto Slice::begin() const -> ConstIterator { return m_data; } @@ -138,7 +138,7 @@ auto Slice::end() -> Iterator { } template -auto Slice::end() const -> Iterator { +auto Slice::end() const -> ConstIterator { return m_data + m_number_of_elements; } @@ -148,7 +148,7 @@ auto Slice::data() -> Iterator { } template -auto Slice::data() const -> Iterator { +auto Slice::data() const -> ConstIterator { return m_data; } diff --git a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp index 9c0cbf95d..d9ff1aeb2 100644 --- a/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/slice_tests.cpp @@ -35,11 +35,12 @@ TEST(SliceTest, const_correctness_is_maintained) { ASSERT_FALSE(std::is_const_v>); ASSERT_FALSE(std::is_const_v>); + // const instances of MutableSlice are also not mutable const auto const_mutable_slice = iox::MutableSlice(elements.data(), SLICE_MAX_LENGTH); - ASSERT_FALSE(std::is_const_v>); - ASSERT_FALSE(std::is_const_v>); - ASSERT_FALSE(std::is_const_v>); - ASSERT_FALSE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); + ASSERT_TRUE(std::is_const_v>); auto immutable_slice = iox::ImmutableSlice(elements.data(), SLICE_MAX_LENGTH); ASSERT_TRUE(std::is_const_v>); From 4dfb681757dcc28a3ae56371c78a252909577cce Mon Sep 17 00:00:00 2001 From: Jeff Ithier Date: Thu, 7 Nov 2024 12:54:31 +0100 Subject: [PATCH 22/22] [#490] Make rust and C++ dynamic example consistent --- .../publish_subscribe_dynamic_data/publisher.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/examples/rust/publish_subscribe_dynamic_data/publisher.rs b/examples/rust/publish_subscribe_dynamic_data/publisher.rs index d729735a5..8baa001c1 100644 --- a/examples/rust/publish_subscribe_dynamic_data/publisher.rs +++ b/examples/rust/publish_subscribe_dynamic_data/publisher.rs @@ -23,24 +23,27 @@ fn main() -> Result<(), Box> { .publish_subscribe::<[u8]>() .open_or_create()?; - let worst_case_memory_size = 1024; + let maximum_elements = 1024; let publisher = service .publisher_builder() - .max_slice_len(worst_case_memory_size) + .max_slice_len(maximum_elements) .create()?; - let mut counter = 1; + let mut counter = 0; while node.wait(CYCLE_TIME).is_ok() { - counter += 1; - - let required_memory_size = (8 + counter) % 16; + let required_memory_size = (counter % 16) + 1; let sample = publisher.loan_slice_uninit(required_memory_size)?; let sample = sample.write_from_fn(|byte_idx| ((byte_idx + counter) % 255) as u8); sample.send()?; - println!("Send sample {} ...", counter); + println!( + "Send sample {} with {} bytes...", + counter, required_memory_size + ); + + counter += 1; } println!("exit");