diff --git a/libuavcan/include/uavcan/transport/transfer.hpp b/libuavcan/include/uavcan/transport/transfer.hpp index 4d6e7a712f..6300eb4b18 100644 --- a/libuavcan/include/uavcan/transport/transfer.hpp +++ b/libuavcan/include/uavcan/transport/transfer.hpp @@ -74,6 +74,7 @@ class UAVCAN_EXPORT TransferID public: static const uint8_t BitLen = 5U; static const uint8_t Max = (1U << BitLen) - 1U; + static const uint8_t Half = (1U << BitLen) / 2U; TransferID() : value_(0) diff --git a/libuavcan/include/uavcan/transport/transfer_receiver.hpp b/libuavcan/include/uavcan/transport/transfer_receiver.hpp index 03a1b1f809..20c4b41eea 100644 --- a/libuavcan/include/uavcan/transport/transfer_receiver.hpp +++ b/libuavcan/include/uavcan/transport/transfer_receiver.hpp @@ -21,8 +21,7 @@ public: static const uint16_t MinTransferIntervalMSec = 1; static const uint16_t MaxTransferIntervalMSec = 0xFFFF; static const uint16_t DefaultTransferIntervalMSec = 1000; - static const uint16_t MessageMaxTFallbackMSec = 1000; - static const uint16_t ServiceTFallbackMSec = 500; + static const uint16_t DefaultTidTimeoutMSec = 1000; static MonotonicDuration getDefaultTransferInterval() { @@ -32,11 +31,9 @@ public: static MonotonicDuration getMaxTransferInterval() { return MonotonicDuration::fromMSec(MaxTransferIntervalMSec); } private: - enum TidRelation { TidSame, TidRepeat, TidFuture }; - enum { IfaceIndexNotSet = MaxCanIfaces }; - enum { ErrorCntMask = 15 }; + enum { ErrorCntMask = 31 }; enum { IfaceIndexMask = MaxCanIfaces }; MonotonicTime prev_transfer_ts_; @@ -50,22 +47,19 @@ private: TransferID tid_; // 1 byte field // 1 byte aligned bitfields: - uint8_t tt_service_ : 1; uint8_t next_toggle_ : 1; uint8_t iface_index_ : 2; - mutable uint8_t error_cnt_ : 4; + mutable uint8_t error_cnt_ : 5; bool isInitialized() const { return iface_index_ != IfaceIndexNotSet; } bool isMidTransfer() const { return buffer_write_pos_ > 0; } - MonotonicDuration getTFallback() const; - MonotonicDuration getTTimeout() const; + MonotonicDuration getIfaceSwitchDelay() const; + MonotonicDuration getTidTimeout() const; void registerError() const; - TidRelation getTidRelation(const RxFrame& frame) const; - void updateTransferTimings(); void prepareForNextTransfer(); @@ -78,17 +72,6 @@ public: transfer_interval_msec_(DefaultTransferIntervalMSec), this_transfer_crc_(0), buffer_write_pos_(0), - tt_service_(false), - next_toggle_(false), - iface_index_(IfaceIndexNotSet), - error_cnt_(0) - { } - - TransferReceiver(const TransferType transfer_type) : - transfer_interval_msec_(DefaultTransferIntervalMSec), - this_transfer_crc_(0), - buffer_write_pos_(0), - tt_service_((transfer_type == TransferTypeServiceRequest) || (transfer_type == TransferTypeServiceResponse)), next_toggle_(false), iface_index_(IfaceIndexNotSet), error_cnt_(0) diff --git a/libuavcan/src/transport/uc_transfer.cpp b/libuavcan/src/transport/uc_transfer.cpp index bb5af287b3..58778ef168 100644 --- a/libuavcan/src/transport/uc_transfer.cpp +++ b/libuavcan/src/transport/uc_transfer.cpp @@ -26,6 +26,7 @@ const TransferPriority TransferPriority::Lowest(NumericallyMax); */ const uint8_t TransferID::BitLen; const uint8_t TransferID::Max; +const uint8_t TransferID::Half; /** * NodeID diff --git a/libuavcan/src/transport/uc_transfer_listener.cpp b/libuavcan/src/transport/uc_transfer_listener.cpp index cf0cd2aa23..59ed97dc5c 100644 --- a/libuavcan/src/transport/uc_transfer_listener.cpp +++ b/libuavcan/src/transport/uc_transfer_listener.cpp @@ -207,7 +207,7 @@ void TransferListenerBase::handleFrame(const RxFrame& frame) return; } - TransferReceiver new_recv(frame.getTransferType()); + TransferReceiver new_recv; recv = receivers_.insert(key, new_recv); if (recv == NULL) { diff --git a/libuavcan/src/transport/uc_transfer_receiver.cpp b/libuavcan/src/transport/uc_transfer_receiver.cpp index 116ba02739..435ccd7f39 100644 --- a/libuavcan/src/transport/uc_transfer_receiver.cpp +++ b/libuavcan/src/transport/uc_transfer_receiver.cpp @@ -14,32 +14,16 @@ namespace uavcan const uint16_t TransferReceiver::MinTransferIntervalMSec; const uint16_t TransferReceiver::MaxTransferIntervalMSec; const uint16_t TransferReceiver::DefaultTransferIntervalMSec; -const uint16_t TransferReceiver::MessageMaxTFallbackMSec; -const uint16_t TransferReceiver::ServiceTFallbackMSec; +const uint16_t TransferReceiver::DefaultTidTimeoutMSec; -MonotonicDuration TransferReceiver::getTFallback() const +MonotonicDuration TransferReceiver::getIfaceSwitchDelay() const { - if (tt_service_) - { - return MonotonicDuration::fromMSec(ServiceTFallbackMSec); - } - else - { - const MonotonicDuration t_fallback = MonotonicDuration::fromMSec(transfer_interval_msec_ * 2); - return min(t_fallback, MonotonicDuration::fromMSec(MessageMaxTFallbackMSec)); - } + return MonotonicDuration::fromMSec(transfer_interval_msec_); } -MonotonicDuration TransferReceiver::getTTimeout() const +MonotonicDuration TransferReceiver::getTidTimeout() const { - if (tt_service_) - { - return getTFallback(); - } - else - { - return getTFallback() * 3; - } + return MonotonicDuration::fromMSec(DefaultTidTimeoutMSec); } void TransferReceiver::registerError() const @@ -47,20 +31,6 @@ void TransferReceiver::registerError() const error_cnt_ = static_cast(error_cnt_ + 1) & ErrorCntMask; } -TransferReceiver::TidRelation TransferReceiver::getTidRelation(const RxFrame& frame) const -{ - const int distance = tid_.computeForwardDistance(frame.getTransferID()); - if (distance == 0) - { - return TidSame; - } - if (distance < ((1 << TransferID::BitLen) / 2)) - { - return TidFuture; - } - return TidRepeat; -} - void TransferReceiver::updateTransferTimings() { UAVCAN_ASSERT(!this_transfer_ts_.isZero()); @@ -114,7 +84,7 @@ bool TransferReceiver::validate(const RxFrame& frame) const registerError(); return false; } - if (getTidRelation(frame) != TidSame) + if (frame.getTransferID() != tid_) { UAVCAN_TRACE("TransferReceiver", "Unexpected TID (current %i), %s", tid_.get(), frame.toString().c_str()); registerError(); @@ -210,13 +180,11 @@ TransferReceiver::ResultCode TransferReceiver::receive(const RxFrame& frame, Tra bool TransferReceiver::isTimedOut(MonotonicTime current_ts) const { - return (current_ts - this_transfer_ts_) > getTTimeout(); + return (current_ts - this_transfer_ts_) > getTidTimeout(); } TransferReceiver::ResultCode TransferReceiver::addFrame(const RxFrame& frame, TransferBufferAccessor& tba) { - UAVCAN_ASSERT((getDataTypeKindForTransferType(frame.getTransferType()) == DataTypeKindService) == tt_service_); - if ((frame.getMonotonicTimestamp().isZero()) || (frame.getMonotonicTimestamp() < prev_transfer_ts_) || (frame.getMonotonicTimestamp() < this_transfer_ts_)) @@ -226,35 +194,37 @@ TransferReceiver::ResultCode TransferReceiver::addFrame(const RxFrame& frame, Tr } const bool not_initialized = !isInitialized(); - const bool receiver_timed_out = isTimedOut(frame.getMonotonicTimestamp()); + const bool tid_timed_out = isTimedOut(frame.getMonotonicTimestamp()); const bool same_iface = frame.getIfaceIndex() == iface_index_; - const bool first_fame = frame.isStartOfTransfer(); - const TidRelation tid_rel = getTidRelation(frame); - const bool iface_timed_out = (frame.getMonotonicTimestamp() - this_transfer_ts_) > getTFallback(); + const bool first_frame = frame.isStartOfTransfer(); + const bool non_wrapped_tid = tid_.computeForwardDistance(frame.getTransferID()) < TransferID::Half; + const bool not_previous_tid = frame.getTransferID().computeForwardDistance(tid_) > 1; + const bool iface_switch_allowed = (frame.getMonotonicTimestamp() - this_transfer_ts_) > getIfaceSwitchDelay(); // FSM, the hard way const bool need_restart = - not_initialized || - receiver_timed_out || - ((same_iface || iface_timed_out) && first_fame && (tid_rel == TidFuture)); + (not_initialized) || + (tid_timed_out) || + (same_iface && first_frame && not_previous_tid) || + (iface_switch_allowed && first_frame && non_wrapped_tid); if (need_restart) { - if (!not_initialized && !receiver_timed_out) + if (!not_initialized && (tid_ != frame.getTransferID())) { registerError(); } - UAVCAN_TRACE("TransferReceiver", - "Restart [not_inited=%i, iface_timeout=%i, recv_timeout=%i, same_iface=%i, first_frame=%i, tid_rel=%i], %s", - int(not_initialized), int(iface_timed_out), int(receiver_timed_out), int(same_iface), - int(first_fame), int(tid_rel), frame.toString().c_str()); + UAVCAN_TRACE("TransferReceiver", "Restart [ni=%d, isa=%d, tt=%d, si=%d, ff=%d, nwtid=%d, nptid=%d, tid=%d], %s", + int(not_initialized), int(iface_switch_allowed), int(tid_timed_out), int(same_iface), + int(first_frame), int(non_wrapped_tid), int(not_previous_tid), int(tid_.get()), + frame.toString().c_str()); tba.remove(); iface_index_ = frame.getIfaceIndex() & IfaceIndexMask; tid_ = frame.getTransferID(); next_toggle_ = false; buffer_write_pos_ = 0; this_transfer_crc_ = 0; - if (!first_fame) + if (!first_frame) { tid_.increment(); return ResultNotComplete; diff --git a/libuavcan/test/protocol/transport_stats_provider.cpp b/libuavcan/test/protocol/transport_stats_provider.cpp index b55f2237f8..dc107b3501 100644 --- a/libuavcan/test/protocol/transport_stats_provider.cpp +++ b/libuavcan/test/protocol/transport_stats_provider.cpp @@ -55,7 +55,7 @@ TEST(TransportStatsProvider, Basic) * Sending a malformed frame, it must be registered as tranfer error */ uavcan::Frame frame(uavcan::protocol::GetTransportStats::DefaultDataTypeID, uavcan::TransferTypeServiceRequest, - 2, 1, 0); + 2, 1, 1); frame.setStartOfTransfer(true); frame.setEndOfTransfer(true); uavcan::CanFrame can_frame; @@ -76,11 +76,11 @@ TEST(TransportStatsProvider, Basic) ASSERT_LE(0, nodes.spinBoth(uavcan::MonotonicDuration::fromMSec(10))); ASSERT_TRUE(tsp_cln.collector.result.get()); - ASSERT_EQ(1, tsp_cln.collector.result->getResponse().transfer_errors); // That broken frame - ASSERT_EQ(3, tsp_cln.collector.result->getResponse().transfers_rx); - ASSERT_EQ(2, tsp_cln.collector.result->getResponse().transfers_tx); - ASSERT_EQ(1, tsp_cln.collector.result->getResponse().can_iface_stats.size()); - ASSERT_EQ(72, tsp_cln.collector.result->getResponse().can_iface_stats[0].errors); - ASSERT_EQ(4, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_rx); // Same here - ASSERT_EQ(12, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_tx); + EXPECT_EQ(1, tsp_cln.collector.result->getResponse().transfer_errors); // That broken frame + EXPECT_EQ(3, tsp_cln.collector.result->getResponse().transfers_rx); + EXPECT_EQ(2, tsp_cln.collector.result->getResponse().transfers_tx); + EXPECT_EQ(1, tsp_cln.collector.result->getResponse().can_iface_stats.size()); + EXPECT_EQ(72, tsp_cln.collector.result->getResponse().can_iface_stats[0].errors); + EXPECT_EQ(4, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_rx); // Same here + EXPECT_EQ(12, tsp_cln.collector.result->getResponse().can_iface_stats[0].frames_tx); } diff --git a/libuavcan/test/transport/transfer_receiver.cpp b/libuavcan/test/transport/transfer_receiver.cpp index f5aeca0252..fad00f45b4 100644 --- a/libuavcan/test/transport/transfer_receiver.cpp +++ b/libuavcan/test/transport/transfer_receiver.cpp @@ -78,8 +78,7 @@ struct Context uavcan::TransferReceiver receiver; // Must be default constructible and copyable uavcan::TransferBufferManager bufmgr; - Context(uavcan::TransferType tt) : - receiver(tt), + Context() : bufmgr(pool) { assert(pool.allocate(1) == NULL); @@ -122,7 +121,7 @@ static bool matchBufferContent(const uavcan::ITransferBuffer* tbb, const std::st TEST(TransferReceiver, Basic) { using uavcan::TransferReceiver; - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -194,9 +193,9 @@ TEST(TransferReceiver, Basic) CHECK_SINGLE_FRAME(rcv.addFrame(gen(0, "", SET110, 3, 2500), bk)); ASSERT_EQ(2500, rcv.getLastTransferTimestampMonotonic().toUSec()); - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "", SET110, 0, 3000), bk)); // Old TID - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "", SET110, 1, 3100), bk)); // Old TID - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "", SET110, 3, 3200), bk)); // Old TID + CHECK_SINGLE_FRAME(rcv.addFrame(gen(0, "", SET110, 0, 3000), bk)); + CHECK_SINGLE_FRAME(rcv.addFrame(gen(0, "", SET110, 1, 3100), bk)); + CHECK_SINGLE_FRAME(rcv.addFrame(gen(0, "", SET110, 3, 3200), bk)); CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "", SET110, 0, 3300), bk)); // Old TID, wrong iface CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "", SET110, 2, 3400), bk)); // Old TID, wrong iface CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "", SET110, 3, 3500), bk)); // Old TID, wrong iface @@ -261,7 +260,7 @@ TEST(TransferReceiver, Basic) TEST(TransferReceiver, OutOfBufferSpace_32bytes) { - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -299,7 +298,7 @@ TEST(TransferReceiver, OutOfBufferSpace_32bytes) TEST(TransferReceiver, OutOfOrderFrames) { - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -323,7 +322,7 @@ TEST(TransferReceiver, OutOfOrderFrames) TEST(TransferReceiver, IntervalMeasurement) { - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -353,7 +352,7 @@ TEST(TransferReceiver, IntervalMeasurement) TEST(TransferReceiver, Restart) { - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -368,38 +367,40 @@ TEST(TransferReceiver, Restart) CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "-------", SET010, 0, 100000200), bk)); // Ignored /* - * Begins immediately after, gets an iface timeout but completes OK + * Begins immediately after, encounters a delay 0.9 sec but completes OK */ CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "1234567", SET100, 0, 100000300), bk)); // Begin - CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "1234567", SET001, 0, 102000300), bk)); // 2 sec later, iface timeout - CHECK_COMPLETE( rcv.addFrame(gen(1, "1234567", SET010, 0, 102000400), bk)); // OK nevertheless + CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "1234567", SET001, 0, 100900300), bk)); // 0.9 sec later + CHECK_COMPLETE( rcv.addFrame(gen(1, "1234567", SET010, 0, 100900400), bk)); // OK nevertheless ASSERT_TRUE(matchBufferContent(bufmgr.access(gen.bufmgr_key), "3456712345671234567")); ASSERT_EQ(0x3231, rcv.getLastTransferCrc()); + std::cout << "Interval: " << rcv.getInterval().toString() << std::endl; + /* - * Begins OK, gets an iface timeout, switches to another iface + * Begins OK, gets a timeout, switches to another iface */ CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "-------", SET100, 1, 103000500), bk)); // Begin - CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "-------", SET001, 1, 104000500), bk)); // 1 sec later, iface timeout - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "-------", SET001, 1, 104000600), bk)); // Same TID, another iface - ignore - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "-------", SET001, 2, 104000700), bk)); // Not first frame - ignore - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "1234567", SET100, 2, 104000800), bk)); // First, another iface - restart - CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "-------", SET010, 1, 104000600), bk)); // Old iface - ignore - CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "1234567", SET001, 2, 104000900), bk)); // Continuing - CHECK_COMPLETE( rcv.addFrame(gen(0, "1234567", SET010, 2, 104000910), bk)); // Done + CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "-------", SET001, 1, 105000500), bk)); // 2 sec later, timeout + CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "-------", SET001, 1, 105000600), bk)); // Same TID, another iface - ignore + CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "-------", SET001, 2, 105000700), bk)); // Not first frame - ignore + CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "1234567", SET100, 2, 105000800), bk)); // First, another iface - restart + CHECK_NOT_COMPLETE(rcv.addFrame(gen(1, "-------", SET010, 1, 105000600), bk)); // Old iface - ignore + CHECK_NOT_COMPLETE(rcv.addFrame(gen(0, "1234567", SET001, 2, 105000900), bk)); // Continuing + CHECK_COMPLETE( rcv.addFrame(gen(0, "1234567", SET010, 2, 105000910), bk)); // Done ASSERT_TRUE(matchBufferContent(bufmgr.access(gen.bufmgr_key), "3456712345671234567")); ASSERT_EQ(0x3231, rcv.getLastTransferCrc()); - ASSERT_EQ(1, rcv.yieldErrorCount()); + ASSERT_EQ(4, rcv.yieldErrorCount()); ASSERT_EQ(0, rcv.yieldErrorCount()); } TEST(TransferReceiver, UtcTransferTimestamping) { - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(789); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -458,7 +459,7 @@ TEST(TransferReceiver, HeaderParsing) * Broadcast */ { - Context<32> context(uavcan::TransferTypeMessageBroadcast); + Context<32> context; RxFrameGenerator gen(123); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr; @@ -505,7 +506,7 @@ TEST(TransferReceiver, HeaderParsing) * Unicast */ { - Context<32> context(uavcan::TransferTypeServiceRequest); + Context<32> context; RxFrameGenerator gen(123); uavcan::TransferReceiver& rcv = context.receiver; uavcan::ITransferBufferManager& bufmgr = context.bufmgr;