diff --git a/libcanard/canard.c b/libcanard/canard.c index f453c57..a99996b 100644 --- a/libcanard/canard.c +++ b/libcanard/canard.c @@ -825,7 +825,14 @@ CANARD_PRIVATE int8_t rxSessionUpdate(CanardInstance* const ins, CANARD_ASSERT(frame->transfer_id <= CANARD_TRANSFER_ID_MAX); // The transfer ID timeout is measured relative to the timestamp of the last start-of-transfer frame. + // Triggering a TID timeout when the TID is the same is undesirable because it may cause the reassembler to + // switch to another interface if the start-of-transfer frame of the current transfer is duplicated + // on the other interface more than (transfer-ID timeout) units of time after the start of + // the transfer while the reassembly of this transfer is still in progress. + // While this behavior is not visible to the application because the transfer will still be reassembled, + // it may delay the delivery of the transfer. const bool tid_timed_out = (frame->timestamp_usec > rxs->transfer_timestamp_usec) && + (frame->transfer_id != rxs->transfer_id) && ((frame->timestamp_usec - rxs->transfer_timestamp_usec) > transfer_id_timeout_usec); // Examples: rxComputeTransferIDDifference(2, 3)==31 // rxComputeTransferIDDifference(2, 2)==0 @@ -836,12 +843,6 @@ CANARD_PRIVATE int8_t rxSessionUpdate(CanardInstance* const ins, const bool need_restart = frame->start_of_transfer && (tid_timed_out || ((rxs->redundant_transport_index == redundant_transport_index) && not_previous_tid)); - // One interesting trait of this implementation is that if the start-of-transfer frame of the current - // transfer is duplicated on another interface more than (transfer-ID timeout) units of time after the start of - // the transfer while the reassembly of this transfer is still in progress, - // the reassembler will switch to the other interface and restart the transfer reassembly from scratch. - // This effect is not visible to the application because the outcome is the same as if the transfer was - // received on the original interface. if (need_restart) { CANARD_ASSERT(frame->start_of_transfer); diff --git a/tests/test_public_rx.cpp b/tests/test_public_rx.cpp index 89d0608..695f8ef 100644 --- a/tests/test_public_rx.cpp +++ b/tests/test_public_rx.cpp @@ -518,17 +518,17 @@ TEST_CASE("Issue212") 0, 0b001'00'0'11'0110011001100'0'0100111, {1, 2, 3, 4, 5, 6, 7, 0b101'00011})); - REQUIRE(0 == accept(112'000'001, // second frame, transport #0 - 0, + REQUIRE(0 == accept(112'000'001, // second frame, transport #1 + 1, 0b001'00'0'11'0110011001100'0'0100111, {8, 9, 10, 11, 12, 13, 14, 0b000'00011})); - REQUIRE(1 == accept(113'000'002, // third and last frame, transport #0 - 0, + REQUIRE(1 == accept(113'000'002, // third and last frame, transport #1 + 1, 0b001'00'0'11'0110011001100'0'0100111, {0x32, 0xF8, 0b011'00011})); REQUIRE(subscription != nullptr); // Subscription exists. REQUIRE(subscription->port_id == 0b0110011001100); - REQUIRE(transfer.timestamp_usec == 111'000'001); + REQUIRE(transfer.timestamp_usec == 110'000'001); REQUIRE(transfer.metadata.priority == CanardPriorityImmediate); REQUIRE(transfer.metadata.transfer_kind == CanardTransferKindMessage); REQUIRE(transfer.metadata.port_id == 0b0110011001100);