Skip to content

Commit c067862

Browse files
aeriqaeriq
committed
Cleanup headers and version check on headers
1 parent 6f5dc9b commit c067862

File tree

8 files changed

+57
-47
lines changed

8 files changed

+57
-47
lines changed

‎README.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
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
44

5-
# Compact Cyphal/UDP v0 in C
5+
# Compact Cyphal/UDP v1 in C
66

77
Libudpard is a compact implementation of the Cyphal/UDP protocol stack in C99/C11 for high-integrity real-time
88
embedded systems.
@@ -24,6 +24,10 @@ cd build
2424
make
2525
make test
2626
```
27+
Or to debug
28+
```
29+
TEST_OUTPUT_ON_FAILURE=TRUE make test
30+
```
2731

2832
## Features, Description, and Usage
2933

‎libudpard/udpard.c‎

Lines changed: 31 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
#define UDPARD_SUBJECT_ID_MASK 32767U /// 0x7FFF
7878
#define UDPARD_SUBNET_OFFSET 17U
7979
#define UDPARD_SUBNET_MASK (31U << UDPARD_SUBNET_OFFSET)
80+
#define UDPARD_TRANSMIT_SUBNET_VALUE 0U
8081
#define UDPARD_RESERVED_1BIT_OFFSET 15U
8182
#define UDPARD_RESERVED_1BIT_MASK (1U << UDPARD_RESERVED_1BIT_OFFSET)
8283
#define UDPARD_SERVICE_NOT_MESSAGE_OFFSET 16U
@@ -97,7 +98,7 @@ IRNR - Is Request, Not Response
9798
#define UDPARD_SERVICE_ID_MASK 16383U /// 0x3FFF
9899
#define UPDARD_DATA_SPECIFIER_MESSAGE (0xFFFF >> 1) // SNM (0) + SubjectID
99100
#define UDPARD_DATA_SPECIFIER_SERVICE_RESPONSE (2U << UDPARD_IRNR_DATA_SPECIFIER_OFFSET) // Set SNM in Cyphal data specifier - SNM (1) + IRNR (0) + ServiceID
100-
#define UDPARD_DATA_SPECIFIER_SERVICE_REQUEST (3U << UDPARD_IRNR_DATA_SPECIFIER_OFFSET) // Set SNM and IRNR in Cyphal data specifier - SNM (1) + IRNR (1) + ServiceID
101+
#define UDPARD_DATA_SPECIFIER_SERVICE_REQUEST (3U << UDPARD_IRNR_DATA_SPECIFIER_OFFSET) // Set SNM and IRNR in Cyphal data specifier - SNM (1) + IRNR (1) + ServiceID
101102

102103
/// Ports align with subject and service ids
103104
/// Subjects use multicast and always use port 16383
@@ -255,7 +256,7 @@ UDPARD_PRIVATE int32_t txMakeMessageSessionSpecifier(const UdpardPortID
255256
(local_node_addr & ~(UdpardIPv4Addr) UDPARD_NODE_ID_MASK) |
256257
(UdpardIPv4Addr) src_node_id;
257258
out_spec->destination_route_specifier =
258-
((local_node_addr & (UdpardIPv4Addr) UDPARD_SUBNET_MASK) |
259+
((UDPARD_TRANSMIT_SUBNET_VALUE & (UdpardIPv4Addr) UDPARD_SUBNET_MASK) |
259260
(UdpardIPv4Addr) UDPARD_MULTICAST_PREFIX |
260261
((UdpardIPv4Addr) UDPARD_SUBJECT_ID_MASK & (UdpardIPv4Addr) subject_id)) &
261262
~(UdpardIPv4Addr) UDPARD_SERVICE_NOT_MESSAGE_MASK &
@@ -265,9 +266,7 @@ UDPARD_PRIVATE int32_t txMakeMessageSessionSpecifier(const UdpardPortID
265266
}
266267

267268
UDPARD_PRIVATE int32_t txMakeServiceSessionSpecifier(const UdpardPortID service_id,
268-
const bool request_not_response,
269269
const UdpardNodeID src_node_id,
270-
const UdpardNodeID dst_node_id,
271270
const UdpardIPv4Addr local_node_addr,
272271
UdpardSessionSpecifier* const out_spec)
273272
{
@@ -278,9 +277,9 @@ UDPARD_PRIVATE int32_t txMakeServiceSessionSpecifier(const UdpardPortID
278277
(local_node_addr & ~(UdpardIPv4Addr) UDPARD_NODE_ID_MASK) |
279278
(UdpardIPv4Addr) src_node_id;
280279
out_spec->destination_route_specifier =
281-
((local_node_addr & (UdpardIPv4Addr) UDPARD_SUBNET_MASK) |
280+
((UDPARD_TRANSMIT_SUBNET_VALUE & (UdpardIPv4Addr) UDPARD_SUBNET_MASK) |
282281
(UdpardIPv4Addr) UDPARD_MULTICAST_PREFIX |
283-
((UdpardIPv4Addr) UDPARD_NODE_ID_MASK & (UdpardIPv4Addr) dst_node_id)) |
282+
((UdpardIPv4Addr) UDPARD_NODE_ID_MASK & (UdpardIPv4Addr) service_id)) |
284283
(UdpardIPv4Addr) UDPARD_SERVICE_NOT_MESSAGE_MASK;
285284
out_spec->data_specifier = (UdpardUdpPortID) UDPARD_UDP_PORT;
286285
return UDPARD_SUCCESS;
@@ -323,9 +322,7 @@ UDPARD_PRIVATE int32_t txMakeSessionSpecifier(const UdpardTransferMetadata* cons
323322
if (local_node_id != UDPARD_NODE_ID_UNSET)
324323
{
325324
out = txMakeServiceSessionSpecifier(tr->port_id,
326-
tr->transfer_kind == UdpardTransferKindRequest,
327325
local_node_id,
328-
tr->remote_node_id,
329326
local_node_addr,
330327
spec);
331328
UDPARD_ASSERT(out >= 0);
@@ -365,20 +362,20 @@ UDPARD_PRIVATE void txMakeFrameHeader(UdpardFrameHeader* const header,
365362
uint32_t end_of_transfer_mask = (uint32_t) (end_of_transfer ? 1 : 0) << (uint32_t) UDPARD_END_OF_TRANSFER_OFFSET;
366363
size_t cyphal_header_size_without_crc = sizeof(UdpardFrameHeader) - CYPHAL_HEADER_CRC_SIZE_BYTES;
367364
header->transfer_id = transfer_id;
365+
header->version = (uint8_t) UDPARD_CYPHAL_HEADER_VERSION;
368366
header->priority = (uint8_t) priority;
369367
header->frame_index_eot = end_of_transfer_mask | frame_index;
370368
header->source_node_id = src_node_id;
371369
header->destination_node_id = dst_node_id;
372370
header->cyphal_header_checksum = cyphalHeaderCrcAdd(CYPHAL_HEADER_CRC_INITIAL, cyphal_header_size_without_crc, header);
373371
if (transfer_kind == UdpardTransferKindMessage)
374372
{
375-
header->data_specifier = (uint16_t) UPDARD_DATA_SPECIFIER_MESSAGE & port_id; // SNM (0) + Subject ID
373+
header->data_specifier = (uint16_t) UPDARD_DATA_SPECIFIER_MESSAGE & port_id & UDPARD_SUBJECT_ID_MASK; // SNM (0) + Subject ID
376374
}
377375
else
378376
{
379377
header->data_specifier =
380-
(transfer_kind == UdpardTransferKindRequest) ? UDPARD_DATA_SPECIFIER_SERVICE_REQUEST | port_id
381-
: UDPARD_DATA_SPECIFIER_SERVICE_RESPONSE | port_id; // SNM (1) + IRNR + ServiceID
378+
((transfer_kind == UdpardTransferKindRequest) ? UDPARD_DATA_SPECIFIER_SERVICE_REQUEST : UDPARD_DATA_SPECIFIER_SERVICE_RESPONSE) | port_id; // SNM (1) + IRNR + ServiceID
382379
}
383380
}
384381

@@ -435,10 +432,10 @@ UDPARD_PRIVATE int32_t txPushSingleFrame(UdpardTxQueue* const que
435432
UdpardInstance* const ins,
436433
const UdpardMicrosecond deadline_usec,
437434
const UdpardSessionSpecifier* const specifier,
438-
const UdpardNodeID src_node_id,
439-
const UdpardNodeID dst_node_id,
440-
const UdpardPortID port_id,
441-
const UdpardTransferKind transfer_kind,
435+
const UdpardNodeID src_node_id,
436+
const UdpardNodeID dst_node_id,
437+
const UdpardPortID port_id,
438+
const UdpardTransferKind transfer_kind,
442439
const UdpardPriority priority,
443440
const UdpardTransferID transfer_id,
444441
const size_t payload_size,
@@ -477,7 +474,7 @@ UDPARD_PRIVATE int32_t txPushSingleFrame(UdpardTxQueue* const que
477474
(uint8_t)(crc >> 8U & CRC_BYTE_MASK),
478475
(uint8_t)(crc >> 16U & CRC_BYTE_MASK),
479476
(uint8_t)(crc >> 24U & CRC_BYTE_MASK)};
480-
for (int i = 0; i < sizeof(crc_as_byte); i++)
477+
for (unsigned int i = 0; i < sizeof(crc_as_byte); i++)
481478
{
482479
tqi->payload_buffer[frame_offset++] = crc_as_byte[i];
483480
}
@@ -590,9 +587,9 @@ UDPARD_PRIVATE TxChain txGenerateMultiFrameChain(UdpardInstance* const in
590587
(void) memcpy(&out.tail->payload_buffer[sizeof(UdpardFrameHeader)], payload_ptr, payload_move_size); // NOLINT
591588
frame_offset += sizeof(UdpardFrameHeader) + payload_move_size;
592589
// Insert crc into same frame
593-
for(int i = 0; i < overrun_amount; i++)
590+
for(unsigned int i = 0; i < overrun_amount; i++)
594591
{
595-
out.tail->payload_buffer[frame_offset++] = crc_as_byte[i+last_crc_index];
592+
out.tail->payload_buffer[frame_offset++] = crc_as_byte[i + last_crc_index];
596593
}
597594
last_crc_index += overrun_amount;
598595
}
@@ -727,6 +724,7 @@ typedef struct UdpardInternalRxSession
727724
typedef struct
728725
{
729726
UdpardMicrosecond timestamp_usec;
727+
UdpardHeaderVersion version;
730728
UdpardPriority priority;
731729
UdpardTransferKind transfer_kind;
732730
UdpardPortID port_id;
@@ -810,19 +808,22 @@ UDPARD_PRIVATE bool rxTryParseFrame(const UdpardMicrosecond timestam
810808
(void) memcpy(&frame->udp_cyphal_header, frame->payload, sizeof(frame->udp_cyphal_header)); // NOLINT
811809
out->timestamp_usec = timestamp_usec;
812810

813-
out->priority = (UdpardPriority) frame->udp_cyphal_header.priority;
814-
out -> source_node_id = frame->udp_cyphal_header.source_node_id;
815-
out->transfer_kind = getTransferKindFromDataSpecifier(frame->udp_cyphal_header.data_specifier);
816-
out ->port_id = getPortIdFromDataSpecifiers(frame->udp_cyphal_header.data_specifier);
817-
out ->destination_node_id = frame->udp_cyphal_header.destination_node_id;
811+
out->version = frame->udp_cyphal_header.version;
812+
out->priority = (UdpardPriority) frame->udp_cyphal_header.priority;
813+
out-> source_node_id = frame->udp_cyphal_header.source_node_id;
814+
out->transfer_kind = getTransferKindFromDataSpecifier(frame->udp_cyphal_header.data_specifier);
815+
out->port_id = getPortIdFromDataSpecifiers(frame->udp_cyphal_header.data_specifier);
816+
out->destination_node_id = frame->udp_cyphal_header.destination_node_id;
818817
// Payload parsing.
819-
out->payload_size = frame->payload_size - sizeof(frame->udp_cyphal_header); // Cut off the header size.
820-
out->payload = (void*) ((uint8_t*) frame->payload + sizeof(frame->udp_cyphal_header));
821-
822-
out->transfer_id = frame->udp_cyphal_header.transfer_id;
823-
out->start_of_transfer = (((frame->udp_cyphal_header.frame_index_eot) & (UDPARD_MAX_FRAME_INDEX)) == 1);
824-
out->end_of_transfer = ((frame->udp_cyphal_header.frame_index_eot >> UDPARD_END_OF_TRANSFER_OFFSET) == 1);
825-
out->frame_index = frame->udp_cyphal_header.frame_index_eot;
818+
out->payload_size = frame->payload_size - sizeof(frame->udp_cyphal_header); // Cut off the header size.
819+
out->payload = (void*) ((uint8_t*) frame->payload + sizeof(frame->udp_cyphal_header));
820+
821+
out->transfer_id = frame->udp_cyphal_header.transfer_id;
822+
out->start_of_transfer = (((frame->udp_cyphal_header.frame_index_eot) & (UDPARD_MAX_FRAME_INDEX)) == 1);
823+
out->end_of_transfer = ((frame->udp_cyphal_header.frame_index_eot >> UDPARD_END_OF_TRANSFER_OFFSET) == 1);
824+
out->frame_index = frame->udp_cyphal_header.frame_index_eot;
825+
// Make sure header version is supported
826+
valid = valid && (out->version >= UDPARD_CYPHAL_HEADER_VERSION);
826827
if (out->transfer_kind != UdpardTransferKindMessage)
827828
{
828829
valid = valid && (out->source_node_id != out->destination_node_id);

‎libudpard/udpard.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ extern "C" {
3434
#define UDPARD_CYPHAL_SPECIFICATION_VERSION_MAJOR 1
3535
#define UDPARD_CYPHAL_SPECIFICATION_VERSION_MINOR 0
3636

37+
/// The version number of the Cyphal Header supported by this library
38+
#define UDPARD_CYPHAL_HEADER_VERSION 1
39+
3740
/// These error codes may be returned from the library API calls whose return type is a signed integer in the negated
3841
/// form (e.g., error code 2 returned as -2). A non-negative return value represents success.
3942
/// API calls whose return type is not a signed integer cannot fail by contract.
@@ -79,6 +82,7 @@ typedef uint16_t UdpardPortID;
7982
typedef uint16_t UdpardUdpPortID;
8083
typedef uint16_t UdpardNodeID;
8184
typedef uint64_t UdpardTransferID; /// TODO - This may need to be broken up for 32bit systems?
85+
typedef uint8_t UdpardHeaderVersion;
8286

8387
/// Transfer priority level mnemonics per the recommendations given in the Cyphal Specification.
8488
typedef enum

‎tests/ethard_config_private.h‎

Lines changed: 0 additions & 4 deletions
This file was deleted.

‎tests/exposed.hpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ struct RxSession
6161
struct RxFrameModel
6262
{
6363
UdpardMicrosecond timestamp_usec = std::numeric_limits<std::uint64_t>::max();
64+
UdpardHeaderVersion version = 1U;
6465
UdpardPriority priority = UdpardPriorityOptional;
6566
UdpardTransferKind transfer_kind = UdpardTransferKindMessage;
6667
UdpardPortID port_id = std::numeric_limits<std::uint16_t>::max();
@@ -89,9 +90,7 @@ auto txMakeMessageSessionSpecifier(const UdpardPortID subject_id,
8990
UdpardSessionSpecifier* const out_spec) -> std::uint32_t;
9091

9192
auto txMakeServiceSessionSpecifier(const UdpardPortID service_id,
92-
const bool request_not_response,
9393
const UdpardNodeID src_node_id,
94-
const UdpardNodeID dst_node_id,
9594
const UdpardIPv4Addr local_node_addr,
9695
UdpardSessionSpecifier* const out_spec) -> std::uint32_t;
9796

‎tests/test_private_rx.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ TEST_CASE("rxTryParseFrame")
195195

196196
// REQUEST
197197
REQUIRE(0 ==
198-
exposed::txMakeServiceSessionSpecifier(0b0000110011, true, 0b0100111, 0b0011010, 0xc0a80000, &specifier));
198+
exposed::txMakeServiceSessionSpecifier(0b0000110011, 0b0100111, 0xc0a80000, &specifier));
199199
REQUIRE(parse(999'999U,
200200
specifier,
201201
{
@@ -228,7 +228,7 @@ TEST_CASE("rxTryParseFrame")
228228
// SIMILAR BUT INVALID (Source Node ID cant be equal to Destination Node ID)
229229
REQUIRE(!parse(999'999U, specifier, {})); // NO HEADER
230230
REQUIRE(0 ==
231-
exposed::txMakeServiceSessionSpecifier(0b0000110011, true, 0b0100111, 0b0100111, 0xc0a80000, &specifier));
231+
exposed::txMakeServiceSessionSpecifier(0b0000110011, 0b0100111, 0xc0a80000, &specifier));
232232
REQUIRE(!parse(999'999U,
233233
specifier,
234234
{
@@ -246,7 +246,7 @@ TEST_CASE("rxTryParseFrame")
246246

247247
// RESPONSE
248248
REQUIRE(0 ==
249-
exposed::txMakeServiceSessionSpecifier(0b0000110011, false, 0b00011010, 0b0100111, 0xc0a80000, &specifier));
249+
exposed::txMakeServiceSessionSpecifier(0b0000110011, 0b00011010, 0xc0a80000, &specifier));
250250
REQUIRE(parse(888'888,
251251
specifier,
252252
{
@@ -276,7 +276,7 @@ TEST_CASE("rxTryParseFrame")
276276
REQUIRE(!parse(888'888U, specifier, {})); // NO TAIL BYTE
277277
REQUIRE(
278278
0 ==
279-
exposed::txMakeServiceSessionSpecifier(0b0000110011, false, 0b00011010, 0b00011010, 0xc0a80000, &specifier));
279+
exposed::txMakeServiceSessionSpecifier(0b0000110011, 0b00011010, 0xc0a80000, &specifier));
280280
REQUIRE(!parse(888'888,
281281
specifier,
282282
{

‎tests/test_private_tx.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,19 @@ TEST_CASE("SessionSpecifier")
1515
UdpardSessionSpecifier specifier = {};
1616
REQUIRE(0 == exposed::txMakeMessageSessionSpecifier(0b0110011001100, 0b0100111, 0xc0a80000, &specifier));
1717
REQUIRE(UDPARD_UDP_PORT == specifier.data_specifier);
18-
REQUIRE(0b11101111'00'10100'0'0'0001100'11001100 == specifier.destination_route_specifier);
18+
REQUIRE(0b11101111'0'0'00000'0'0'0001100'11001100 == specifier.destination_route_specifier);
1919
REQUIRE(0b11000000'10101000'00000000'00100111 == specifier.source_route_specifier);
2020
// Service Request
2121
REQUIRE(0 ==
22-
exposed::txMakeServiceSessionSpecifier(0b0100110011, true, 0b1010101, 0b0101010, 0xc0a80000, &specifier));
22+
exposed::txMakeServiceSessionSpecifier(0b0100110011, 0b1010101, 0xc0a80000, &specifier));
2323
REQUIRE(UDPARD_UDP_PORT == specifier.data_specifier);
24-
REQUIRE(0b11101111'00'10100'1'00000000'00101010 == specifier.destination_route_specifier);
24+
REQUIRE(0b11101111'0'0'00000'1'0'0000001'00110011 == specifier.destination_route_specifier);
2525
REQUIRE(0b11000000'10101000'00000000'01010101 == specifier.source_route_specifier);
2626
// Service Response
2727
REQUIRE(0 ==
28-
exposed::txMakeServiceSessionSpecifier(0b0100110011, false, 0b1010101, 0b0101010, 0xc0a80000, &specifier));
28+
exposed::txMakeServiceSessionSpecifier(0b0100110011, 0b1010101, 0xc0a80000, &specifier));
2929
REQUIRE(UDPARD_UDP_PORT == specifier.data_specifier);
30-
REQUIRE(0b11101111'00'10100'1'00000000'00101010 == specifier.destination_route_specifier);
30+
REQUIRE(0b11101111'0'0'00000'1'0'0000001'00110011 == specifier.destination_route_specifier);
3131
REQUIRE(0b11000000'10101000'00000000'01010101 == specifier.source_route_specifier);
3232
}
3333

@@ -75,7 +75,7 @@ TEST_CASE("txMakeSessionSpecifier")
7575
&specifier));
7676

7777
REQUIRE(UDPARD_UDP_PORT == specifier.data_specifier);
78-
REQUIRE(0b11101111'00'10100'0'0'0010011'00110011 == specifier.destination_route_specifier);
78+
REQUIRE(0b11101111'0'0'00000'0'0'0010011'00110011 == specifier.destination_route_specifier);
7979
REQUIRE(0b11000000'10101000'00000000'01010101 == specifier.source_route_specifier);
8080
REQUIRE(-UDPARD_ERROR_INVALID_ARGUMENT == // Bad subject-ID.
8181
txMakeSessionSpecifier(mk_meta(UdpardPriorityExceptional,

‎tests/test_public_rx.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,12 @@ TEST_CASE("RxAnonymous")
437437
REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 1); // The PAYLOAD BUFFER only! No session for anons.
438438
REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 10); // Smaller allocation.
439439
REQUIRE(ensureAllNullptr(ins.getMessageSubs().at(0)->sessions)); // No RX states!
440+
441+
// Version mismatch will be ignored
442+
header.version = 0;
443+
specifier.destination_route_specifier = 0b11101111'00'01000'0'0'000110011001100;
444+
specifier.source_route_specifier = 0b11000000'10101000'00000000'00000000;
445+
REQUIRE(0 == accept(0, 100'000'001, header, specifier, {1, 2, 3, 4, 5, 6, 171, 251, 77, 79}));
440446
}
441447

442448
TEST_CASE("RxSubscriptionErrors")

0 commit comments

Comments
 (0)