v4 tx poll

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

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)
             {

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,

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
     //