diff --git a/MIGRATION_v3.x_to_v4.0.md b/MIGRATION_v3.x_to_v4.0.md index 37d5b61..669ffd0 100644 --- a/MIGRATION_v3.x_to_v4.0.md +++ b/MIGRATION_v3.x_to_v4.0.md @@ -217,7 +217,7 @@ Several functions have updated prototypes and usage patterns: Frames in the TX queue that have exceeded their `tx_deadline_usec` can now be automatically dropped when `now_usec` is provided to `canardTxPush()` or `canardTxPoll()`. -- **Benefit**: Reduces the need for manual management of timed-out frames. +- **Benefit**: Reduces the worst-case peak memory footprint. - **Optional**: Feature can be disabled by passing `0` for `now_usec`. #### Migration Steps diff --git a/libcanard/canard.c b/libcanard/canard.c index aff91b3..563025c 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -597,6 +597,10 @@ CANARD_PRIVATE void txPopAndFreeTransfer(struct CanardTxQueue* const que, struct CanardTxQueueItem* const tx_item, const bool drop_whole_transfer) { + CANARD_ASSERT(que != NULL); + CANARD_ASSERT(ins != NULL); + CANARD_ASSERT(tx_item != NULL); + struct CanardTxQueueItem* next_tx_item = tx_item; struct CanardTxQueueItem* tx_item_to_free = canardTxPop(que, next_tx_item); while (NULL != tx_item_to_free) @@ -619,10 +623,12 @@ CANARD_PRIVATE void txFlushExpiredTransfers(struct CanardTxQueue* const q const struct CanardInstance* const ins, const CanardMicrosecond now_usec) { - struct CanardTxQueueItem* tx_item = MUTABLE_CONTAINER_OF( // - struct CanardTxQueueItem, - cavlFindExtremum(que->deadline_root, false), - deadline_base); + CANARD_ASSERT(que != NULL); + CANARD_ASSERT(ins != NULL); + CANARD_ASSERT(now_usec > 0); + + struct CanardTreeNode* tx_node = cavlFindExtremum(que->deadline_root, false); + struct CanardTxQueueItem* tx_item = MUTABLE_CONTAINER_OF(struct CanardTxQueueItem, tx_node, deadline_base); while (NULL != tx_item) { if (now_usec <= tx_item->tx_deadline_usec) @@ -634,10 +640,8 @@ CANARD_PRIVATE void txFlushExpiredTransfers(struct CanardTxQueue* const q // All frames of the transfer are dropped at once b/c they all have the same deadline. txPopAndFreeTransfer(que, ins, tx_item, true); // drop the whole transfer - tx_item = MUTABLE_CONTAINER_OF( // - struct CanardTxQueueItem, - cavlFindExtremum(que->deadline_root, false), - deadline_base); + tx_node = cavlFindExtremum(que->deadline_root, false); + tx_item = MUTABLE_CONTAINER_OF(struct CanardTxQueueItem, tx_node, deadline_base); } } @@ -1245,7 +1249,7 @@ void canardTxFree(struct CanardTxQueue* const que, const struct CanardInstance* const ins, struct CanardTxQueueItem* item) { - if (item != NULL) + if ((que != NULL) && (ins != NULL) && (item != NULL)) { if (item->frame.payload.data != NULL) { @@ -1264,20 +1268,19 @@ int8_t canardTxPoll(struct CanardTxQueue* const que, void* const user_reference, const CanardTxFrameHandler frame_handler) { - int8_t out = 0; - - // Before peeking a frame to transmit, we need to try to flush any expired transfers. - // This will not only ensure ASAP freeing of the queue capacity, but also makes sure that the following - // `canardTxPeek` will return a not expired item (if any), so we don't need to check the deadline again. - // 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) + int8_t out = -CANARD_ERROR_INVALID_ARGUMENT; + if ((que != NULL) && (ins != NULL) && (frame_handler != NULL)) { - txFlushExpiredTransfers(que, ins, now_usec); - } + // Before peeking a frame to transmit, we need to try to flush any expired transfers. + // This will not only ensure ASAP freeing of the queue capacity, but also makes sure that the following + // `canardTxPeek` will return a not expired item (if any), so we don't need to check the deadline again. + // 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); + } - if (frame_handler != NULL) - { struct CanardTxQueueItem* const tx_item = canardTxPeek(que); if (tx_item != NULL) { @@ -1291,11 +1294,15 @@ int8_t canardTxPoll(struct CanardTxQueue* const que, if (out != 0) { // In case of a failure, it makes sense to drop the whole transfer immediately - // b/c at least this frame has been rejected, so the whole transfer is useless. + // b/c at least this frame has been rejected, so the whole transfer is useless. const bool drop_whole_transfer = (out < 0); txPopAndFreeTransfer(que, ins, tx_item, drop_whole_transfer); } } + else + { + out = 0; // No frames to transmit. + } } CANARD_ASSERT(out <= 1); @@ -1321,13 +1328,12 @@ 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 = MUTABLE_CONTAINER_OF( // - struct CanardRxSubscription, - cavlSearch(&ins->rx_subscriptions[(size_t) model.transfer_kind], - &model.port_id, - &rxSubscriptionPredicateOnPortID, - NULL), - base); + struct CanardTreeNode* const sub_node = cavlSearch(&ins->rx_subscriptions[(size_t) model.transfer_kind], + &model.port_id, + &rxSubscriptionPredicateOnPortID, + NULL); + struct CanardRxSubscription* const sub = + MUTABLE_CONTAINER_OF(struct CanardRxSubscription, sub_node, base); if (out_subscription != NULL) { *out_subscription = sub; // Expose selected instance to the caller. @@ -1405,13 +1411,15 @@ int8_t canardRxUnsubscribe(struct CanardInstance* const ins, { CanardPortID port_id_mutable = port_id; - struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF( // - struct CanardRxSubscription, - cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL), - base); - if (sub != NULL) + struct CanardTreeNode* const sub_node = cavlSearch( // + &ins->rx_subscriptions[tk], + &port_id_mutable, + &rxSubscriptionPredicateOnPortID, + NULL); + if (sub_node != NULL) { - cavlRemove(&ins->rx_subscriptions[tk], &sub->base); + struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF(struct CanardRxSubscription, sub_node, base); + cavlRemove(&ins->rx_subscriptions[tk], sub_node); CANARD_ASSERT(sub->port_id == port_id); out = 1; for (size_t i = 0; i < RX_SESSIONS_PER_SUBSCRIPTION; i++) @@ -1446,12 +1454,14 @@ int8_t canardRxGetSubscription(struct CanardInstance* const ins, { CanardPortID port_id_mutable = port_id; - struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF( // - struct CanardRxSubscription, - cavlSearch(&ins->rx_subscriptions[tk], &port_id_mutable, &rxSubscriptionPredicateOnPortID, NULL), - base); - if (sub != NULL) + struct CanardTreeNode* const sub_node = cavlSearch( // + &ins->rx_subscriptions[tk], + &port_id_mutable, + &rxSubscriptionPredicateOnPortID, + NULL); + if (sub_node != NULL) { + struct CanardRxSubscription* const sub = MUTABLE_CONTAINER_OF(struct CanardRxSubscription, sub_node, base); CANARD_ASSERT(sub->port_id == port_id); if (out_subscription != NULL) { diff --git a/libcanard/canard.h b/libcanard/canard.h index d54f088..170806e 100644 --- a/libcanard/canard.h +++ b/libcanard/canard.h @@ -313,14 +313,10 @@ struct CanardTxQueueStats size_t dropped_frames; }; -/// Defines the signature of the TX frame handler function. -/// /// The handler function is intended to be invoked from Canard TX polling (see details for the `canardTxPoll()`). /// -/// @param user_reference The user reference passed to `canardTxPoll()`. -/// @param deadline_usec The deadline of the frame that is being handled. -/// @param frame The mutable frame that is being handled. -/// @return The result of the handling operation: +/// The user reference parameter what was passed to canardTxPoll. +/// The return result of the handling operation: /// - Any positive value: the frame was successfully handled. /// This indicates that the frame payload was accepted (and its payload ownership could be potentially moved, /// see `canardTxPeek` for the details), and the frame can be safely removed from the queue. @@ -642,24 +638,23 @@ void canardTxFree(struct CanardTxQueue* const que, struct CanardTxQueueItem* const item); /// This is a helper that combines several Canard TX calls (`canardTxPeek`, `canardTxPop` and `canardTxFree`) -/// into one "polling" algorythm. It simplifies the whole process of transmitting frames to just two function calls: +/// into one "polling" algorithm. It simplifies the whole process of transmitting frames to just two function calls: /// - `canardTxPush` to enqueue the frames /// - `canardTxPoll` to dequeue, transmit and free a single frame /// -/// The algorythm implements a typical pattern of de-queuing, transmitting and freeing a TX queue item, +/// The algorithm implements a typical pattern of de-queuing, transmitting and freeing a TX queue item, /// as well as handling transmission failures, retries, and deadline timeouts. /// /// The function is intended to be periodically called, most probably on a signal that the previous TX frame /// transmission has been completed, and so the next TX frame (if any) could be polled from the TX queue. /// -/// @param que The TX queue to poll. -/// @param ins The Canard instance. -/// @param now_usec The current time in microseconds. It is used to determine if the frame has timed out. -/// Use zero value to disable automatic dropping of timed-out frames. -/// @param user_reference The user reference to be passed to the frame handler. -/// @param frame_handler The frame handler function that will be called to transmit the frame. -/// @return Zero if the queue is empty or there is no frame handler (NULL). -/// Otherwise, the result from the frame handler call. See `CanardTxFrameHandler` documentation. +/// The current time is used to determine if the frame has timed out. Use zero value to disable automatic dropping +/// of timed-out frames. The user reference will be passed to the frame handler (see CanardTxFrameHandler), which +/// will be called to transmit the frame. +/// +/// Return value is zero if the queue is empty, +/// or `-CANARD_ERROR_INVALID_ARGUMENT` if there is no (NULL) queue, instance or handler. +/// Otherwise, the value will be from the result of the frame handler call (see CanardTxFrameHandler). /// int8_t canardTxPoll(struct CanardTxQueue* const que, const struct CanardInstance* const ins, diff --git a/tests/test_public_tx.cpp b/tests/test_public_tx.cpp index b7cbed8..b253860 100644 --- a/tests/test_public_tx.cpp +++ b/tests/test_public_tx.cpp @@ -970,9 +970,14 @@ TEST_CASE("TxPollSingleFrame") REQUIRE(1 == ins_alloc.getNumAllocatedFragments()); REQUIRE(sizeof(CanardTxQueueItem) * 1 == ins_alloc.getTotalAllocatedAmount()); - // 2. Poll without time and handler. + // 2. Poll with invalid arguments. // - REQUIRE(0 == que.poll(ins, 0, nullptr)); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == // null queue + canardTxPoll(nullptr, &ins.getInstance(), 0, nullptr, [](auto*, auto, auto*) -> std::int8_t { return 0; })); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == // null instance + canardTxPoll(&que.getInstance(), nullptr, 0, nullptr, [](auto*, auto, auto*) -> std::int8_t { return 0; })); + REQUIRE(-CANARD_ERROR_INVALID_ARGUMENT == // null handler + canardTxPoll(&que.getInstance(), &ins.getInstance(), 0, nullptr, nullptr)); // 3. Poll; emulate media is busy @ 10s + 100us //