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/.travis.yml b/.travis.yml index 6b1c321a..5f2a9dc4 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,15 +29,16 @@ 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 -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 @@ -50,46 +51,42 @@ matrix: - make all VERBOSE=1 && make test - make clean - # -------------------- Clang 9 (Part 1) -------------------- + # -------------------- Clang -------------------- - language: cpp addons: apt: 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. + - 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. - # DEBUG - - cmake -DCMAKE_C_COMPILER=clang-9 -DCMAKE_CXX_COMPILER=clang++-9 tests -DCMAKE_BUILD_TYPE=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 - - cmake -DCMAKE_C_COMPILER=clang-9 -DCMAKE_CXX_COMPILER=clang++-9 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-9 -DCMAKE_CXX_COMPILER=clang++-9 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 - # 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: diff --git a/README.md b/README.md index e3364071..d9ae7e42 100644 --- a/README.md +++ b/README.md @@ -12,13 +12,14 @@ 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: [`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 @@ -138,20 +139,32 @@ 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). - 7, // The maximum payload size (max DSDL object size) from the DSDL definition. + 7509, // The fixed Subject-ID of the Heartbeat message type (see DSDL definition). + 16, // 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 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; +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: @@ -187,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: @@ -198,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); @@ -210,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. diff --git a/libcanard/canard.c b/libcanard/canard.c index 79b23966..13103826 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, @@ -637,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; @@ -666,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; @@ -681,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; @@ -710,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); @@ -738,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); @@ -787,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); @@ -834,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; @@ -895,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); } } @@ -905,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) { @@ -1071,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) { @@ -1093,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 f822ce68..0a6ecc17 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 @@ -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 @@ -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,11 @@ 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(); 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 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 +493,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 +515,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 +555,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 +585,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/libcanard/canard_dsdl.c b/libcanard/canard_dsdl.c index cd0e3bd0..98cd1c4c 100644 --- a/libcanard/canard_dsdl.c +++ b/libcanard/canard_dsdl.c @@ -324,7 +324,15 @@ 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 & 0x7FFFFFUL) != 0) // NOLINT NOSONAR + { + out = 0x7E00U; // NOLINT NOSONAR + } + else + { + out = (in.bits > f32inf.bits) ? (uint16_t) 0x7FFFU : (uint16_t) 0x7C00U; // NOLINT NOSONAR + } } else { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c1598231..71428c6c 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 () diff --git a/tests/exposed.hpp b/tests/exposed.hpp index 07d1a513..8b1e0a53 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. @@ -99,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; @@ -110,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_dsdl.cpp b/tests/test_dsdl.cpp index 17078b40..43b9c2f2 100644 --- a/tests/test_dsdl.cpp +++ b/tests/test_dsdl.cpp @@ -5,7 +5,8 @@ #include "exposed.hpp" #include #include -#include +#include +#include TEST_CASE("float16Pack") { @@ -13,9 +14,17 @@ 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(0b0111111111111111 == float16Pack(std::nanf(""))); // 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. 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(0b0111111000000000 == float16Pack(+std::numeric_limits::signaling_NaN())); + REQUIRE(0b1111111000000000 == float16Pack(-std::numeric_limits::signaling_NaN())); } TEST_CASE("float16Unpack") @@ -27,7 +36,53 @@ TEST_CASE("float16Unpack") REQUIRE(Approx(-65504.0F) == float16Unpack(0b1111101111111111)); REQUIRE(std::isinf(float16Unpack(0b0111110000000000))); - REQUIRE(bool(std::isnan(float16Unpack(0b0111111111111111)))); + 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") @@ -43,7 +98,15 @@ TEST_CASE("canardDSDLFloat16") REQUIRE(0b0111110000000000 == float16Pack(float16Unpack(0b0111110000000000))); // +inf REQUIRE(0b1111110000000000 == float16Pack(float16Unpack(0b1111110000000000))); // -inf - REQUIRE(0b0111111111111111 == float16Pack(float16Unpack(0b0111111111111111))); // nan + + // 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(0b0111111000000000 == float16Pack(float16Unpack(0b0111110111111111))); // +sNaN, extra bits stripped + REQUIRE(0b1111111000000000 == float16Pack(float16Unpack(0b1111111111111111))); // -qNaN, extra bits stripped + REQUIRE(0b1111111000000000 == float16Pack(float16Unpack(0b1111110111111111))); // -sNaN, extra bits stripped } TEST_CASE("canardDSDLCopyBits") @@ -272,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)); @@ -281,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)); @@ -506,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 } diff --git a/tests/test_private_rx.cpp b/tests/test_private_rx.cpp index da514914..9fe6c4db 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; @@ -310,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); }; @@ -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..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{}; }; @@ -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}, @@ -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 5d012820..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; @@ -35,21 +42,18 @@ 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]->_payload_size_max == 16); + 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); @@ -61,12 +65,9 @@ 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) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[2]->_sessions)); // Create a response subscription. CanardRxSubscription sub_res{}; @@ -75,12 +76,9 @@ 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) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[1]->_sessions)); REQUIRE(ins.getInstance()._rx_subscriptions[2] == &sub_req); // Create a second response subscription. @@ -90,20 +88,17 @@ 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) - { - REQUIRE(_session == nullptr); - } + REQUIRE(ensureAllNullptr(ins.getInstance()._rx_subscriptions[1]->_sessions)); 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); @@ -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); @@ -155,8 +150,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,49 +210,43 @@ 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. 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 == - 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. 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); @@ -265,21 +254,18 @@ 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. 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_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); 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());