minor fixes based on pr review

Author: serges147

include/libcyphal/transport/can/delegate.hpp

@@ -225,6 +225,10 @@ class TransportDelegate
 
     };  // CanardConcreteTree
 
+    /// Umbrella type for all session-related events.
+    ///
+    /// These are passed to the `onSessionEvent` method of the transport implementation.
+    ///
     struct SessionEvent
     {
         struct MsgCreated

include/libcyphal/transport/can/rx_session_tree_node.hpp

@@ -23,6 +23,11 @@ namespace detail
 
 class IRxSessionDelegate;
 
+/// Umbrella type for various RX session tree nodes in use at the CAN transport.
+///
+/// Currently, it contains only one `Response` subtype,
+/// but still kept nested to match the UDP transport approach (where there are several subtypes).
+///
 struct RxSessionTreeNode
 {
     /// @brief Represents a service response RX session node.

include/libcyphal/transport/session_tree.hpp

@@ -23,13 +23,16 @@ namespace libcyphal
 namespace transport
 {
 
-/// Internal implementation details of the UDP transport.
+/// Internal implementation details of a transport.
 /// Not supposed to be used directly by the users of the library.
 ///
 namespace detail
 {
 
-/// @brief Defines a tree of sessions for the UDP transport.
+/// @brief Defines a tree of sessions for a transport.
+///
+/// @tparam Node The type of the session node. Expected to be a subclass of `SessionTree::NodeBase`,
+///              and to have a method `compareByParams` that compares nodes by its parameters.
 ///
 template <typename Node>
 class SessionTree final
@@ -74,10 +77,21 @@ class SessionTree final
         return nodes_.empty();
     }
 
+    /// @brief Ensures that a node for the given parameters exists in the tree.
+    ///
+    /// @tparam ShouldBeNew If `true`, the function will return an error if node with given
+    //                      parametes already exists (see also `Node::compareByParams` method).
+    /// @tparam Params The type of the parameters to be used to find or create the node.
+    /// @tparam Args The types of the arguments to be passed to the constructor of the node.
+    /// @param params The parameters to be used to find or create the node.
+    /// @param args The extra arguments to be forwarded to the constructor of the node (as a tuple).
+    /// @return The reference to the node, or an error if the node could not be created.
+    ///
     template <bool ShouldBeNew = false, typename Params, typename... Args>
     CETL_NODISCARD auto ensureNodeFor(const Params& params,
                                       Args&&... args) -> Expected<typename NodeBase::RefWrapper, AnyFailure>
     {
+        // In c++14 we can't capture `args` with forwarding, so we pack them into a tuple.
         auto args_tuple = std::make_tuple(std::forward<Args>(args)...);
 
         const auto node_existing = nodes_.search(
@@ -167,6 +181,9 @@ template <typename RxSessionDelegate>
 class ResponseRxSessionNode final : public SessionTree<ResponseRxSessionNode<RxSessionDelegate>>::NodeBase
 {
 public:
+    // Empty tuple parameter is used to allow for the same constructor signature
+    // as for other session nodes (see also `SessionTree::ensureNodeFor` method).
+    //
     explicit ResponseRxSessionNode(const ResponseRxParams& params, const std::tuple<>&)
         : service_id_{params.service_id}
         , server_node_id{params.server_node_id}

include/libcyphal/transport/udp/delegate.hpp

@@ -253,6 +253,10 @@ class IMsgRxSessionDelegate : public IRxSessionDelegate
 class TransportDelegate
 {
 public:
+    /// Umbrella type for all session-related events.
+    ///
+    /// These are passed to the `onSessionEvent` method of the transport implementation.
+    ///
     struct SessionEvent
     {
         struct MsgDestroyed
@@ -301,24 +305,13 @@ class TransportDelegate
         return rx_rpc_dispatcher_;
     }
 
-    void listenForRxRpcPort(UdpardRxRPCPort& rpc_port, const RequestRxParams& params)
-    {
-        const std::int8_t result = ::udpardRxRPCDispatcherListen(&rx_rpc_dispatcher_,
-                                                                 &rpc_port,
-                                                                 params.service_id,
-                                                                 true,  // request
-                                                                 params.extent_bytes);
-        (void) result;
-        CETL_DEBUG_ASSERT(result >= 0, "There is no way currently to get an error here.");
-        CETL_DEBUG_ASSERT(result == 1, "A new registration has been expected to be created.");
-    }
-
-    void listenForRxRpcPort(UdpardRxRPCPort& rpc_port, const ResponseRxParams& params)
+    template <bool IsRequest, typename Params>
+    void listenForRxRpcPort(UdpardRxRPCPort& rpc_port, const Params& params)
     {
         const std::int8_t result = ::udpardRxRPCDispatcherListen(&rx_rpc_dispatcher_,
                                                                  &rpc_port,
                                                                  params.service_id,
-                                                                 false,  // response
+                                                                 IsRequest,
                                                                  params.extent_bytes);
         (void) result;
         CETL_DEBUG_ASSERT(result >= 0, "There is no way currently to get an error here.");
@@ -485,7 +478,7 @@ class TransportDelegate
             , ref_count_{0}
             , port_{}
         {
-            transport_delegate_.listenForRxRpcPort(port_, params);
+            transport_delegate_.listenForRxRpcPort<false>(port_, params);
 
             // No Sonar `cpp:S5356` b/c we integrate here with C libudpard API.
             port_.user_reference = static_cast<IRxSessionDelegate*>(this);  // NOSONAR cpp:S5356

include/libcyphal/transport/udp/rx_session_tree_node.hpp

@@ -46,13 +46,18 @@ struct SocketState
 class IRxSessionDelegate;
 class IMsgRxSessionDelegate;
 
+/// Umbrella type for various RX session tree nodes in use at the UDP transport.
+///
 struct RxSessionTreeNode
 {
     /// @brief Represents a message RX session node.
     ///
     class Message final : public transport::detail::SessionTree<Message>::NodeBase
     {
     public:
+        // Empty tuple parameter is used to allow for the same constructor signature
+        // as for other session nodes (see also `SessionTree::ensureNodeFor` method).
+        //
         explicit Message(const MessageRxParams& params, const std::tuple<>&)
             : subject_id_{params.subject_id}
         {
@@ -90,6 +95,9 @@ struct RxSessionTreeNode
     class Request final : public transport::detail::SessionTree<Request>::NodeBase
     {
     public:
+        // Empty tuple parameter is used to allow for the same constructor signature
+        // as for other session nodes (see also `SessionTree::ensureNodeFor` method).
+        //
         explicit Request(const RequestRxParams& params, const std::tuple<>&)
             : service_id_{params.service_id}
         {

include/libcyphal/transport/udp/svc_rx_sessions.hpp

@@ -148,7 +148,7 @@ class SvcRequestRxSession final : public SvcRxSessionBase<IRequestRxSession, Req
         : Base{delegate, params}
         , rpc_port_{}
     {
-        delegate.listenForRxRpcPort(rpc_port_, params);
+        delegate.listenForRxRpcPort<true>(rpc_port_, params);
 
         // No Sonar `cpp:S5356` b/c we integrate here with C libudpard API.
         rpc_port_.user_reference = static_cast<IRxSessionDelegate*>(this);  // NOSONAR cpp:S5356

include/libcyphal/types.hpp

@@ -15,6 +15,7 @@
 #include <chrono>
 #include <cstddef>
 #include <cstdint>
+#include <exception>
 #include <memory>
 #include <ratio>
 #include <type_traits>
@@ -149,8 +150,15 @@ CETL_NODISCARD UpVariant upcastVariant(Variant&& variant)
                        std::forward<Variant>(variant));
 }
 
-template <typename Action>
-bool performWithoutThrowing(Action&& action) noexcept
+/// @brief Wraps the given action into a lambda and performs it without throwing exceptions.
+///
+/// In use f.e. for `cetl::visit` which might hypothetically throw an exceptions.
+///
+/// @return `true` if the action was performed successfully, `false` if an exception was thrown.
+///         Always `true` if exceptions are disabled.
+///
+template <typename Exception = std::exception, typename Action>
+CETL_NODISCARD bool performWithoutThrowing(Action&& action) noexcept
 {
 #if defined(__cpp_exceptions)
     try
@@ -159,8 +167,9 @@ bool performWithoutThrowing(Action&& action) noexcept
         std::forward<Action>(action)();
         return true;
 #if defined(__cpp_exceptions)
-    } catch (...)
+    } catch (const Exception& ex)
     {
+        CETL_DEBUG_ASSERT(false, ex.what());
         return false;
     }
 #endif