Remove topic hash from the transport layer; remove ordered delivery and use simplified ACK matching based on the transfer-ID alone#72
Merged
pavel-kirienko merged 13 commits intomainfrom Feb 13, 2026
Conversation
…sion heap footprint" This reverts commit 6ac82bc.
There was a problem hiding this comment.
Pull request overview
This PR significantly refactors the Cyphal/UDP protocol implementation by simplifying the transfer identification scheme and removing the ordered reassembly mode. The changes consolidate responsibility separation between layers and streamline the codebase.
Changes:
- Removed topic hash from transfer identification, replacing it with a unique transfer-ID per subject
- Eliminated the ordered reassembly mode, keeping only unordered and stateless modes
- Reduced the Cyphal/UDP header size from 48 to 40 bytes by removing the topic hash field
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/src/test_intrusive_tx.c | Updated tests to reflect removal of topic hash, new transfer indexing by ID+seq_no, and simplified ACK handling |
| tests/src/test_intrusive_rx.c | Removed ordered mode tests, updated port initialization to remove topic hash and mode parameters |
| tests/src/test_intrusive_header.c | Updated header serialization tests for new 40-byte format with frame_kind replacing flags |
| tests/src/test_intrusive_guards.c | Added guard tests for new P2P push validation and updated existing guards for API changes |
| tests/src/test_integration_sockets.cpp | Removed topic hash and collision tracking from integration tests |
| tests/src/test_fragment.cpp | Added test for seeking before first fragment boundary |
| tests/src/test_e2e_responses.cpp | Updated header size constant and removed topic hash from response flow tests |
| tests/src/test_e2e_reliable_ordered.cpp | Entire file deleted - removed ordered mode test suite |
| tests/src/test_e2e_random.cpp | Simplified transfer key to use only transfer-ID, removed collision tracking |
| tests/src/test_e2e_edge.cpp | Removed ordered mode tests and topic hash collision detection tests |
| tests/src/test_e2e_api.cpp | Updated port initialization and removed collision callbacks from API tests |
| tests/CMakeLists.txt | Removed test_e2e_reliable_ordered from build |
| libudpard/udpard.h | Updated API to remove topic hash, ordered mode, and reordering window parameters |
| libudpard/udpard.c | Implemented new transfer indexing scheme, removed ordered mode logic, reduced header to 40 bytes |
| cyphal_udp_header.dsdl | Added new header specification documenting 40-byte format with kind field |
| README.md | Updated feature list to remove ordered mode mentions |
Files not reviewed (1)
- .idea/dictionaries/project.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
pavel-kirienko
changed the title
pavel-kirienko
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Ordered delivery cannot be correct given the risk of multiple topics temporarily sharing the same subject.