diff --git a/include/libcyphal/application/node/heartbeat_producer.hpp b/include/libcyphal/application/node/heartbeat_producer.hpp index 95c36d182..60a71bff9 100644 --- a/include/libcyphal/application/node/heartbeat_producer.hpp +++ b/include/libcyphal/application/node/heartbeat_producer.hpp @@ -169,7 +169,7 @@ class HeartbeatProducer final // NOSONAR cpp:S3624 void publishMessage(const TimePoint approx_now) { // Publishing of heartbeats makes sense only if the local node ID is known. - if (presentation_.transport().getLocalNodeId() == cetl::nullopt) + if (!presentation_.transport().getLocalNodeId().has_value()) { return; } diff --git a/include/libcyphal/presentation/client.hpp b/include/libcyphal/presentation/client.hpp index 536571a9c..57617fdbb 100644 --- a/include/libcyphal/presentation/client.hpp +++ b/include/libcyphal/presentation/client.hpp @@ -237,8 +237,8 @@ class Client final : public detail::ClientBase // For request (and the following response) we need to allocate a transfer ID, // which will be in use to pair the request with the response. // - auto& shared_client = getSharedClient(); - auto opt_transfer_id = shared_client.nextTransferId(); + auto& shared_client = getSharedClient(); + const auto opt_transfer_id = shared_client.nextTransferId(); if (!opt_transfer_id) { return TooManyPendingRequestsError{}; @@ -325,8 +325,8 @@ class RawServiceClient final : public detail::ClientBase // 1. For request (and following response) we need to allocate a transfer ID, // which will be in use to pair the request with the response. // - auto& shared_client = getSharedClient(); - auto opt_transfer_id = shared_client.nextTransferId(); + auto& shared_client = getSharedClient(); + const auto opt_transfer_id = shared_client.nextTransferId(); if (!opt_transfer_id) { return TooManyPendingRequestsError{}; diff --git a/include/libcyphal/presentation/client_impl.hpp b/include/libcyphal/presentation/client_impl.hpp index b2f557852..426811927 100644 --- a/include/libcyphal/presentation/client_impl.hpp +++ b/include/libcyphal/presentation/client_impl.hpp @@ -13,7 +13,7 @@ #include "libcyphal/executor.hpp" #include "libcyphal/transport/errors.hpp" #include "libcyphal/transport/svc_sessions.hpp" -#include "libcyphal/transport/transfer_id_generators.hpp" +#include "libcyphal/transport/transfer_id_map.hpp" #include "libcyphal/transport/types.hpp" #include "libcyphal/types.hpp" @@ -34,7 +34,9 @@ namespace presentation namespace detail { -class SharedClient : public common::cavl::Node, public SharedObject +class SharedClient : public common::cavl::Node, + public SharedObject, + protected transport::detail::ITransferIdStorage { public: using Node::remove; @@ -134,17 +136,24 @@ class SharedClient : public common::cavl::Node, public SharedObjec , svc_request_tx_session_{std::move(svc_request_tx_session)} , svc_response_rx_session_{std::move(svc_response_rx_session)} , response_rx_params_{svc_response_rx_session_->getParams()} + , next_transfer_id_{0} , nearest_deadline_{DistantFuture()} { CETL_DEBUG_ASSERT(svc_request_tx_session_ != nullptr, ""); CETL_DEBUG_ASSERT(svc_response_rx_session_ != nullptr, ""); + if (const auto* const transfer_id_map = delegate.getTransferIdMap()) + { + const SessionSpec session_spec{response_rx_params_.service_id, response_rx_params_.server_node_id}; + next_transfer_id_ = transfer_id_map->getIdFor(session_spec); + } + // Override the default (2s) timeout value of the response session. // This is done to allow multiple overlapping responses to be handled properly. // Otherwise, the responses would be rejected (as "duplicates") if their transfer IDs are in order. // Real duplicates (f.e. caused by redundant transports) won't cause any issues // b/c shared RPC client expects/accepts only one response per transfer ID, - // and corresponding promise callback node will be removed after the first response. + // and the corresponding promise callback node will be removed after the first response. svc_response_rx_session_->setTransferIdTimeout({}); svc_response_rx_session_->setOnReceiveCallback([this](const auto& arg) { @@ -199,7 +208,7 @@ class SharedClient : public common::cavl::Node, public SharedObjec { if (timeout_node.isTimeoutLinked()) { - // Remove previous timeout node (if any), + // Remove the previous timeout node (if any), // and then reinsert the node with updated/given new deadline time. // timeout_nodes_by_deadline_.remove(&timeout_node); @@ -240,9 +249,27 @@ class SharedClient : public common::cavl::Node, public SharedObjec void destroy() noexcept override { + if (auto* const transfer_id_map = delegate_.getTransferIdMap()) + { + const SessionSpec session_spec{response_rx_params_.service_id, response_rx_params_.server_node_id}; + transfer_id_map->setIdFor(session_spec, next_transfer_id_); + } + delegate_.forgetSharedClient(*this); } + // MARK: ITransferIdStorage + + transport::TransferId load() const noexcept override + { + return next_transfer_id_; + } + + void save(const transport::TransferId transfer_id) noexcept override + { + next_transfer_id_ = transfer_id; + } + protected: virtual void insertNewCallbackNode(CallbackNode& callback_node) { @@ -273,7 +300,8 @@ class SharedClient : public common::cavl::Node, public SharedObjec } private: - using Schedule = IExecutor::Callback::Schedule; + using Schedule = IExecutor::Callback::Schedule; + using SessionSpec = transport::ITransferIdMap::SessionSpec; static constexpr TimePoint DistantFuture() { @@ -388,6 +416,7 @@ class SharedClient : public common::cavl::Node, public SharedObjec const UniquePtr svc_request_tx_session_; const UniquePtr svc_response_rx_session_; const transport::ResponseRxParams response_rx_params_; + transport::TransferId next_transfer_id_; common::cavl::Tree cb_nodes_by_transfer_id_; TimePoint nearest_deadline_; common::cavl::Tree timeout_nodes_by_deadline_; @@ -399,8 +428,8 @@ class SharedClient : public common::cavl::Node, public SharedObjec /// @brief Defines a shared client implementation that uses a generic transfer ID generator. /// -template -class ClientImpl final : public SharedClient, private TransferIdGeneratorMixin +template +class ClientImpl final : public SharedClient { public: ClientImpl(IPresentationDelegate& delegate, @@ -409,7 +438,7 @@ class ClientImpl final : public SharedClient, private TransferIdGeneratorMixin UniquePtr svc_response_rx_session, const transport::TransferId transfer_id_modulo) : SharedClient{delegate, executor, std::move(svc_request_tx_session), std::move(svc_response_rx_session)} - , TransferIdGeneratorMixin{transfer_id_modulo} + , transfer_id_generator_{transfer_id_modulo, *this} { } @@ -424,33 +453,33 @@ class ClientImpl final : public SharedClient, private TransferIdGeneratorMixin private: using Base = SharedClient; - // MARK: SharedClient - - CETL_NODISCARD cetl::optional nextTransferId() noexcept override - { - return TransferIdGeneratorMixin::nextTransferId(); - } - void insertNewCallbackNode(CallbackNode& callback_node) override { SharedClient::insertNewCallbackNode(callback_node); - TransferIdGeneratorMixin::retainTransferId(callback_node.getTransferId()); + transfer_id_generator_.retainTransferId(callback_node.getTransferId()); } void removeCallbackNode(CallbackNode& callback_node) override { - TransferIdGeneratorMixin::releaseTransferId(callback_node.getTransferId()); + transfer_id_generator_.releaseTransferId(callback_node.getTransferId()); SharedClient::removeCallbackNode(callback_node); } + // MARK: SharedClient + + CETL_NODISCARD cetl::optional nextTransferId() noexcept override + { + return transfer_id_generator_.nextTransferId(); + } + + TransferIdGenerator transfer_id_generator_; + }; // ClientImpl /// @brief Defines a shared client specialization that uses a trivial transfer ID generator. /// template <> -class ClientImpl final - : public SharedClient, - private transport::detail::TrivialTransferIdGenerator +class ClientImpl final : public SharedClient { public: ClientImpl(IPresentationDelegate& delegate, @@ -458,6 +487,7 @@ class ClientImpl final UniquePtr svc_request_tx_session, UniquePtr svc_response_rx_session) : Base{delegate, executor, std::move(svc_request_tx_session), std::move(svc_response_rx_session)} + , transfer_id_generator_{*this} { } @@ -476,9 +506,11 @@ class ClientImpl final CETL_NODISCARD cetl::optional nextTransferId() noexcept override { - return TrivialTransferIdGenerator::nextTransferId(); + return transfer_id_generator_.nextTransferId(); } + transport::detail::TrivialTransferIdGenerator transfer_id_generator_; + }; // ClientImpl } // namespace detail diff --git a/include/libcyphal/presentation/presentation.hpp b/include/libcyphal/presentation/presentation.hpp index 464d84cd0..e6f1f66ba 100644 --- a/include/libcyphal/presentation/presentation.hpp +++ b/include/libcyphal/presentation/presentation.hpp @@ -23,7 +23,7 @@ #include "libcyphal/transport/errors.hpp" #include "libcyphal/transport/msg_sessions.hpp" #include "libcyphal/transport/svc_sessions.hpp" -#include "libcyphal/transport/transfer_id_generators.hpp" +#include "libcyphal/transport/transfer_id_map.hpp" #include "libcyphal/transport/transport.hpp" #include "libcyphal/transport/types.hpp" #include "libcyphal/types.hpp" @@ -100,7 +100,7 @@ struct IsFixedPortIdMessageTrait class Presentation final : private detail::IPresentationDelegate { public: - /// @brief Defines failure type of various `make...` methods of the presentation layer. + /// @brief Defines a failure type for various `make...` methods of the presentation layer. /// /// The set of possible make errors includes transport layer failures. /// @@ -112,6 +112,7 @@ class Presentation final : private detail::IPresentationDelegate : memory_{memory} , executor_{executor} , transport_{transport} + , transfer_id_map_{nullptr} , unreferenced_nodes_{&unreferenced_nodes_, &unreferenced_nodes_} { unref_nodes_deleter_callback_ = executor_.registerCallback([this](const auto&) { @@ -194,6 +195,8 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a typed publisher bound to its fixed subject id. /// + /// The publisher must never outlive this presentation object. + /// /// @tparam Message The message type generated by DSDL tool. The type expected to have a fixed port ID. /// template , MakeFailure>> @@ -244,6 +247,8 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a typed subscriber bound to its fixed subject id. /// + /// The subscriber must never outlive this presentation object. + /// /// @tparam Message The message type generated by DSDL tool. The type expected to have a fixed port ID. /// @param on_receive_cb_fn Optional callback function to be called when a message is received. /// Can be assigned (or reset) later via `Subscriber::setOnReceiveCallback`. @@ -296,6 +301,8 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a custom typed RPC server bound to a specific service id. /// + /// The server must never outlive this presentation object. + /// /// @tparam Request The request type of the server. See `Server` for more details. /// @tparam Response The response type of the server. See `Server<..., Response>` for more details. /// @param service_id The service ID of the server. @@ -325,6 +332,8 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a service typed RPC server bound to a specific service id. /// + /// The server must never outlive this presentation object. + /// /// @tparam Service The service type generated by DSDL tool. See `ServiceServer` for more details. /// @param service_id The service ID of the server. /// @param on_request_cb_fn Optional callback function to be called when a request is received. @@ -341,6 +350,8 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a typed RPC server bound to its fixed service id. /// + /// The server must never outlive this presentation object. + /// /// @tparam Service The service type generated by DSDL tool. The type expected to have a fixed port ID. /// @param on_request_cb_fn Optional callback function to be called when a request is received. /// Can be assigned (or reset) later via `Server::setOnRequestCallback`. @@ -354,6 +365,8 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a raw (aka untyped) RPC server bound to a specific service id. /// + /// The server must never outlive this presentation object. + /// /// @param service_id The service ID of the server. /// @param extent_bytes Defines the size of the transfer payload memory buffer; /// or, in other words, the maximum possible size of received objects, @@ -384,7 +397,9 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a custom typed RPC client bound to a specific server node and service ids. /// - /// Notice that a client is bound to a specific remote server node. To query multiple servers one has to create + /// The client must never outlive this presentation object. + /// + /// Notice that a client is bound to a specific remote server node. To query multiple servers, one has to create /// multiple clients. It's also possible to create multiple clients bound to the same server node and service id - /// it can be done either by making multiple `makeClient` calls or just by copying the client object. /// @@ -412,7 +427,9 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a service typed RPC client bound to a specific server node and service ids. /// - /// Notice that a client is bound to a specific remote server node. To query multiple servers one has to create + /// The client must never outlive this presentation object. + /// + /// Notice that a client is bound to a specific remote server node. To query multiple, servers one has to create /// multiple clients. It's also possible to create multiple clients bound to the same server node and service id - /// it can be done either by making multiple `makeClient` calls or just by copying the client object. /// @@ -431,7 +448,9 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a typed RPC server bound to its standard (aka fixed) service id. /// - /// Notice that a client is bound to a specific remote server node. To query multiple servers one has to create + /// The client must never outlive this presentation object. + /// + /// Notice that a client is bound to a specific remote server node. To query multiple, servers one has to create /// multiple clients. It's also possible to create multiple clients bound to the same server node and service id - /// it can be done either by making multiple `makeClient` calls or just by copying the client object. /// @@ -449,7 +468,9 @@ class Presentation final : private detail::IPresentationDelegate /// @brief Makes a raw (aka untyped) RPC client bound to a specific server node and service ids. /// - /// Notice that a client is bound to a specific remote server node. To query multiple servers one has to create + /// The client must never outlive this presentation object. + /// + /// Notice that a client is bound to a specific remote server node. To query multiple, servers one has to create /// multiple clients. It's also possible to create multiple clients bound to the same server node and service id - /// it can be done either by making multiple `makeClient` calls or just by copying the client object. /// @@ -476,6 +497,28 @@ class Presentation final : private detail::IPresentationDelegate return RawServiceClient{shared_client}; } + /// @brief Sets an external transfer ID map provided by the user. + /// + /// By default, this presentation layer object operates without a transfer ID map. + /// It means that continuous transfer IDs are not guaranteed across recreation of entities. + /// In this mode, every new transmission session will start with the default (zero) initial transfer ID. + /// Such default behavior is suitable for most applications, especially when an application + /// is completely initialized at the start, and does not create presentation entities dynamically. + /// + /// However, in some cases, it might be required to have continuous transfer IDs across recreation of entities. + /// For long-running applications (like a daemon), it might be enough to have a transfer ID map in memory only. + /// For short-running applications (like a CLI tool), it might be necessary to add a persistent storage. + /// + /// @param transfer_id_map The transfer ID map object to be used by the presentation layer. + /// Could be reset at any moment to other value (or `nullptr` to return default mode). + /// Already existing entities will automatically switch to the latest state. + /// While being set, it must outlive this presentation object. + /// + void setTransferIdMap(transport::ITransferIdMap* const transfer_id_map) noexcept + { + transfer_id_map_ = transfer_id_map; + } + // MARK: IPresentationDelegate cetl::pmr::memory_resource& memory() const noexcept override @@ -654,8 +697,7 @@ class Presentation final : private detail::IPresentationDelegate tf_id_modulo); } - using ClientImpl = detail::ClientImpl< // - transport::detail::TrivialTransferIdGenerator>; + using ClientImpl = detail::ClientImpl; return detail::SharedObject::createWithPmr(memory_, out_failure, @@ -696,6 +738,16 @@ class Presentation final : private detail::IPresentationDelegate // MARK: IPresentationDelegate + cetl::optional getLocalNodeId() const noexcept override + { + return transport_.getLocalNodeId(); + } + + transport::ITransferIdMap* getTransferIdMap() const noexcept override + { + return transfer_id_map_; + } + void markSharedObjAsUnreferenced(detail::SharedObject& shared_obj) noexcept override { // We are not going to destroy the shared object immediately, but schedule it for deletion. @@ -730,6 +782,7 @@ class Presentation final : private detail::IPresentationDelegate cetl::pmr::memory_resource& memory_; IExecutor& executor_; transport::ITransport& transport_; + transport::ITransferIdMap* transfer_id_map_; common::cavl::Tree shared_client_nodes_; common::cavl::Tree publisher_impl_nodes_; common::cavl::Tree subscriber_impl_nodes_; diff --git a/include/libcyphal/presentation/presentation_delegate.hpp b/include/libcyphal/presentation/presentation_delegate.hpp index 3c49d74b6..2f2429319 100644 --- a/include/libcyphal/presentation/presentation_delegate.hpp +++ b/include/libcyphal/presentation/presentation_delegate.hpp @@ -6,6 +6,7 @@ #ifndef LIBCYPHAL_PRESENTATION_DELEGATE_HPP_INCLUDED #define LIBCYPHAL_PRESENTATION_DELEGATE_HPP_INCLUDED +#include "libcyphal/transport/transfer_id_map.hpp" #include "shared_object.hpp" #include @@ -84,7 +85,9 @@ class IPresentationDelegate IPresentationDelegate& operator=(const IPresentationDelegate&) = delete; IPresentationDelegate& operator=(IPresentationDelegate&&) noexcept = delete; - virtual cetl::pmr::memory_resource& memory() const noexcept = 0; + virtual cetl::pmr::memory_resource& memory() const noexcept = 0; + virtual cetl::optional getLocalNodeId() const noexcept = 0; + virtual transport::ITransferIdMap* getTransferIdMap() const noexcept = 0; virtual void markSharedObjAsUnreferenced(SharedObject& shared_obj) noexcept = 0; virtual void forgetSharedClient(SharedClient& shared_client) noexcept = 0; diff --git a/include/libcyphal/presentation/publisher_impl.hpp b/include/libcyphal/presentation/publisher_impl.hpp index 953338bfd..00a9c2322 100644 --- a/include/libcyphal/presentation/publisher_impl.hpp +++ b/include/libcyphal/presentation/publisher_impl.hpp @@ -12,6 +12,7 @@ #include "libcyphal/common/cavl/cavl.hpp" #include "libcyphal/transport/errors.hpp" #include "libcyphal/transport/msg_sessions.hpp" +#include "libcyphal/transport/transfer_id_map.hpp" #include "libcyphal/transport/types.hpp" #include "libcyphal/types.hpp" @@ -32,7 +33,9 @@ namespace presentation namespace detail { -class PublisherImpl final : public common::cavl::Node, public SharedObject +class PublisherImpl final : public common::cavl::Node, + public SharedObject, + private transport::detail::ITransferIdStorage { public: using Node::remove; @@ -42,9 +45,19 @@ class PublisherImpl final : public common::cavl::Node, public Sha : delegate_{delegate} , msg_tx_session_{std::move(msg_tx_session)} , subject_id_{msg_tx_session_->getParams().subject_id} - , transfer_id_{0} + , next_transfer_id_{0} + , transfer_id_generator_{*this} { CETL_DEBUG_ASSERT(msg_tx_session_ != nullptr, ""); + + if (const auto* const transfer_id_map = delegate.getTransferIdMap()) + { + if (const auto local_node_id = delegate.getLocalNodeId()) + { + const SessionSpec session_spec{subject_id_, *local_node_id}; + next_transfer_id_ = transfer_id_map->getIdFor(session_spec); + } + } } CETL_NODISCARD cetl::pmr::memory_resource& memory() const noexcept @@ -59,11 +72,11 @@ class PublisherImpl final : public common::cavl::Node, public Sha cetl::optional publishRawData(const TimePoint deadline, const transport::Priority priority, - const transport::PayloadFragments payload_fragments) + const transport::PayloadFragments payload_fragments) const { - transfer_id_ += 1; - const transport::TransferTxMetadata metadata{{transfer_id_, priority}, deadline}; + const auto transfer_id = transfer_id_generator_.nextTransferId(); + const transport::TransferTxMetadata metadata{{transfer_id, priority}, deadline}; return msg_tx_session_->send(metadata, payload_fragments); } @@ -85,20 +98,44 @@ class PublisherImpl final : public common::cavl::Node, public Sha } private: + using SessionSpec = transport::ITransferIdMap::SessionSpec; + // MARK: SharedObject void destroy() noexcept override { + if (auto* const transfer_id_map = delegate_.getTransferIdMap()) + { + if (const auto local_node_id = delegate_.getLocalNodeId()) + { + const SessionSpec session_spec{subject_id_, *local_node_id}; + transfer_id_map->setIdFor(session_spec, next_transfer_id_); + } + } + delegate_.forgetPublisherImpl(*this); destroyWithPmr(this, delegate_.memory()); } + // MARK: ITransferIdStorage + + transport::TransferId load() const noexcept override + { + return next_transfer_id_; + } + + void save(const transport::TransferId transfer_id) noexcept override + { + next_transfer_id_ = transfer_id; + } + // MARK: Data members: IPresentationDelegate& delegate_; const UniquePtr msg_tx_session_; const transport::PortId subject_id_; - transport::TransferId transfer_id_; + transport::TransferId next_transfer_id_; + transport::detail::TrivialTransferIdGenerator transfer_id_generator_; }; // PublisherImpl diff --git a/include/libcyphal/transport/can/can_transport_impl.hpp b/include/libcyphal/transport/can/can_transport_impl.hpp index d4be277bd..3f422b1f4 100644 --- a/include/libcyphal/transport/can/can_transport_impl.hpp +++ b/include/libcyphal/transport/can/can_transport_impl.hpp @@ -564,12 +564,11 @@ class TransportImpl final : private TransportDelegate, public ICanTransport return; } const auto& pop_success = cetl::get(pop_result); - if (!pop_success.has_value()) + if (!pop_success) { return; } - - const IMedia::PopResult::Metadata& pop_meta = pop_success.value(); + const IMedia::PopResult::Metadata& pop_meta = *pop_success; const auto timestamp_us = std::chrono::duration_cast(pop_meta.timestamp.time_since_epoch()); diff --git a/include/libcyphal/transport/transfer_id_generators.hpp b/include/libcyphal/transport/transfer_id_generators.hpp deleted file mode 100644 index 91349bcfc..000000000 --- a/include/libcyphal/transport/transfer_id_generators.hpp +++ /dev/null @@ -1,134 +0,0 @@ -/// @copyright -/// Copyright (C) OpenCyphal Development Team -/// Copyright Amazon.com Inc. or its affiliates. -/// SPDX-License-Identifier: MIT - -#ifndef LIBCYPHAL_TRANSPORT_TRANSFER_ID_GENERATORS_HPP_INCLUDED -#define LIBCYPHAL_TRANSPORT_TRANSFER_ID_GENERATORS_HPP_INCLUDED - -#include "types.hpp" - -#include -#include - -#include -#include -#include - -namespace libcyphal -{ -namespace transport -{ - -/// Internal implementation details of the Presentation layer. -/// Not supposed to be used directly by the users of the library. -/// -namespace detail -{ - -/// @brief Defines a trivial transfer ID generator. -/// -/// The generator is trivial in the sense that it simply increments the transfer ID. -/// B/c modulo is expected to be quite big (like >= 2^48), collisions of transfer ids are unlikely. -/// Normally in use for UDP transport, where the modulo is `2^64 - 1`. -/// -class TrivialTransferIdGenerator -{ -public: - /// @brief Returns the next transfer ID. - /// - CETL_NODISCARD TransferId nextTransferId() noexcept - { - return std::exchange(next_transfer_id_, next_transfer_id_ + 1); - } - - /// @brief Sets next transfer ID. - /// - /// In use for testing purposes. - /// - void setNextTransferId(const TransferId transfer_id) noexcept - { - next_transfer_id_ = transfer_id; - } - -private: - // MARK: Data members: - - TransferId next_transfer_id_{0}; - -}; // TrivialTransferIdGenerator - -// MARK: - - -/// @brief Defines a small range transfer ID generator. -/// -/// The generator tracks allocated transfer ids by using bitset operations. -/// Its `Size` and modulo is expected to be quite small (like <= 2^8). -/// Normally in use for CAN transport, where the modulo is `2^5`. -/// -template -class SmallRangeTransferIdGenerator -{ - static_assert(Size > 0, "Size must be greater than 0."); - -public: - explicit SmallRangeTransferIdGenerator(const TransferId modulo) noexcept - : modulo_{modulo} - , next_transfer_id_{0} - , in_use_transfer_ids_{} - { - CETL_DEBUG_ASSERT(modulo > 0, "Transfer ID modulo must be greater than 0."); - CETL_DEBUG_ASSERT(modulo <= Size, "Transfer ID modulo must be <= than `Size`."); - } - - /// @brief Returns the next available (not in use) transfer ID. - /// - /// The worst-case complexity is linear of the number of pending requests. - /// - CETL_NODISCARD cetl::optional nextTransferId() noexcept - { - const auto end = next_transfer_id_; - while (in_use_transfer_ids_.test(next_transfer_id_)) - { - next_transfer_id_ = (next_transfer_id_ + 1) % modulo_; - if (next_transfer_id_ == end) - { - return cetl::nullopt; - } - } - - return std::exchange(next_transfer_id_, (next_transfer_id_ + 1) % modulo_); - } - - /// @brief Marks given transfer ID as in use. - /// - /// Such retained transfer IDs will be skipped by the `nextTransferId` method. - /// - void retainTransferId(const TransferId transfer_id) noexcept - { - CETL_DEBUG_ASSERT(transfer_id < modulo_, "Valid Transfer ID must be less than modulo."); - in_use_transfer_ids_[transfer_id] = true; - } - - /// @brief Marks given transfer ID as not in use anymore. - /// - void releaseTransferId(const TransferId transfer_id) noexcept - { - CETL_DEBUG_ASSERT(transfer_id < modulo_, "Valid Transfer ID must be less than modulo."); - in_use_transfer_ids_[transfer_id] = false; - } - -private: - // MARK: Data members: - - const TransferId modulo_; - TransferId next_transfer_id_; - std::bitset in_use_transfer_ids_; - -}; // SmallRangeTransferIdGenerator - -} // namespace detail -} // namespace transport -} // namespace libcyphal - -#endif // LIBCYPHAL_TRANSPORT_TRANSFER_ID_GENERATORS_HPP_INCLUDED diff --git a/include/libcyphal/transport/transfer_id_map.hpp b/include/libcyphal/transport/transfer_id_map.hpp new file mode 100644 index 000000000..5906b9200 --- /dev/null +++ b/include/libcyphal/transport/transfer_id_map.hpp @@ -0,0 +1,228 @@ +/// @copyright +/// Copyright (C) OpenCyphal Development Team +/// Copyright Amazon.com Inc. or its affiliates. +/// SPDX-License-Identifier: MIT + +#ifndef LIBCYPHAL_TRANSPORT_TRANSFER_ID_GENERATORS_HPP_INCLUDED +#define LIBCYPHAL_TRANSPORT_TRANSFER_ID_GENERATORS_HPP_INCLUDED + +#include "types.hpp" + +#include +#include + +#include +#include + +namespace libcyphal +{ +namespace transport +{ + +/// @brief Defines an abstract interface of a transport id map. +/// +/// Presentation layer uses this interface to map session specifiers to their transfer ids. +/// Users may provide a custom implementation of this interface to maintain/persist transfer IDs. +/// See `Presentation::setTransferIdMap` method for more details. +/// +class ITransferIdMap +{ +public: + /// Hashable specifier of a session. + /// + struct SessionSpec + { + const PortId port_id; + const NodeId node_id; + + friend bool operator==(const SessionSpec& lhs, const SessionSpec& rhs) noexcept + { + return (lhs.port_id == rhs.port_id) && (lhs.node_id == rhs.node_id); + } + + }; // SessionSpec + + ITransferIdMap(const ITransferIdMap&) = delete; + ITransferIdMap(ITransferIdMap&&) noexcept = delete; + ITransferIdMap& operator=(const ITransferIdMap&) = delete; + ITransferIdMap& operator=(ITransferIdMap&&) noexcept = delete; + + /// Gets the transfer ID for the given session specifier. + /// + /// An implementation is expected to be fast (at least O(log), better O(1)) and non-blocking. + /// + /// @param session_spec The unique session specifier. + /// @return The transfer ID which was last set (by the `setIdFor`). + /// Or some default value (zero) if not set yet. + /// + virtual TransferId getIdFor(const SessionSpec& session_spec) const noexcept = 0; + + /// Sets the transfer ID for the given session specifier. + /// + /// An implementation is expected to be fast (at least O(log), better O(1)) and non-blocking. + /// Depending on the implementation, the previously set transfer ids may be stored in memory + /// or also persisted to a file (but probably on exit to fulfill the above performance expectations). + /// + virtual void setIdFor(const SessionSpec& session_spec, const TransferId transfer_id) noexcept = 0; + +protected: + ITransferIdMap() = default; + ~ITransferIdMap() = default; + +}; // ITransferIdMap + +/// Internal implementation details of the Transport layer. +/// Not supposed to be used directly by the users of the library. +/// +namespace detail +{ + +/// @brief Defines an abstract storage interface of a transport id. +/// +class ITransferIdStorage +{ +public: + ITransferIdStorage(const ITransferIdStorage&) = delete; + ITransferIdStorage(ITransferIdStorage&&) noexcept = delete; + ITransferIdStorage& operator=(const ITransferIdStorage&) = delete; + ITransferIdStorage& operator=(ITransferIdStorage&&) noexcept = delete; + + /// Loads last saved transfer ID. + /// + /// An implementation is expected to be fast (at least O(log), better O(1)) and non-blocking. + /// + /// @return The transfer ID which was last saved (by the `save` method). + /// Or some default value (zero) if not set yet. + /// + virtual TransferId load() const noexcept = 0; + + /// Saves the transfer ID. + /// + /// An implementation is expected to be fast (at least O(log), better O(1)) and non-blocking. + /// + virtual void save(const TransferId transfer_id) noexcept = 0; + +protected: + ITransferIdStorage() = default; + ~ITransferIdStorage() = default; + +}; // ITransferIdStorage + +/// @brief Defines a trivial transfer ID generator. +/// +/// The generator is trivial in the sense that it simply increments the transfer ID. +/// B/c modulo is expected to be quite big (like >= 2^48), collisions of transfer ids are unlikely. +/// Normally in use for UDP transport, where the modulo is `2^64 - 1`. +/// +class TrivialTransferIdGenerator +{ +public: + explicit TrivialTransferIdGenerator(ITransferIdStorage& transfer_id_storage) noexcept + : transfer_id_storage_{transfer_id_storage} + { + } + + /// @brief Generates the next transfer ID for an output session. + /// + CETL_NODISCARD TransferId nextTransferId() const noexcept + { + const auto curr_transfer_id = transfer_id_storage_.load(); + const auto next_transfer_id = curr_transfer_id + 1; + transfer_id_storage_.save(next_transfer_id); + return curr_transfer_id; + } + +private: + // MARK: Data members: + + ITransferIdStorage& transfer_id_storage_; + +}; // TrivialTransferIdGenerator + +// MARK: - + +/// @brief Defines a small range transfer ID generator. +/// +/// The generator tracks allocated transfer ids by using bitset operations. +/// Its `Size` and modulo is expected to be quite small (like <= 2^8). +/// Normally in use for CAN transport, where the modulo is `2^5`. +/// +template +class SmallRangeTransferIdGenerator +{ + static_assert(Size > 0, "Size must be greater than 0."); + +public: + SmallRangeTransferIdGenerator(const TransferId modulo, ITransferIdStorage& transfer_id_storage) noexcept + : modulo_{modulo} + , transfer_id_storage_{transfer_id_storage} + , in_use_transfer_ids_{} + { + CETL_DEBUG_ASSERT(modulo > 0, "Transfer ID modulo must be greater than 0."); + CETL_DEBUG_ASSERT(modulo <= Size, "Transfer ID modulo must be <= than `Size`."); + } + + /// @brief Generates the next available (not in use) transfer ID for an output session. + /// + /// The worst-case complexity is linear of the number of pending requests. + /// + CETL_NODISCARD cetl::optional nextTransferId() noexcept + { + auto curr_transfer_id = transfer_id_storage_.load() % modulo_; + const auto final_transfer_id = curr_transfer_id; + while (in_use_transfer_ids_.test(curr_transfer_id)) + { + curr_transfer_id = (curr_transfer_id + 1) % modulo_; + if (curr_transfer_id == final_transfer_id) + { + return cetl::nullopt; + } + } + const auto next_transfer_id = (curr_transfer_id + 1) % modulo_; + transfer_id_storage_.save(next_transfer_id); + return curr_transfer_id; + } + + /// @brief Marks given transfer ID as in use. + /// + /// Such retained transfer IDs will be skipped by the `nextTransferId` method. + /// + void retainTransferId(const TransferId transfer_id) noexcept + { + CETL_DEBUG_ASSERT(transfer_id < modulo_, "Valid Transfer ID must be less than modulo."); + in_use_transfer_ids_[transfer_id] = true; + } + + /// @brief Marks given transfer ID as not in use anymore. + /// + void releaseTransferId(const TransferId transfer_id) noexcept + { + CETL_DEBUG_ASSERT(transfer_id < modulo_, "Valid Transfer ID must be less than modulo."); + in_use_transfer_ids_[transfer_id] = false; + } + +private: + // MARK: Data members: + + const TransferId modulo_; + ITransferIdStorage& transfer_id_storage_; + std::bitset in_use_transfer_ids_; + +}; // SmallRangeTransferIdGenerator + +} // namespace detail +} // namespace transport +} // namespace libcyphal + +template <> +struct std::hash +{ + std::size_t operator()(const libcyphal::transport::ITransferIdMap::SessionSpec& spec) const noexcept + { + const std::size_t h1 = std::hash{}(spec.port_id); + const std::size_t h2 = std::hash{}(spec.node_id); + return h1 ^ (h2 << 1U); + } +}; + +#endif // LIBCYPHAL_TRANSPORT_TRANSFER_ID_GENERATORS_HPP_INCLUDED diff --git a/test/unittest/application/node/test_heartbeat_producer.cpp b/test/unittest/application/node/test_heartbeat_producer.cpp index ead10f4ea..5792f0584 100644 --- a/test/unittest/application/node/test_heartbeat_producer.cpp +++ b/test/unittest/application/node/test_heartbeat_producer.cpp @@ -126,7 +126,7 @@ TEST_F(TestHeartbeatProducer, make) }); scheduler_.scheduleAt(3s, [&](const auto&) { // - EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{1, Priority::Nominal}, now() + 1s}), _)) // + EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{0, Priority::Nominal}, now() + 1s}), _)) // .WillOnce(Return(cetl::nullopt)); }); scheduler_.scheduleAt(3s + 500ms, [&](const auto&) { @@ -143,12 +143,12 @@ TEST_F(TestHeartbeatProducer, make) }); scheduler_.scheduleAt(4s, [&](const auto&) { // - EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{2, Priority::Nominal}, now() + 1s}), _)) // + EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{1, Priority::Nominal}, now() + 1s}), _)) // .WillOnce(Return(cetl::nullopt)); }); scheduler_.scheduleAt(5s, [&](const auto&) { // - EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{3, Priority::Nominal}, now() + 1s}), _)) // + EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{2, Priority::Nominal}, now() + 1s}), _)) // .WillOnce(Return(cetl::nullopt)); }); scheduler_.scheduleAt(5s + 500ms, [&](const auto&) { @@ -161,7 +161,7 @@ TEST_F(TestHeartbeatProducer, make) }); scheduler_.scheduleAt(6s, [&](const auto&) { // - EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{4, Priority::Nominal}, now() + 1s}), _)) // + EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{3, Priority::Nominal}, now() + 1s}), _)) // .WillOnce(Return(cetl::nullopt)); }); scheduler_.scheduleAt(6s + 500ms, [&](const auto&) { @@ -173,12 +173,12 @@ TEST_F(TestHeartbeatProducer, make) }); scheduler_.scheduleAt(7s, [&](const auto&) { // - EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{5, Priority::Nominal}, now() + 1s}), _)) // + EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{4, Priority::Nominal}, now() + 1s}), _)) // .WillOnce(Return(cetl::nullopt)); }); scheduler_.scheduleAt(8s, [&](const auto&) { // - EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{6, Priority::Nominal}, now() + 1s}), _)) // + EXPECT_CALL(msg_tx_session_mock, send(TransferTxMetadataEq({{5, Priority::Nominal}, now() + 1s}), _)) // .WillOnce(Return(cetl::nullopt)); heartbeat_producer->message().health.value = uavcan::node::Health_1_0::CAUTION; diff --git a/test/unittest/presentation/test_client.cpp b/test/unittest/presentation/test_client.cpp index 558516417..a5e5a2b06 100644 --- a/test/unittest/presentation/test_client.cpp +++ b/test/unittest/presentation/test_client.cpp @@ -11,6 +11,7 @@ #include "tracking_memory_resource.hpp" #include "transport/scattered_buffer_storage_mock.hpp" #include "transport/svc_sessions_mock.hpp" +#include "transport/transfer_id_map_mock.hpp" #include "transport/transport_gtest_helpers.hpp" #include "transport/transport_mock.hpp" #include "virtual_time_scheduler.hpp" @@ -23,6 +24,7 @@ #include #include #include +#include #include #include @@ -789,7 +791,7 @@ TEST_F(TestClient, raw_request_response_failures) State state{mr_, transport_mock_, rx_params}; // Emulate that transport supports only 2 concurrent transfers by having module equal to 2^1. - // This will make the client fail to make more than 2 request. + // This will make the client fail to make more than 2 requests. EXPECT_CALL(transport_mock_, getProtocolParams()).WillRepeatedly(Return(ProtocolParams{2, 0, 0})); Presentation presentation{mr_, scheduler_, transport_mock_}; @@ -829,6 +831,84 @@ TEST_F(TestClient, raw_request_response_failures) scheduler_.spinFor(10s); } +TEST_F(TestClient, multiple_clients_with_transfer_id_map) +{ + using SvcResPromise = ResponsePromise; + using SessionSpec = ITransferIdMap::SessionSpec; + + StrictMock transfer_id_map_mock; + + constexpr ResponseRxParams rx_params_n31{4, 147, 0x31}; + constexpr ResponseRxParams rx_params_n32{4, 147, 0x32}; + + State state_31{mr_, transport_mock_, rx_params_n31}; + State state_32{mr_, transport_mock_, rx_params_n32}; + + Presentation presentation{mr_, scheduler_, transport_mock_}; + presentation.setTransferIdMap(&transfer_id_map_mock); + + EXPECT_CALL(transfer_id_map_mock, getIdFor(SessionSpec{rx_params_n31.service_id, rx_params_n31.server_node_id})) + .WillOnce(Return(0x310)); + EXPECT_CALL(transfer_id_map_mock, getIdFor(SessionSpec{rx_params_n32.service_id, rx_params_n32.server_node_id})) + .WillOnce(Return(0x320)); + + auto maybe_client_n31 = presentation.makeClient( // + rx_params_n31.server_node_id, + rx_params_n31.service_id, + rx_params_n31.extent_bytes); + ASSERT_THAT(maybe_client_n31, VariantWith(_)); + cetl::optional client_n31 = cetl::get(std::move(maybe_client_n31)); + + auto maybe_client_n32 = presentation.makeClient( // + rx_params_n32.server_node_id, + rx_params_n32.service_id, + rx_params_n32.extent_bytes); + ASSERT_THAT(maybe_client_n32, VariantWith(_)); + cetl::optional client_n32 = cetl::get(std::move(maybe_client_n32)); + + scheduler_.scheduleAt(1s, [&](const auto&) { + // + EXPECT_CALL(state_31.req_tx_session_mock_, send(_, _)) // + .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { + // + EXPECT_THAT(metadata.base.transfer_id, 0x310); + return cetl::nullopt; + })); + + const auto maybe_promise = client_n31->request(now() + 100ms, {}, now() + 2s); + EXPECT_THAT(maybe_promise, VariantWith(_)); + }); + scheduler_.scheduleAt(2s, [&](const auto&) { + // + EXPECT_CALL(state_32.req_tx_session_mock_, send(_, _)) // + .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { + // + EXPECT_THAT(metadata.base.transfer_id, 0x320); + return cetl::nullopt; + })); + + const auto maybe_promise = client_n32->request(now() + 100ms, {}, now() + 2s); + EXPECT_THAT(maybe_promise, VariantWith(_)); + }); + scheduler_.scheduleAt(8s, [&](const auto&) { + // + EXPECT_CALL(transfer_id_map_mock, + setIdFor(SessionSpec{rx_params_n32.service_id, rx_params_n32.server_node_id}, 0x320 + 1)) + .WillOnce(Return()); + + client_n32.reset(); + }); + scheduler_.scheduleAt(9s, [&](const auto&) { + // + EXPECT_CALL(transfer_id_map_mock, + setIdFor(SessionSpec{rx_params_n31.service_id, rx_params_n31.server_node_id}, 0x310 + 1)) + .WillOnce(Return()); + + client_n31.reset(); + }); + scheduler_.spinFor(10s); +} + // NOLINTEND(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers, *-function-cognitive-complexity) } // namespace diff --git a/test/unittest/presentation/test_presentation.cpp b/test/unittest/presentation/test_presentation.cpp index 63106446c..22bdbfdc1 100644 --- a/test/unittest/presentation/test_presentation.cpp +++ b/test/unittest/presentation/test_presentation.cpp @@ -25,7 +25,7 @@ #include #include #include -#include +#include #include #include diff --git a/test/unittest/presentation/test_publisher.cpp b/test/unittest/presentation/test_publisher.cpp index b76e8ca04..5d63490e5 100644 --- a/test/unittest/presentation/test_publisher.cpp +++ b/test/unittest/presentation/test_publisher.cpp @@ -7,6 +7,7 @@ #include "my_custom/bar_1_0.hpp" #include "tracking_memory_resource.hpp" #include "transport/msg_sessions_mock.hpp" +#include "transport/transfer_id_map_mock.hpp" #include "transport/transport_gtest_helpers.hpp" #include "transport/transport_mock.hpp" #include "verification_utilities.hpp" @@ -17,6 +18,7 @@ #include #include #include +#include #include #include @@ -176,7 +178,7 @@ TEST_F(TestPublisher, publish) EXPECT_CALL(msg_tx_session_mock, send(_, _)) // .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { // - EXPECT_THAT(metadata.base.transfer_id, 1); + EXPECT_THAT(metadata.base.transfer_id, 0); EXPECT_THAT(metadata.base.priority, Priority::Exceptional); EXPECT_THAT(metadata.deadline, now + 200ms); return cetl::nullopt; @@ -189,7 +191,7 @@ TEST_F(TestPublisher, publish) EXPECT_CALL(msg_tx_session_mock, send(_, _)) // .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { // - EXPECT_THAT(metadata.base.transfer_id, 2); + EXPECT_THAT(metadata.base.transfer_id, 1); EXPECT_THAT(metadata.base.priority, Priority::Fast); EXPECT_THAT(metadata.deadline, now + 100ms); return cetl::nullopt; @@ -283,7 +285,7 @@ TEST_F(TestPublisher, publishRawData) EXPECT_CALL(msg_tx_session_mock, send(_, _)) // .WillOnce(Invoke([now = now()](const auto& metadata, const auto frags) { // - EXPECT_THAT(metadata.base.transfer_id, 1); + EXPECT_THAT(metadata.base.transfer_id, 0); EXPECT_THAT(metadata.base.priority, Priority::Nominal); EXPECT_THAT(metadata.deadline, now + 200ms); EXPECT_THAT(frags.size(), 1); @@ -303,6 +305,103 @@ TEST_F(TestPublisher, publishRawData) scheduler_.spinFor(10s); } +TEST_F(TestPublisher, multiple_publishers_with_transfer_id_map) +{ + using Message = uavcan::node::Heartbeat_1_0; + using SessionSpec = ITransferIdMap::SessionSpec; + + StrictMock transfer_id_map_mock; + + Presentation presentation{mr_, scheduler_, transport_mock_}; + presentation.setTransferIdMap(&transfer_id_map_mock); + + EXPECT_CALL(transport_mock_, getLocalNodeId()) // + .WillOnce(Return(cetl::nullopt)); + + StrictMock msg7_tx_session_mock; + constexpr MessageTxParams tx7_params{7}; + cetl::optional> publisher7; + { + EXPECT_CALL(msg7_tx_session_mock, getParams()).WillOnce(Return(tx7_params)); + + EXPECT_CALL(transport_mock_, makeMessageTxSession(MessageTxParamsEq(tx7_params))) // + .WillOnce(Invoke([&](const auto&) { // + return libcyphal::detail::makeUniquePtr(mr_, msg7_tx_session_mock); + })); + + auto maybe_pub7 = presentation.makePublisher(tx7_params.subject_id); + ASSERT_THAT(maybe_pub7, VariantWith>(_)); + + publisher7.emplace(cetl::get>(std::move(maybe_pub7))); + } + + constexpr NodeId local_node_id = 0x13; + EXPECT_CALL(transport_mock_, getLocalNodeId()) // + .WillRepeatedly(Return(cetl::optional{NodeId{local_node_id}})); + + StrictMock msg9_tx_session_mock; + constexpr MessageTxParams tx9_params{9}; + cetl::optional> publisher9; + { + EXPECT_CALL(msg9_tx_session_mock, getParams()).WillOnce(Return(tx9_params)); + + EXPECT_CALL(transport_mock_, makeMessageTxSession(MessageTxParamsEq(tx9_params))) // + .WillOnce(Invoke([&](const auto&) { // + return libcyphal::detail::makeUniquePtr(mr_, msg9_tx_session_mock); + })); + + EXPECT_CALL(transfer_id_map_mock, getIdFor(SessionSpec{tx9_params.subject_id, local_node_id})) + .WillOnce(Return(90)); + + auto maybe_pub9 = presentation.makePublisher(tx9_params.subject_id); + ASSERT_THAT(maybe_pub9, VariantWith>(_)); + + publisher9.emplace(cetl::get>(std::move(maybe_pub9))); + } + + scheduler_.scheduleAt(1s, [&](const auto&) { + // + EXPECT_CALL(msg7_tx_session_mock, send(_, _)) // + .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { + // + EXPECT_THAT(metadata.base.transfer_id, 0); + return cetl::nullopt; + })); + + EXPECT_THAT(publisher7->publish(now() + 200ms, Message{&mr_}), Eq(cetl::nullopt)); + }); + scheduler_.scheduleAt(2s, [&](const auto&) { + // + EXPECT_CALL(msg9_tx_session_mock, send(_, _)) // + .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { + // + EXPECT_THAT(metadata.base.transfer_id, 90); + return cetl::nullopt; + })); + + EXPECT_THAT(publisher9->publish(now() + 200ms, Message{&mr_}), Eq(cetl::nullopt)); + }); + scheduler_.scheduleAt(8s, [&](const auto&) { + // + EXPECT_CALL(transfer_id_map_mock, setIdFor(SessionSpec{tx9_params.subject_id, local_node_id}, 90 + 1)) // + .WillOnce(Return()); + + publisher9.reset(); + testing::Mock::VerifyAndClearExpectations(&msg9_tx_session_mock); + EXPECT_CALL(msg9_tx_session_mock, deinit()).Times(1); + }); + scheduler_.scheduleAt(9s, [&](const auto&) { + // + EXPECT_CALL(transfer_id_map_mock, setIdFor(SessionSpec{tx7_params.subject_id, local_node_id}, 1)) // + .WillOnce(Return()); + + publisher7.reset(); + testing::Mock::VerifyAndClearExpectations(&msg7_tx_session_mock); + EXPECT_CALL(msg7_tx_session_mock, deinit()).Times(1); + }); + scheduler_.spinFor(10s); +} + // NOLINTEND(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) } // namespace diff --git a/test/unittest/presentation/test_publisher_custom_config.cpp b/test/unittest/presentation/test_publisher_custom_config.cpp index b39c5510c..a9ef45050 100644 --- a/test/unittest/presentation/test_publisher_custom_config.cpp +++ b/test/unittest/presentation/test_publisher_custom_config.cpp @@ -110,7 +110,7 @@ TEST_F(TestPublisherCustomConfig, publish_with_custom_buffer_size) EXPECT_CALL(msg_tx_session_mock, send(_, _)) // .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { // - EXPECT_THAT(metadata.base.transfer_id, 1); + EXPECT_THAT(metadata.base.transfer_id, 0); EXPECT_THAT(metadata.base.priority, Priority::Exceptional); EXPECT_THAT(metadata.deadline, now + 200ms); return cetl::nullopt; @@ -123,7 +123,7 @@ TEST_F(TestPublisherCustomConfig, publish_with_custom_buffer_size) EXPECT_CALL(msg_tx_session_mock, send(_, _)) // .WillOnce(Invoke([now = now()](const auto& metadata, const auto) { // - EXPECT_THAT(metadata.base.transfer_id, 2); + EXPECT_THAT(metadata.base.transfer_id, 1); EXPECT_THAT(metadata.base.priority, Priority::Fast); EXPECT_THAT(metadata.deadline, now + 100ms); return cetl::nullopt; diff --git a/test/unittest/transport/test_transfer_id_generators.cpp b/test/unittest/transport/test_transfer_id_map.cpp similarity index 53% rename from test/unittest/transport/test_transfer_id_generators.cpp rename to test/unittest/transport/test_transfer_id_map.cpp index 1f5381dbf..ca0a6b5c6 100644 --- a/test/unittest/transport/test_transfer_id_generators.cpp +++ b/test/unittest/transport/test_transfer_id_map.cpp @@ -3,7 +3,9 @@ /// Copyright Amazon.com Inc. or its affiliates. /// SPDX-License-Identifier: MIT -#include +#include "transfer_id_storage_mock.hpp" + +#include #include #include @@ -20,50 +22,93 @@ using namespace libcyphal::transport; // NOLINT This our main concern here in t using testing::Eq; using testing::Optional; +using testing::StrictMock; // NOLINTBEGIN(cppcoreguidelines-avoid-magic-numbers, readability-magic-numbers) -class TestTransferIdGenerators : public testing::Test -{}; +class TestTransferIdMapAndGens : public testing::Test +{ +protected: + class TransferIdStorage final : public detail::ITransferIdStorage + { + public: + explicit TransferIdStorage(const TransferId initial_transfer_id = 0) noexcept + : transfer_id_{initial_transfer_id} + { + } + + // detail::ITransferIdStorage + + TransferId load() const noexcept override + { + return transfer_id_; + } + + void save(const TransferId transfer_id) noexcept override + { + transfer_id_ = transfer_id; + } + + private: + TransferId transfer_id_; + + }; // TransferIdStorage + +}; // TestTransferIdMapAndGens // MARK: - Tests: -TEST_F(TestTransferIdGenerators, trivial_default) +TEST_F(TestTransferIdMapAndGens, trivial_default) { - // Default starting value is 0. - detail::TrivialTransferIdGenerator tf_id_gen; + StrictMock transfer_id_storage_mock; - EXPECT_THAT(tf_id_gen.nextTransferId(), 0); - EXPECT_THAT(tf_id_gen.nextTransferId(), 1); - EXPECT_THAT(tf_id_gen.nextTransferId(), 2); - EXPECT_THAT(tf_id_gen.nextTransferId(), 3); - EXPECT_THAT(tf_id_gen.nextTransferId(), 4); - EXPECT_THAT(tf_id_gen.nextTransferId(), 5); + const detail::TrivialTransferIdGenerator tf_id_gen{transfer_id_storage_mock}; + + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(42)); + EXPECT_CALL(transfer_id_storage_mock, save(43)); + EXPECT_THAT(tf_id_gen.nextTransferId(), 42); + + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(43)); + EXPECT_CALL(transfer_id_storage_mock, save(44)); + EXPECT_THAT(tf_id_gen.nextTransferId(), 43); + + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(44)); + EXPECT_CALL(transfer_id_storage_mock, save(45)); + EXPECT_THAT(tf_id_gen.nextTransferId(), 44); } -TEST_F(TestTransferIdGenerators, trivial_max_tf_id) +TEST_F(TestTransferIdMapAndGens, trivial_max_tf_id) { - // Starting value is 2^64 - 3. - constexpr auto max = std::numeric_limits::max(); + StrictMock transfer_id_storage_mock; - detail::TrivialTransferIdGenerator tf_id_gen; - tf_id_gen.setNextTransferId(max - 3); + const detail::TrivialTransferIdGenerator tf_id_gen{transfer_id_storage_mock}; - EXPECT_THAT(tf_id_gen.nextTransferId(), max - 3); + // The starting value is 2^64 - 2. + constexpr auto max = std::numeric_limits::max(); + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(max - 2)); + EXPECT_CALL(transfer_id_storage_mock, save(max - 1)); EXPECT_THAT(tf_id_gen.nextTransferId(), max - 2); + + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(max - 1)); + EXPECT_CALL(transfer_id_storage_mock, save(max)); EXPECT_THAT(tf_id_gen.nextTransferId(), max - 1); + + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(max)); + EXPECT_CALL(transfer_id_storage_mock, save(0)); EXPECT_THAT(tf_id_gen.nextTransferId(), max); + + EXPECT_CALL(transfer_id_storage_mock, load()).WillOnce(testing::Return(0)); + EXPECT_CALL(transfer_id_storage_mock, save(1)); EXPECT_THAT(tf_id_gen.nextTransferId(), 0); - EXPECT_THAT(tf_id_gen.nextTransferId(), 1); - EXPECT_THAT(tf_id_gen.nextTransferId(), 2); } -TEST_F(TestTransferIdGenerators, small_range) +TEST_F(TestTransferIdMapAndGens, small_range_with_default_map) { - detail::SmallRangeTransferIdGenerator<8> tf_id_gen{4}; + TransferIdStorage transfer_id_storage{9}; // on purpose bigger than modulo 4 - EXPECT_THAT(tf_id_gen.nextTransferId(), Optional(0)); - EXPECT_THAT(tf_id_gen.nextTransferId(), Optional(1)); + detail::SmallRangeTransferIdGenerator<8> tf_id_gen{4, transfer_id_storage}; + + EXPECT_THAT(tf_id_gen.nextTransferId(), Optional(1)); // 9 % 4 -> 1 EXPECT_THAT(tf_id_gen.nextTransferId(), Optional(2)); EXPECT_THAT(tf_id_gen.nextTransferId(), Optional(3)); EXPECT_THAT(tf_id_gen.nextTransferId(), Optional(0)); diff --git a/test/unittest/transport/transfer_id_map_mock.hpp b/test/unittest/transport/transfer_id_map_mock.hpp new file mode 100644 index 000000000..71e3ba490 --- /dev/null +++ b/test/unittest/transport/transfer_id_map_mock.hpp @@ -0,0 +1,37 @@ +/// @copyright +/// Copyright (C) OpenCyphal Development Team +/// Copyright Amazon.com Inc. or its affiliates. +/// SPDX-License-Identifier: MIT + +#ifndef LIBCYPHAL_TRANSPORT_TRANSFER_ID_MAP_MOCK_HPP_INCLUDED +#define LIBCYPHAL_TRANSPORT_TRANSFER_ID_MAP_MOCK_HPP_INCLUDED + +#include + +#include + +namespace libcyphal +{ +namespace transport +{ + +class TransferIdMapMock : public ITransferIdMap +{ +public: + TransferIdMapMock() = default; + virtual ~TransferIdMapMock() = default; + + TransferIdMapMock(const TransferIdMapMock&) = delete; + TransferIdMapMock(TransferIdMapMock&&) noexcept = delete; + TransferIdMapMock& operator=(const TransferIdMapMock&) = delete; + TransferIdMapMock& operator=(TransferIdMapMock&&) noexcept = delete; + + MOCK_METHOD(TransferId, getIdFor, (const SessionSpec& session_spec), (const, noexcept, override)); + MOCK_METHOD(void, setIdFor, (const SessionSpec& session_spec, const TransferId transfer_id), (noexcept, override)); + +}; // TransferIdMapMock + +} // namespace transport +} // namespace libcyphal + +#endif // LIBCYPHAL_TRANSPORT_TRANSFER_ID_MAP_MOCK_HPP_INCLUDED diff --git a/test/unittest/transport/transfer_id_storage_mock.hpp b/test/unittest/transport/transfer_id_storage_mock.hpp new file mode 100644 index 000000000..d8a81df4f --- /dev/null +++ b/test/unittest/transport/transfer_id_storage_mock.hpp @@ -0,0 +1,40 @@ +/// @copyright +/// Copyright (C) OpenCyphal Development Team +/// Copyright Amazon.com Inc. or its affiliates. +/// SPDX-License-Identifier: MIT + +#ifndef LIBCYPHAL_TRANSPORT_TRANSFER_ID_STORAGE_MOCK_HPP_INCLUDED +#define LIBCYPHAL_TRANSPORT_TRANSFER_ID_STORAGE_MOCK_HPP_INCLUDED + +#include + +#include + +namespace libcyphal +{ +namespace transport +{ +namespace detail +{ + +class TransferIdStorageMock : public ITransferIdStorage +{ +public: + TransferIdStorageMock() = default; + virtual ~TransferIdStorageMock() = default; + + TransferIdStorageMock(const TransferIdStorageMock&) = delete; + TransferIdStorageMock(TransferIdStorageMock&&) noexcept = delete; + TransferIdStorageMock& operator=(const TransferIdStorageMock&) = delete; + TransferIdStorageMock& operator=(TransferIdStorageMock&&) noexcept = delete; + + MOCK_METHOD(TransferId, load, (), (const, noexcept, override)); + MOCK_METHOD(void, save, (const TransferId transfer_id), (noexcept, override)); + +}; // TransferIdStorageMock + +} // namespace detail +} // namespace transport +} // namespace libcyphal + +#endif // LIBCYPHAL_TRANSPORT_TRANSFER_ID_STORAGE_MOCK_HPP_INCLUDED