Replace the implementation #51
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Update the API according to the new design principle that the library is to be used with a third-party UDP/IP+IGMP stack. > Q: Could you be a bit more specific about the multiplexing that you saw in the old version? Do you mean redundant interfaces? > A: No, I mean the socket layer (or the lack-of-socket layer, as Erik puts it) already demultiplexes incoming datagrams per subject. The original design joins the demultiplexed datagram feeds into one feed which is then demultiplexed back through AVL tree lookup. This is suboptimal. The new design eliminates one (of the two) AVL lookups from the RX pipeline. To subscribe to a subject, you would call `udpardRxSubscribtionInit` and then create a socket for this subscription. The socket and the subscription will exist side-by-side. 2. Set up static analysis and verification: - Clang-Tidy and SonarCloud with a large subset of MISRA rules enabled. - Build for i386, AMD64, ARM, AVR. - Run tests on i386 and AMD64 using both GCC and Clang with coverage. 3. Switch from Catch2 to Google Test.
…O reassembly support (#38) Close #36 Add RX frame parser (a simple function) with tests. Update the RX pipeline as described in https://forum.opencyphal.org/t/refined-requirements-for-the-rx-pipeline-in-libudpard/1938. Pros: - Zero-copy RX pipeline Cons: - The `udpardRx*Receive` functions take ownership of the payload buffer. - The memory usage pattern depends on the MTU of the transmitting node even for the same extent. Small MTU --> more payload fragments --> more memory fragments to keep, and vice versa. One missing feature is a trivial helper function that takes payload fragments and returns a contiguous chunk of memory instead (this is expensive both memory-wise and time-wise).
* Implement the common parts of the RX pipeline. At this point, about 90% of the work is done. All that is left is to add the basic public API wrappers for subscriptions and RPC-services. For details about the implementation of the RX pipeline refer to the line that says "Internally, the RX pipeline is arranged as follows". * Slightly refactor the code added earlier to uphold orthogonality and avoid unnecessary duplication. * Trivial renamings to keep names shorter, e.g.: * `UdpardConstPayload` -> `UdpardPayload` (const implied by default; there's also `UdpardMutablePayload`) * `UdpardPayloadFragmentHandle` -> `UdpardFragment` * Establish an internal convention that functions named "Destroy" deallocate the passed memory while "Free" only deallocate the memory of the associated resources. This is not API-visible. * Remove the `port_id` field from `UdpardRxPort` because it is not needed. After this is merged, the remaining task is to simply implement the public RX pipeline functions on top of: - `rxPortInit` - `rxPortAcceptFrame` - `rxPortFree`
It is now possible to subscribe to subjects and receive data from them. The only missing bit is the RPC services API, all other functionality of the library is already implemented and can be used. This changeset also brings `udpardGather` that, well, gathers a fragmented payload buffer into one contiguous chunk of memory that can be passed into Nunavut. Later we should amend Nunavut to support fragmented buffers directly to eliminate extra data copying and memory utilization. This changeset also fixes an internal documentation error that provided an incorrect estimation of the worst-case recursion depth.
This is the final feature. This changeset slightly modifies the error handling behavior of `udpardRxSubscriptionReceive` such that the datagram is deallocated if the passed arguments are invalid, unless `self` is also invalid. This is needed to prevent memory leak when the function is invoked incorrectly. An alternative solution is to state it explicitly in the API that the caller is required to free the datagram memory if `-UDPARD_ERROR_ARGUMENT` is returned, but this convention is comparatively fragile. A new test is added for this. `UdpardRxRPC` is renamed into `UdpardRxRPCPort` for clarity. `UdpardRxRPCDispatcher` is extended with `local_node_id` to facilitate destination address check when accepting frames.
- Add e2e tests. - Create a top-level CMakeLists.txt to simplify development. - Remove a redundant .clang-format file. This changeset completes the functionality of the library and its test suite. The missing part is the usage overview for the README and the contributing documentation.
|
Kudos, SonarCloud Quality Gate passed!
|
lydia-at-amazon
approved these changes
Aug 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As the library is functionally complete and is usable, I would like to continue further development against the main branch. The current version number is set to v0.1.
There have been no pushes to
main2since #48.