Amend the RX RPC API and populate README.md & CONTRIBUTING.md (#53)

pavel-kirienko · web-flow · commit 6e72fe5e4414 · 2023-09-01T21:56:31.000+03:00
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -1,3 +1,32 @@
-# WORK IN PROGRESS, NOT READY FOR USE
+# LibUDPard contribution guidelines
 
-While this is a work in progress, contribute via the forums at [https://forum.opencyphal.org/](https://forum.opencyphal.org/)
+## Standards
+
+The library shall be implemented in ISO C99/C11 following MISRA C:2012.
+The MISRA compliance is enforced by Clang-Tidy and SonarQube.
+Deviations are documented directly in the source code as follows:
+
+```c
+// Intentional violation of MISRA: <some valid reason>
+<... deviant construct ...>
+```
+
+The full list of deviations with the accompanying explanation can be found by grepping the sources.
+
+Do not suppress compliance warnings using the means provided by static analysis tools because such deviations
+are impossible to track at the source code level.
+An exception applies for the case of false-positive (invalid) warnings -- those should not be mentioned in the codebase.
+
+Unfortunately, some rules are hard or impractical to enforce automatically,
+so code reviewers shall be aware of MISRA and general high-reliability coding practices
+to prevent non-compliant code from being accepted into upstream.
+
+## Build & test
+
+Consult with the CI workflow files for the required tools and build & test instructions.
+You may want to use the [toolshed](https://github.com/OpenCyphal/docker_toolchains/pkgs/container/toolshed)
+container for this.
+
+## Releasing
+
+Simply create a new release & tag on GitHub.
diff --git a/README.md b/README.md
@@ -1,46 +1,63 @@
-# NOTICE
-
-This package is a staging package to make changes before committing a pull request for the github repo: https://github.com/OpenCyphal-Garage/libudpard based on @schoberm's prototype work
-
-# Compact Cyphal/UDP v1 in C
+# Compact Cyphal/UDP in C
 
 [![Main Workflow](https://github.com/OpenCyphal-Garage/libudpard/actions/workflows/main.yml/badge.svg)](https://github.com/OpenCyphal-Garage/libudpard/actions/workflows/main.yml)
 [![Reliability Rating](https://sonarcloud.io/api/project_badges/measure?project=libudpard&metric=reliability_rating)](https://sonarcloud.io/summary?id=libudpard)
 [![Coverage](https://sonarcloud.io/api/project_badges/measure?project=libudpard&metric=coverage)](https://sonarcloud.io/summary?id=libudpard)
 [![Forum](https://img.shields.io/discourse/users.svg?server=https%3A%2F%2Fforum.opencyphal.org&color=1700b3)](https://forum.opencyphal.org)
 
-LibUDPard is a compact implementation of the Cyphal/UDP protocol stack in C99/C11 for high-integrity real-time
+LibUDPard is a compact implementation of the Cyphal/UDP protocol in C99/C11 for high-integrity real-time
 embedded systems.
 
 [Cyphal](https://opencyphal.org) is an open lightweight data bus standard designed for reliable intravehicular
 communication in aerospace and robotic applications via CAN bus, UDP, and other robust transports.
 
 We pronounce LibUDPard as *lib-you-dee-pee-ard*.
 
-## WORK IN PROGRESS, NOT READY FOR FORMAL USE
+## Features
+
+Some of the features listed here are intrinsic properties of Cyphal.
+
+- Full branch coverage and extensive static analysis.
+
+- Compliance with automatically enforceable MISRA C rules (reach out to https://forum.opencyphal.org for details).
+
+- Detailed time complexity and memory requirement models for the benefit of real-time high-integrity applications.
+
+- Purely reactive time-deterministic API without the need for background servicing.
+
+- Zero-copy data pipeline on reception --
+  payload is moved from the underlying NIC driver all the way to the application without copying.
 
-**Read the docs in [`libudpard/udpard.h`](/libudpard/udpard.h).**
+- Support for redundant network interfaces with seamless interface aggregation and no fail-over delay.
 
-Building
-```
-cmake -B ./build -DCMAKE_BUILD_TYPE=Debug -DNO_STATIC_ANALYSIS=1 -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ -DCMAKE_EXPORT_COMPILE_COMMANDS=1 tests
-```
-Testing
-```
-cd build
-make
-make test
-```
-Or to debug
-```
-TEST_OUTPUT_ON_FAILURE=TRUE make test
-```
+- Out-of-order multi-frame transfer reassembly, including cross-transfer interleaved frames.
 
-## Features, Description, and Usage
+- Support for repetition-coding forward error correction (FEC) for lossy links (e.g., wireless)
+  transparent to the application.
 
-To be added at a later date.
+- No dependency on heap memory; the library can be used with fixed-size block pool allocators.
+
+- Compatibility with all conventional 8/16/32/64-bit platforms.
+
+- Compatibility with extremely resource-constrained baremetal environments starting from 64K ROM and 64K RAM.
+
+- Implemented in ≈2000 lines of code.
+
+## Usage
+
+The library implements the Cyphal/UDP protocol, which is a transport-layer entity.
+An application using this library will need to implement the presentation layer above the library,
+perhaps with the help of the [Nunavut transpiler](https://github.com/OpenCyphal/nunavut),
+and the network layer below the library using a third-party UDP/IP stack implementation with multicast/IGMP support
+(TCP and ARP are not needed).
+In the most straightforward case, the network layer can be based on the standard Berkeley socket API
+or a lightweight embedded stack such as LwIP.
+
+**Read the API docs in [`libudpard/udpard.h`](libudpard/udpard.h).**
+For complete usage examples, please refer to <https://github.com/OpenCyphal-Garage/demos>.
 
 ## Revisions
-### v0.0
 
-Prototype commit
+### v1.0
+
+Initial release.
diff --git a/libudpard/udpard.c b/libudpard/udpard.c
@@ -1721,19 +1721,32 @@ int_fast8_t udpardRxSubscriptionReceive(struct UdpardRxSubscription* const self,
 }
 
 int_fast8_t udpardRxRPCDispatcherInit(struct UdpardRxRPCDispatcher* const  self,
-                                      const UdpardNodeID                   local_node_id,
                                       const struct UdpardRxMemoryResources memory)
 {
     int_fast8_t result = -UDPARD_ERROR_ARGUMENT;
-    if ((self != NULL) && (local_node_id <= UDPARD_NODE_ID_MAX) && rxValidateMemoryResources(memory))
+    if ((self != NULL) && rxValidateMemoryResources(memory))
     {
         memZero(sizeof(*self), self);
-        self->local_node_id   = local_node_id;
-        self->udp_ip_endpoint = makeServiceUDPIPEndpoint(local_node_id);
-        self->memory          = memory;
-        self->request_ports   = NULL;
-        self->response_ports  = NULL;
-        result                = 0;
+        self->local_node_id  = UDPARD_NODE_ID_UNSET;
+        self->memory         = memory;
+        self->request_ports  = NULL;
+        self->response_ports = NULL;
+        result               = 0;
+    }
+    return result;
+}
+
+int_fast8_t udpardRxRPCDispatcherStart(struct UdpardRxRPCDispatcher* const self,
+                                       const UdpardNodeID                  local_node_id,
+                                       struct UdpardUDPIPEndpoint* const   out_udp_ip_endpoint)
+{
+    int_fast8_t result = -UDPARD_ERROR_ARGUMENT;
+    if ((self != NULL) && (out_udp_ip_endpoint != NULL) && (local_node_id <= UDPARD_NODE_ID_MAX) &&
+        (self->local_node_id > UDPARD_NODE_ID_MAX))
+    {
+        self->local_node_id  = local_node_id;
+        *out_udp_ip_endpoint = makeServiceUDPIPEndpoint(local_node_id);
+        result               = 0;
     }
     return result;
 }
diff --git a/libudpard/udpard.h b/libudpard/udpard.h
@@ -915,29 +915,14 @@ struct UdpardRxRPCPort
 };
 
 /// A service dispatcher is a collection of RPC-service RX ports.
-///
-/// In Cyphal/UDP, each node has a specific IP multicast group address where RPC-service transfers destined to that
-/// node are sent to. This is similar to subject (topic) multicast group addressed except that the node-ID takes
-/// the place of the subject-ID. The IP multicast group address is derived from the local node-ID.
-/// This address is available in the field named "udp_ip_endpoint".
-/// The application is expected to open a separate socket bound to that endpoint per redundant interface,
-/// and then feed the UDP datagrams received from these sockets into udpardRxRPCDispatcherReceive,
-/// collecting UdpardRxRPCTransfer instances at the output.
-///
 /// Anonymous nodes (nodes without a node-ID of their own) cannot use RPC-services.
 struct UdpardRxRPCDispatcher
 {
     /// The local node-ID has to be stored to facilitate correctness checking of incoming transfers.
-    /// This value shall not be modified after initialization. If the local node needs to change its node-ID,
-    /// this dispatcher instance must be destroyed and a new one created instead.
+    /// This value shall not be modified.
     /// READ-ONLY
     UdpardNodeID local_node_id;
 
-    /// The IP address and UDP port number where UDP/IP datagrams carrying RPC-service transfers destined to this node
-    /// will be sent.
-    /// READ-ONLY
-    struct UdpardUDPIPEndpoint udp_ip_endpoint;
-
     /// Refer to UdpardRxMemoryResources.
     struct UdpardRxMemoryResources memory;
 
@@ -956,23 +941,29 @@ struct UdpardRxRPCTransfer
 
 /// To begin receiving RPC-service requests and/or responses, the application should do this:
 ///
-///     1. Create a new UdpardRxRPCDispatcher instance.
+///     1. Create a new UdpardRxRPCDispatcher instance and initialize it by calling udpardRxRPCDispatcherInit.
 ///
-///     2. Initialize it by calling udpardRxRPCDispatcherInit. Observe that a valid node-ID is required here.
-///        If the application has to perform a plug-and-play node-ID allocation, it has to complete that beforehand.
-///        The dispatcher is not needed to perform PnP node-ID allocation.
+///     2. Announce its interest in specific RPC-services (requests and/or responses) by calling
+///        udpardRxRPCDispatcherListen per each. This can be done at any later point as well.
 ///
-///     3. Per redundant network interface:
-///        - Create a new socket bound to the IP multicast group address and UDP port number specified in the
-///          udp_ip_endpoint field of the initialized RPC dispatcher instance. The library will determine the
-///          endpoint to use based on the node-ID.
+///     3. When the local node-ID is known, invoke udpardRxRPCDispatcherStart to inform the library of the
+///        node-ID value of the local node, and at the same time obtain the address of the UDP/IP multicast group
+///        to bind the socket(s) to. This step can be taken before or after the RPC-service port registration.
+///        If the application has to perform a plug-and-play node-ID allocation, it has to complete that beforehand
+///        (the dispatcher is not needed for PnP node-ID allocation).
 ///
-///     4. Announce its interest in specific RPC-services (requests and/or responses) by calling
-///        udpardRxRPCDispatcherListen per each. This can be done at any later point as well.
+///     4. Having obtained the UDP/IP endpoint in the previous step, do per redundant network interface:
+///        - Create a new socket bound to the IP multicast group address and UDP port number obtained earlier.
+///          The multicast group address depends on the local node-ID.
 ///
 ///     5. Read data from the sockets continuously and forward each received UDP datagram to
 ///        udpardRxRPCDispatcherReceive, along with the index of the redundant interface
-///        the datagram was received on. Only those services that were announced in step 4 will be processed.
+///        the datagram was received on. Only those services that were announced in step 3 will be processed.
+///
+/// The reason the local node-ID has to be specified via a separate call is to allow the application to set up the
+/// RPC ports early, without having to be aware of its own node-ID. This is useful for applications that perform
+/// plug-and-play node-ID allocation. Applications where PnP is not needed will simply call both functions
+/// at the same time during early initialization.
 ///
 /// There is no resource deallocation function ("free") for the RPC dispatcher. This is because the dispatcher
 /// does not own any resources. To dispose of a dispatcher safely, the application shall invoke
@@ -983,9 +974,31 @@ struct UdpardRxRPCTransfer
 ///
 /// The time complexity is constant. This function does not invoke the dynamic memory manager.
 int_fast8_t udpardRxRPCDispatcherInit(struct UdpardRxRPCDispatcher* const  self,
-                                      const UdpardNodeID                   local_node_id,
                                       const struct UdpardRxMemoryResources memory);
 
+/// This function must be called exactly once to complete the initialization of the RPC dispatcher.
+/// It takes the node-ID of the local node, which is used to derive the UDP/IP multicast group address
+/// to bind the sockets to, which is returned via the out parameter.
+///
+/// In Cyphal/UDP, each node has a specific IP multicast group address where RPC-service transfers destined to that
+/// node are sent to. This is similar to subject (topic) multicast group addressed except that the node-ID takes
+/// the place of the subject-ID. The IP multicast group address is derived from the local node-ID.
+///
+/// The application is expected to open a separate socket bound to that endpoint per redundant interface,
+/// and then feed the UDP datagrams received from these sockets into udpardRxRPCDispatcherReceive,
+/// collecting UdpardRxRPCTransfer instances at the output.
+///
+/// This function shall not be called more than once per dispatcher. If the local node needs to change its node-ID,
+/// this dispatcher instance must be destroyed and a new one created instead.
+///
+/// The return value is 0 on success.
+/// The return value is a negated UDPARD_ERROR_ARGUMENT if any of the input arguments are invalid.
+///
+/// The time complexity is constant. This function does not invoke the dynamic memory manager.
+int_fast8_t udpardRxRPCDispatcherStart(struct UdpardRxRPCDispatcher* const self,
+                                       const UdpardNodeID                  local_node_id,
+                                       struct UdpardUDPIPEndpoint* const   out_udp_ip_endpoint);
+
 /// This function lets the application register its interest in a particular service-ID and kind (request/response)
 /// by creating an RPC-service RX port. The port pointer shall retain validity until its unregistration or until
 /// the dispatcher is destroyed. The service instance shall not be moved or destroyed.
diff --git a/tests/src/test_e2e.cpp b/tests/src/test_e2e.cpp
@@ -427,7 +427,9 @@ void testRPC()
     }
     // Initialize the RPC dispatcher and the RPC services.
     UdpardRxRPCDispatcher dispatcher{};
-    TEST_ASSERT_EQUAL(0, udpardRxRPCDispatcherInit(&dispatcher, 4321, mem_rx));
+    TEST_ASSERT_EQUAL(0, udpardRxRPCDispatcherInit(&dispatcher, mem_rx));
+    UdpardUDPIPEndpoint udp_ip_endpoint{};
+    TEST_ASSERT_EQUAL(0, udpardRxRPCDispatcherStart(&dispatcher, 4321, &udp_ip_endpoint));
     UdpardRxRPCPort port_foo_a{};
     UdpardRxRPCPort port_foo_q{};
     TEST_ASSERT_EQUAL(1, udpardRxRPCDispatcherListen(&dispatcher, &port_foo_a, 200, false, 500));
@@ -473,7 +475,7 @@ void testRPC()
     TEST_ASSERT_EQUAL(0, alloc_rx_payload.allocated_fragments);
     const UdpardTxItem* tx_item = udpardTxPeek(&tx);
     TEST_ASSERT_NOT_NULL(tx_item);
-    TEST_ASSERT_EQUAL(dispatcher.udp_ip_endpoint.ip_address, tx_item->destination.ip_address);
+    TEST_ASSERT_EQUAL(udp_ip_endpoint.ip_address, tx_item->destination.ip_address);
     TEST_ASSERT_NULL(tx_item->next_in_transfer);
     TEST_ASSERT_EQUAL(10'001'000, tx_item->deadline_usec);
     TEST_ASSERT_EQUAL(0xA1, tx_item->dscp);
@@ -533,7 +535,7 @@ void testRPC()
     // Second transfer.
     tx_item = udpardTxPeek(&tx);
     TEST_ASSERT_NOT_NULL(tx_item);
-    TEST_ASSERT_EQUAL(dispatcher.udp_ip_endpoint.ip_address, tx_item->destination.ip_address);
+    TEST_ASSERT_EQUAL(udp_ip_endpoint.ip_address, tx_item->destination.ip_address);
     TEST_ASSERT_NULL(tx_item->next_in_transfer);
     TEST_ASSERT_EQUAL(10'000'000, tx_item->deadline_usec);
     TEST_ASSERT_EQUAL(0xA2, tx_item->dscp);
diff --git a/tests/src/test_rx.cpp b/tests/src/test_rx.cpp
@@ -209,25 +209,15 @@ void testRxRPCDispatcher()
 
     // Initialize the RPC dispatcher.
     UdpardRxRPCDispatcher self{};
-    TEST_ASSERT_EQUAL(-UDPARD_ERROR_ARGUMENT,
-                      udpardRxRPCDispatcherInit(nullptr,
-                                                0xFFFFU,
-                                                {
-                                                    .session  = instrumentedAllocatorMakeMemoryResource(&mem_session),
-                                                    .fragment = instrumentedAllocatorMakeMemoryResource(&mem_fragment),
-                                                    .payload  = instrumentedAllocatorMakeMemoryDeleter(&mem_payload),
-                                                }));
     TEST_ASSERT_EQUAL(-UDPARD_ERROR_ARGUMENT,
                       udpardRxRPCDispatcherInit(&self,
-                                                0x1042,
                                                 {
                                                     .session  = {nullptr, nullptr, nullptr},
                                                     .fragment = instrumentedAllocatorMakeMemoryResource(&mem_fragment),
                                                     .payload  = instrumentedAllocatorMakeMemoryDeleter(&mem_payload),
                                                 }));
     TEST_ASSERT_EQUAL(0,
                       udpardRxRPCDispatcherInit(&self,
-                                                0x1042,
                                                 {
                                                     .session  = instrumentedAllocatorMakeMemoryResource(&mem_session),
                                                     .fragment = instrumentedAllocatorMakeMemoryResource(&mem_fragment),
@@ -237,13 +227,21 @@ void testRxRPCDispatcher()
     TEST_ASSERT_EQUAL(&instrumentedAllocatorDeallocate, self.memory.session.deallocate);
     TEST_ASSERT_NULL(self.request_ports);
     TEST_ASSERT_NULL(self.response_ports);
-    TEST_ASSERT_EQUAL(0x1042, self.local_node_id);
-    TEST_ASSERT_EQUAL(0xEF011042UL, self.udp_ip_endpoint.ip_address);
-    TEST_ASSERT_EQUAL(9382, self.udp_ip_endpoint.udp_port);
+    TEST_ASSERT_EQUAL(0xFFFF, self.local_node_id);
     TEST_ASSERT_EQUAL(0, mem_session.allocated_fragments);
     TEST_ASSERT_EQUAL(0, mem_fragment.allocated_fragments);
     TEST_ASSERT_EQUAL(0, mem_payload.allocated_fragments);
 
+    // Start the dispatcher by setting the local node ID.
+    UdpardUDPIPEndpoint udp_ip_endpoint{};
+    TEST_ASSERT_EQUAL(-UDPARD_ERROR_ARGUMENT, udpardRxRPCDispatcherStart(&self, 0xFFFF, &udp_ip_endpoint));
+    TEST_ASSERT_EQUAL(-UDPARD_ERROR_ARGUMENT, udpardRxRPCDispatcherStart(&self, 0x1042, nullptr));
+    TEST_ASSERT_EQUAL(0, udpardRxRPCDispatcherStart(&self, 0x1042, &udp_ip_endpoint));
+    TEST_ASSERT_EQUAL(-UDPARD_ERROR_ARGUMENT, udpardRxRPCDispatcherStart(&self, 0x1042, &udp_ip_endpoint));
+    TEST_ASSERT_EQUAL(0x1042, self.local_node_id);
+    TEST_ASSERT_EQUAL(0xEF011042UL, udp_ip_endpoint.ip_address);
+    TEST_ASSERT_EQUAL(9382, udp_ip_endpoint.udp_port);
+
     // Add a request port.
     UdpardRxRPCPort port_request_foo{};
     TEST_ASSERT_EQUAL(-UDPARD_ERROR_ARGUMENT, udpardRxRPCDispatcherListen(&self, nullptr, 511, true, 100));
