From da357f599291f6049d9bec68b67f017666869915 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Thu, 27 Mar 2014 02:19:27 +0400 Subject: [PATCH] TransportPerfCounter - counting transfers and transport errors --- .../uavcan/node/generic_subscriber.hpp | 2 +- .../include/uavcan/transport/dispatcher.hpp | 5 +++ .../include/uavcan/transport/perf_counter.hpp | 40 +++++++++++++++++++ .../uavcan/transport/transfer_listener.hpp | 14 ++++--- .../uavcan/transport/transfer_receiver.hpp | 4 +- .../uavcan/transport/transfer_sender.hpp | 2 + libuavcan/src/transport/dispatcher.cpp | 4 +- libuavcan/src/transport/transfer_listener.cpp | 13 +++--- libuavcan/src/transport/transfer_receiver.cpp | 12 +++--- libuavcan/src/transport/transfer_sender.cpp | 12 +++++- libuavcan/test/transport/dispatcher.cpp | 26 +++++++++--- .../test/transport/transfer_listener.cpp | 19 +++++---- .../test/transport/transfer_receiver.cpp | 2 +- libuavcan/test/transport/transfer_sender.cpp | 21 ++++++++-- .../test/transport/transfer_test_helpers.hpp | 5 ++- 15 files changed, 138 insertions(+), 43 deletions(-) create mode 100644 libuavcan/include/uavcan/transport/perf_counter.hpp diff --git a/libuavcan/include/uavcan/node/generic_subscriber.hpp b/libuavcan/include/uavcan/node/generic_subscriber.hpp index bdb1d8240d..b518098db5 100644 --- a/libuavcan/include/uavcan/node/generic_subscriber.hpp +++ b/libuavcan/include/uavcan/node/generic_subscriber.hpp @@ -98,7 +98,7 @@ class GenericSubscriber : Noncopyable public: TransferForwarder(SelfType& obj, const DataTypeDescriptor& data_type, IAllocator& allocator) - : TransferListenerType(data_type, allocator) + : TransferListenerType(obj.node_.getDispatcher().getTransportPerfCounter(), data_type, allocator) , obj_(obj) { } }; diff --git a/libuavcan/include/uavcan/transport/dispatcher.hpp b/libuavcan/include/uavcan/transport/dispatcher.hpp index c10a7e1118..4e7c7ec269 100644 --- a/libuavcan/include/uavcan/transport/dispatcher.hpp +++ b/libuavcan/include/uavcan/transport/dispatcher.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -57,6 +58,7 @@ class Dispatcher : Noncopyable CanIOManager canio_; ISystemClock& sysclock_; IOutgoingTransferRegistry& outgoing_transfer_reg_; + TransportPerfCounter perf_; class ListenerRegistry { @@ -141,6 +143,9 @@ public: ISystemClock& getSystemClock() { return sysclock_; } const CanIOManager& getCanIOManager() const { return canio_; } + + const TransportPerfCounter& getTransportPerfCounter() const { return perf_; } + TransportPerfCounter& getTransportPerfCounter() { return perf_; } }; } diff --git a/libuavcan/include/uavcan/transport/perf_counter.hpp b/libuavcan/include/uavcan/transport/perf_counter.hpp new file mode 100644 index 0000000000..788a164978 --- /dev/null +++ b/libuavcan/include/uavcan/transport/perf_counter.hpp @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2014 Pavel Kirienko + */ + +#pragma once + +#include + +namespace uavcan +{ + +class TransportPerfCounter +{ + uint64_t transfers_tx_; + uint64_t transfers_rx_; + uint64_t errors_; + +public: + TransportPerfCounter() + : transfers_tx_(0) + , transfers_rx_(0) + , errors_(0) + { } + + void addTxTransfer() { transfers_tx_++; } + void addRxTransfer() { transfers_rx_++; } + + void addError() { errors_++; } + + void addErrors(unsigned int errors) + { + errors_ += errors; + } + + uint64_t getTxTransferCount() const { return transfers_tx_; } + uint64_t getRxTransferCount() const { return transfers_rx_; } + uint64_t getErrorCount() const { return errors_; } +}; + +} diff --git a/libuavcan/include/uavcan/transport/transfer_listener.hpp b/libuavcan/include/uavcan/transport/transfer_listener.hpp index f8183c5734..ba1b2f9942 100644 --- a/libuavcan/include/uavcan/transport/transfer_listener.hpp +++ b/libuavcan/include/uavcan/transport/transfer_listener.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -88,13 +89,15 @@ class TransferListenerBase : public LinkedListNode, Noncop { const DataTypeDescriptor& data_type_; const TransferCRC crc_base_; ///< Pre-initialized with data type hash, thus constant + TransportPerfCounter& perf_; bool checkPayloadCrc(const uint16_t compare_with, const ITransferBuffer& tbb) const; protected: - TransferListenerBase(const DataTypeDescriptor& data_type) + TransferListenerBase(TransportPerfCounter& perf, const DataTypeDescriptor& data_type) : data_type_(data_type) , crc_base_(data_type.getSignature().toTransferCRC()) + , perf_(perf) { } virtual ~TransferListenerBase() { } @@ -181,8 +184,8 @@ protected: } public: - TransferListener(const DataTypeDescriptor& data_type, IAllocator& allocator) - : TransferListenerBase(data_type) + TransferListener(TransportPerfCounter& perf, const DataTypeDescriptor& data_type, IAllocator& allocator) + : TransferListenerBase(perf, data_type) , bufmgr_(allocator) , receivers_(allocator) { @@ -246,8 +249,9 @@ private: } public: - ServiceResponseTransferListener(const DataTypeDescriptor& data_type, IAllocator& allocator) - : BaseType(data_type, allocator) + ServiceResponseTransferListener(TransportPerfCounter& perf, const DataTypeDescriptor& data_type, + IAllocator& allocator) + : BaseType(perf, data_type, allocator) { } void setExpectedResponseParams(const ExpectedResponseParams& erp) diff --git a/libuavcan/include/uavcan/transport/transfer_receiver.hpp b/libuavcan/include/uavcan/transport/transfer_receiver.hpp index 124983dbf3..e84069f77e 100644 --- a/libuavcan/include/uavcan/transport/transfer_receiver.hpp +++ b/libuavcan/include/uavcan/transport/transfer_receiver.hpp @@ -41,11 +41,11 @@ private: TransferID tid_; uint8_t iface_index_; uint8_t next_frame_index_; - uint8_t error_cnt_; + mutable uint8_t error_cnt_; bool isInitialized() const { return iface_index_ != IfaceIndexNotSet; } - void registerError(); + void registerError() const; TidRelation getTidRelation(const RxFrame& frame) const; diff --git a/libuavcan/include/uavcan/transport/transfer_sender.hpp b/libuavcan/include/uavcan/transport/transfer_sender.hpp index 003e352ba7..a10f47b7b2 100644 --- a/libuavcan/include/uavcan/transport/transfer_sender.hpp +++ b/libuavcan/include/uavcan/transport/transfer_sender.hpp @@ -25,6 +25,8 @@ class TransferSender Dispatcher& dispatcher_; + void registerError(); + public: enum { AllIfacesMask = 0xFF }; diff --git a/libuavcan/src/transport/dispatcher.cpp b/libuavcan/src/transport/dispatcher.cpp index 981d8708e9..1445ae9139 100644 --- a/libuavcan/src/transport/dispatcher.cpp +++ b/libuavcan/src/transport/dispatcher.cpp @@ -143,6 +143,7 @@ void Dispatcher::handleFrame(const CanRxFrame& can_frame) RxFrame frame; if (!frame.parse(can_frame)) { + // This is not counted as a transport error UAVCAN_TRACE("Dispatcher", "Invalid CAN frame received: %s", can_frame.toString().c_str()); return; } @@ -161,19 +162,16 @@ void Dispatcher::handleFrame(const CanRxFrame& can_frame) lmsg_.handleFrame(frame); break; } - case TransferTypeServiceRequest: { lsrv_req_.handleFrame(frame); break; } - case TransferTypeServiceResponse: { lsrv_resp_.handleFrame(frame); break; } - default: assert(0); } diff --git a/libuavcan/src/transport/transfer_listener.cpp b/libuavcan/src/transport/transfer_listener.cpp index 15cf399ab7..4238ba3650 100644 --- a/libuavcan/src/transport/transfer_listener.cpp +++ b/libuavcan/src/transport/transfer_listener.cpp @@ -113,32 +113,35 @@ void TransferListenerBase::handleReception(TransferReceiver& receiver, const RxF { case TransferReceiver::ResultNotComplete: { - return; + perf_.addErrors(receiver.yieldErrorCount()); + break; } case TransferReceiver::ResultSingleFrame: { + perf_.addRxTransfer(); SingleFrameIncomingTransfer it(frame); handleIncomingTransfer(it); - return; + break; } case TransferReceiver::ResultComplete: { + perf_.addRxTransfer(); const ITransferBuffer* tbb = tba.access(); if (tbb == NULL) { UAVCAN_TRACE("TransferListenerBase", "Buffer access failure, last frame: %s", frame.toString().c_str()); - return; + break; } if (!checkPayloadCrc(receiver.getLastTransferCrc(), *tbb)) { UAVCAN_TRACE("TransferListenerBase", "CRC error, last frame: %s", frame.toString().c_str()); - return; + break; } MultiFrameIncomingTransfer it(receiver.getLastTransferTimestampMonotonic(), receiver.getLastTransferTimestampUtc(), frame, tba); handleIncomingTransfer(it); it.release(); - return; + break; } default: { diff --git a/libuavcan/src/transport/transfer_receiver.cpp b/libuavcan/src/transport/transfer_receiver.cpp index ad022690aa..551fcc479f 100644 --- a/libuavcan/src/transport/transfer_receiver.cpp +++ b/libuavcan/src/transport/transfer_receiver.cpp @@ -17,7 +17,7 @@ const uint32_t TransferReceiver::MinTransferIntervalUSec; const uint32_t TransferReceiver::MaxTransferIntervalUSec; const uint32_t TransferReceiver::DefaultTransferIntervalUSec; -void TransferReceiver::registerError() +void TransferReceiver::registerError() const { if (error_cnt_ < 0xFF) { @@ -72,29 +72,29 @@ bool TransferReceiver::validate(const RxFrame& frame) const { return false; } - if (frame.isFirst() && !frame.isLast() && (frame.getPayloadLen() < TransferCRC::NumBytes)) { UAVCAN_TRACE("TransferReceiver", "CRC expected, %s", frame.toString().c_str()); + registerError(); return false; } - if ((frame.getIndex() == Frame::MaxIndex) && !frame.isLast()) { UAVCAN_TRACE("TransferReceiver", "Unterminated transfer, %s", frame.toString().c_str()); + registerError(); return false; } - if (frame.getIndex() != next_frame_index_) { UAVCAN_TRACE("TransferReceiver", "Unexpected frame index (not %i), %s", int(next_frame_index_), frame.toString().c_str()); + registerError(); return false; } - if (getTidRelation(frame) != TidSame) { UAVCAN_TRACE("TransferReceiver", "Unexpected TID (current %i), %s", tid_.get(), frame.toString().c_str()); + registerError(); return false; } return true; @@ -245,10 +245,8 @@ TransferReceiver::ResultCode TransferReceiver::addFrame(const RxFrame& frame, Tr if (!validate(frame)) { - registerError(); return ResultNotComplete; } - return receive(frame, tba); } diff --git a/libuavcan/src/transport/transfer_sender.cpp b/libuavcan/src/transport/transfer_sender.cpp index 85301f7d50..240d3dada1 100644 --- a/libuavcan/src/transport/transfer_sender.cpp +++ b/libuavcan/src/transport/transfer_sender.cpp @@ -10,10 +10,17 @@ namespace uavcan { +void TransferSender::registerError() +{ + dispatcher_.getTransportPerfCounter().addError(); +} + int TransferSender::send(const uint8_t* payload, int payload_len, MonotonicTime tx_deadline, MonotonicTime blocking_deadline, TransferType transfer_type, NodeID dst_node_id, TransferID tid) { + dispatcher_.getTransportPerfCounter().addTxTransfer(); + Frame frame(data_type_.getID(), transfer_type, dispatcher_.getNodeID(), dst_node_id, 0, tid); if (frame.getMaxPayloadLen() >= payload_len) // Single Frame Transfer @@ -23,6 +30,7 @@ int TransferSender::send(const uint8_t* payload, int payload_len, MonotonicTime { assert(0); UAVCAN_TRACE("TransferSender", "Frame payload write failure, %i", res); + registerError(); return (res < 0) ? res : -1; } frame.makeLast(); @@ -47,6 +55,7 @@ int TransferSender::send(const uint8_t* payload, int payload_len, MonotonicTime if (write_res < 2) { UAVCAN_TRACE("TransferSender", "Frame payload write failure, %i", write_res); + registerError(); return write_res; } offset = write_res - 2; @@ -60,13 +69,13 @@ int TransferSender::send(const uint8_t* payload, int payload_len, MonotonicTime const int send_res = dispatcher_.send(frame, tx_deadline, blocking_deadline, qos_, flags_, iface_mask_); if (send_res < 0) { + registerError(); return send_res; } if (frame.isLast()) { return next_frame_index; // Number of frames transmitted - } frame.setIndex(next_frame_index++); @@ -74,6 +83,7 @@ int TransferSender::send(const uint8_t* payload, int payload_len, MonotonicTime if (write_res < 0) { UAVCAN_TRACE("TransferSender", "Frame payload write failure, %i", write_res); + registerError(); return write_res; } diff --git a/libuavcan/test/transport/dispatcher.cpp b/libuavcan/test/transport/dispatcher.cpp index 5df43a9a78..46d21fdbee 100644 --- a/libuavcan/test/transport/dispatcher.cpp +++ b/libuavcan/test/transport/dispatcher.cpp @@ -68,12 +68,12 @@ TEST(Dispatcher, Reception) static const int NUM_SUBSCRIBERS = 6; SubscriberPtr subscribers[NUM_SUBSCRIBERS] = { - SubscriberPtr(new Subscriber(TYPES[0], poolmgr)), // msg - SubscriberPtr(new Subscriber(TYPES[0], poolmgr)), // msg // Two similar, yes - SubscriberPtr(new Subscriber(TYPES[1], poolmgr)), // msg - SubscriberPtr(new Subscriber(TYPES[2], poolmgr)), // srv - SubscriberPtr(new Subscriber(TYPES[3], poolmgr)), // srv - SubscriberPtr(new Subscriber(TYPES[3], poolmgr)) // srv // Repeat again + SubscriberPtr(new Subscriber(dispatcher.getTransportPerfCounter(), TYPES[0], poolmgr)), // msg + SubscriberPtr(new Subscriber(dispatcher.getTransportPerfCounter(), TYPES[0], poolmgr)), // msg // Two similar + SubscriberPtr(new Subscriber(dispatcher.getTransportPerfCounter(), TYPES[1], poolmgr)), // msg + SubscriberPtr(new Subscriber(dispatcher.getTransportPerfCounter(), TYPES[2], poolmgr)), // srv + SubscriberPtr(new Subscriber(dispatcher.getTransportPerfCounter(), TYPES[3], poolmgr)), // srv + SubscriberPtr(new Subscriber(dispatcher.getTransportPerfCounter(), TYPES[3], poolmgr)) // srv // Repeat again }; static const std::string DATA[6] = @@ -212,6 +212,13 @@ TEST(Dispatcher, Reception) ASSERT_EQ(0, dispatcher.getNumMessageListeners()); ASSERT_EQ(0, dispatcher.getNumServiceRequestListeners()); ASSERT_EQ(0, dispatcher.getNumServiceResponseListeners()); + + /* + * Perf counters + */ + EXPECT_LT(0, dispatcher.getTransportPerfCounter().getErrorCount()); // Repeated transfers + EXPECT_EQ(0, dispatcher.getTransportPerfCounter().getTxTransferCount()); + EXPECT_EQ(9, dispatcher.getTransportPerfCounter().getRxTransferCount()); } @@ -260,6 +267,13 @@ TEST(Dispatcher, Transmission) ASSERT_TRUE(driver.ifaces.at(0).tx.empty()); ASSERT_TRUE(driver.ifaces.at(1).tx.empty()); + + /* + * Perf counters - all empty because dispatcher itself does not count TX transfers + */ + EXPECT_EQ(0, dispatcher.getTransportPerfCounter().getErrorCount()); + EXPECT_EQ(0, dispatcher.getTransportPerfCounter().getTxTransferCount()); + EXPECT_EQ(0, dispatcher.getTransportPerfCounter().getRxTransferCount()); } diff --git a/libuavcan/test/transport/transfer_listener.cpp b/libuavcan/test/transport/transfer_listener.cpp index 4c62e997c3..2505b85993 100644 --- a/libuavcan/test/transport/transfer_listener.cpp +++ b/libuavcan/test/transport/transfer_listener.cpp @@ -38,7 +38,8 @@ TEST(TransferListener, BasicMFT) uavcan::PoolManager<1> poolmgr; poolmgr.addPool(&pool); - TestListener<256, 1, 1> subscriber(type, poolmgr); + uavcan::TransportPerfCounter perf; + TestListener<256, 1, 1> subscriber(perf, type, poolmgr); /* * Test data @@ -89,7 +90,8 @@ TEST(TransferListener, CrcFailure) const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); uavcan::PoolManager<1> poolmgr; // No dynamic memory - TestListener<256, 2, 2> subscriber(type, poolmgr); // Static buffer only, 2 entries + uavcan::TransportPerfCounter perf; + TestListener<256, 2, 2> subscriber(perf, type, poolmgr); // Static buffer only, 2 entries /* * Generating transfers with damaged payload (CRC is not valid) @@ -131,8 +133,9 @@ TEST(TransferListener, BasicSFT) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - uavcan::PoolManager<1> poolmgr; // No dynamic memory. At all. - TestListener<0, 0, 5> subscriber(type, poolmgr); // Max buf size is 0, i.e. SFT-only + uavcan::PoolManager<1> poolmgr; // No dynamic memory. At all. + uavcan::TransportPerfCounter perf; + TestListener<0, 0, 5> subscriber(perf, type, poolmgr); // Max buf size is 0, i.e. SFT-only TransferListenerEmulator emulator(subscriber, type); const Transfer transfers[] = @@ -166,8 +169,9 @@ TEST(TransferListener, Cleanup) { const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); - uavcan::PoolManager<1> poolmgr; // No dynamic memory - TestListener<256, 1, 2> subscriber(type, poolmgr); // Static buffer only, 1 entry + uavcan::PoolManager<1> poolmgr; // No dynamic memory + uavcan::TransportPerfCounter perf; + TestListener<256, 1, 2> subscriber(perf, type, poolmgr); // Static buffer only, 1 entry /* * Generating transfers @@ -221,7 +225,8 @@ TEST(TransferListener, MaximumTransferLength) const uavcan::DataTypeDescriptor type(uavcan::DataTypeKindMessage, 123, uavcan::DataTypeSignature(123456789), "A"); uavcan::PoolManager<1> poolmgr; - TestListener subscriber(type, poolmgr); + uavcan::TransportPerfCounter perf; + TestListener subscriber(perf, type, poolmgr); static const std::string DATA_OK(uavcan::MaxTransferPayloadLen, 'z'); diff --git a/libuavcan/test/transport/transfer_receiver.cpp b/libuavcan/test/transport/transfer_receiver.cpp index a7cfdcfd37..df5af641bf 100644 --- a/libuavcan/test/transport/transfer_receiver.cpp +++ b/libuavcan/test/transport/transfer_receiver.cpp @@ -369,7 +369,7 @@ TEST(TransferReceiver, Restart) ASSERT_TRUE(matchBufferContent(bufmgr.access(gen.bufmgr_key), "3456781234567812345678")); ASSERT_EQ(0x3231, rcv.getLastTransferCrc()); - ASSERT_EQ(3, rcv.yieldErrorCount()); + ASSERT_EQ(1, rcv.yieldErrorCount()); ASSERT_EQ(0, rcv.yieldErrorCount()); } diff --git a/libuavcan/test/transport/transfer_sender.cpp b/libuavcan/test/transport/transfer_sender.cpp index c3dc5a07c0..2335203052 100644 --- a/libuavcan/test/transport/transfer_sender.cpp +++ b/libuavcan/test/transport/transfer_sender.cpp @@ -113,9 +113,9 @@ TEST(TransferSender, Basic) } } - TestListener<512, 2, 2> sub_msg(TYPES[0], poolmgr); - TestListener<512, 2, 2> sub_srv_req(TYPES[1], poolmgr); - TestListener<512, 2, 2> sub_srv_resp(TYPES[1], poolmgr); + TestListener<512, 2, 2> sub_msg(dispatcher_rx.getTransportPerfCounter(), TYPES[0], poolmgr); + TestListener<512, 2, 2> sub_srv_req(dispatcher_rx.getTransportPerfCounter(), TYPES[1], poolmgr); + TestListener<512, 2, 2> sub_srv_resp(dispatcher_rx.getTransportPerfCounter(), TYPES[1], poolmgr); dispatcher_rx.registerMessageListener(&sub_msg); dispatcher_rx.registerServiceRequestListener(&sub_srv_req); @@ -145,6 +145,17 @@ TEST(TransferSender, Basic) ASSERT_TRUE(sub_srv_resp.matchAndPop(TRANSFERS[5])); ASSERT_TRUE(sub_srv_resp.matchAndPop(TRANSFERS[7])); + + /* + * Perf counters + */ + EXPECT_EQ(0, dispatcher_tx.getTransportPerfCounter().getErrorCount()); + EXPECT_EQ(8, dispatcher_tx.getTransportPerfCounter().getTxTransferCount()); + EXPECT_EQ(0, dispatcher_tx.getTransportPerfCounter().getRxTransferCount()); + + EXPECT_EQ(0, dispatcher_rx.getTransportPerfCounter().getErrorCount()); + EXPECT_EQ(0, dispatcher_rx.getTransportPerfCounter().getTxTransferCount()); + EXPECT_EQ(8, dispatcher_rx.getTransportPerfCounter().getRxTransferCount()); } @@ -202,4 +213,8 @@ TEST(TransferSender, Loopback) ASSERT_EQ(3, listener.last_frame.getPayloadLen()); ASSERT_TRUE(TX_NODE_ID == listener.last_frame.getSrcNodeID()); ASSERT_TRUE(listener.last_frame.isLast()); + + EXPECT_EQ(0, dispatcher.getTransportPerfCounter().getErrorCount()); + EXPECT_EQ(1, dispatcher.getTransportPerfCounter().getTxTransferCount()); + EXPECT_EQ(0, dispatcher.getTransportPerfCounter().getRxTransferCount()); } diff --git a/libuavcan/test/transport/transfer_test_helpers.hpp b/libuavcan/test/transport/transfer_test_helpers.hpp index 9d84139d18..d2c0b8113e 100644 --- a/libuavcan/test/transport/transfer_test_helpers.hpp +++ b/libuavcan/test/transport/transfer_test_helpers.hpp @@ -119,8 +119,9 @@ class TestListener : public uavcan::TransferListener transfers_; public: - TestListener(const uavcan::DataTypeDescriptor& data_type, uavcan::IAllocator& allocator) - : Base(data_type, allocator) + TestListener(uavcan::TransportPerfCounter& perf, const uavcan::DataTypeDescriptor& data_type, + uavcan::IAllocator& allocator) + : Base(perf, data_type, allocator) { } void handleIncomingTransfer(uavcan::IncomingTransfer& transfer)