From c9fc32fe1240d093f46515e9b0c07bf6ab197a0b Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 28 Aug 2020 22:19:27 +0300 Subject: [PATCH 01/22] Subject-ID range review https://github.com/UAVCAN/specification/issues/94 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9eaae32b..8ccaec47 100644 --- a/README.md +++ b/README.md @@ -138,7 +138,7 @@ But first, we need to subscribe: CanardRxSubscription heartbeat_subscription; (void) canardRxSubscribe(&ins, // Subscribe to messages uavcan.node.Heartbeat. CanardTransferKindMessage, - 32085, // The fixed Subject-ID of the Heartbeat message type (see DSDL definition). + 7509, // The fixed Subject-ID of the Heartbeat message type (see DSDL definition). 7, // The maximum payload size (max DSDL object size) from the DSDL definition. CANARD_DEFAULT_TRANSFER_ID_TIMEOUT_USEC, &heartbeat_subscription); From b5c859da9ac01899ed99986d2efe73682a8db914 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Fri, 28 Aug 2020 22:21:36 +0300 Subject: [PATCH 02/22] Subject-ID range review See https://github.com/UAVCAN/specification/issues/94 --- libcanard/canard.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcanard/canard.h b/libcanard/canard.h index 7861f51f..1428190c 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -130,7 +130,7 @@ extern "C" { #define CANARD_MTU_CAN_FD 64U /// Parameter ranges are inclusive; the lower bound is zero for all. See UAVCAN/CAN Specification for background. -#define CANARD_SUBJECT_ID_MAX 32767U +#define CANARD_SUBJECT_ID_MAX 8191U #define CANARD_SERVICE_ID_MAX 511U #define CANARD_NODE_ID_MAX 127U #define CANARD_PRIORITY_MAX 7U From 29772fe4827abf0bf4ad9556046b1129a221662a Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Mon, 14 Sep 2020 21:43:24 +0300 Subject: [PATCH 03/22] Update the CI to Ubuntu 20.04 Focal, GCC 10, CLang 10 --- .travis.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.travis.yml b/.travis.yml index d07cb99d..ac26c522 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,4 +1,4 @@ -dist: xenial +dist: focal env: - CTEST_OUTPUT_ON_FAILURE=1 @@ -15,13 +15,13 @@ matrix: sources: - ubuntu-toolchain-r-test packages: - - g++-9 - - g++-9-multilib - - gcc-9-multilib + - g++-10 + - g++-10-multilib + - gcc-10-multilib - linux-libc-dev:i386 env: - - CC=gcc-9 - - CXX=g++-9 + - CC=gcc-10 + - CXX=g++-10 script: # ANALYSIS # Using the build wrapper from Sonar and collecting the code coverage. @@ -29,9 +29,9 @@ matrix: - cmake tests -DCMAKE_BUILD_TYPE=Debug -DNO_STATIC_ANALYSIS=1 -DCMAKE_C_FLAGS='-DNDEBUG=1' - build-wrapper-linux-x86-64 --out-dir sonar-dump make all - make test - - gcov-9 --preserve-paths --long-file-names $(find CMakeFiles/test_private_cov.dir -name '*.gcno') - - gcov-9 --preserve-paths --long-file-names $(find CMakeFiles/test_private_le_cov.dir -name '*.gcno') - - gcov-9 --preserve-paths --long-file-names $(find CMakeFiles/test_public_cov.dir -name '*.gcno') + - gcov-10 --preserve-paths --long-file-names $(find CMakeFiles/test_private_cov.dir -name '*.gcno') + - gcov-10 --preserve-paths --long-file-names $(find CMakeFiles/test_private_le_cov.dir -name '*.gcno') + - gcov-10 --preserve-paths --long-file-names $(find CMakeFiles/test_public_cov.dir -name '*.gcno') - 'sonar-scanner -Dsonar.projectKey=libcanard -Dsonar.organization=uavcan -Dsonar.sources=libcanard @@ -57,35 +57,35 @@ matrix: sources: - ubuntu-toolchain-r-test packages: # Install a newer GCC because https://stackoverflow.com/a/51512150/1007777. - - g++-9 - - g++-9-multilib - - gcc-9-multilib + - g++-10 + - g++-10-multilib + - gcc-10-multilib - linux-libc-dev:i386 - libc6-dev-i386 - libstdc++-7-dev:i386 script: # Set up the toolchain. - wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && sudo ./llvm.sh 9 - - sudo apt install clang-tidy-9 clang-format-9 - - clang++-9 -E -x c++ - -v < /dev/null # Print the Clang configuration for troubleshooting purposes. + - sudo apt install clang-tidy-10 clang-format-10 + - clang++-10 -E -x c++ - -v < /dev/null # Print the Clang configuration for troubleshooting purposes. # DEBUG - - cmake -DCMAKE_C_COMPILER=clang-9 -DCMAKE_CXX_COMPILER=clang++-9 tests -DCMAKE_BUILD_TYPE=Debug + - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=Debug - make VERBOSE=1 && make test - make clean # RELEASE - - cmake -DCMAKE_C_COMPILER=clang-9 -DCMAKE_CXX_COMPILER=clang++-9 tests -DCMAKE_BUILD_TYPE=Release + - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=Release - make VERBOSE=1 && make test - make clean # RELWITHDEBINFO - - cmake -DCMAKE_C_COMPILER=clang-9 -DCMAKE_CXX_COMPILER=clang++-9 tests -DCMAKE_BUILD_TYPE=RelWithDebInfo + - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=RelWithDebInfo - make VERBOSE=1 && make test - make clean # MINSIZEREL - - cmake -DCMAKE_C_COMPILER=clang-9 -DCMAKE_CXX_COMPILER=clang++-9 tests -DCMAKE_BUILD_TYPE=MinSizeRel + - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=MinSizeRel - make VERBOSE=1 && make test - make clean From 3959615e3da700cdd01d54d3523328d2b853aca3 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Mon, 14 Sep 2020 21:44:19 +0300 Subject: [PATCH 04/22] Fix the build against CLang-10 --- tests/exposed.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/exposed.hpp b/tests/exposed.hpp index 07d1a513..3e296b30 100644 --- a/tests/exposed.hpp +++ b/tests/exposed.hpp @@ -7,6 +7,7 @@ #include "catch.hpp" #include #include +#include /// Definitions that are not exposed by the library but that are needed for testing. /// Please keep them in sync with the library by manually updating as necessary. From 1bcd2a2a8f63f5ef6d75be8c236e75aed4e160d0 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Mon, 14 Sep 2020 21:46:36 +0300 Subject: [PATCH 05/22] Bump the version to v1.0 --- libcanard/canard.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcanard/canard.h b/libcanard/canard.h index 1428190c..eef1c20b 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -108,8 +108,8 @@ extern "C" { /// Semantic version of this library (not the UAVCAN specification). /// API will be backward compatible within the same major version. -#define CANARD_VERSION_MAJOR 0 -#define CANARD_VERSION_MINOR 100 +#define CANARD_VERSION_MAJOR 1 +#define CANARD_VERSION_MINOR 0 /// The version number of the UAVCAN specification implemented by this library. #define CANARD_UAVCAN_SPECIFICATION_VERSION_MAJOR 1 From 428004cf334f232d76468f5a80b8b774ff111cf9 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 00:47:07 +0300 Subject: [PATCH 06/22] Fix the unit tests --- .idea/dictionaries/pavel.xml | 1 + libcanard/canard.c | 3 ++- tests/test_private_rx.cpp | 47 ++++++++++++++++++--------------- tests/test_private_tx.cpp | 22 +++++++-------- tests/test_public_roundtrip.cpp | 2 +- tests/test_public_rx.cpp | 32 +++++++++++----------- tests/test_public_tx.cpp | 2 +- 7 files changed, 57 insertions(+), 52 deletions(-) diff --git a/.idea/dictionaries/pavel.xml b/.idea/dictionaries/pavel.xml index f54ad737..da8c8e2f 100644 --- a/.idea/dictionaries/pavel.xml +++ b/.idea/dictionaries/pavel.xml @@ -66,6 +66,7 @@ storages submoduling subtreeing + supremum uavcan uint unicast diff --git a/libcanard/canard.c b/libcanard/canard.c index a3dfcd07..f83be56f 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -119,7 +119,8 @@ CANARD_PRIVATE uint32_t txMakeMessageSessionSpecifier(const CanardPortID subject { CANARD_ASSERT(src_node_id <= CANARD_NODE_ID_MAX); CANARD_ASSERT(subject_id <= CANARD_SUBJECT_ID_MAX); - return src_node_id | ((uint32_t) subject_id << OFFSET_SUBJECT_ID); + const uint32_t tmp = subject_id | (CANARD_SUBJECT_ID_MAX + 1) | ((CANARD_SUBJECT_ID_MAX + 1) * 2); + return src_node_id | (tmp << OFFSET_SUBJECT_ID); } CANARD_PRIVATE uint32_t txMakeServiceSessionSpecifier(const CanardPortID service_id, diff --git a/tests/test_private_rx.cpp b/tests/test_private_rx.cpp index da514914..52d036ee 100644 --- a/tests/test_private_rx.cpp +++ b/tests/test_private_rx.cpp @@ -52,11 +52,11 @@ TEST_CASE("rxTryParseFrame") REQUIRE(!parse(543210U, 0U, {0, 1, 2, 3, 4, 5, 6})); // MFT NON-LAST FRAME PAYLOAD CAN'T BE SHORTER THAN 7 // MESSAGE - REQUIRE(parse(123456U, 0b001'00'0'110110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); + REQUIRE(parse(123456U, 0b001'00'0'11'0110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); REQUIRE(model.timestamp_usec == 123456U); REQUIRE(model.priority == CanardPriorityImmediate); REQUIRE(model.transfer_kind == CanardTransferKindMessage); - REQUIRE(model.port_id == 0b110110011001100U); + REQUIRE(model.port_id == 0b0110011001100U); REQUIRE(model.source_node_id == 0b0100111U); REQUIRE(model.destination_node_id == CANARD_NODE_ID_UNSET); REQUIRE(model.transfer_id == 23U); @@ -73,24 +73,24 @@ TEST_CASE("rxTryParseFrame") REQUIRE(model.payload[6] == 6); // SIMILAR BUT INVALID // NO TAIL BYTE - REQUIRE(!parse(123456U, 0b001'00'0'110110011001100'0'0100111U, {})); + REQUIRE(!parse(123456U, 0b001'00'0'11'0110011001100'0'0100111U, {})); // BAD TOGGLE - REQUIRE(!parse(123456U, 0b001'00'0'110110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b100'00000U | 23U})); + REQUIRE(!parse(123456U, 0b001'00'0'11'0110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b100'00000U | 23U})); // BAD RESERVED R07 - REQUIRE(!parse(123456U, 0b001'00'0'110110011001100'1'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); + REQUIRE(!parse(123456U, 0b001'00'0'11'0110011001100'1'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); // BAD RESERVED R23 - REQUIRE(!parse(123456U, 0b001'00'1'110110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); + REQUIRE(!parse(123456U, 0b001'00'1'11'0110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); // BAD RESERVED R07 R23 - REQUIRE(!parse(123456U, 0b001'00'1'110110011001100'1'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); + REQUIRE(!parse(123456U, 0b001'00'1'11'0110011001100'1'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); // ANON NOT SINGLE FRAME - REQUIRE(!parse(123456U, 0b001'01'0'110110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); + REQUIRE(!parse(123456U, 0b001'01'0'11'0110011001100'0'0100111U, {0, 1, 2, 3, 4, 5, 6, 0b101'00000U | 23U})); // ANONYMOUS MESSAGE - REQUIRE(parse(12345U, 0b010'01'0'000110011001101'0'0100111U, {0b111'00000U | 0U})); + REQUIRE(parse(12345U, 0b010'01'0'00'0110011001101'0'0100111U, {0b111'00000U | 0U})); REQUIRE(model.timestamp_usec == 12345U); REQUIRE(model.priority == CanardPriorityFast); REQUIRE(model.transfer_kind == CanardTransferKindMessage); - REQUIRE(model.port_id == 0b000110011001101U); + REQUIRE(model.port_id == 0b0110011001101U); REQUIRE(model.source_node_id == CANARD_NODE_ID_UNSET); REQUIRE(model.destination_node_id == CANARD_NODE_ID_UNSET); REQUIRE(model.transfer_id == 0U); @@ -98,12 +98,15 @@ TEST_CASE("rxTryParseFrame") REQUIRE(model.end_of_transfer); REQUIRE(model.toggle); REQUIRE(model.payload_size == 0); + // SAME BUT RESERVED 21 22 SET (and ignored) + REQUIRE(parse(12345U, 0b010'01'0'11'0110011001101'0'0100111U, {0b111'00000U | 0U})); + REQUIRE(model.port_id == 0b0110011001101U); // SIMILAR BUT INVALID - REQUIRE(!parse(12345U, 0b010'01'0'110110011001100'0'0100111U, {})); // NO TAIL BYTE - REQUIRE(!parse(12345U, 0b010'01'0'110110011001100'0'0100111U, {0b110'00000U | 0U})); // BAD TOGGLE - REQUIRE(!parse(12345U, 0b010'01'0'110110011001100'1'0100111U, {0b111'00000U | 0U})); // BAD RESERVED 07 - REQUIRE(!parse(12345U, 0b010'01'1'110110011001100'0'0100111U, {0b111'00000U | 0U})); // BAD RESERVED 23 - REQUIRE(!parse(12345U, 0b010'01'1'110110011001100'1'0100111U, {0b111'00000U | 0U})); // BAD RESERVED 07 23 + REQUIRE(!parse(12345U, 0b010'01'0'11'0110011001100'0'0100111U, {})); // NO TAIL BYTE + REQUIRE(!parse(12345U, 0b010'01'0'11'0110011001100'0'0100111U, {0b110'00000U | 0U})); // BAD TOGGLE + REQUIRE(!parse(12345U, 0b010'01'0'11'0110011001100'1'0100111U, {0b111'00000U | 0U})); // BAD RESERVED 07 + REQUIRE(!parse(12345U, 0b010'01'1'11'0110011001100'0'0100111U, {0b111'00000U | 0U})); // BAD RESERVED 23 + REQUIRE(!parse(12345U, 0b010'01'1'11'0110011001100'1'0100111U, {0b111'00000U | 0U})); // BAD RESERVED 07 23 // REQUEST REQUIRE(parse(999'999U, 0b011'11'0000110011'0011010'0100111U, {0, 1, 2, 3, 0b011'00000U | 31U})); @@ -292,7 +295,7 @@ TEST_CASE("rxSessionUpdate") frame.timestamp_usec = 10'000'000; frame.priority = CanardPrioritySlow; frame.transfer_kind = CanardTransferKindMessage; - frame.port_id = 22'222; + frame.port_id = 2'222; frame.source_node_id = 55; frame.destination_node_id = CANARD_NODE_ID_UNSET; frame.transfer_id = 11; @@ -334,7 +337,7 @@ TEST_CASE("rxSessionUpdate") REQUIRE(transfer.timestamp_usec == 10'000'000); REQUIRE(transfer.priority == CanardPrioritySlow); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 22'222); + REQUIRE(transfer.port_id == 2'222); REQUIRE(transfer.remote_node_id == 55); REQUIRE(transfer.transfer_id == 11); REQUIRE(transfer.payload_size == 3); @@ -370,7 +373,7 @@ TEST_CASE("rxSessionUpdate") REQUIRE(transfer.timestamp_usec == 10'000'050); REQUIRE(transfer.priority == CanardPrioritySlow); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 22'222); + REQUIRE(transfer.port_id == 2'222); REQUIRE(transfer.remote_node_id == 55); REQUIRE(transfer.transfer_id == 12); REQUIRE(transfer.payload_size == 3); @@ -407,7 +410,7 @@ TEST_CASE("rxSessionUpdate") REQUIRE(transfer.timestamp_usec == 20'000'000); REQUIRE(transfer.priority == CanardPrioritySlow); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 22'222); + REQUIRE(transfer.port_id == 2'222); REQUIRE(transfer.remote_node_id == 55); REQUIRE(transfer.transfer_id == 12); REQUIRE(transfer.payload_size == 3); @@ -485,7 +488,7 @@ TEST_CASE("rxSessionUpdate") REQUIRE(transfer.timestamp_usec == 20'000'100); REQUIRE(transfer.priority == CanardPrioritySlow); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 22'222); + REQUIRE(transfer.port_id == 2'222); REQUIRE(transfer.remote_node_id == 55); REQUIRE(transfer.transfer_id == 13); REQUIRE(transfer.payload_size == 16); @@ -570,7 +573,7 @@ TEST_CASE("rxSessionUpdate") REQUIRE(transfer.timestamp_usec == 20'000'200); REQUIRE(transfer.priority == CanardPrioritySlow); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 22'222); + REQUIRE(transfer.port_id == 2'222); REQUIRE(transfer.remote_node_id == 55); REQUIRE(transfer.transfer_id == 11); REQUIRE(transfer.payload_size == 10); @@ -617,7 +620,7 @@ TEST_CASE("rxSessionUpdate") REQUIRE(transfer.timestamp_usec == 30'000'000); REQUIRE(transfer.priority == CanardPrioritySlow); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 22'222); + REQUIRE(transfer.port_id == 2'222); REQUIRE(transfer.remote_node_id == 55); REQUIRE(transfer.transfer_id == 0); REQUIRE(transfer.payload_size == 7); // ONE CRC BYTE BACKTRACKED! diff --git a/tests/test_private_tx.cpp b/tests/test_private_tx.cpp index 983d7da0..a904b866 100644 --- a/tests/test_private_tx.cpp +++ b/tests/test_private_tx.cpp @@ -6,8 +6,8 @@ TEST_CASE("SessionSpecifier") { - REQUIRE(0b000'00'0011001100110011'0'1010101 == - exposed::txMakeMessageSessionSpecifier(0b0011001100110011, 0b1010101)); + REQUIRE(0b000'00'0'11'1001100110011'0'1010101 == + exposed::txMakeMessageSessionSpecifier(0b1001100110011, 0b1010101)); REQUIRE(0b000'11'0100110011'0101010'1010101 == exposed::txMakeServiceSessionSpecifier(0b0100110011, true, 0b1010101, 0b0101010)); REQUIRE(0b000'10'0100110011'1010101'0101010 == @@ -59,24 +59,24 @@ TEST_CASE("txMakeCANID") }; // MESSAGE TRANSFERS - REQUIRE(0b000'00'0011001100110011'0'1010101 == // Regular message. + REQUIRE(0b000'00'0'11'1001100110011'0'1010101 == // Regular message. txMakeCANID(mk_transfer(CanardPriorityExceptional, CanardTransferKindMessage, - 0b0011001100110011, + 0b1001100110011, CANARD_NODE_ID_UNSET), 0b1010101, 7U)); - REQUIRE(0b111'00'0011001100110011'0'1010101 == // Regular message. + REQUIRE(0b111'00'0'11'1001100110011'0'1010101 == // Regular message. txMakeCANID(mk_transfer(CanardPriorityOptional, CanardTransferKindMessage, - 0b0011001100110011, + 0b1001100110011, CANARD_NODE_ID_UNSET), 0b1010101, 7U)); - REQUIRE((0b010'01'0011001100110011'0'0000000U | (crc123 & CANARD_NODE_ID_MAX)) == // Anonymous message. + REQUIRE((0b010'01'0'11'1001100110011'0'0000000U | (crc123 & CANARD_NODE_ID_MAX)) == // Anonymous message. txMakeCANID(mk_transfer(CanardPriorityFast, CanardTransferKindMessage, - 0b0011001100110011, + 0b1001100110011, CANARD_NODE_ID_UNSET, {1, 2, 3}), 128U, // Invalid local node-ID. @@ -84,20 +84,20 @@ TEST_CASE("txMakeCANID") REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == // Multi-frame anonymous messages are not allowed. txMakeCANID(mk_transfer(CanardPriorityImmediate, CanardTransferKindMessage, - 0b0011001100110011, + 0b1001100110011, CANARD_NODE_ID_UNSET, {1, 2, 3, 4, 5, 6, 7, 8}), 128U, // Invalid local node-ID is treated as anonymous/unset. 7U)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == // Bad remote node-ID -- unicast messages not supported. - txMakeCANID(mk_transfer(CanardPriorityHigh, CanardTransferKindMessage, 0b0011001100110011, 123U), 0U, 7U)); + txMakeCANID(mk_transfer(CanardPriorityHigh, CanardTransferKindMessage, 0b1001100110011, 123U), 0U, 7U)); REQUIRE( -CANARD_ERROR_INVALID_ARGUMENT == // Bad subject-ID. txMakeCANID(mk_transfer(CanardPriorityLow, CanardTransferKindMessage, 0xFFFFU, CANARD_NODE_ID_UNSET), 0U, 7U)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == // Bad priority. txMakeCANID(mk_transfer(PriorityAlias{123}.prio, CanardTransferKindMessage, - 0b0011001100110011, + 0b1001100110011, CANARD_NODE_ID_UNSET), 0b1010101, 7U)); diff --git a/tests/test_public_roundtrip.cpp b/tests/test_public_roundtrip.cpp index efe51485..1f16e2d6 100644 --- a/tests/test_public_roundtrip.cpp +++ b/tests/test_public_roundtrip.cpp @@ -35,7 +35,7 @@ TEST_CASE("RoundtripSimple") return static_cast(getRandomNatural(CANARD_PRIORITY_MAX + 1U)); }; std::array tx_states{ - TxState{CanardTransferKindMessage, get_random_priority(), 32767, 1000}, + TxState{CanardTransferKindMessage, get_random_priority(), 8191, 1000}, TxState{CanardTransferKindMessage, get_random_priority(), 511, 0}, TxState{CanardTransferKindMessage, get_random_priority(), 0, 13}, TxState{CanardTransferKindRequest, get_random_priority(), 511, 900}, diff --git a/tests/test_public_rx.cpp b/tests/test_public_rx.cpp index 5d012820..6b41f2a6 100644 --- a/tests/test_public_rx.cpp +++ b/tests/test_public_rx.cpp @@ -35,15 +35,15 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[2] == nullptr); // A valid single-frame transfer for which there is no subscription. - REQUIRE(0 == accept(0, 100'000'000, 0b001'00'0'110110011001100'0'0100111, {0b111'00000})); + REQUIRE(0 == accept(0, 100'000'000, 0b001'00'0'11'0110011001100'0'0100111, {0b111'00000})); // Create a message subscription. CanardRxSubscription sub_msg{}; - REQUIRE(1 == ins.rxSubscribe(CanardTransferKindMessage, 0b110110011001100, 32, 2'000'000, sub_msg)); // New. - REQUIRE(0 == ins.rxSubscribe(CanardTransferKindMessage, 0b110110011001100, 16, 1'000'000, sub_msg)); // Replaced. + REQUIRE(1 == ins.rxSubscribe(CanardTransferKindMessage, 0b0110011001100, 32, 2'000'000, sub_msg)); // New. + REQUIRE(0 == ins.rxSubscribe(CanardTransferKindMessage, 0b0110011001100, 16, 1'000'000, sub_msg)); // Replaced. REQUIRE(ins.getInstance()._rx_subscriptions[0] == &sub_msg); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_next == nullptr); - REQUIRE(ins.getInstance()._rx_subscriptions[0]->_port_id == 0b110110011001100); + REQUIRE(ins.getInstance()._rx_subscriptions[0]->_port_id == 0b0110011001100); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_payload_size_max == 16); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_transfer_id_timeout_usec == 1'000'000); for (auto _session : ins.getInstance()._rx_subscriptions[0]->_sessions) @@ -99,11 +99,11 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[2] == &sub_req); // Accepted message. - REQUIRE(1 == accept(0, 100'000'001, 0b001'00'0'110110011001100'0'0100111, {0b111'00000})); + REQUIRE(1 == accept(0, 100'000'001, 0b001'00'0'11'0110011001100'0'0100111, {0b111'00000})); REQUIRE(transfer.timestamp_usec == 100'000'001); REQUIRE(transfer.priority == CanardPriorityImmediate); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 0b110110011001100); + REQUIRE(transfer.port_id == 0b0110011001100); REQUIRE(transfer.remote_node_id == 0b0100111); REQUIRE(transfer.transfer_id == 0); REQUIRE(transfer.payload_size == 0); @@ -155,8 +155,8 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == (3 * sizeof(RxSession) + 16 + 20)); // Destroy the message subscription and the buffer to free up memory. - REQUIRE(1 == ins.rxUnsubscribe(CanardTransferKindMessage, 0b110110011001100)); - REQUIRE(0 == ins.rxUnsubscribe(CanardTransferKindMessage, 0b110110011001100)); // Repeat, nothing to do. + REQUIRE(1 == ins.rxUnsubscribe(CanardTransferKindMessage, 0b0110011001100)); + REQUIRE(0 == ins.rxUnsubscribe(CanardTransferKindMessage, 0b0110011001100)); // Repeat, nothing to do. REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 4); REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == (2 * sizeof(RxSession) + 16 + 20)); ins.getAllocator().deallocate(msg_payload); @@ -215,21 +215,21 @@ TEST_CASE("RxAnonymous") ins.getAllocator().setAllocationCeiling(16); // A valid anonymous transfer for which there is no subscription. - REQUIRE(0 == accept(0, 100'000'000, 0b001'01'0'110110011001100'0'0100111, {0b111'00000})); + REQUIRE(0 == accept(0, 100'000'000, 0b001'01'0'11'0110011001100'0'0100111, {0b111'00000})); // Create a message subscription. CanardRxSubscription sub_msg{}; - REQUIRE(1 == ins.rxSubscribe(CanardTransferKindMessage, 0b110110011001100, 16, 2'000'000, sub_msg)); // New. + REQUIRE(1 == ins.rxSubscribe(CanardTransferKindMessage, 0b0110011001100, 16, 2'000'000, sub_msg)); // New. // Accepted anonymous message. REQUIRE(1 == accept(0, 100'000'001, - 0b001'01'0'110110011001100'0'0100111, // + 0b001'01'0'11'0110011001100'0'0100111, // {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 0b111'00000})); REQUIRE(transfer.timestamp_usec == 100'000'001); REQUIRE(transfer.priority == CanardPriorityImmediate); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 0b110110011001100); + REQUIRE(transfer.port_id == 0b0110011001100); REQUIRE(transfer.remote_node_id == CANARD_NODE_ID_UNSET); REQUIRE(transfer.transfer_id == 0); REQUIRE(transfer.payload_size == 16); // Truncated. @@ -243,11 +243,11 @@ TEST_CASE("RxAnonymous") // Anonymous message not accepted because OOM. The transfer shall remain unmodified by the call, so we re-check it. REQUIRE(-CANARD_ERROR_OUT_OF_MEMORY == - accept(0, 100'000'001, 0b001'01'0'110110011001100'0'0100111, {3, 2, 1, 0b111'00000})); + accept(0, 100'000'001, 0b001'01'0'11'0110011001100'0'0100111, {3, 2, 1, 0b111'00000})); REQUIRE(transfer.timestamp_usec == 100'000'001); REQUIRE(transfer.priority == CanardPriorityImmediate); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 0b110110011001100); + REQUIRE(transfer.port_id == 0b0110011001100); REQUIRE(transfer.remote_node_id == CANARD_NODE_ID_UNSET); REQUIRE(transfer.transfer_id == 0); REQUIRE(transfer.payload_size == 16); // Truncated. @@ -265,11 +265,11 @@ TEST_CASE("RxAnonymous") REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 0); // Accepted anonymous message with small payload. - REQUIRE(1 == accept(0, 100'000'001, 0b001'01'0'110110011001100'0'0100111, {1, 2, 3, 4, 5, 6, 0b111'00000})); + REQUIRE(1 == accept(0, 100'000'001, 0b001'01'0'11'0110011001100'0'0100111, {1, 2, 3, 4, 5, 6, 0b111'00000})); REQUIRE(transfer.timestamp_usec == 100'000'001); REQUIRE(transfer.priority == CanardPriorityImmediate); REQUIRE(transfer.transfer_kind == CanardTransferKindMessage); - REQUIRE(transfer.port_id == 0b110110011001100); + REQUIRE(transfer.port_id == 0b0110011001100); REQUIRE(transfer.remote_node_id == CANARD_NODE_ID_UNSET); REQUIRE(transfer.transfer_id == 0); REQUIRE(transfer.payload_size == 6); // NOT truncated. diff --git a/tests/test_public_tx.cpp b/tests/test_public_tx.cpp index 3deca819..0c162d40 100644 --- a/tests/test_public_tx.cpp +++ b/tests/test_public_tx.cpp @@ -75,7 +75,7 @@ TEST_CASE("TxBasic0") // Check the TX queue. { - auto q = ins.getTxQueueRoot(); + const auto* q = ins.getTxQueueRoot(); REQUIRE(q != nullptr); REQUIRE(q->frame.timestamp_usec == 1'000'000'000'000ULL); REQUIRE(q->frame.payload_size == 12); From 0d4ee50cc205c5f593f108c076d68495a71ff0e6 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 01:20:28 +0300 Subject: [PATCH 07/22] Tooling update --- .travis.yml | 6 ++---- tests/CMakeLists.txt | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index ac26c522..358748a0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -50,7 +50,7 @@ matrix: - make all VERBOSE=1 && make test - make clean - # -------------------- Clang 9 -------------------- + # -------------------- Clang -------------------- - language: cpp addons: apt: @@ -61,11 +61,9 @@ matrix: - g++-10-multilib - gcc-10-multilib - linux-libc-dev:i386 - - libc6-dev-i386 - - libstdc++-7-dev:i386 script: # Set up the toolchain. - - wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && sudo ./llvm.sh 9 + - wget https://apt.llvm.org/llvm.sh && chmod +x llvm.sh && sudo ./llvm.sh 10 - sudo apt install clang-tidy-10 clang-format-10 - clang++-10 -E -x c++ - -v < /dev/null # Print the Clang configuration for troubleshooting purposes. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 094b7ff8..9d6f9e6b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,7 +16,7 @@ set(library_dir "${CMAKE_SOURCE_DIR}/../libcanard") # If not suppressed, the tools used here shall be available, otherwise the build will fail. if (NOT NO_STATIC_ANALYSIS) # clang-tidy (separate config files per directory) - find_program(clang_tidy NAMES clang-tidy-10 clang-tidy-9 clang-tidy) + find_program(clang_tidy NAMES clang-tidy-12 clang-tidy-11 clang-tidy-10 clang-tidy) if (NOT clang_tidy) message(FATAL_ERROR "Could not locate clang-tidy") endif () @@ -25,7 +25,7 @@ if (NOT NO_STATIC_ANALYSIS) set(CMAKE_CXX_CLANG_TIDY ${clang_tidy}) # clang-format - find_program(clang_format NAMES clang-format-10 clang-format-9 clang-format) + find_program(clang_format NAMES clang-format-12 clang-format-11 clang-format-10 clang-format) if (NOT clang_format) message(FATAL_ERROR "Could not locate clang-format") endif () From c9b1629356e88a906baec09b35690cf75e6af5be Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 01:24:53 +0300 Subject: [PATCH 08/22] Terminology update per Specification v1.0-beta: use 'extent'; update the docs accordingly --- libcanard/canard.c | 45 ++++++++++++++++----------------- libcanard/canard.h | 26 ++++++++++--------- tests/exposed.hpp | 4 +-- tests/helpers.hpp | 9 ++----- tests/test_private_rx.cpp | 4 +-- tests/test_public_roundtrip.cpp | 8 +++--- tests/test_public_rx.cpp | 8 +++--- 7 files changed, 50 insertions(+), 54 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index f83be56f..10c6e6fa 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -638,28 +638,28 @@ CANARD_PRIVATE uint8_t rxComputeTransferIDDifference(const uint8_t a, const uint CANARD_PRIVATE int8_t rxSessionWritePayload(CanardInstance* const ins, CanardInternalRxSession* const rxs, - const size_t payload_size_max, + const size_t extent, const size_t payload_size, const void* const payload); CANARD_PRIVATE int8_t rxSessionWritePayload(CanardInstance* const ins, CanardInternalRxSession* const rxs, - const size_t payload_size_max, + const size_t extent, const size_t payload_size, const void* const payload) { CANARD_ASSERT(ins != NULL); CANARD_ASSERT(rxs != NULL); CANARD_ASSERT((payload != NULL) || (payload_size == 0U)); - CANARD_ASSERT(rxs->payload_size <= payload_size_max); // This invariant is enforced by the subscription logic. + CANARD_ASSERT(rxs->payload_size <= extent); // This invariant is enforced by the subscription logic. CANARD_ASSERT(rxs->payload_size <= rxs->total_payload_size); rxs->total_payload_size += payload_size; // Allocate the payload lazily, as late as possible. - if ((NULL == rxs->payload) && (payload_size_max > 0U)) + if ((NULL == rxs->payload) && (extent > 0U)) { CANARD_ASSERT(rxs->payload_size == 0); - rxs->payload = ins->memory_allocate(ins, payload_size_max); + rxs->payload = ins->memory_allocate(ins, extent); } int8_t out = 0; @@ -667,11 +667,11 @@ CANARD_PRIVATE int8_t rxSessionWritePayload(CanardInstance* const ins, { // Copy the payload into the contiguous buffer. Apply the implicit truncation rule if necessary. size_t bytes_to_copy = payload_size; - if ((rxs->payload_size + bytes_to_copy) > payload_size_max) + if ((rxs->payload_size + bytes_to_copy) > extent) { - CANARD_ASSERT(rxs->payload_size <= payload_size_max); - bytes_to_copy = payload_size_max - rxs->payload_size; - CANARD_ASSERT((rxs->payload_size + bytes_to_copy) == payload_size_max); + CANARD_ASSERT(rxs->payload_size <= extent); + bytes_to_copy = extent - rxs->payload_size; + CANARD_ASSERT((rxs->payload_size + bytes_to_copy) == extent); CANARD_ASSERT(bytes_to_copy < payload_size); } // This memcpy() call here is one of the two variable-complexity operations in the RX pipeline; @@ -682,12 +682,12 @@ CANARD_PRIVATE int8_t rxSessionWritePayload(CanardInstance* const ins, // We ignore it because the safe functions are poorly supported; reliance on them may limit the portability. (void) memcpy(&rxs->payload[rxs->payload_size], payload, bytes_to_copy); // NOLINT NOSONAR rxs->payload_size += bytes_to_copy; - CANARD_ASSERT(rxs->payload_size <= payload_size_max); + CANARD_ASSERT(rxs->payload_size <= extent); } else { CANARD_ASSERT(rxs->payload_size == 0); - out = (payload_size_max > 0U) ? -CANARD_ERROR_OUT_OF_MEMORY : 0; + out = (extent > 0U) ? -CANARD_ERROR_OUT_OF_MEMORY : 0; } CANARD_ASSERT(out <= 0); return out; @@ -711,12 +711,12 @@ CANARD_PRIVATE void rxSessionRestart(CanardInstance* const ins, CanardInternalRx CANARD_PRIVATE int8_t rxSessionAcceptFrame(CanardInstance* const ins, CanardInternalRxSession* const rxs, const RxFrameModel* const frame, - const size_t payload_size_max, + const size_t extent, CanardTransfer* const out_transfer); CANARD_PRIVATE int8_t rxSessionAcceptFrame(CanardInstance* const ins, CanardInternalRxSession* const rxs, const RxFrameModel* const frame, - const size_t payload_size_max, + const size_t extent, CanardTransfer* const out_transfer) { CANARD_ASSERT(ins != NULL); @@ -739,7 +739,7 @@ CANARD_PRIVATE int8_t rxSessionAcceptFrame(CanardInstance* const ins, rxs->calculated_crc = crcAdd(rxs->calculated_crc, frame->payload_size, frame->payload); } - int8_t out = rxSessionWritePayload(ins, rxs, payload_size_max, frame->payload_size, frame->payload); + int8_t out = rxSessionWritePayload(ins, rxs, extent, frame->payload_size, frame->payload); if (out < 0) { CANARD_ASSERT(-CANARD_ERROR_OUT_OF_MEMORY == out); @@ -788,14 +788,14 @@ CANARD_PRIVATE int8_t rxSessionUpdate(CanardInstance* const ins, const RxFrameModel* const frame, const uint8_t redundant_transport_index, const CanardMicrosecond transfer_id_timeout_usec, - const size_t payload_size_max, + const size_t extent, CanardTransfer* const out_transfer); CANARD_PRIVATE int8_t rxSessionUpdate(CanardInstance* const ins, CanardInternalRxSession* const rxs, const RxFrameModel* const frame, const uint8_t redundant_transport_index, const CanardMicrosecond transfer_id_timeout_usec, - const size_t payload_size_max, + const size_t extent, CanardTransfer* const out_transfer) { CANARD_ASSERT(ins != NULL); @@ -835,7 +835,7 @@ CANARD_PRIVATE int8_t rxSessionUpdate(CanardInstance* const ins, const bool correct_tid = (frame->transfer_id == rxs->transfer_id); if (correct_transport && correct_toggle && correct_tid) { - out = rxSessionAcceptFrame(ins, rxs, frame, payload_size_max, out_transfer); + out = rxSessionAcceptFrame(ins, rxs, frame, extent, out_transfer); } } return out; @@ -896,7 +896,7 @@ CANARD_PRIVATE int8_t rxAcceptFrame(CanardInstance* const ins, frame, redundant_transport_index, subscription->_transfer_id_timeout_usec, - subscription->_payload_size_max, + subscription->_extent, out_transfer); } } @@ -906,9 +906,8 @@ CANARD_PRIVATE int8_t rxAcceptFrame(CanardInstance* const ins, // Anonymous transfers are stateless. No need to update the state machine, just blindly accept it. // We have to copy the data into an allocated storage because the API expects it: the lifetime shall be // independent of the input data and the memory shall be free-able. - const size_t payload_size = (subscription->_payload_size_max < frame->payload_size) - ? subscription->_payload_size_max - : frame->payload_size; + const size_t payload_size = + (subscription->_extent < frame->payload_size) ? subscription->_extent : frame->payload_size; void* const payload = ins->memory_allocate(ins, payload_size); if (payload != NULL) { @@ -1072,7 +1071,7 @@ int8_t canardRxAccept(CanardInstance* const ins, int8_t canardRxSubscribe(CanardInstance* const ins, const CanardTransferKind transfer_kind, const CanardPortID port_id, - const size_t payload_size_max, + const size_t extent, const CanardMicrosecond transfer_id_timeout_usec, CanardRxSubscription* const out_subscription) { @@ -1094,7 +1093,7 @@ int8_t canardRxSubscribe(CanardInstance* const ins, out_subscription->_sessions[i] = NULL; } out_subscription->_transfer_id_timeout_usec = transfer_id_timeout_usec; - out_subscription->_payload_size_max = payload_size_max; + out_subscription->_extent = extent; out_subscription->_port_id = port_id; out_subscription->_next = ins->_rx_subscriptions[tk]; ins->_rx_subscriptions[tk] = out_subscription; diff --git a/libcanard/canard.h b/libcanard/canard.h index eef1c20b..3c9e12b1 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -289,7 +289,7 @@ typedef struct CanardRxSubscription struct CanardInternalRxSession* _sessions[CANARD_NODE_ID_MAX + 1U]; CanardMicrosecond _transfer_id_timeout_usec; ///< Internal use only. - size_t _payload_size_max; ///< Internal use only. + size_t _extent; ///< Internal use only. CanardPortID _port_id; ///< Internal use only. } CanardRxSubscription; @@ -480,10 +480,10 @@ void canardTxPop(CanardInstance* const ins); /// was already allocated at the time. /// This event occurs when a transport frame that matches a known subscription is received and it begins a /// new transfer (that is, the start-of-frame flag is set and it is not a duplicate). -/// The amount of the allocated memory is payload_size_max as configured via canardRxSubscribe(). +/// The amount of the allocated memory equals the extent as configured via canardRxSubscribe(). /// The worst case occurs when every node on the bus initiates a multi-frame transfer for which there is a -/// matching subscription: in this case, the library will allocate number_of_nodes allocations of size -/// payload_size_max. +/// matching subscription: in this case, the library will allocate number_of_nodes allocations, where each +/// allocation is the same size as the configured extent. /// /// 3. Memory allocated for the transfer payload buffer may be deallocated at the discretion of the library. /// This operation does not increase the worst case execution time and does not improve the worst case memory @@ -492,9 +492,9 @@ void canardTxPop(CanardInstance* const ins); /// /// The worst case dynamic memory consumption per subscription is: /// -/// (sizeof(session instance) + payload_size_max) * number_of_nodes +/// (sizeof(session instance) + extent) * number_of_nodes /// -/// Where sizeof(session instance) and payload_size_max are defined above, and number_of_nodes is the number of remote +/// Where sizeof(session instance) and extent are defined above, and number_of_nodes is the number of remote /// nodes emitting transfers that match the subscription (which cannot exceed (CANARD_NODE_ID_MAX-1) by design). /// If the dynamic memory pool is sized correctly, the application is guaranteed to never encounter an /// out-of-memory (OOM) error at runtime. The actual size of the dynamic memory pool is typically larger; @@ -514,7 +514,7 @@ void canardTxPop(CanardInstance* const ins); /// are stored into out_transfer, and the transfer payload buffer ownership is passed to that object. The lifetime /// of the resulting transfer object is not related to the lifetime of the input transport frame (that is, even if /// it is a single-frame transfer, its payload is copied out into a new dynamically allocated buffer storage). -/// If the payload_size_max is zero, the payload pointer may be NULL, since there is no data to store and so a +/// If the extent is zero, the payload pointer may be NULL, since there is no data to store and so a /// buffer is not needed. The application is responsible for deallocating the payload buffer when the processing /// is done by invoking memory_free on the transfer payload pointer. /// @@ -554,10 +554,12 @@ int8_t canardRxAccept(CanardInstance* const ins, /// If such subscription already exists, it will be removed first as if canardRxUnsubscribe() was /// invoked by the application, and then re-created anew with the new parameters. /// -/// The payload_size_max defines the size of the transfer payload memory buffer. Transfers that carry larger payloads -/// will be accepted but the excess payload will be truncated, as mandated by the Specification. This behavior is -/// called the Implicit Truncation Rule (ITR) and it is intended to facilitate extensibility of data types while -/// preserving backward compatibility. The transfer CRC is validated regardless of whether its payload is truncated. +/// The extent defines the size of the transfer payload memory buffer; or, in other words, the maximum possible size +/// of received objects, considering also possible future versions with new fields. It is safe to pick larger values. +/// Note well that the extent is not the same thing as the maximum size of the object, it is usually larger! +/// Transfers that carry payloads that exceed the specified extent will be accepted anyway but the excess payload +/// will be truncated away, as mandated by the Specification. The transfer CRC is always validated regardless of +/// whether its payload is truncated. /// /// The default transfer-ID timeout value is defined as CANARD_DEFAULT_TRANSFER_ID_TIMEOUT_USEC; use it if not sure. /// The redundant transport fail-over timeout (if redundant transports are used) is the same as the transfer-ID timeout. @@ -582,7 +584,7 @@ int8_t canardRxAccept(CanardInstance* const ins, int8_t canardRxSubscribe(CanardInstance* const ins, const CanardTransferKind transfer_kind, const CanardPortID port_id, - const size_t payload_size_max, + const size_t extent, const CanardMicrosecond transfer_id_timeout_usec, CanardRxSubscription* const out_subscription); diff --git a/tests/exposed.hpp b/tests/exposed.hpp index 3e296b30..8b1e0a53 100644 --- a/tests/exposed.hpp +++ b/tests/exposed.hpp @@ -100,7 +100,7 @@ auto rxTryParseFrame(const CanardFrame* const frame, RxFrameModel* const out_res auto rxSessionWritePayload(CanardInstance* const ins, RxSession* const rxs, - const std::size_t payload_size_max, + const std::size_t extent, const std::size_t payload_size, const void* const payload) -> std::int8_t; @@ -111,7 +111,7 @@ auto rxSessionUpdate(CanardInstance* const ins, const RxFrameModel* const frame, const std::uint8_t redundant_transport_index, const CanardMicrosecond transfer_id_timeout_usec, - const std::size_t payload_size_max, + const std::size_t extent, CanardTransfer* const out_transfer) -> std::int8_t; auto float16Pack(const float value) -> std::uint16_t; diff --git a/tests/helpers.hpp b/tests/helpers.hpp index 235d63eb..df1fb24d 100644 --- a/tests/helpers.hpp +++ b/tests/helpers.hpp @@ -183,16 +183,11 @@ class Instance [[nodiscard]] auto rxSubscribe(const CanardTransferKind transfer_kind, const CanardPortID port_id, - const std::size_t payload_size_max, + const std::size_t extent, const CanardMicrosecond transfer_id_timeout_usec, CanardRxSubscription& out_subscription) { - return canardRxSubscribe(&canard_, - transfer_kind, - port_id, - payload_size_max, - transfer_id_timeout_usec, - &out_subscription); + return canardRxSubscribe(&canard_, transfer_kind, port_id, extent, transfer_id_timeout_usec, &out_subscription); } [[nodiscard]] auto rxUnsubscribe(const CanardTransferKind transfer_kind, const CanardPortID port_id) diff --git a/tests/test_private_rx.cpp b/tests/test_private_rx.cpp index 52d036ee..9fe6c4db 100644 --- a/tests/test_private_rx.cpp +++ b/tests/test_private_rx.cpp @@ -313,13 +313,13 @@ TEST_CASE("rxSessionUpdate") const auto update = [&](const std::uint8_t redundant_transport_index, const std::uint64_t tid_timeout_usec, - const std::size_t payload_size_max) { + const std::size_t extent) { return rxSessionUpdate(&ins.getInstance(), &rxs, &frame, redundant_transport_index, tid_timeout_usec, - payload_size_max, + extent, &transfer); }; diff --git a/tests/test_public_roundtrip.cpp b/tests/test_public_roundtrip.cpp index 1f16e2d6..d9636e6c 100644 --- a/tests/test_public_roundtrip.cpp +++ b/tests/test_public_roundtrip.cpp @@ -23,7 +23,7 @@ TEST_CASE("RoundtripSimple") CanardTransferKind transfer_kind{}; CanardPriority priority{}; CanardPortID port_id{}; - std::size_t payload_size_max{}; + std::size_t extent{}; CanardTransferID transfer_id{}; CanardRxSubscription subscription{}; }; @@ -47,11 +47,11 @@ TEST_CASE("RoundtripSimple") { REQUIRE(1 == ins_rx.rxSubscribe(s.transfer_kind, s.port_id, - s.payload_size_max, + s.extent, CANARD_DEFAULT_TRANSFER_ID_TIMEOUT_USEC, s.subscription)); // The true worst case is 128 times larger, but there is only one transmitting node. - rx_worst_case_memory_consumption += sizeof(exposed::RxSession) + s.payload_size_max; + rx_worst_case_memory_consumption += sizeof(exposed::RxSession) + s.extent; } ins_rx.getAllocator().setAllocationCeiling(rx_worst_case_memory_consumption); // This is guaranteed to be enough. @@ -74,7 +74,7 @@ TEST_CASE("RoundtripSimple") auto& st = tx_states.at(getRandomNatural(std::size(tx_states))); // Generate random payload. The size may be larger than expected to test the implicit truncation rule. - const auto payload_size = getRandomNatural(st.payload_size_max + 1024U); + const auto payload_size = getRandomNatural(st.extent + 1024U); auto* const payload = static_cast(std::malloc(payload_size)); // NOLINT std::generate_n(payload, payload_size, [&]() { return static_cast(getRandomNatural(256U)); }); diff --git a/tests/test_public_rx.cpp b/tests/test_public_rx.cpp index 6b41f2a6..67867689 100644 --- a/tests/test_public_rx.cpp +++ b/tests/test_public_rx.cpp @@ -44,7 +44,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[0] == &sub_msg); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_next == nullptr); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_port_id == 0b0110011001100); - REQUIRE(ins.getInstance()._rx_subscriptions[0]->_payload_size_max == 16); + REQUIRE(ins.getInstance()._rx_subscriptions[0]->_extent == 16); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_transfer_id_timeout_usec == 1'000'000); for (auto _session : ins.getInstance()._rx_subscriptions[0]->_sessions) { @@ -61,7 +61,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[2] == &sub_req); REQUIRE(ins.getInstance()._rx_subscriptions[2]->_next == nullptr); REQUIRE(ins.getInstance()._rx_subscriptions[2]->_port_id == 0b0000110011); - REQUIRE(ins.getInstance()._rx_subscriptions[2]->_payload_size_max == 20); + REQUIRE(ins.getInstance()._rx_subscriptions[2]->_extent == 20); REQUIRE(ins.getInstance()._rx_subscriptions[2]->_transfer_id_timeout_usec == 3'000'000); for (auto _session : ins.getInstance()._rx_subscriptions[2]->_sessions) { @@ -75,7 +75,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[1] == &sub_res); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_next == nullptr); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_port_id == 0b0000111100); - REQUIRE(ins.getInstance()._rx_subscriptions[1]->_payload_size_max == 10); + REQUIRE(ins.getInstance()._rx_subscriptions[1]->_extent == 10); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_transfer_id_timeout_usec == 100'000); for (auto _session : ins.getInstance()._rx_subscriptions[1]->_sessions) { @@ -90,7 +90,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[1] == &sub_res2); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_next == &sub_res); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_port_id == 0b0000000000); - REQUIRE(ins.getInstance()._rx_subscriptions[1]->_payload_size_max == 10); + REQUIRE(ins.getInstance()._rx_subscriptions[1]->_extent == 10); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_transfer_id_timeout_usec == 1'000); for (auto _session : ins.getInstance()._rx_subscriptions[1]->_sessions) { From fe14582921857a03e4c1c788e54fb4910ee4c2ff Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 02:03:21 +0300 Subject: [PATCH 09/22] Fix clang-tidy complaints --- tests/test_public_rx.cpp | 44 ++++++++++++++-------------------------- tests/test_self.cpp | 8 ++++---- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/tests/test_public_rx.cpp b/tests/test_public_rx.cpp index 67867689..f5253b40 100644 --- a/tests/test_public_rx.cpp +++ b/tests/test_public_rx.cpp @@ -5,6 +5,13 @@ #include "helpers.hpp" #include +// clang-tidy mistakenly suggests to avoid C arrays here, which is clearly an error +template +auto ensureAllNullptr(P* (&arr)[N]) -> bool // NOLINT +{ + return std::all_of(std::begin(arr), std::end(arr), [](const auto* const x) { return x == nullptr; }); +} + TEST_CASE("RxBasic0") { using helpers::Instance; @@ -46,10 +53,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[0]->_port_id == 0b0110011001100); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_extent == 16); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_transfer_id_timeout_usec == 1'000'000); - for (auto _session : ins.getInstance()._rx_subscriptions[0]->_sessions) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[0]->_sessions)); REQUIRE(ins.getInstance()._rx_subscriptions[1] == nullptr); REQUIRE(ins.getInstance()._rx_subscriptions[2] == nullptr); @@ -63,10 +67,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[2]->_port_id == 0b0000110011); REQUIRE(ins.getInstance()._rx_subscriptions[2]->_extent == 20); REQUIRE(ins.getInstance()._rx_subscriptions[2]->_transfer_id_timeout_usec == 3'000'000); - for (auto _session : ins.getInstance()._rx_subscriptions[2]->_sessions) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[2]->_sessions)); // Create a response subscription. CanardRxSubscription sub_res{}; @@ -77,10 +78,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[1]->_port_id == 0b0000111100); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_extent == 10); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_transfer_id_timeout_usec == 100'000); - for (auto _session : ins.getInstance()._rx_subscriptions[1]->_sessions) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[1]->_sessions)); REQUIRE(ins.getInstance()._rx_subscriptions[2] == &sub_req); // Create a second response subscription. @@ -92,10 +90,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getInstance()._rx_subscriptions[1]->_port_id == 0b0000000000); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_extent == 10); REQUIRE(ins.getInstance()._rx_subscriptions[1]->_transfer_id_timeout_usec == 1'000); - for (auto _session : ins.getInstance()._rx_subscriptions[1]->_sessions) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[1]->_sessions)); REQUIRE(ins.getInstance()._rx_subscriptions[2] == &sub_req); // Accepted message. @@ -111,7 +106,7 @@ TEST_CASE("RxBasic0") REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 2); // The SESSION and the PAYLOAD BUFFER. REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == (sizeof(RxSession) + 16)); REQUIRE(ins.getInstance()._rx_subscriptions[0]->_sessions[0b0100111] != nullptr); - auto msg_payload = transfer.payload; // Will need it later. + const auto* msg_payload = transfer.payload; // Will need it later. // Provide the space for an extra session and its payload. ins.getAllocator().setAllocationCeiling(sizeof(RxSession) * 2 + 16 + 20); @@ -236,10 +231,7 @@ TEST_CASE("RxAnonymous") REQUIRE(0 == std::memcmp(transfer.payload, "\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F\x10", 0)); REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 1); // The PAYLOAD BUFFER only! No session for anons. REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 16); - for (auto _session : ins.getInstance()._rx_subscriptions[0]->_sessions) // No RX states! - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[0]->_sessions)); // No RX states! // Anonymous message not accepted because OOM. The transfer shall remain unmodified by the call, so we re-check it. REQUIRE(-CANARD_ERROR_OUT_OF_MEMORY == @@ -254,10 +246,7 @@ TEST_CASE("RxAnonymous") REQUIRE(0 == std::memcmp(transfer.payload, "\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F\x10", 0)); REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 1); // The PAYLOAD BUFFER only! No session for anons. REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 16); - for (auto _session : ins.getInstance()._rx_subscriptions[0]->_sessions) // No RX states! - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[0]->_sessions)); // No RX states! // Release the memory. ins.getAllocator().deallocate(transfer.payload); @@ -276,10 +265,7 @@ TEST_CASE("RxAnonymous") REQUIRE(0 == std::memcmp(transfer.payload, "\x01\x02\x03\x04\x05\x06", 0)); REQUIRE(ins.getAllocator().getNumAllocatedFragments() == 1); // The PAYLOAD BUFFER only! No session for anons. REQUIRE(ins.getAllocator().getTotalAllocatedAmount() == 6); // Smaller allocation. - for (auto _session : ins.getInstance()._rx_subscriptions[0]->_sessions) // No RX states! - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[0]->_sessions)); // No RX states! } TEST_CASE("RxSubscriptionErrors") diff --git a/tests/test_self.cpp b/tests/test_self.cpp index a83dde5b..a79f9bde 100644 --- a/tests/test_self.cpp +++ b/tests/test_self.cpp @@ -11,11 +11,11 @@ TEST_CASE("TestAllocator") REQUIRE(0 == al.getNumAllocatedFragments()); REQUIRE(std::numeric_limits::max() == al.getAllocationCeiling()); - auto a = al.allocate(123); + auto* a = al.allocate(123); REQUIRE(1 == al.getNumAllocatedFragments()); REQUIRE(123 == al.getTotalAllocatedAmount()); - auto b = al.allocate(456); + auto* b = al.allocate(456); REQUIRE(2 == al.getNumAllocatedFragments()); REQUIRE(579 == al.getTotalAllocatedAmount()); @@ -25,7 +25,7 @@ TEST_CASE("TestAllocator") REQUIRE(2 == al.getNumAllocatedFragments()); REQUIRE(579 == al.getTotalAllocatedAmount()); - auto c = al.allocate(21); + auto* c = al.allocate(21); REQUIRE(3 == al.getNumAllocatedFragments()); REQUIRE(600 == al.getTotalAllocatedAmount()); @@ -33,7 +33,7 @@ TEST_CASE("TestAllocator") REQUIRE(2 == al.getNumAllocatedFragments()); REQUIRE(477 == al.getTotalAllocatedAmount()); - auto d = al.allocate(100); + auto* d = al.allocate(100); REQUIRE(3 == al.getNumAllocatedFragments()); REQUIRE(577 == al.getTotalAllocatedAmount()); From e8daea2e1cc05aa1a5c2f59c4ebda0c6075f2cfe Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 02:16:06 +0300 Subject: [PATCH 10/22] Describe 'extent' in the README briefly --- README.md | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 8ccaec47..0604a3ed 100644 --- a/README.md +++ b/README.md @@ -139,19 +139,31 @@ CanardRxSubscription heartbeat_subscription; (void) canardRxSubscribe(&ins, // Subscribe to messages uavcan.node.Heartbeat. CanardTransferKindMessage, 7509, // The fixed Subject-ID of the Heartbeat message type (see DSDL definition). - 7, // The maximum payload size (max DSDL object size) from the DSDL definition. + 7, // The extent (the maximum possible payload size); pick a huge value if not sure. CANARD_DEFAULT_TRANSFER_ID_TIMEOUT_USEC, &heartbeat_subscription); CanardRxSubscription my_service_subscription; -(void) canardRxSubscribe(&ins, // Subscribe to an arbitrary service response. - CanardTransferKindResponse, - 123, // The Service-ID to subscribe to. - 1024, // The maximum payload size (max DSDL object size). +(void) canardRxSubscribe(&ins, // Subscribe to an arbitrary service response. + CanardTransferKindResponse, // Specify that we want service responses, not requests. + 123, // The Service-ID whose responses we will receive. + 1024, // The extent (the maximum payload size); pick a huge value if not sure. CANARD_DEFAULT_TRANSFER_ID_TIMEOUT_USEC, &my_service_subscription); ``` +The "extent" refers to the minimum amount of memory required to hold any serialized representation of any compatible +version of a data type. +This parameter is determined by the data type author at the data type definition time. +It is typically larger than the maximum object size in order to allow the data type author to introduce more +fields in the future versions of the type; +for example, `MyMessage.1.0` may have the maximum size of 100 bytes and the extent 200 bytes; +a revised version `MyMessage.1.1` may have the maximum size anywhere between 0 and 200 bytes. +It is always safe to pick a larger value if not sure. +You will find a more rigorous description in the UAVCAN Specification. + +In Libcanard we use the term "subscription" not only for subjects (messages), but also for services, for simplicity. + We can subscribe and unsubscribe at runtime as many times as we want. Normally, however, an embedded application would subscribe once and roll with it. Okay, this is how we receive transfers: From a97073e7eb9d8b1dbd3687729f72856aad1b8d61 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 02:32:59 +0300 Subject: [PATCH 11/22] Reword the README, add link to the examples posted on the forum. --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 0604a3ed..fa383a3e 100644 --- a/README.md +++ b/README.md @@ -14,11 +14,12 @@ embedded systems. communication in aerospace and robotic applications via CAN bus, Ethernet, and other robust transports. The acronym UAVCAN stands for *Uncomplicated Application-level Vehicular Communication And Networking*. -**READ THE DOCS: [`libcanard/canard.h`](/libcanard/canard.h)** +**Read the docs in [`libcanard/canard.h`](/libcanard/canard.h)** -Contribute: [`CONTRIBUTING.md`](/CONTRIBUTING.md) +Find examples, starters, tutorials on the +[UAVCAN forum](https://forum.uavcan.org/t/libcanard-examples-starters-tutorials/935). -Ask questions: [forum.uavcan.org](https://forum.uavcan.org) +If you want to contribute, please read [`CONTRIBUTING.md`](/CONTRIBUTING.md). ## Features From 7240cf8e668976ffbb19b6b34d1ef63d1c59be75 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 02:35:51 +0300 Subject: [PATCH 12/22] Update SonarQube config to fix the environment configuration warning --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 358748a0..b7fab839 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,7 +37,8 @@ matrix: -Dsonar.sources=libcanard -Dsonar.cfamily.gcov.reportsPath=. -Dsonar.cfamily.build-wrapper-output=sonar-dump - -Dsonar.cfamily.cache.enabled=false' + -Dsonar.cfamily.cache.enabled=false + -Dsonar.cfamily.threads=1' - make clean # DEBUG From 58a6aff5c50519c6d6d4d8bc24668e8f6db7b8e5 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 02:47:17 +0300 Subject: [PATCH 13/22] Add revisions --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index fa383a3e..274e81e8 100644 --- a/README.md +++ b/README.md @@ -223,3 +223,9 @@ result = canardTxPush(&ins, &my_transfer); Full API specification is available in the documentation. If you find the examples to be unclear or incorrect, please, open a ticket. + +## Revisions + +### v1.0 + +The initial release. From ca83b06c8389ca1131eef4247bc0996d25926ccb Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 15 Sep 2020 10:46:06 +0300 Subject: [PATCH 14/22] Use the correct extent of the Heartbeat type in the README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 274e81e8..6b527cb1 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,7 @@ CanardRxSubscription heartbeat_subscription; (void) canardRxSubscribe(&ins, // Subscribe to messages uavcan.node.Heartbeat. CanardTransferKindMessage, 7509, // The fixed Subject-ID of the Heartbeat message type (see DSDL definition). - 7, // The extent (the maximum possible payload size); pick a huge value if not sure. + 16, // The extent (the maximum possible payload size); pick a huge value if not sure. CANARD_DEFAULT_TRANSFER_ID_TIMEOUT_USEC, &heartbeat_subscription); From affb14dca65917d63b03517f8412133892d42204 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Thu, 1 Oct 2020 00:44:23 +0300 Subject: [PATCH 15/22] Add a couple of clarifications per Peter's request --- README.md | 2 +- libcanard/canard.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 1e8b2345..48bc367c 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ CanardRxSubscription my_service_subscription; ``` The "extent" refers to the minimum amount of memory required to hold any serialized representation of any compatible -version of a data type. +version of the data type; or, on other words, it is the the maximum possible size of received objects. This parameter is determined by the data type author at the data type definition time. It is typically larger than the maximum object size in order to allow the data type author to introduce more fields in the future versions of the type; diff --git a/libcanard/canard.h b/libcanard/canard.h index 599ef919..0a6ecc17 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -480,7 +480,8 @@ void canardTxPop(CanardInstance* const ins); /// was already allocated at the time. /// This event occurs when a transport frame that matches a known subscription is received and it begins a /// new transfer (that is, the start-of-frame flag is set and it is not a duplicate). -/// The amount of the allocated memory equals the extent as configured via canardRxSubscribe(). +/// The amount of the allocated memory equals the extent as configured via canardRxSubscribe(); please read +/// its documentation for further information about the extent and related edge cases. /// The worst case occurs when every node on the bus initiates a multi-frame transfer for which there is a /// matching subscription: in this case, the library will allocate number_of_nodes allocations, where each /// allocation is the same size as the configured extent. From 3089e708379c9bfd25afd485e0273fc7076f8b87 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Sun, 4 Oct 2020 15:48:48 +0300 Subject: [PATCH 16/22] Apply Scott's fixes to float16Pack(); see https://github.com/UAVCAN/nunavut/pull/115\#issuecomment-703177071 --- libcanard/canard_dsdl.c | 19 ++++++++++++++++++- tests/test_dsdl.cpp | 10 ++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/libcanard/canard_dsdl.c b/libcanard/canard_dsdl.c index cd0e3bd0..bf166afc 100644 --- a/libcanard/canard_dsdl.c +++ b/libcanard/canard_dsdl.c @@ -324,7 +324,24 @@ CANARD_PRIVATE uint16_t float16Pack(const CanardDSDLFloat32 value) uint16_t out = 0; if (in.bits >= f32inf.bits) { - out = (in.bits > f32inf.bits) ? (uint16_t) 0x7FFFU : (uint16_t) 0x7C00U; // NOLINT NOSONAR + // The no-lint statements suppress the warnings about magic numbers. + if ((in.bits & 0x3FFFFFUL) != 0) // NOLINT NOSONAR + { + if ((0x400000UL & in.bits) != 0) // NOLINT NOSONAR + { + out = 0x7E00U; // NOLINT NOSONAR + } + else + { + // Signalling NaN. We're allowed to discard any diagnostic data in the mantissa but at least one bit, + // other than the signalling bit, must be set to distinguish from INFINITY. + out = 0x7D00U; // NOLINT NOSONAR + } + } + else + { + out = (in.bits > f32inf.bits) ? (uint16_t) 0x7FFFU : (uint16_t) 0x7C00U; // NOLINT NOSONAR + } } else { diff --git a/tests/test_dsdl.cpp b/tests/test_dsdl.cpp index 17078b40..33a5daea 100644 --- a/tests/test_dsdl.cpp +++ b/tests/test_dsdl.cpp @@ -27,7 +27,10 @@ TEST_CASE("float16Unpack") REQUIRE(Approx(-65504.0F) == float16Unpack(0b1111101111111111)); REQUIRE(std::isinf(float16Unpack(0b0111110000000000))); - REQUIRE(bool(std::isnan(float16Unpack(0b0111111111111111)))); + REQUIRE(bool(std::isnan(float16Unpack(0b0111111111111111)))); // quiet + REQUIRE(bool(std::isnan(float16Unpack(0b0111111000000000)))); // quiet + REQUIRE(bool(std::isnan(float16Unpack(0b0111110111111111)))); // signaling + REQUIRE(bool(std::isnan(float16Unpack(0b0111110000000001)))); // signaling } TEST_CASE("canardDSDLFloat16") @@ -43,7 +46,10 @@ TEST_CASE("canardDSDLFloat16") REQUIRE(0b0111110000000000 == float16Pack(float16Unpack(0b0111110000000000))); // +inf REQUIRE(0b1111110000000000 == float16Pack(float16Unpack(0b1111110000000000))); // -inf - REQUIRE(0b0111111111111111 == float16Pack(float16Unpack(0b0111111111111111))); // nan + REQUIRE(0b0111111000000000 == float16Pack(float16Unpack(0b0111111111111111))); // qNaN, extra bits stripped + + // https://github.com/UAVCAN/nunavut/pull/115#issuecomment-703248946 + // REQUIRE(0b0111110100000000 == float16Pack(float16Unpack(0b0111110111111111))); // sNaN, extra bits stripped } TEST_CASE("canardDSDLCopyBits") From 48d7563db8b0a6c251a17305fbcb91c59028c836 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Sun, 4 Oct 2020 16:39:05 +0300 Subject: [PATCH 17/22] Add float16 packing tests that reveal issues; the CI is expected to be broken. See https://github.com/UAVCAN/nunavut/pull/115\#issuecomment-703257086 --- tests/test_dsdl.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/test_dsdl.cpp b/tests/test_dsdl.cpp index 33a5daea..b4ee86ad 100644 --- a/tests/test_dsdl.cpp +++ b/tests/test_dsdl.cpp @@ -5,7 +5,7 @@ #include "exposed.hpp" #include #include -#include +#include TEST_CASE("float16Pack") { @@ -15,7 +15,8 @@ TEST_CASE("float16Pack") REQUIRE(0b1100000000000000 == float16Pack(-2.0F)); REQUIRE(0b0111110000000000 == float16Pack(999999.0F)); // +inf REQUIRE(0b1111101111111111 == float16Pack(-65519.0F)); // -max - REQUIRE(0b0111111111111111 == float16Pack(std::nanf(""))); // nan + REQUIRE(0b0111111000000000 == float16Pack(std::numeric_limits::quiet_NaN())); + REQUIRE(0b0111110100000000 == float16Pack(std::numeric_limits::signaling_NaN())); } TEST_CASE("float16Unpack") @@ -47,9 +48,7 @@ TEST_CASE("canardDSDLFloat16") REQUIRE(0b0111110000000000 == float16Pack(float16Unpack(0b0111110000000000))); // +inf REQUIRE(0b1111110000000000 == float16Pack(float16Unpack(0b1111110000000000))); // -inf REQUIRE(0b0111111000000000 == float16Pack(float16Unpack(0b0111111111111111))); // qNaN, extra bits stripped - - // https://github.com/UAVCAN/nunavut/pull/115#issuecomment-703248946 - // REQUIRE(0b0111110100000000 == float16Pack(float16Unpack(0b0111110111111111))); // sNaN, extra bits stripped + REQUIRE(0b0111110100000000 == float16Pack(float16Unpack(0b0111110111111111))); // sNaN, extra bits stripped } TEST_CASE("canardDSDLCopyBits") From 64a6f4c91419ff0b4fd15af8a7eb6b4d6aebbfba Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 6 Oct 2020 13:14:38 +0300 Subject: [PATCH 18/22] Fix one issue in float16Pack(), one remains --- libcanard/canard_dsdl.c | 2 +- tests/test_dsdl.cpp | 75 +++++++++++++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/libcanard/canard_dsdl.c b/libcanard/canard_dsdl.c index bf166afc..d013f9ca 100644 --- a/libcanard/canard_dsdl.c +++ b/libcanard/canard_dsdl.c @@ -325,7 +325,7 @@ CANARD_PRIVATE uint16_t float16Pack(const CanardDSDLFloat32 value) if (in.bits >= f32inf.bits) { // The no-lint statements suppress the warnings about magic numbers. - if ((in.bits & 0x3FFFFFUL) != 0) // NOLINT NOSONAR + if ((in.bits & 0x7FFFFFUL) != 0) // NOLINT NOSONAR { if ((0x400000UL & in.bits) != 0) // NOLINT NOSONAR { diff --git a/tests/test_dsdl.cpp b/tests/test_dsdl.cpp index b4ee86ad..66812e9c 100644 --- a/tests/test_dsdl.cpp +++ b/tests/test_dsdl.cpp @@ -5,6 +5,7 @@ #include "exposed.hpp" #include #include +#include #include TEST_CASE("float16Pack") @@ -13,10 +14,16 @@ TEST_CASE("float16Pack") REQUIRE(0b0000000000000000 == float16Pack(0.0F)); REQUIRE(0b0011110000000000 == float16Pack(1.0F)); REQUIRE(0b1100000000000000 == float16Pack(-2.0F)); - REQUIRE(0b0111110000000000 == float16Pack(999999.0F)); // +inf - REQUIRE(0b1111101111111111 == float16Pack(-65519.0F)); // -max - REQUIRE(0b0111111000000000 == float16Pack(std::numeric_limits::quiet_NaN())); - REQUIRE(0b0111110100000000 == float16Pack(std::numeric_limits::signaling_NaN())); + REQUIRE(0b0111110000000000 == float16Pack(999999.0F)); // +inf + REQUIRE(0b1111101111111111 == float16Pack(-65519.0F)); // -max + + // These are intrusive tests, they make assumptions about the specific implementation of the conversion logic. + // Normally, one wouldn't be able to compare a NaN against a particular number because there are many ways to + // represent it. + REQUIRE(0b0111111000000000 == float16Pack(+std::numeric_limits::quiet_NaN())); + REQUIRE(0b1111111000000000 == float16Pack(-std::numeric_limits::quiet_NaN())); + REQUIRE(0b0111110100000000 == float16Pack(+std::numeric_limits::signaling_NaN())); + REQUIRE(0b1111110100000000 == float16Pack(-std::numeric_limits::signaling_NaN())); } TEST_CASE("float16Unpack") @@ -28,10 +35,53 @@ TEST_CASE("float16Unpack") REQUIRE(Approx(-65504.0F) == float16Unpack(0b1111101111111111)); REQUIRE(std::isinf(float16Unpack(0b0111110000000000))); - REQUIRE(bool(std::isnan(float16Unpack(0b0111111111111111)))); // quiet - REQUIRE(bool(std::isnan(float16Unpack(0b0111111000000000)))); // quiet - REQUIRE(bool(std::isnan(float16Unpack(0b0111110111111111)))); // signaling - REQUIRE(bool(std::isnan(float16Unpack(0b0111110000000001)))); // signaling + const auto explode_sign_exp_mantissa = [](const float f) -> std::tuple { + std::uint32_t n = 0; + std::memcpy(&n, &f, 4); + return std::make_tuple((n & (1UL << 31U)) != 0, + static_cast((n >> 23U) & 0xFFU), + n & ((1UL << 23U) - 1U)); + }; + + { + const auto [sign, exp, man] = explode_sign_exp_mantissa(float16Unpack(0b0111111111111111)); // +qNaN + REQUIRE(!sign); + REQUIRE(exp == 0xFFU); + REQUIRE(man != 0); + } + { + const auto [sign, exp, man] = explode_sign_exp_mantissa(float16Unpack(0b0111111000000000)); // +qNaN + REQUIRE(!sign); + REQUIRE(exp == 0xFFU); + REQUIRE(man != 0); + } + { + const auto [sign, exp, man] = explode_sign_exp_mantissa(float16Unpack(0b1111111111111111)); // -qNaN + REQUIRE(sign); + REQUIRE(exp == 0xFFU); + REQUIRE(man != 0); + } + { + const auto [sign, exp, man] = explode_sign_exp_mantissa(float16Unpack(0b0111110111111111)); // +sNaN + REQUIRE(!sign); + REQUIRE(exp == 0xFFU); + REQUIRE(man != 0); + } + { + const auto [sign, exp, man] = explode_sign_exp_mantissa(float16Unpack(0b1111110000000001)); // -sNaN + REQUIRE(sign); + REQUIRE(exp == 0xFFU); + REQUIRE(man != 0); + } + + REQUIRE(bool(std::isnan(float16Unpack(0b0111111111111111)))); // +quiet + REQUIRE(bool(std::isnan(float16Unpack(0b0111111000000000)))); // +quiet + REQUIRE(bool(std::isnan(float16Unpack(0b1111111111111111)))); // -quiet + REQUIRE(bool(std::isnan(float16Unpack(0b1111111000000000)))); // -quiet + REQUIRE(bool(std::isnan(float16Unpack(0b0111110111111111)))); // +signaling + REQUIRE(bool(std::isnan(float16Unpack(0b0111110000000001)))); // +signaling + REQUIRE(bool(std::isnan(float16Unpack(0b1111110111111111)))); // -signaling + REQUIRE(bool(std::isnan(float16Unpack(0b1111110000000001)))); // -signaling } TEST_CASE("canardDSDLFloat16") @@ -45,10 +95,15 @@ TEST_CASE("canardDSDLFloat16") x += 0.5F; } + // These are intrusive tests, they make assumptions about the specific implementation of the conversion logic. + // Normally, one wouldn't be able to compare a NaN against a particular number because there are many ways to + // represent it. REQUIRE(0b0111110000000000 == float16Pack(float16Unpack(0b0111110000000000))); // +inf REQUIRE(0b1111110000000000 == float16Pack(float16Unpack(0b1111110000000000))); // -inf - REQUIRE(0b0111111000000000 == float16Pack(float16Unpack(0b0111111111111111))); // qNaN, extra bits stripped - REQUIRE(0b0111110100000000 == float16Pack(float16Unpack(0b0111110111111111))); // sNaN, extra bits stripped + REQUIRE(0b0111111000000000 == float16Pack(float16Unpack(0b0111111111111111))); // +qNaN, extra bits stripped + REQUIRE(0b0111110100000000 == float16Pack(float16Unpack(0b0111110111111111))); // +sNaN, extra bits stripped + REQUIRE(0b1111111000000000 == float16Pack(float16Unpack(0b1111111111111111))); // -qNaN, extra bits stripped + REQUIRE(0b1111110100000000 == float16Pack(float16Unpack(0b1111110111111111))); // -sNaN, extra bits stripped } TEST_CASE("canardDSDLCopyBits") From e603be3ef8b2f49c55f9d0be9f2cf81a7c0c26c4 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 6 Oct 2020 13:49:54 +0300 Subject: [PATCH 19/22] Do not differentiate between sNaN and qNaN; see https://github.com/UAVCAN/nunavut/pull/115\#issuecomment-704185463 --- libcanard/canard_dsdl.c | 11 +---------- tests/test_dsdl.cpp | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/libcanard/canard_dsdl.c b/libcanard/canard_dsdl.c index d013f9ca..98cd1c4c 100644 --- a/libcanard/canard_dsdl.c +++ b/libcanard/canard_dsdl.c @@ -327,16 +327,7 @@ CANARD_PRIVATE uint16_t float16Pack(const CanardDSDLFloat32 value) // The no-lint statements suppress the warnings about magic numbers. if ((in.bits & 0x7FFFFFUL) != 0) // NOLINT NOSONAR { - if ((0x400000UL & in.bits) != 0) // NOLINT NOSONAR - { - out = 0x7E00U; // NOLINT NOSONAR - } - else - { - // Signalling NaN. We're allowed to discard any diagnostic data in the mantissa but at least one bit, - // other than the signalling bit, must be set to distinguish from INFINITY. - out = 0x7D00U; // NOLINT NOSONAR - } + out = 0x7E00U; // NOLINT NOSONAR } else { diff --git a/tests/test_dsdl.cpp b/tests/test_dsdl.cpp index 66812e9c..24a32765 100644 --- a/tests/test_dsdl.cpp +++ b/tests/test_dsdl.cpp @@ -19,11 +19,12 @@ TEST_CASE("float16Pack") // These are intrusive tests, they make assumptions about the specific implementation of the conversion logic. // Normally, one wouldn't be able to compare a NaN against a particular number because there are many ways to - // represent it. + // represent it. We do not differentiate between sNaN and qNaN because there is no platform-agnostic way to do + // that; see https://github.com/UAVCAN/nunavut/pull/115#issuecomment-704185463 REQUIRE(0b0111111000000000 == float16Pack(+std::numeric_limits::quiet_NaN())); REQUIRE(0b1111111000000000 == float16Pack(-std::numeric_limits::quiet_NaN())); - REQUIRE(0b0111110100000000 == float16Pack(+std::numeric_limits::signaling_NaN())); - REQUIRE(0b1111110100000000 == float16Pack(-std::numeric_limits::signaling_NaN())); + REQUIRE(0b0111111000000000 == float16Pack(+std::numeric_limits::signaling_NaN())); + REQUIRE(0b1111111000000000 == float16Pack(-std::numeric_limits::signaling_NaN())); } TEST_CASE("float16Unpack") @@ -95,15 +96,17 @@ TEST_CASE("canardDSDLFloat16") x += 0.5F; } - // These are intrusive tests, they make assumptions about the specific implementation of the conversion logic. - // Normally, one wouldn't be able to compare a NaN against a particular number because there are many ways to - // represent it. REQUIRE(0b0111110000000000 == float16Pack(float16Unpack(0b0111110000000000))); // +inf REQUIRE(0b1111110000000000 == float16Pack(float16Unpack(0b1111110000000000))); // -inf + + // These are intrusive tests, they make assumptions about the specific implementation of the conversion logic. + // Normally, one wouldn't be able to compare a NaN against a particular number because there are many ways to + // represent it. We do not differentiate between sNaN and qNaN because there is no platform-agnostic way to do + // that; see https://github.com/UAVCAN/nunavut/pull/115#issuecomment-704185463 REQUIRE(0b0111111000000000 == float16Pack(float16Unpack(0b0111111111111111))); // +qNaN, extra bits stripped - REQUIRE(0b0111110100000000 == float16Pack(float16Unpack(0b0111110111111111))); // +sNaN, extra bits stripped + REQUIRE(0b0111111000000000 == float16Pack(float16Unpack(0b0111110111111111))); // +sNaN, extra bits stripped REQUIRE(0b1111111000000000 == float16Pack(float16Unpack(0b1111111111111111))); // -qNaN, extra bits stripped - REQUIRE(0b1111110100000000 == float16Pack(float16Unpack(0b1111110111111111))); // -sNaN, extra bits stripped + REQUIRE(0b1111111000000000 == float16Pack(float16Unpack(0b1111110111111111))); // -sNaN, extra bits stripped } TEST_CASE("canardDSDLCopyBits") From c563bfa59b7cb75ddbdea1b64890ae34f4218740 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Mon, 12 Oct 2020 21:53:09 +0300 Subject: [PATCH 20/22] Final update after the Spec changes have been accepted; good to merge --- README.md | 14 +++++++------- tests/test_dsdl.cpp | 18 ++++++++---------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 48bc367c..d9ae7e42 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ embedded systems. [UAVCAN](https://uavcan.org) is an open lightweight data bus standard designed for reliable intravehicular communication in aerospace and robotic applications via CAN bus, Ethernet, and other robust transports. -The acronym UAVCAN stands for *Uncomplicated Application-level Vehicular Communication And Networking*. +The acronym UAVCAN stands for *Uncomplicated Application-level Vehicular Computing And Networking*. **Read the docs in [`libcanard/canard.h`](/libcanard/canard.h).** @@ -200,10 +200,10 @@ The DSDL serialization helper library can be used to (de-)serialize DSDL objects Here's a simple deserialization example for a `uavcan.node.Heartbeat.1.0` message: ```c -uint8_t mode = canardDSDLGetU8(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 34, 3); +uint8_t mode = canardDSDLGetU8(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 40, 8); uint32_t uptime = canardDSDLGetU32(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 0, 32); -uint32_t vssc = canardDSDLGetU32(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 37, 19); -uint8_t health = canardDSDLGetU8(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 32, 2); +uint8_t vssc = canardDSDLGetU32(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 48, 8); +uint8_t health = canardDSDLGetU8(heartbeat_transfer->payload, heartbeat_transfer->payload_size, 32, 8); ``` And the opposite: @@ -211,10 +211,10 @@ And the opposite: ```c uint8_t buffer[7]; // destination offset value bit-length -canardDSDLSetUxx(&buffer[0], 34, 2, 3); // mode +canardDSDLSetUxx(&buffer[0], 40, 2, 8); // mode canardDSDLSetUxx(&buffer[0], 0, 0xDEADBEEF, 32); // uptime -canardDSDLSetUxx(&buffer[0], 37, 0x7FFFF, 19); // vssc -canardDSDLSetUxx(&buffer[0], 32, 2, 2); // health +canardDSDLSetUxx(&buffer[0], 48, 0x7F, 8); // vssc +canardDSDLSetUxx(&buffer[0], 32, 2, 8); // health // Now it can be transmitted: my_transfer->payload = &buffer[0]; my_transfer->payload_size = sizeof(buffer); diff --git a/tests/test_dsdl.cpp b/tests/test_dsdl.cpp index 24a32765..43b9c2f2 100644 --- a/tests/test_dsdl.cpp +++ b/tests/test_dsdl.cpp @@ -335,8 +335,7 @@ TEST_CASE("canardDSDLSerialize_unaligned") TEST_CASE("canardDSDLSerialize_heartbeat") { - // The reference values were taken from the PyUAVCAN test. - const std::vector Reference({239, 190, 173, 222, 234, 255, 255, 0}); + const std::vector Reference({239, 190, 173, 222, 3, 2, 127, 0}); std::vector dest(std::size(Reference)); @@ -344,10 +343,10 @@ TEST_CASE("canardDSDLSerialize_heartbeat") canardDSDLSetUxx(dest.data(), offset_bit, value, length_bit); }; - set_u(34, 2, 3); // mode + set_u(40, 2, 8); // mode set_u(0, 0xDEADBEEF, 32); // uptime - set_u(37, 0x7FFFF, 19); // vssc - set_u(32, 2, 2); // health + set_u(48, 0x7F, 8); // vssc + set_u(32, 3, 8); // health REQUIRE(std::size(dest) == std::size(Reference)); REQUIRE_THAT(dest, Catch::Matchers::Equals(Reference)); @@ -569,11 +568,10 @@ TEST_CASE("canardDSDLDeserialize_unaligned") TEST_CASE("canardDSDLDeserialize_heartbeat") { - // The reference values were taken from the PyUAVCAN test. - const std::vector Reference({239, 190, 173, 222, 234, 255, 255}); + const std::vector Reference({239, 190, 173, 222, 3, 2, 127, 0}); const std::uint8_t* const buf = Reference.data(); - REQUIRE(2 == canardDSDLGetU8(buf, 7, 34, 3)); // mode + REQUIRE(2 == canardDSDLGetU8(buf, 7, 40, 8)); // mode REQUIRE(0xDEADBEEF == canardDSDLGetU32(buf, 7, 0, 32)); // uptime - REQUIRE(0x7FFFF == canardDSDLGetU32(buf, 7, 37, 19)); // vssc - REQUIRE(2 == canardDSDLGetU8(buf, 7, 32, 2)); // health + REQUIRE(0x7F == canardDSDLGetU32(buf, 7, 48, 8)); // vssc + REQUIRE(3 == canardDSDLGetU8(buf, 7, 32, 8)); // health } From dc1f829e4837066aeb6ca9f4c00e13e5ed6aec91 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Mon, 12 Oct 2020 23:43:01 +0300 Subject: [PATCH 21/22] Travis CI keeps aborting our jobs because of the time limit. Do not run Clang-Tidy in release builds to reduce the workload for the CI worker. --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 571bc6da..d82e7355 100644 --- a/.travis.yml +++ b/.travis.yml @@ -73,13 +73,13 @@ matrix: - make VERBOSE=1 && make test - make clean - # RELEASE - - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=Release + # RELEASE (skip static analysis because it is done in the DEBUG configuration) + - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=Release -DNO_STATIC_ANALYSIS=1 - make VERBOSE=1 && make test - make clean - # MINSIZEREL - - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=MinSizeRel + # MINSIZEREL (skip static analysis because it is done in the DEBUG configuration) + - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=MinSizeRel -DNO_STATIC_ANALYSIS=1 - make VERBOSE=1 && make test - make clean From 977e313597476e909777938ddc0c158db0c97875 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Tue, 13 Oct 2020 00:20:48 +0300 Subject: [PATCH 22/22] Another stab at fixing the CI build --- .travis.yml | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/.travis.yml b/.travis.yml index d82e7355..5f2a9dc4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -68,9 +68,13 @@ matrix: - sudo apt install clang-tidy-10 clang-format-10 - clang++-10 -E -x c++ - -v < /dev/null # Print the Clang configuration for troubleshooting purposes. - # DEBUG + # DEBUG + format check - cmake -DCMAKE_C_COMPILER=clang-10 -DCMAKE_CXX_COMPILER=clang++-10 tests -DCMAKE_BUILD_TYPE=Debug - make VERBOSE=1 && make test + - make format VERBOSE=1 + - 'modified="$(git status --porcelain --untracked-files=no)"' + - echo "${modified}" + - 'if [ -n "$modified" ]; then echo "Run make format to reformat the code properly."; exit 1; fi' - make clean # RELEASE (skip static analysis because it is done in the DEBUG configuration) @@ -83,12 +87,6 @@ matrix: - make VERBOSE=1 && make test - make clean - # Format check - - make format VERBOSE=1 - - 'modified="$(git status --porcelain --untracked-files=no)"' - - echo "${modified}" - - 'if [ -n "$modified" ]; then echo "Run make format to reformat the code properly."; exit 1; fi' - # -------------------- AVR GCC -------------------- - language: c addons: