ScatteredBuffer::IFragmentsObserver

.clang-tidy

@@ -12,6 +12,7 @@ Checks: >-
   performance-*,
   portability-*,
   readability-*,
+  -boost-use-ranges,
   -clang-analyzer-core.uninitialized.Assign,
   -cppcoreguidelines-avoid-const-or-ref-data-members,
   -cppcoreguidelines-use-default-member-init,
@@ -22,7 +23,9 @@ Checks: >-
   -modernize-type-traits,
   -modernize-use-constraints,
   -modernize-use-default-member-init,
+  -modernize-use-designated-initializers,
   -modernize-use-nodiscard,
+  -modernize-use-ranges,
   -readability-avoid-const-params-in-decls,
   -readability-identifier-length,
   -*-use-trailing-return-type,

.devcontainer/devcontainer.json

@@ -1,6 +1,6 @@
 {
     "name": "linux development environment for libcyphal",
-    "image": "ghcr.io/opencyphal/toolshed:ts22.4.10",
+    "image": "ghcr.io/opencyphal/toolshed:ts24.4.3",
     "customizations": {
         "vscode": {
             "extensions": [

.github/workflows/sonar.yml

@@ -15,14 +15,14 @@ jobs:
       contains(github.ref, '/issue/') ||
       (github.event_name == 'pull_request')
     runs-on: ubuntu-latest
-    container: ghcr.io/opencyphal/toolshed:ts22.4.10
+    container: ghcr.io/opencyphal/toolshed:ts24.4.3
     steps:
       - uses: actions/checkout@v4
         with:
           fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis
       - name: get nunavut
-        run: >
-          pip install git+https://github.com/OpenCyphal/[email protected]
+        run: |
+          pip install --break-system-packages git+https://github.com/OpenCyphal/[email protected]
       - name: Install sonar-scanner and build-wrapper
         uses: SonarSource/sonarcloud-github-c-cpp@v2
       - name: Run tests

.github/workflows/tests.yml

@@ -16,7 +16,7 @@ jobs:
       contains(github.ref, '/issue/') ||
       (github.event_name == 'pull_request')
     runs-on: ubuntu-latest
-    container: ghcr.io/opencyphal/toolshed:ts22.4.10
+    container: ghcr.io/opencyphal/toolshed:ts24.4.3
     steps:
       - uses: actions/checkout@v4
       - name: Cache ext modules
@@ -29,8 +29,8 @@ jobs:
           key: ${{ runner.os }}-${{ hashFiles('cmake/modules/*.cmake') }}
       - name: get nunavut
         # TODO: setup a venv, cache, and distribute to the other jobs.
-        run: >
-          pip install git+https://github.com/OpenCyphal/[email protected]
+        run: |
+          pip install --break-system-packages git+https://github.com/OpenCyphal/[email protected]
       - name: configure
         run: >
           ./build-tools/bin/verify.py
@@ -47,7 +47,7 @@ jobs:
       contains(github.ref, '/issue/') ||
       (github.event_name == 'pull_request')
     runs-on: ubuntu-latest
-    container: ghcr.io/opencyphal/toolshed:ts22.4.10
+    container: ghcr.io/opencyphal/toolshed:ts24.4.3
     needs: [warmup]
     strategy:
       matrix:
@@ -69,8 +69,8 @@ jobs:
         path: external
         key: ${{ runner.os }}-${{ hashFiles('cmake/modules/*.cmake') }}
     - name: get nunavut
-      run: >
-        pip install git+https://github.com/OpenCyphal/[email protected]
+      run: |
+        pip install --break-system-packages git+https://github.com/OpenCyphal/[email protected]
     - name: run tests
       env:
         GTEST_COLOR: yes
@@ -101,7 +101,7 @@ jobs:
       contains(github.ref, '/issue/') ||
       (github.event_name == 'pull_request')
     runs-on: ubuntu-latest
-    container: ghcr.io/opencyphal/toolshed:ts22.4.10
+    container: ghcr.io/opencyphal/toolshed:ts24.4.3
     needs: [warmup]
     steps:
     - uses: actions/checkout@v4
@@ -114,8 +114,8 @@ jobs:
         path: external
         key: ${{ runner.os }}-${{ hashFiles('cmake/modules/*.cmake') }}
     - name: get nunavut
-      run: >
-        pip install git+https://github.com/OpenCyphal/[email protected]
+      run: |
+        pip install --break-system-packages git+https://github.com/OpenCyphal/[email protected]
     - name: doc-gen
       run: >
         ./build-tools/bin/verify.py

CONTRIBUTING.md

@@ -147,10 +147,10 @@ and manually run it.
 
 ### TLDR
 ```
-docker pull ghcr.io/opencyphal/toolshed:ts22.4.10
+docker pull ghcr.io/opencyphal/toolshed:ts24.4.3
 git clone {this repo}
 cd {this repo}
-docker run --rm -it -v ${PWD}:/repo ghcr.io/opencyphal/toolshed:ts22.4.10
+docker run --rm -it -v ${PWD}:/repo ghcr.io/opencyphal/toolshed:ts24.4.3
 mkdir build
 cd build
 cmake ..
@@ -200,7 +200,7 @@ To ensure that the formatting matches the expectations of the CI suite,
 invoke Clang-Format of the correct version from the container (be sure to use the correct image tag):
 
 ```
-docker run --rm -v ${PWD}:/repo ghcr.io/opencyphal/toolshed:ts22.4.10 ./build-tools/bin/verify.py build-danger-danger-repo-clang-format-in-place
+docker run --rm -v ${PWD}:/repo ghcr.io/opencyphal/toolshed:ts24.4.3 ./build-tools/bin/verify.py build-danger-danger-repo-clang-format-in-place
 ```
 
 ### `issue/*` and hashtag-based CI triggering

docs/examples/1_presentation/example_1_presentation_0_hello_raw_pubsub_udp.cpp

@@ -151,8 +151,8 @@ TEST_F(Example_1_Presentation_0_HelloRawPubSub_Udp, main)
         const TimePoint                  msg_deadline = arg.approx_now + 1s;
         constexpr cetl::span<const char> message{"Hello, World!"};
         //
-        const std::array<const cetl::span<const cetl::byte>, 1> payload_fragments{
-            cetl::span<const cetl::byte>{reinterpret_cast<const cetl::byte*>(message.data()),  // NOLINT
+        const std::array<const PayloadFragment, 1> payload_fragments{
+            PayloadFragment{reinterpret_cast<const cetl::byte*>(message.data()),  // NOLINT
                                          message.size()}};
         EXPECT_THAT(raw_publisher.publish(msg_deadline, payload_fragments), testing::Eq(cetl::nullopt));
     });

docs/examples/1_presentation/example_1_presentation_2_heartbeat_getinfo_udp.cpp

@@ -176,7 +176,7 @@ TEST_F(Example_1_Presentation_2_Heartbeat_GetInfo_Udp, main)
 
     // Bring up 'GetInfo' server.
     //
-    using GetInfo_1_0 = uavcan::node::GetInfo_1_0;
+    using uavcan::node::GetInfo_1_0;
     const std::string node_name{"org.opencyphal.Ex_1_Pres_2_HB_GetInfo_UDP"};
     std::copy_n(node_name.begin(), std::min(node_name.size(), 50UL), std::back_inserter(state.get_info_response.name));
     //

docs/examples/1_presentation/example_1_presentation_3_hb_getinfo_ping_linux_can.cpp

@@ -424,8 +424,8 @@ TEST_F(Example_1_Presentation_3_HB_GetInfo_Ping_Can, main)
 
     // 7. Bring up 'GetInfo' server.
     //
-    using GetInfo_1_0 = uavcan::node::GetInfo_1_0;
-    uavcan::node::GetInfo_1_0::Response get_info_response{{&mr_}};
+    using uavcan::node::GetInfo_1_0;
+    GetInfo_1_0::Response get_info_response{{&mr_}};
     get_info_response.protocol_version.major = 1;
     const std::string node_name{"org.opencyphal.Ex_1_Pres_3_HB_GetInfo_Ping_CAN"};
     std::copy_n(node_name.begin(), std::min(node_name.size(), 50UL), std::back_inserter(get_info_response.name));

docs/examples/platform/node_helpers.hpp

@@ -70,13 +70,15 @@ struct NodeHelpers
                                                                              const TxMetadata& metadata)
     {
         using traits = typename T::_traits_;
+        using libcyphal::transport::PayloadFragment;
+
         std::array<std::uint8_t, traits::SerializationBufferSizeBytes> buffer{};
 
         const auto data_size = serialize(value, buffer).value();
 
         // NOLINTNEXTLINE
-        const cetl::span<const cetl::byte> fragment{reinterpret_cast<cetl::byte*>(buffer.data()), data_size};
-        const std::array<const cetl::span<const cetl::byte>, 1> payload{fragment};
+        const PayloadFragment                      fragment{reinterpret_cast<cetl::byte*>(buffer.data()), data_size};
+        const std::array<const PayloadFragment, 1> payload{fragment};
 
         return tx_session.send(metadata, payload);
     }

include/libcyphal/common/cavl/cavl.hpp

@@ -81,12 +81,12 @@ class Node  // NOSONAR cpp:S1448
     using DerivedType = Derived;
 
     // Tree nodes cannot be copied for obvious reasons.
-    Node(const Node&)                    = delete;
+    Node(const Node&)                    = delete;  // NOLINT bugprone-crtp-constructor-accessibility
     auto operator=(const Node&) -> Node& = delete;
 
     // Tree nodes can be moved. We update the pointers in the adjacent nodes to keep the tree valid,
     // as well as root node pointer if needed (see `moveFrom`). This operation is constant time.
-    Node(Node&& other) noexcept
+    Node(Node&& other) noexcept  // NOLINT bugprone-crtp-constructor-accessibility
     {
         moveFrom(other);
     }
@@ -102,7 +102,7 @@ class Node  // NOSONAR cpp:S1448
     }
 
 protected:
-    Node()  = default;
+    Node()  = default;  // NOLINT bugprone-crtp-constructor-accessibility
     ~Node() = default;
 
     /// Accessors for advanced tree introspection. Not needed for typical usage.

include/libcyphal/presentation/client.hpp

@@ -92,11 +92,13 @@ class ClientBase
 
     ClientBase& operator=(ClientBase&& other) noexcept
     {
-        CETL_DEBUG_ASSERT(shared_client_ != nullptr, "Not supposed to move construct to already moved `this`.");
         CETL_DEBUG_ASSERT(other.shared_client_ != nullptr,
                           "Not supposed to move construct from already moved `other`.");
 
-        (void) shared_client_->release();
+        if (nullptr != shared_client_)
+        {
+            (void) shared_client_->release();
+        }
 
         shared_client_ = std::exchange(other.shared_client_, nullptr);
         priority_      = other.priority_;

include/libcyphal/presentation/common_helpers.hpp

@@ -100,8 +100,8 @@ static auto tryPerformOnSerialized(const Message&                    message,
         return result_size.error();
     }
 
-    const cetl::span<const cetl::byte>                      data_span{buffer.data(), result_size.value()};
-    const std::array<const cetl::span<const cetl::byte>, 1> fragments{data_span};
+    const transport::PayloadFragment                      data_span{buffer.data(), result_size.value()};
+    const std::array<const transport::PayloadFragment, 1> fragments{data_span};
 
     return std::forward<Action>(action)(fragments);
 }
@@ -135,8 +135,8 @@ static auto tryPerformOnSerialized(const Message&              message,
         return result_size.error();
     }
 
-    const cetl::span<const cetl::byte>                      data_span{buffer.get(), result_size.value()};
-    const std::array<const cetl::span<const cetl::byte>, 1> fragments{data_span};
+    const transport::PayloadFragment                      data_span{buffer.get(), result_size.value()};
+    const std::array<const transport::PayloadFragment, 1> fragments{data_span};
 
     return std::forward<Action>(action)(fragments);
 }

include/libcyphal/presentation/publisher.hpp

@@ -78,10 +78,12 @@ class PublisherBase
 
     PublisherBase& operator=(PublisherBase&& other) noexcept
     {
-        CETL_DEBUG_ASSERT(impl_ != nullptr, "Not supposed to move to already moved `this`.");
         CETL_DEBUG_ASSERT(other.impl_ != nullptr, "Not supposed to move from already moved `other`.");
 
-        (void) impl_->release();
+        if (nullptr != impl_)
+        {
+            (void) impl_->release();
+        }
 
         impl_     = std::exchange(other.impl_, nullptr);
         priority_ = other.priority_;

include/libcyphal/presentation/subscriber.hpp

@@ -50,9 +50,12 @@ class SubscriberBase : public SubscriberImpl::CallbackNode
 
     SubscriberBase& operator=(SubscriberBase&& other) noexcept
     {
-        CETL_DEBUG_ASSERT(impl_ != nullptr, "Not supposed to move to already moved `this`.");
+        CETL_DEBUG_ASSERT(other.impl_ != nullptr, "Not supposed to move from already moved `other`.");
 
-        impl_->releaseCallbackNode(*this);
+        if (nullptr != impl_)
+        {
+            impl_->releaseCallbackNode(*this);
+        }
 
         impl_ = std::exchange(other.impl_, nullptr);
 

include/libcyphal/transport/can/can_transport_impl.hpp

@@ -109,7 +109,7 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
     private:
         CETL_NODISCARD static CanardMemoryResource makeTxMemoryResource(IMedia& media_interface)
         {
-            using LizardHelpers = libcyphal::transport::detail::LizardHelpers;
+            using transport::detail::LizardHelpers;
 
             // TX memory resource is used for raw bytes block allocations only.
             // So it has no alignment requirements.
@@ -147,10 +147,8 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
             return ArgumentError{};
         }
 
-        // False positive of clang-tidy - we move `media_array` to the `transport` instance, so can't make it const.
-        // NOLINTNEXTLINE(misc-const-correctness)
-        MediaArray media_array = makeMediaArray(memory, media_count, media, tx_capacity);
-        if (media_array.size() != media_count)
+        MediaArray media_array{media_count, &memory};
+        if (!makeMediaArray(media_array, media_count, media, tx_capacity))
         {
             return MemoryError{};
         }
@@ -514,18 +512,23 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
         return tryHandleTransientFailure<Report>(std::move(*failure), media.index(), canardInstance());
     }
 
-    CETL_NODISCARD static MediaArray makeMediaArray(cetl::pmr::memory_resource& memory,
-                                                    const std::size_t           media_count,
-                                                    const cetl::span<IMedia*>   media_interfaces,
-                                                    const std::size_t           tx_capacity)
+    CETL_NODISCARD static bool makeMediaArray(MediaArray&               media_array,
+                                              const std::size_t         media_count,
+                                              const cetl::span<IMedia*> media_interfaces,
+                                              const std::size_t         tx_capacity)
     {
-        MediaArray media_array{media_count, &memory};
-
-        // Reserve the space for the whole array (to avoid reallocations).
-        // Capacity will be less than requested in case of out of memory.
-        media_array.reserve(media_count);
-        if (media_array.capacity() >= media_count)
+#if defined(__cpp_exceptions)
+        try
         {
+#endif
+            // Reserve the space for the whole array (to avoid reallocations).
+            // Capacity will be less than requested in case of out of memory.
+            media_array.reserve(media_count);
+            if (media_array.capacity() < media_count)
+            {
+                return false;
+            }
+
             std::size_t index = 0;
             for (IMedia* const media_interface : media_interfaces)
             {
@@ -538,9 +541,14 @@ class TransportImpl final : private TransportDelegate, public ICanTransport
             }
             CETL_DEBUG_ASSERT(index == media_count, "");
             CETL_DEBUG_ASSERT(media_array.size() == media_count, "");
-        }
+            return true;
 
-        return media_array;
+#if defined(__cpp_exceptions)
+        } catch (const std::bad_alloc&)
+        {
+            return false;
+        }
+#endif
     }
 
     static void flushCanardTxQueue(CanardTxQueue& canard_tx_queue, const CanardInstance& canard_instance)

include/libcyphal/transport/can/delegate.hpp

@@ -101,6 +101,14 @@ class CanardMemory final : public ScatteredBuffer::IStorage
         return bytes_to_copy;
     }
 
+    void forEachFragment(ScatteredBuffer::IFragmentsVisitor& visitor) const override
+    {
+        if ((buffer_ != nullptr) && (payload_size_ > 0))
+        {
+            visitor.onNext({buffer_, payload_size_});
+        }
+    }
+
 private:
     // MARK: Data members:
 
@@ -471,13 +479,23 @@ class TransportDelegate
         // Now we know that we have at least one active port,
         // so we need preallocate temp memory for the total number of active ports.
         //
-        filters.reserve(total_active_ports);
-        if (filters.capacity() < total_active_ports)
+#if defined(__cpp_exceptions)
+        try
+        {
+#endif
+            filters.reserve(total_active_ports);
+            if (filters.capacity() < total_active_ports)
+            {
+                // This is out of memory situation.
+                return false;
+            }
+
+#if defined(__cpp_exceptions)
+        } catch (const std::bad_alloc&)
         {
-            // This is out of memory situation.
             return false;
         }
-
+#endif
         // `ports_count` counting is just for the sake of debug verification.
         std::size_t ports_count = 0;