Skip to content

Commit c888d48

Browse files
SchoberMJSchoberMJ
authored
Merge pull request #2 from keningatamazon/main
Add unit tests for crcAdd() Fixes 32bit crcAdd function and adds unit tests in preparation for Multiframe Transfers.
2 parents 0a3a03f + 78df98c commit c888d48

File tree

4 files changed

+33
-13
lines changed

4 files changed

+33
-13
lines changed

‎libudpard/udpard.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ static const uint32_t CRCTable[256] =
139139

140140
UDPARD_PRIVATE TransferCRC crcAddByte(const TransferCRC crc, const uint8_t byte)
141141
{
142-
TransferCRC crc32c = (uint32_t) (crc ^ CRC_XOR);
142+
TransferCRC crc32c = (uint32_t) (crc != CRC_INITIAL ? (crc ^ CRC_XOR) : crc);
143143
crc32c = CRCTable[(uint32_t) ((uint32_t) (crc32c ^ byte) & BYTE_MAX)] ^ (crc32c >> BITS_PER_BYTE);
144144
return (uint32_t) (crc32c ^ CRC_XOR);
145145
}

‎tests/exposed.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
/// Please keep them in sync with the library by manually updating as necessary.
1515
namespace exposed
1616
{
17-
using TransferCRC = std::uint16_t;
17+
using TransferCRC = std::uint32_t;
1818

1919
struct TxItem final : UdpardTxQueueItem
2020
{
@@ -76,7 +76,7 @@ struct RxFrameModel
7676
// Extern C effectively discards the outer namespaces.
7777
extern "C" {
7878

79-
auto crcAdd(const std::uint16_t crc, const std::size_t size, const void* const bytes) -> std::uint16_t;
79+
auto crcAdd(const std::uint32_t crc, const std::size_t size, const void* const bytes) -> std::uint32_t;
8080

8181
auto txMakeMessageSessionSpecifier(const UdpardPortID subject_id,
8282
const UdpardNodeID src_node_id,

‎tests/test_private_crc.cpp‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,22 @@
11
// This software is distributed under the terms of the MIT License.
22
// Copyright (c) 2016-2020 OpenCyphal Development Team.
3+
// Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
4+
5+
#include "exposed.hpp"
6+
#include "catch.hpp"
7+
8+
TEST_CASE("TransferCRC")
9+
{
10+
using exposed::crcAdd;
11+
std::uint32_t crc = 0xFFFFFFFFU;
12+
13+
crc = crcAdd(crc, 1, "1");
14+
REQUIRE(0x90F599E3U == crc);
15+
crc = crcAdd(crc, 1, "2");
16+
REQUIRE(0x7355C460U == crc);
17+
crc = crcAdd(crc, 1, "3");
18+
REQUIRE(0x107B2FB2U == crc);
19+
20+
crc = crcAdd(crc, 6, "456789");
21+
REQUIRE(0xE3069283U == crc);
22+
}

‎tests/test_private_rx.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ TEST_CASE("rxSessionWritePayload")
339339
REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 0);
340340
REQUIRE(rxs.payload_size == 0);
341341
REQUIRE(rxs.payload == nullptr);
342-
REQUIRE(rxs.calculated_crc == 0xFFFFU);
342+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
343343
REQUIRE(rxs.transfer_id == 1);
344344

345345
// Double restart has no effect on memory.
@@ -350,7 +350,7 @@ TEST_CASE("rxSessionWritePayload")
350350
REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 0);
351351
REQUIRE(rxs.payload_size == 0);
352352
REQUIRE(rxs.payload == nullptr);
353-
REQUIRE(rxs.calculated_crc == 0xFFFFU);
353+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
354354
REQUIRE(rxs.transfer_id == 24U);
355355

356356
// Restart with a transfer-ID overflow.
@@ -361,7 +361,7 @@ TEST_CASE("rxSessionWritePayload")
361361
REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 0);
362362
REQUIRE(rxs.payload_size == 0);
363363
REQUIRE(rxs.payload == nullptr);
364-
REQUIRE(rxs.calculated_crc == 0xFFFFU);
364+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
365365
REQUIRE(rxs.transfer_id == 0U);
366366

367367
// Write into a zero-capacity storage. NULL at the output.
@@ -429,7 +429,7 @@ TEST_CASE("rxSessionUpdate")
429429
REQUIRE(rxs.transfer_timestamp_usec == 10'000'000);
430430
REQUIRE(rxs.payload_size == 0); // Handed over to the output transfer.
431431
REQUIRE(rxs.payload == nullptr); // Handed over to the output transfer.
432-
REQUIRE(rxs.calculated_crc == 0xFFFF);
432+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
433433
REQUIRE(rxs.transfer_id == 12U); // Incremented.
434434
REQUIRE(rxs.redundant_transport_index == 1);
435435
REQUIRE(transfer.timestamp_usec == 10'000'000);
@@ -452,7 +452,7 @@ TEST_CASE("rxSessionUpdate")
452452
REQUIRE(rxs.transfer_timestamp_usec == 10'000'000);
453453
REQUIRE(rxs.payload_size == 0); // Handed over to the output transfer.
454454
REQUIRE(rxs.payload == nullptr); // Handed over to the output transfer.
455-
REQUIRE(rxs.calculated_crc == 0xFFFF);
455+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
456456
REQUIRE(rxs.transfer_id == 12U); // Incremented.
457457
REQUIRE(rxs.redundant_transport_index == 1);
458458

@@ -463,7 +463,7 @@ TEST_CASE("rxSessionUpdate")
463463
REQUIRE(rxs.transfer_timestamp_usec == 10'000'050);
464464
REQUIRE(rxs.payload_size == 0);
465465
REQUIRE(rxs.payload == nullptr);
466-
REQUIRE(rxs.calculated_crc == 0xFFFF);
466+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
467467
REQUIRE(rxs.transfer_id == 13U);
468468
REQUIRE(rxs.redundant_transport_index == 1);
469469
REQUIRE(transfer.timestamp_usec == 10'000'050);
@@ -486,7 +486,7 @@ TEST_CASE("rxSessionUpdate")
486486
REQUIRE(rxs.transfer_timestamp_usec == 10'000'050);
487487
REQUIRE(rxs.payload_size == 0);
488488
REQUIRE(rxs.payload == nullptr);
489-
REQUIRE(rxs.calculated_crc == 0xFFFF);
489+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
490490
REQUIRE(rxs.transfer_id == 13U);
491491
REQUIRE(rxs.redundant_transport_index == 1);
492492

@@ -498,7 +498,7 @@ TEST_CASE("rxSessionUpdate")
498498
REQUIRE(rxs.transfer_timestamp_usec == 20'000'000);
499499
REQUIRE(rxs.payload_size == 0);
500500
REQUIRE(rxs.payload == nullptr);
501-
REQUIRE(rxs.calculated_crc == 0xFFFF);
501+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
502502
REQUIRE(rxs.transfer_id == 13U);
503503
REQUIRE(rxs.redundant_transport_index == 0);
504504
REQUIRE(transfer.timestamp_usec == 20'000'000);
@@ -524,7 +524,7 @@ TEST_CASE("rxSessionUpdate")
524524
REQUIRE(rxs.transfer_timestamp_usec == 20'000'000); // No change.
525525
REQUIRE(rxs.payload_size == 0);
526526
REQUIRE(rxs.payload == nullptr);
527-
REQUIRE(rxs.calculated_crc == 0xFFFF);
527+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
528528
REQUIRE(rxs.transfer_id == 13U);
529529
REQUIRE(rxs.redundant_transport_index == 2);
530530
REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 0);
@@ -541,7 +541,7 @@ TEST_CASE("rxSessionUpdate")
541541
REQUIRE(rxs.transfer_timestamp_usec == 20'000'200);
542542
REQUIRE(rxs.payload_size == 0);
543543
REQUIRE(rxs.payload == nullptr);
544-
REQUIRE(rxs.calculated_crc == 0xFFFF);
544+
REQUIRE(rxs.calculated_crc == 0xFFFFFFFFU);
545545
REQUIRE(rxs.transfer_id == 31U); // Reset.
546546
REQUIRE(rxs.redundant_transport_index == 2);
547547
REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 0);

0 commit comments

Comments
 (0)