From 5670ba5e6033d25e131024df765352f053fa67b2 Mon Sep 17 00:00:00 2001 From: Sergei Shirokov Date: Tue, 3 Dec 2024 17:20:49 +0200 Subject: [PATCH 1/7] introduce deadline queue #sonar --- libcanard/canard.c | 124 ++++++++++++++++++++++++++++----------- libcanard/canard.h | 27 +++++++-- tests/helpers.hpp | 21 +++++-- tests/test_public_tx.cpp | 34 +++++------ 4 files changed, 148 insertions(+), 58 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index 95d8280..f884015 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -3,6 +3,7 @@ /// Author: Pavel Kirienko #include "canard.h" +#include #include // --------------------------------------------- BUILD CONFIGURATION --------------------------------------------- @@ -299,10 +300,15 @@ CANARD_PRIVATE struct CanardTxQueueItem* txAllocateQueueItem(struct CanardTxQueu struct CanardTxQueueItem* out = ins->memory.allocate(ins->memory.user_reference, sizeof(struct CanardTxQueueItem)); if (out != NULL) { - out->base.up = NULL; - out->base.lr[0] = NULL; - out->base.lr[1] = NULL; - out->base.bf = 0; + out->priority_base.up = NULL; + out->priority_base.lr[0] = NULL; + out->priority_base.lr[1] = NULL; + out->priority_base.bf = 0; + + out->deadline_base.up = NULL; + out->deadline_base.lr[0] = NULL; + out->deadline_base.lr[1] = NULL; + out->deadline_base.bf = 0; out->next_in_transfer = NULL; // Last by default. out->tx_deadline_usec = deadline_usec; @@ -326,18 +332,39 @@ CANARD_PRIVATE struct CanardTxQueueItem* txAllocateQueueItem(struct CanardTxQueu return out; } +#define CONTAINER_OF(type, ptr, member) ((type*) (((ptr) == NULL) ? NULL : ((char*) (ptr) - offsetof(type, member)))) + /// Frames with identical CAN ID that are added later always compare greater than their counterparts with same CAN ID. /// This ensures that CAN frames with the same CAN ID are transmitted in the FIFO order. /// Frames that should be transmitted earlier compare smaller (i.e., put on the left side of the tree). -CANARD_PRIVATE int8_t txAVLPredicate(void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. - const struct CanardTreeNode* const node) +CANARD_PRIVATE int8_t txAVLPriorityPredicate( // + void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. + const struct CanardTreeNode* const node) { - const struct CanardTxQueueItem* const target = (const struct CanardTxQueueItem*) user_reference; - const struct CanardTxQueueItem* const other = (const struct CanardTxQueueItem*) (const void*) node; + typedef const struct CanardTxQueueItem ConstTxItem; + + ConstTxItem* const target = CONTAINER_OF(ConstTxItem, user_reference, priority_base); + ConstTxItem* const other = CONTAINER_OF(ConstTxItem, node, priority_base); CANARD_ASSERT((target != NULL) && (other != NULL)); return (target->frame.extended_can_id >= other->frame.extended_can_id) ? +1 : -1; } +/// Frames with identical deadline +/// that are added later always compare greater than their counterparts with the same deadline. +/// This ensures that CAN frames with the same deadline are, when timed out, dropped in the FIFO order. +/// Frames that should be dropped earlier compare smaller (i.e., put on the left side of the tree). +CANARD_PRIVATE int8_t txAVLDeadlinePredicate( // + void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. + const struct CanardTreeNode* const node) +{ + typedef const struct CanardTxQueueItem ConstTxItem; + + ConstTxItem* const target = CONTAINER_OF(ConstTxItem, user_reference, deadline_base); + ConstTxItem* const other = CONTAINER_OF(ConstTxItem, node, deadline_base); + CANARD_ASSERT((target != NULL) && (other != NULL)); + return (target->tx_deadline_usec >= other->tx_deadline_usec) ? +1 : -1; +} + /// Returns the number of frames enqueued or error (i.e., =1 or <0). CANARD_PRIVATE int32_t txPushSingleFrame(struct CanardTxQueue* const que, const struct CanardInstance* const ins, @@ -369,11 +396,19 @@ CANARD_PRIVATE int32_t txPushSingleFrame(struct CanardTxQueue* const que, uint8_t* const frame_bytes = tqi->frame.payload.data; (void) memset(frame_bytes + payload.size, PADDING_BYTE_VALUE, padding_size); // NOLINT *(frame_bytes + (frame_payload_size - 1U)) = txMakeTailByte(true, true, true, transfer_id); - // Insert the newly created TX item into the queue. - const struct CanardTreeNode* const res = - cavlSearch(&que->root, &tqi->base, &txAVLPredicate, &avlTrivialFactory); - (void) res; - CANARD_ASSERT(res == &tqi->base); + + // Insert the newly created TX item into the priority queue. + const struct CanardTreeNode* const priority_queue_res = + cavlSearch(&que->priority_root, &tqi->priority_base, &txAVLPriorityPredicate, &avlTrivialFactory); + (void) priority_queue_res; + CANARD_ASSERT(priority_queue_res == &tqi->priority_base); + + // Insert the newly created TX item into the deadline queue. + const struct CanardTreeNode* const deadline_queue_res = + cavlSearch(&que->deadline_root, &tqi->deadline_base, &txAVLDeadlinePredicate, &avlTrivialFactory); + (void) deadline_queue_res; + CANARD_ASSERT(deadline_queue_res == &tqi->deadline_base); + que->size++; CANARD_ASSERT(que->size <= que->capacity); out = 1; // One frame enqueued. @@ -514,11 +549,18 @@ CANARD_PRIVATE int32_t txPushMultiFrame(struct CanardTxQueue* const que, struct CanardTxQueueItem* next = sq.head; do { - const struct CanardTreeNode* const res = - cavlSearch(&que->root, &next->base, &txAVLPredicate, &avlTrivialFactory); - (void) res; - CANARD_ASSERT(res == &next->base); - CANARD_ASSERT(que->root != NULL); + const struct CanardTreeNode* const priority_queue_res = + cavlSearch(&que->priority_root, &next->priority_base, &txAVLPriorityPredicate, &avlTrivialFactory); + (void) priority_queue_res; + CANARD_ASSERT(priority_queue_res == &next->priority_base); + CANARD_ASSERT(que->priority_root != NULL); + + const struct CanardTreeNode* const deadline_queue_res = + cavlSearch(&que->deadline_root, &next->deadline_base, &txAVLDeadlinePredicate, &avlTrivialFactory); + (void) deadline_queue_res; + CANARD_ASSERT(deadline_queue_res == &next->deadline_base); + CANARD_ASSERT(que->deadline_root != NULL); + next = next->next_in_transfer; } while (next != NULL); CANARD_ASSERT(num_frames == sq.size); @@ -1006,7 +1048,7 @@ rxSubscriptionPredicateOnPortID(void* const user_reference, // NOSONAR Cavl API const struct CanardTreeNode* const node) { const CanardPortID sought = *((const CanardPortID*) user_reference); - const CanardPortID other = ((const struct CanardRxSubscription*) (const void*) node)->port_id; + const CanardPortID other = CONTAINER_OF(const struct CanardRxSubscription, node, base)->port_id; static const int8_t NegPos[2] = {-1, +1}; // Clang-Tidy mistakenly identifies a narrowing cast to int8_t here, which is incorrect. return (sought == other) ? 0 : NegPos[sought > other]; // NOLINT no narrowing conversion is taking place here @@ -1016,7 +1058,9 @@ CANARD_PRIVATE int8_t rxSubscriptionPredicateOnStruct(void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. const struct CanardTreeNode* const node) { - return rxSubscriptionPredicateOnPortID(&((struct CanardRxSubscription*) user_reference)->port_id, node); + return rxSubscriptionPredicateOnPortID( // + &CONTAINER_OF(struct CanardRxSubscription, user_reference, base)->port_id, + node); } // --------------------------------------------- PUBLIC API --------------------------------------------- @@ -1056,7 +1100,8 @@ struct CanardTxQueue canardTxInit(const size_t capacity, .capacity = capacity, .mtu_bytes = mtu_bytes, .size = 0, - .root = NULL, + .priority_root = NULL, + .deadline_root = NULL, .memory = memory, .user_reference = NULL, }; @@ -1067,8 +1112,11 @@ int32_t canardTxPush(struct CanardTxQueue* const que, const struct CanardInstance* const ins, const CanardMicrosecond tx_deadline_usec, const struct CanardTransferMetadata* const metadata, - const struct CanardPayload payload) + const struct CanardPayload payload, + const CanardMicrosecond now_usec) { + (void) now_usec; + int32_t out = -CANARD_ERROR_INVALID_ARGUMENT; if ((ins != NULL) && (que != NULL) && (metadata != NULL) && ((payload.data != NULL) || (0U == payload.size))) { @@ -1114,7 +1162,8 @@ struct CanardTxQueueItem* canardTxPeek(const struct CanardTxQueue* const que) { // Paragraph 6.7.2.1.15 of the C standard says: // A pointer to a structure object, suitably converted, points to its initial member, and vice versa. - out = (struct CanardTxQueueItem*) (void*) cavlFindExtremum(que->root, false); + struct CanardTreeNode* const priority_node = cavlFindExtremum(que->priority_root, false); + out = CONTAINER_OF(struct CanardTxQueueItem, priority_node, priority_base); } return out; } @@ -1127,7 +1176,8 @@ struct CanardTxQueueItem* canardTxPop(struct CanardTxQueue* const que, struct Ca // A pointer to a structure object, suitably converted, points to its initial member, and vice versa. // Note that the highest-priority frame is always a leaf node in the AVL tree, which means that it is very // cheap to remove. - cavlRemove(&que->root, &item->base); + cavlRemove(&que->priority_root, &item->priority_base); + cavlRemove(&que->deadline_root, &item->deadline_base); que->size--; } return item; @@ -1169,11 +1219,13 @@ int8_t canardRxAccept(struct CanardInstance* const ins, // This is the reason the function has a logarithmic time complexity of the number of subscriptions. // Note also that this one of the two variable-complexity operations in the RX pipeline; the other one // is memcpy(). Excepting these two cases, the entire RX pipeline contains neither loops nor recursion. - struct CanardRxSubscription* const sub = (struct CanardRxSubscription*) (void*) + struct CanardRxSubscription* const sub = CONTAINER_OF( // + struct CanardRxSubscription, cavlSearch(&ins->rx_subscriptions[(size_t) model.transfer_kind], &model.port_id, &rxSubscriptionPredicateOnPortID, - NULL); + NULL), + base); if (out_subscription != NULL) { *out_subscription = sub; // Expose selected instance to the caller. @@ -1230,7 +1282,7 @@ int8_t canardRxSubscribe(struct CanardInstance* const ins, out_subscription->sessions[i] = NULL; } const struct CanardTreeNode* const res = cavlSearch(&ins->rx_subscriptions[tk], - out_subscription, + &out_subscription->base, &rxSubscriptionPredicateOnStruct, &avlTrivialFactory); (void) res; @@ -1249,9 +1301,12 @@ int8_t canardRxUnsubscribe(struct CanardInstance* const ins, const size_t tk = (size_t) transfer_kind; if ((ins != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS)) { - CanardPortID port_id_mutable = port_id; - struct CanardRxSubscription* const sub = (struct CanardRxSubscription*) (void*) - cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL); + CanardPortID port_id_mutable = port_id; + + struct CanardRxSubscription* const sub = CONTAINER_OF( // + struct CanardRxSubscription, + cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL), + base); if (sub != NULL) { cavlRemove(&ins->rx_subscriptions[tk], &sub->base); @@ -1287,9 +1342,12 @@ int8_t canardRxGetSubscription(struct CanardInstance* const ins, const size_t tk = (size_t) transfer_kind; if ((ins != NULL) && (tk < CANARD_NUM_TRANSFER_KINDS)) { - CanardPortID port_id_mutable = port_id; - struct CanardRxSubscription* const sub = (struct CanardRxSubscription*) (void*) - cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL); + CanardPortID port_id_mutable = port_id; + + struct CanardRxSubscription* const sub = CONTAINER_OF( // + struct CanardRxSubscription, + cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL), + base); if (sub != NULL) { CANARD_ASSERT(sub->port_id == port_id); diff --git a/libcanard/canard.h b/libcanard/canard.h index 443ccf2..d76bb7c 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -306,6 +306,13 @@ struct CanardMemoryResource CanardMemoryAllocate allocate; ///< Shall be a valid pointer. }; +/// Holds the statistics of a transmission queue. +struct CanardTxQueueStats +{ + /// Holds number of dropped TX frames (due to timeout when `now > deadline`). + size_t dropped_frames; +}; + /// Prioritized transmission queue that keeps CAN frames destined for transmission via one CAN interface. /// Applications with redundant interfaces are expected to have one instance of this type per interface. /// Applications that are not interested in transmission may have zero queues. @@ -335,7 +342,10 @@ struct CanardTxQueue size_t size; /// The root of the priority queue is NULL if the queue is empty. Do not modify this field! - struct CanardTreeNode* root; + struct CanardTreeNode* priority_root; + + /// The root of the deadline queue is NULL if the queue is empty. Do not modify this field! + struct CanardTreeNode* deadline_root; /// The memory resource used by this queue for allocating the payload data (CAN frames). /// There is exactly one allocation of payload buffer per enqueued item (not considering the item itself @@ -354,13 +364,19 @@ struct CanardTxQueue /// This field can be arbitrarily mutated by the user. It is never accessed by the library. /// Its purpose is to simplify integration with OOP interfaces. void* user_reference; + + /// Holds the statistics of this TX queue. + struct CanardTxQueueStats stats; }; /// One frame stored in the transmission queue along with its metadata. struct CanardTxQueueItem { /// Internal use only; do not access this field. - struct CanardTreeNode base; + struct CanardTreeNode priority_base; + + /// Internal use only; do not access this field. + struct CanardTreeNode deadline_base; /// Points to the next frame in this transfer or NULL. This field is mostly intended for own needs of the library. /// Normally, the application would not use it because transfer frame ordering is orthogonal to global TX ordering. @@ -502,7 +518,9 @@ struct CanardTxQueue canardTxInit(const size_t capacity, /// frames (so all frames will have the same timestamp value). This feature is intended to facilitate transmission /// deadline tracking, i.e., aborting frames that could not be transmitted before the specified deadline. /// Therefore, normally, the timestamp value should be in the future. -/// The library itself, however, does not use or check this value in any way, so it can be zero if not needed. +/// The library uses `now > deadline` comparison to determine which frames timed out, and so could +/// be dropped (incrementing `CanardTxQueueStats::dropped_frames` field per such a frame). +/// If this timeout behavior is not needed, the timestamp value can be set to zero. /// /// The function returns the number of frames enqueued into the prioritized TX queue (which is always a positive /// number) in case of success (so that the application can track the number of items in the TX queue if necessary). @@ -539,7 +557,8 @@ int32_t canardTxPush(struct CanardTxQueue* const que, const struct CanardInstance* const ins, const CanardMicrosecond tx_deadline_usec, const struct CanardTransferMetadata* const metadata, - const struct CanardPayload payload); + const struct CanardPayload payload, + const CanardMicrosecond now_usec); /// This function accesses the top element of the prioritized transmission queue. The queue itself is not modified /// (i.e., the accessed element is not removed). The application should invoke this function to collect the transport diff --git a/tests/helpers.hpp b/tests/helpers.hpp index 12fdd33..322d90d 100644 --- a/tests/helpers.hpp +++ b/tests/helpers.hpp @@ -296,11 +296,12 @@ class TxQueue [[nodiscard]] auto push(CanardInstance* const ins, const CanardMicrosecond transmission_deadline_usec, const CanardTransferMetadata& metadata, - const struct CanardPayload payload) + const struct CanardPayload payload, + const CanardMicrosecond now_usec = 0ULL) { checkInvariants(); const auto size_before = que_.size; - const auto ret = canardTxPush(&que_, ins, transmission_deadline_usec, &metadata, payload); + const auto ret = canardTxPush(&que_, ins, transmission_deadline_usec, &metadata, payload, now_usec); const auto num_added = static_cast(ret); enforce((ret < 0) || ((size_before + num_added) == que_.size), "Unexpected size change after push"); checkInvariants(); @@ -341,7 +342,18 @@ class TxQueue [[nodiscard]] auto getSize() const { std::size_t out = 0; - traverse(que_.root, [&](auto* _) { + traverse(que_.priority_root, [&](auto* _) { + (void) _; + out++; + }); + enforce(que_.size == out, "Size miscalculation"); + return out; + } + + [[nodiscard]] auto getDeadlineQueueSize() const + { + std::size_t out = 0; + traverse(que_.deadline_root, [&](auto* _) { (void) _; out++; }); @@ -352,7 +364,7 @@ class TxQueue [[nodiscard]] auto linearize() const -> std::vector { std::vector out; - traverse(que_.root, [&](const CanardTreeNode* const item) { + traverse(que_.priority_root, [&](const CanardTreeNode* const item) { out.push_back(reinterpret_cast(item)); }); enforce(out.size() == getSize(), "Internal error"); @@ -393,6 +405,7 @@ class TxQueue { enforce(que_.user_reference == this, "User reference damaged"); enforce(que_.size == getSize(), "Size miscalculation"); + enforce(que_.size == getDeadlineQueueSize(), "Deadline queue size miscalculation"); } TestAllocator allocator_; diff --git a/tests/test_public_tx.cpp b/tests/test_public_tx.cpp index 416816b..c852a2d 100644 --- a/tests/test_public_tx.cpp +++ b/tests/test_public_tx.cpp @@ -30,7 +30,7 @@ TEST_CASE("TxBasic0") REQUIRE(0 == que.getSize()); REQUIRE(0 == alloc.getNumAllocatedFragments()); - alloc.setAllocationCeiling(400); + alloc.setAllocationCeiling(496); CanardTransferMetadata meta{}; @@ -72,7 +72,7 @@ TEST_CASE("TxBasic0") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(20 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Check the TX queue. { @@ -123,7 +123,7 @@ TEST_CASE("TxBasic0") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(20 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Pop the queue. // hex(pycyphal.transport.commons.crc.CRC16CCITT.new(list(range(8))).value) @@ -190,7 +190,7 @@ TEST_CASE("TxBasic0") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(40 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Read the generated frames. ti = que.peek(); REQUIRE(nullptr != ti); @@ -235,7 +235,7 @@ TEST_CASE("TxBasic0") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(40 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Read the generated frames. ti = que.peek(); REQUIRE(nullptr != ti); @@ -341,11 +341,11 @@ TEST_CASE("TxBasic0") REQUIRE(nullptr == ti); // Error handling. - REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, nullptr, {0, nullptr})); - REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, &meta, {0, nullptr})); - REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, &ins.getInstance(), 0, &meta, {0, nullptr})); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, nullptr, {0, nullptr}, 0)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, &meta, {0, nullptr}, 0)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, &ins.getInstance(), 0, &meta, {0, nullptr}, 0)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == - canardTxPush(&que.getInstance(), &ins.getInstance(), 0, nullptr, {0, nullptr})); + canardTxPush(&que.getInstance(), &ins.getInstance(), 0, nullptr, {0, nullptr}, 0)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == que.push(&ins.getInstance(), 1'000'000'006'000ULL, meta, {1, nullptr})); REQUIRE(nullptr == canardTxPeek(nullptr)); @@ -408,7 +408,7 @@ TEST_CASE("TxBasic1") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(20 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Check the TX queue. { @@ -449,7 +449,7 @@ TEST_CASE("TxBasic1") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(20 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Pop the queue. // hex(pycyphal.transport.commons.crc.CRC16CCITT.new(list(range(8))).value) @@ -514,7 +514,7 @@ TEST_CASE("TxBasic1") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(40 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Read the generated frames. ti = que.peek(); REQUIRE(nullptr != ti); @@ -559,7 +559,7 @@ TEST_CASE("TxBasic1") REQUIRE(3 == que.getSize()); REQUIRE(6 == alloc.getNumAllocatedFragments()); REQUIRE(40 < alloc.getTotalAllocatedAmount()); - REQUIRE(400 > alloc.getTotalAllocatedAmount()); + REQUIRE(496 > alloc.getTotalAllocatedAmount()); // Read the generated frames. ti = que.peek(); REQUIRE(nullptr != ti); @@ -665,11 +665,11 @@ TEST_CASE("TxBasic1") REQUIRE(nullptr == ti); // Error handling. - REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, nullptr, {0, nullptr})); - REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, &meta, {0, nullptr})); - REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, &ins.getInstance(), 0, &meta, {0, nullptr})); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, nullptr, {0, nullptr}, 0)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, nullptr, 0, &meta, {0, nullptr}, 0)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == canardTxPush(nullptr, &ins.getInstance(), 0, &meta, {0, nullptr}, 0)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == - canardTxPush(&que.getInstance(), &ins.getInstance(), 0, nullptr, {0, nullptr})); + canardTxPush(&que.getInstance(), &ins.getInstance(), 0, nullptr, {0, nullptr}, 0)); REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == que.push(&ins.getInstance(), 1'000'000'006'000ULL, meta, {1, nullptr})); REQUIRE(nullptr == canardTxPeek(nullptr)); From 3c1535f4e816f9c70d913ab356ee513915bec716 Mon Sep 17 00:00:00 2001 From: Sergei Date: Tue, 3 Dec 2024 23:48:14 +0200 Subject: [PATCH 2/7] pr review and build fixes #sonar --- libcanard/canard.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index f884015..3f3b000 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -71,6 +71,11 @@ #define INITIAL_TOGGLE_STATE true +#define CONTAINER_OF(type, ptr, member) \ + ((type*) (((ptr) == NULL) ? NULL : (void*) ((char*) (ptr) - offsetof(type, member)))) +#define CONST_CONTAINER_OF(type, ptr, member) \ + ((const type*) (((ptr) == NULL) ? NULL : (const void*) ((const char*) (ptr) - offsetof(type, member)))) + /// Used for inserting new items into AVL trees. CANARD_PRIVATE struct CanardTreeNode* avlTrivialFactory(void* const user_reference) { @@ -332,8 +337,6 @@ CANARD_PRIVATE struct CanardTxQueueItem* txAllocateQueueItem(struct CanardTxQueu return out; } -#define CONTAINER_OF(type, ptr, member) ((type*) (((ptr) == NULL) ? NULL : ((char*) (ptr) - offsetof(type, member)))) - /// Frames with identical CAN ID that are added later always compare greater than their counterparts with same CAN ID. /// This ensures that CAN frames with the same CAN ID are transmitted in the FIFO order. /// Frames that should be transmitted earlier compare smaller (i.e., put on the left side of the tree). @@ -341,10 +344,10 @@ CANARD_PRIVATE int8_t txAVLPriorityPredicate( // void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. const struct CanardTreeNode* const node) { - typedef const struct CanardTxQueueItem ConstTxItem; + typedef struct CanardTxQueueItem TxItem; - ConstTxItem* const target = CONTAINER_OF(ConstTxItem, user_reference, priority_base); - ConstTxItem* const other = CONTAINER_OF(ConstTxItem, node, priority_base); + const TxItem* const target = CONST_CONTAINER_OF(TxItem, user_reference, priority_base); + const TxItem* const other = CONST_CONTAINER_OF(TxItem, node, priority_base); CANARD_ASSERT((target != NULL) && (other != NULL)); return (target->frame.extended_can_id >= other->frame.extended_can_id) ? +1 : -1; } @@ -357,10 +360,10 @@ CANARD_PRIVATE int8_t txAVLDeadlinePredicate( // void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. const struct CanardTreeNode* const node) { - typedef const struct CanardTxQueueItem ConstTxItem; + typedef struct CanardTxQueueItem TxItem; - ConstTxItem* const target = CONTAINER_OF(ConstTxItem, user_reference, deadline_base); - ConstTxItem* const other = CONTAINER_OF(ConstTxItem, node, deadline_base); + const TxItem* const target = CONST_CONTAINER_OF(TxItem, user_reference, deadline_base); + const TxItem* const other = CONST_CONTAINER_OF(TxItem, node, deadline_base); CANARD_ASSERT((target != NULL) && (other != NULL)); return (target->tx_deadline_usec >= other->tx_deadline_usec) ? +1 : -1; } @@ -1048,7 +1051,7 @@ rxSubscriptionPredicateOnPortID(void* const user_reference, // NOSONAR Cavl API const struct CanardTreeNode* const node) { const CanardPortID sought = *((const CanardPortID*) user_reference); - const CanardPortID other = CONTAINER_OF(const struct CanardRxSubscription, node, base)->port_id; + const CanardPortID other = CONST_CONTAINER_OF(struct CanardRxSubscription, node, base)->port_id; static const int8_t NegPos[2] = {-1, +1}; // Clang-Tidy mistakenly identifies a narrowing cast to int8_t here, which is incorrect. return (sought == other) ? 0 : NegPos[sought > other]; // NOLINT no narrowing conversion is taking place here From 4f1a28c4a0888bc9d898b783821cabe2bb457f52 Mon Sep 17 00:00:00 2001 From: Sergei Date: Wed, 4 Dec 2024 00:14:35 +0200 Subject: [PATCH 3/7] build fixes #sonar --- libcanard/canard.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index 3f3b000..3661dad 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -72,9 +72,9 @@ #define INITIAL_TOGGLE_STATE true #define CONTAINER_OF(type, ptr, member) \ - ((type*) (((ptr) == NULL) ? NULL : (void*) ((char*) (ptr) - offsetof(type, member)))) + ((type*) (((ptr) == NULL) ? NULL : (void*) ((char*) (ptr) -offsetof(type, member)))) #define CONST_CONTAINER_OF(type, ptr, member) \ - ((const type*) (((ptr) == NULL) ? NULL : (const void*) ((const char*) (ptr) - offsetof(type, member)))) + ((const type*) (((ptr) == NULL) ? NULL : (const void*) ((const char*) (ptr) -offsetof(type, member)))) /// Used for inserting new items into AVL trees. CANARD_PRIVATE struct CanardTreeNode* avlTrivialFactory(void* const user_reference) @@ -1050,6 +1050,7 @@ CANARD_PRIVATE int8_t rxSubscriptionPredicateOnPortID(void* const user_reference, // NOSONAR Cavl API requires pointer to non-const. const struct CanardTreeNode* const node) { + CANARD_ASSERT((user_reference != NULL) && (node != NULL)); const CanardPortID sought = *((const CanardPortID*) user_reference); const CanardPortID other = CONST_CONTAINER_OF(struct CanardRxSubscription, node, base)->port_id; static const int8_t NegPos[2] = {-1, +1}; From 7ec9051e9ccbd627ecec2a1acee716426c76daf7 Mon Sep 17 00:00:00 2001 From: Sergei Date: Wed, 4 Dec 2024 00:43:15 +0200 Subject: [PATCH 4/7] minor fix --- libcanard/canard.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index 3661dad..7f01803 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -72,9 +72,9 @@ #define INITIAL_TOGGLE_STATE true #define CONTAINER_OF(type, ptr, member) \ - ((type*) (((ptr) == NULL) ? NULL : (void*) ((char*) (ptr) -offsetof(type, member)))) + ((type*) (((ptr) == NULL) ? NULL : (void*) (((char*) (ptr)) -offsetof(type, member)))) #define CONST_CONTAINER_OF(type, ptr, member) \ - ((const type*) (((ptr) == NULL) ? NULL : (const void*) ((const char*) (ptr) -offsetof(type, member)))) + ((const type*) (((ptr) == NULL) ? NULL : (const void*) (((const char*) (ptr)) -offsetof(type, member)))) /// Used for inserting new items into AVL trees. CANARD_PRIVATE struct CanardTreeNode* avlTrivialFactory(void* const user_reference) From c67a59c42a87d9e4485fc68535c6dc40c3ff3017 Mon Sep 17 00:00:00 2001 From: Sergei Date: Wed, 4 Dec 2024 00:44:56 +0200 Subject: [PATCH 5/7] style fix --- libcanard/canard.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index 7f01803..dcf9bfd 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -72,9 +72,9 @@ #define INITIAL_TOGGLE_STATE true #define CONTAINER_OF(type, ptr, member) \ - ((type*) (((ptr) == NULL) ? NULL : (void*) (((char*) (ptr)) -offsetof(type, member)))) + ((type*) (((ptr) == NULL) ? NULL : (void*) (((char*) (ptr)) - offsetof(type, member)))) #define CONST_CONTAINER_OF(type, ptr, member) \ - ((const type*) (((ptr) == NULL) ? NULL : (const void*) (((const char*) (ptr)) -offsetof(type, member)))) + ((const type*) (((ptr) == NULL) ? NULL : (const void*) (((const char*) (ptr)) - offsetof(type, member)))) /// Used for inserting new items into AVL trees. CANARD_PRIVATE struct CanardTreeNode* avlTrivialFactory(void* const user_reference) From ee67adc57b2a9efa02bd14ed28c61a7dc04a394b Mon Sep 17 00:00:00 2001 From: Sergei Shirokov Date: Wed, 4 Dec 2024 16:07:50 +0200 Subject: [PATCH 6/7] `canardTxPush` now flushes expired frames --- libcanard/canard.c | 62 +++++++++++++++++---- libcanard/canard.h | 7 ++- tests/helpers.hpp | 14 +++-- tests/test_public_tx.cpp | 116 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 19 deletions(-) diff --git a/libcanard/canard.c b/libcanard/canard.c index dcf9bfd..098d362 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -72,9 +72,9 @@ #define INITIAL_TOGGLE_STATE true #define CONTAINER_OF(type, ptr, member) \ - ((type*) (((ptr) == NULL) ? NULL : (void*) (((char*) (ptr)) - offsetof(type, member)))) -#define CONST_CONTAINER_OF(type, ptr, member) \ ((const type*) (((ptr) == NULL) ? NULL : (const void*) (((const char*) (ptr)) - offsetof(type, member)))) +#define MUTABLE_CONTAINER_OF(type, ptr, member) \ + ((type*) (((ptr) == NULL) ? NULL : (void*) (((char*) (ptr)) - offsetof(type, member)))) /// Used for inserting new items into AVL trees. CANARD_PRIVATE struct CanardTreeNode* avlTrivialFactory(void* const user_reference) @@ -346,8 +346,8 @@ CANARD_PRIVATE int8_t txAVLPriorityPredicate( // { typedef struct CanardTxQueueItem TxItem; - const TxItem* const target = CONST_CONTAINER_OF(TxItem, user_reference, priority_base); - const TxItem* const other = CONST_CONTAINER_OF(TxItem, node, priority_base); + const TxItem* const target = CONTAINER_OF(TxItem, user_reference, priority_base); + const TxItem* const other = CONTAINER_OF(TxItem, node, priority_base); CANARD_ASSERT((target != NULL) && (other != NULL)); return (target->frame.extended_can_id >= other->frame.extended_can_id) ? +1 : -1; } @@ -362,8 +362,8 @@ CANARD_PRIVATE int8_t txAVLDeadlinePredicate( // { typedef struct CanardTxQueueItem TxItem; - const TxItem* const target = CONST_CONTAINER_OF(TxItem, user_reference, deadline_base); - const TxItem* const other = CONST_CONTAINER_OF(TxItem, node, deadline_base); + const TxItem* const target = CONTAINER_OF(TxItem, user_reference, deadline_base); + const TxItem* const other = CONTAINER_OF(TxItem, node, deadline_base); CANARD_ASSERT((target != NULL) && (other != NULL)); return (target->tx_deadline_usec >= other->tx_deadline_usec) ? +1 : -1; } @@ -592,6 +592,35 @@ CANARD_PRIVATE int32_t txPushMultiFrame(struct CanardTxQueue* const que, return out; } +/// Flushes expired transfers by comparing deadline timestamps of the pending transfers with the current time. +CANARD_PRIVATE void txFlushExpiredTransfers(struct CanardTxQueue* const que, + const struct CanardInstance* const ins, + const CanardMicrosecond now_usec) +{ + struct CanardTxQueueItem* tx_item = NULL; + while (NULL != (tx_item = MUTABLE_CONTAINER_OF( // + struct CanardTxQueueItem, + cavlFindExtremum(que->deadline_root, false), + deadline_base))) + { + if (now_usec <= tx_item->tx_deadline_usec) + { + // The queue is sorted by deadline, so we can stop here. + break; + } + + // All frames of the transfer are released at once b/c they all have the same deadline. + struct CanardTxQueueItem* tx_item_to_free = NULL; + while (NULL != (tx_item_to_free = canardTxPop(que, tx_item))) + { + tx_item = tx_item_to_free->next_in_transfer; + canardTxFree(que, ins, tx_item_to_free); + + que->stats.dropped_frames++; + } + } +} + // --------------------------------------------- RECEPTION --------------------------------------------- #define RX_SESSIONS_PER_SUBSCRIPTION (CANARD_NODE_ID_MAX + 1U) @@ -1052,7 +1081,7 @@ rxSubscriptionPredicateOnPortID(void* const user_reference, // NOSONAR Cavl API { CANARD_ASSERT((user_reference != NULL) && (node != NULL)); const CanardPortID sought = *((const CanardPortID*) user_reference); - const CanardPortID other = CONST_CONTAINER_OF(struct CanardRxSubscription, node, base)->port_id; + const CanardPortID other = CONTAINER_OF(struct CanardRxSubscription, node, base)->port_id; static const int8_t NegPos[2] = {-1, +1}; // Clang-Tidy mistakenly identifies a narrowing cast to int8_t here, which is incorrect. return (sought == other) ? 0 : NegPos[sought > other]; // NOLINT no narrowing conversion is taking place here @@ -1063,7 +1092,7 @@ rxSubscriptionPredicateOnStruct(void* const user_reference, // NOSONAR Cavl API const struct CanardTreeNode* const node) { return rxSubscriptionPredicateOnPortID( // - &CONTAINER_OF(struct CanardRxSubscription, user_reference, base)->port_id, + &MUTABLE_CONTAINER_OF(struct CanardRxSubscription, user_reference, base)->port_id, node); } @@ -1119,6 +1148,15 @@ int32_t canardTxPush(struct CanardTxQueue* const que, const struct CanardPayload payload, const CanardMicrosecond now_usec) { + // Before pushing payload (potentially in multiple frames), we need to try to flush any expired transfers. + // This is necessary to ensure that we don't exhaust the capacity of the queue by holding outdated frames. + // The flushing is done by comparing deadline timestamps of the pending transfers with the current time, + // which makes sense only if the current time is known (bigger than zero). + if (now_usec > 0) + { + txFlushExpiredTransfers(que, ins, now_usec); + } + (void) now_usec; int32_t out = -CANARD_ERROR_INVALID_ARGUMENT; @@ -1167,7 +1205,7 @@ struct CanardTxQueueItem* canardTxPeek(const struct CanardTxQueue* const que) // Paragraph 6.7.2.1.15 of the C standard says: // A pointer to a structure object, suitably converted, points to its initial member, and vice versa. struct CanardTreeNode* const priority_node = cavlFindExtremum(que->priority_root, false); - out = CONTAINER_OF(struct CanardTxQueueItem, priority_node, priority_base); + out = MUTABLE_CONTAINER_OF(struct CanardTxQueueItem, priority_node, priority_base); } return out; } @@ -1223,7 +1261,7 @@ int8_t canardRxAccept(struct CanardInstance* const ins, // This is the reason the function has a logarithmic time complexity of the number of subscriptions. // Note also that this one of the two variable-complexity operations in the RX pipeline; the other one // is memcpy(). Excepting these two cases, the entire RX pipeline contains neither loops nor recursion. - struct CanardRxSubscription* const sub = CONTAINER_OF( // + struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF( // struct CanardRxSubscription, cavlSearch(&ins->rx_subscriptions[(size_t) model.transfer_kind], &model.port_id, @@ -1307,7 +1345,7 @@ int8_t canardRxUnsubscribe(struct CanardInstance* const ins, { CanardPortID port_id_mutable = port_id; - struct CanardRxSubscription* const sub = CONTAINER_OF( // + struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF( // struct CanardRxSubscription, cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL), base); @@ -1348,7 +1386,7 @@ int8_t canardRxGetSubscription(struct CanardInstance* const ins, { CanardPortID port_id_mutable = port_id; - struct CanardRxSubscription* const sub = CONTAINER_OF( // + struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF( // struct CanardRxSubscription, cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL), base); diff --git a/libcanard/canard.h b/libcanard/canard.h index d76bb7c..8ef472e 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -309,7 +309,7 @@ struct CanardMemoryResource /// Holds the statistics of a transmission queue. struct CanardTxQueueStats { - /// Holds number of dropped TX frames (due to timeout when `now > deadline`). + /// Holds number of dropped TX frames due to timeout (when `now > deadline`) or b/c of transmission failures. size_t dropped_frames; }; @@ -458,8 +458,9 @@ struct CanardInstance /// The time complexity models given in the API documentation are made on the assumption that the memory management /// functions have constant complexity O(1). /// - /// The following API functions may allocate memory: canardRxAccept(), canardTxPush(). - /// The following API functions may deallocate memory: canardRxAccept(), canardRxSubscribe(), canardRxUnsubscribe(). + /// The following API functions may allocate memory: canardTxPush(), canardRxAccept(). + /// The following API functions may deallocate memory: canardTxPush(), canardTxFree(), canardRxAccept(), + /// canardRxSubscribe(), canardRxUnsubscribe(). /// The exact memory requirement and usage model is specified for each function in its documentation. struct CanardMemoryResource memory; diff --git a/tests/helpers.hpp b/tests/helpers.hpp index 322d90d..94d2c26 100644 --- a/tests/helpers.hpp +++ b/tests/helpers.hpp @@ -300,11 +300,17 @@ class TxQueue const CanardMicrosecond now_usec = 0ULL) { checkInvariants(); - const auto size_before = que_.size; - const auto ret = canardTxPush(&que_, ins, transmission_deadline_usec, &metadata, payload, now_usec); - const auto num_added = static_cast(ret); - enforce((ret < 0) || ((size_before + num_added) == que_.size), "Unexpected size change after push"); + + const auto size_before = que_.size; + const auto dropped_before = que_.stats.dropped_frames; + + const auto ret = canardTxPush(&que_, ins, transmission_deadline_usec, &metadata, payload, now_usec); + const auto num_added = static_cast(ret); + + enforce((ret < 0) || ((size_before + num_added + dropped_before - que_.stats.dropped_frames) == que_.size), + "Unexpected size change after push"); checkInvariants(); + return ret; } diff --git a/tests/test_public_tx.cpp b/tests/test_public_tx.cpp index c852a2d..c45ab03 100644 --- a/tests/test_public_tx.cpp +++ b/tests/test_public_tx.cpp @@ -814,3 +814,119 @@ TEST_CASE("TxPayloadOwnership") } } } + +TEST_CASE("TxFlushExpired") +{ + helpers::Instance ins; + helpers::TxQueue que{2, CANARD_MTU_CAN_FD}; // Limit capacity at 2 frames. + + auto& tx_alloc = que.getAllocator(); + auto& ins_alloc = ins.getAllocator(); + + std::array payload{}; + std::iota(payload.begin(), payload.end(), 0U); + + REQUIRE(CANARD_NODE_ID_UNSET == ins.getNodeID()); + REQUIRE(CANARD_MTU_CAN_FD == que.getMTU()); + REQUIRE(0 == que.getSize()); + REQUIRE(0 == tx_alloc.getNumAllocatedFragments()); + REQUIRE(0 == ins_alloc.getNumAllocatedFragments()); + + CanardMicrosecond now = 10'000'000ULL; // 10s + const CanardMicrosecond deadline = 1'000'000ULL; // 1s + + CanardTransferMetadata meta{}; + + // 1. Push single-frame with padding, peek. @ 10s + { + meta.priority = CanardPriorityNominal; + meta.transfer_kind = CanardTransferKindMessage; + meta.port_id = 321; + meta.remote_node_id = CANARD_NODE_ID_UNSET; + meta.transfer_id = 21; + REQUIRE(1 == que.push(&ins.getInstance(), now + deadline, meta, {8, payload.data()}, now)); + REQUIRE(1 == que.getSize()); + REQUIRE(1 == tx_alloc.getNumAllocatedFragments()); + REQUIRE((8 + 4) == tx_alloc.getTotalAllocatedAmount()); + REQUIRE(1 == ins_alloc.getNumAllocatedFragments()); + REQUIRE(sizeof(CanardTxQueueItem) * 1 == ins_alloc.getTotalAllocatedAmount()); + + // Peek and check the payload. + CanardTxQueueItem* ti = que.peek(); + REQUIRE(nullptr != ti); // Make sure we get the same frame again. + REQUIRE(ti->frame.payload.size == 12); + REQUIRE(ti->frame.payload.allocated_size == 12); + REQUIRE(0 == std::memcmp(ti->frame.payload.data, payload.data(), 8)); + REQUIRE(ti->tx_deadline_usec == now + deadline); + REQUIRE(1 == tx_alloc.getNumAllocatedFragments()); + REQUIRE((8 + 4) == tx_alloc.getTotalAllocatedAmount()); + REQUIRE(1 == ins_alloc.getNumAllocatedFragments()); + REQUIRE(sizeof(CanardTxQueueItem) * 1 == ins_alloc.getTotalAllocatedAmount()); + + // Don't pop and free the item - we gonna flush it by the next push at 12s. + } + + now += 2 * deadline; // 10s -> 12s + + // 2. Push two-frames, peek. @ 12s (after 2x deadline) + // These 2 frames should still fit into the queue (with capacity 2) despite one expired frame still there.` + { + que.setMTU(8); + ins.setNodeID(42); + meta.transfer_id = 22; + REQUIRE(2 == que.push(&ins.getInstance(), now + deadline, meta, {8, payload.data()}, now)); + REQUIRE(2 == que.getSize()); + REQUIRE(2 == tx_alloc.getNumAllocatedFragments()); + REQUIRE((8 + 4) == tx_alloc.getTotalAllocatedAmount()); + REQUIRE(2 == ins_alloc.getNumAllocatedFragments()); + REQUIRE(sizeof(CanardTxQueueItem) * 2 == ins_alloc.getTotalAllocatedAmount()); + REQUIRE(1 == que.getInstance().stats.dropped_frames); + + // a) Peek and check the payload of the 1st frame + CanardTxQueueItem* ti = NULL; + { + ti = que.peek(); + REQUIRE(nullptr != ti); + REQUIRE(ti->frame.payload.size == 8); + REQUIRE(ti->frame.payload.allocated_size == 8); + REQUIRE(0 == std::memcmp(ti->frame.payload.data, payload.data(), 7)); + REQUIRE(ti->tx_deadline_usec == now + deadline); + REQUIRE(2 == tx_alloc.getNumAllocatedFragments()); + REQUIRE((8 + 4) == tx_alloc.getTotalAllocatedAmount()); + REQUIRE(2 == ins_alloc.getNumAllocatedFragments()); + REQUIRE(sizeof(CanardTxQueueItem) * 2 == ins_alloc.getTotalAllocatedAmount()); + + // Don't pop and free the item - we gonna flush it by the next push @ 14s. + } + // b) Check the payload of the 2nd frame + { + ti = ti->next_in_transfer; + REQUIRE(nullptr != ti); + REQUIRE(ti->frame.payload.size == 4); + REQUIRE(ti->frame.payload.allocated_size == 4); + REQUIRE(0 == std::memcmp(ti->frame.payload.data, payload.data() + 7, 1)); + REQUIRE(ti->tx_deadline_usec == now + deadline); + + // Don't pop and free the item - we gonna flush it by the next push @ 14s. + } + } + + now += 2 * deadline; // 12s -> 14s + + // 3. Push three-frames, peek. @ 14s (after another 2x deadline) + // These 3 frames should not fit into the queue (with capacity 2), + // but as a side effect, the expired frames (from push @ 12s) should be flushed as well. + { + meta.transfer_id = 23; + REQUIRE(-CANARD_ERROR_OUT_OF_MEMORY == + que.push(&ins.getInstance(), now + deadline, meta, {8 * 2, payload.data()}, now)); + REQUIRE(0 == que.getSize()); + REQUIRE(0 == tx_alloc.getNumAllocatedFragments()); + REQUIRE(0 == tx_alloc.getTotalAllocatedAmount()); + REQUIRE(0 == ins_alloc.getNumAllocatedFragments()); + REQUIRE(0 == ins_alloc.getTotalAllocatedAmount()); + REQUIRE(1 + 2 == que.getInstance().stats.dropped_frames); + + REQUIRE(nullptr == que.peek()); + } +} From 78b62fa05b8878f3c79f753795bc0774c4a4e2a5 Mon Sep 17 00:00:00 2001 From: Sergei Shirokov Date: Wed, 4 Dec 2024 17:37:46 +0200 Subject: [PATCH 7/7] build fix --- tests/test_public_tx.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_public_tx.cpp b/tests/test_public_tx.cpp index c45ab03..3047cfd 100644 --- a/tests/test_public_tx.cpp +++ b/tests/test_public_tx.cpp @@ -883,7 +883,7 @@ TEST_CASE("TxFlushExpired") REQUIRE(1 == que.getInstance().stats.dropped_frames); // a) Peek and check the payload of the 1st frame - CanardTxQueueItem* ti = NULL; + CanardTxQueueItem* ti = nullptr; { ti = que.peek(); REQUIRE(nullptr != ti); @@ -919,7 +919,7 @@ TEST_CASE("TxFlushExpired") { meta.transfer_id = 23; REQUIRE(-CANARD_ERROR_OUT_OF_MEMORY == - que.push(&ins.getInstance(), now + deadline, meta, {8 * 2, payload.data()}, now)); + que.push(&ins.getInstance(), now + deadline, meta, {8ULL * 2ULL, payload.data()}, now)); REQUIRE(0 == que.getSize()); REQUIRE(0 == tx_alloc.getNumAllocatedFragments()); REQUIRE(0 == tx_alloc.getTotalAllocatedAmount());