From 899aae44f7ce8b077a0bdfa478ce5a45f8c652cf Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 14 Oct 2015 20:15:19 +0300 Subject: [PATCH] OTR deinterfaced --- libuavcan/include/uavcan/node/node.hpp | 4 +- libuavcan/include/uavcan/node/scheduler.hpp | 4 +- libuavcan/include/uavcan/node/sub_node.hpp | 4 +- .../include/uavcan/transport/dispatcher.hpp | 8 +-- .../transport/outgoing_transfer_registry.hpp | 62 ++----------------- .../uc_outgoing_transfer_registry.cpp | 32 +++++++++- .../src/transport/uc_transfer_sender.cpp | 2 +- libuavcan/test/node/test_node.hpp | 4 +- libuavcan/test/transport/dispatcher.cpp | 19 ++---- libuavcan/test/transport/transfer_sender.cpp | 14 ++--- 10 files changed, 56 insertions(+), 97 deletions(-) diff --git a/libuavcan/include/uavcan/node/node.hpp b/libuavcan/include/uavcan/node/node.hpp index d9b6502736..4b573bf42a 100644 --- a/libuavcan/include/uavcan/node/node.hpp +++ b/libuavcan/include/uavcan/node/node.hpp @@ -47,7 +47,6 @@ class UAVCAN_EXPORT Node : public INode typedef PoolAllocator Allocator; Allocator pool_allocator_; - OutgoingTransferRegistry outgoing_trans_reg_; Scheduler scheduler_; NodeStatusProvider proto_nsp_; @@ -75,8 +74,7 @@ protected: public: Node(ICanDriver& can_driver, ISystemClock& system_clock) - : outgoing_trans_reg_(pool_allocator_) - , scheduler_(can_driver, pool_allocator_, system_clock, outgoing_trans_reg_) + : scheduler_(can_driver, pool_allocator_, system_clock) , proto_nsp_(*this) #if !UAVCAN_TINY , proto_dtp_(*this) diff --git a/libuavcan/include/uavcan/node/scheduler.hpp b/libuavcan/include/uavcan/node/scheduler.hpp index 01e58eb877..8f008054a8 100644 --- a/libuavcan/include/uavcan/node/scheduler.hpp +++ b/libuavcan/include/uavcan/node/scheduler.hpp @@ -92,8 +92,8 @@ class UAVCAN_EXPORT Scheduler : Noncopyable void pollCleanup(MonotonicTime mono_ts, uint32_t num_frames_processed_with_last_spin); public: - Scheduler(ICanDriver& can_driver, IPoolAllocator& allocator, ISystemClock& sysclock, IOutgoingTransferRegistry& otr) - : dispatcher_(can_driver, allocator, sysclock, otr) + Scheduler(ICanDriver& can_driver, IPoolAllocator& allocator, ISystemClock& sysclock) + : dispatcher_(can_driver, allocator, sysclock) , prev_cleanup_ts_(sysclock.getMonotonic()) , deadline_resolution_(MonotonicDuration::fromMSec(DefaultDeadlineResolutionMs)) , cleanup_period_(MonotonicDuration::fromMSec(DefaultCleanupPeriodMs)) diff --git a/libuavcan/include/uavcan/node/sub_node.hpp b/libuavcan/include/uavcan/node/sub_node.hpp index 2a7de956ef..ef5881cb12 100644 --- a/libuavcan/include/uavcan/node/sub_node.hpp +++ b/libuavcan/include/uavcan/node/sub_node.hpp @@ -31,7 +31,6 @@ class UAVCAN_EXPORT SubNode : public INode typedef PoolAllocator Allocator; Allocator pool_allocator_; - OutgoingTransferRegistry outgoing_trans_reg_; Scheduler scheduler_; uint64_t internal_failure_cnt_; @@ -46,8 +45,7 @@ protected: public: SubNode(ICanDriver& can_driver, ISystemClock& system_clock) : - outgoing_trans_reg_(pool_allocator_), - scheduler_(can_driver, pool_allocator_, system_clock, outgoing_trans_reg_), + scheduler_(can_driver, pool_allocator_, system_clock), internal_failure_cnt_(0) { } diff --git a/libuavcan/include/uavcan/transport/dispatcher.hpp b/libuavcan/include/uavcan/transport/dispatcher.hpp index 33ac1f2e5c..c4f40e00a3 100644 --- a/libuavcan/include/uavcan/transport/dispatcher.hpp +++ b/libuavcan/include/uavcan/transport/dispatcher.hpp @@ -81,7 +81,7 @@ class UAVCAN_EXPORT Dispatcher : Noncopyable { CanIOManager canio_; ISystemClock& sysclock_; - IOutgoingTransferRegistry& outgoing_transfer_reg_; + OutgoingTransferRegistry outgoing_transfer_reg_; TransferPerfCounter perf_; class ListenerRegistry @@ -133,10 +133,10 @@ class UAVCAN_EXPORT Dispatcher : Noncopyable void notifyRxFrameListener(const CanRxFrame& can_frame, CanIOFlags flags); public: - Dispatcher(ICanDriver& driver, IPoolAllocator& allocator, ISystemClock& sysclock, IOutgoingTransferRegistry& otr) + Dispatcher(ICanDriver& driver, IPoolAllocator& allocator, ISystemClock& sysclock) : canio_(driver, allocator, sysclock) , sysclock_(sysclock) - , outgoing_transfer_reg_(otr) + , outgoing_transfer_reg_(allocator) #if !UAVCAN_TINY , rx_listener_(NULL) #endif @@ -201,7 +201,7 @@ public: * @} */ - IOutgoingTransferRegistry& getOutgoingTransferRegistry() { return outgoing_transfer_reg_; } + OutgoingTransferRegistry& getOutgoingTransferRegistry() { return outgoing_transfer_reg_; } #if !UAVCAN_TINY LoopbackFrameListenerRegistry& getLoopbackFrameListenerRegistry() { return loopback_listeners_; } diff --git a/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp b/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp index c6d2c0c412..f92636e540 100644 --- a/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp +++ b/libuavcan/include/uavcan/transport/outgoing_transfer_registry.hpp @@ -60,23 +60,8 @@ public: * Outgoing transfer registry keeps track of Transfer ID values for all currently existing local transfer senders. * If a local transfer sender was inactive for a sufficiently long time, the outgoing transfer registry will * remove the respective Transfer ID tracking object. - * - * TODO: Deinterface this. */ -class UAVCAN_EXPORT IOutgoingTransferRegistry -{ -public: - static const MonotonicDuration MinEntryLifetime; - - virtual ~IOutgoingTransferRegistry() { } - - virtual TransferID* accessOrCreate(const OutgoingTransferRegistryKey& key, MonotonicTime new_deadline) = 0; - virtual bool exists(DataTypeID dtid, TransferType tt) const = 0; - virtual void cleanup(MonotonicTime deadline) = 0; -}; - - -class UAVCAN_EXPORT OutgoingTransferRegistry : public IOutgoingTransferRegistry, Noncopyable +class UAVCAN_EXPORT OutgoingTransferRegistry : Noncopyable { struct Value { @@ -127,54 +112,19 @@ class UAVCAN_EXPORT OutgoingTransferRegistry : public IOutgoingTransferRegistry, Map map_; public: + static const MonotonicDuration MinEntryLifetime; + explicit OutgoingTransferRegistry(IPoolAllocator& allocator) : map_(allocator) { } - virtual TransferID* accessOrCreate(const OutgoingTransferRegistryKey& key, MonotonicTime new_deadline); + TransferID* accessOrCreate(const OutgoingTransferRegistryKey& key, MonotonicTime new_deadline); - virtual bool exists(DataTypeID dtid, TransferType tt) const; + bool exists(DataTypeID dtid, TransferType tt) const; - virtual void cleanup(MonotonicTime ts); + void cleanup(MonotonicTime ts); }; -// ---------------------------------------------------------------------------- - -/* - * OutgoingTransferRegistry - * TODO: deinterface and move to .cpp - */ -inline -TransferID* OutgoingTransferRegistry::accessOrCreate(const OutgoingTransferRegistryKey& key, - MonotonicTime new_deadline) -{ - UAVCAN_ASSERT(!new_deadline.isZero()); - Value* p = map_.access(key); - if (p == NULL) - { - p = map_.insert(key, Value()); - if (p == NULL) - { - return NULL; - } - UAVCAN_TRACE("OutgoingTransferRegistry", "Created %s", key.toString().c_str()); - } - p->deadline = new_deadline; - return &p->tid; -} - -inline -bool OutgoingTransferRegistry::exists(DataTypeID dtid, TransferType tt) const -{ - return NULL != map_.find(ExistenceCheckingPredicate(dtid, tt)); -} - -inline -void OutgoingTransferRegistry::cleanup(MonotonicTime ts) -{ - map_.removeAllWhere(DeadlineExpiredPredicate(ts)); -} - } #endif // UAVCAN_TRANSPORT_OUTGOING_TRANSFER_REGISTRY_HPP_INCLUDED diff --git a/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp b/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp index f840fa8cc6..852e850ccb 100644 --- a/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp +++ b/libuavcan/src/transport/uc_outgoing_transfer_registry.cpp @@ -20,8 +20,36 @@ std::string OutgoingTransferRegistryKey::toString() const #endif /* - * IOutgoingTransferRegistry + * OutgoingTransferRegistry */ -const MonotonicDuration IOutgoingTransferRegistry::MinEntryLifetime = MonotonicDuration::fromMSec(2000); +const MonotonicDuration OutgoingTransferRegistry::MinEntryLifetime = MonotonicDuration::fromMSec(2000); + +TransferID* OutgoingTransferRegistry::accessOrCreate(const OutgoingTransferRegistryKey& key, + MonotonicTime new_deadline) +{ + UAVCAN_ASSERT(!new_deadline.isZero()); + Value* p = map_.access(key); + if (p == NULL) + { + p = map_.insert(key, Value()); + if (p == NULL) + { + return NULL; + } + UAVCAN_TRACE("OutgoingTransferRegistry", "Created %s", key.toString().c_str()); + } + p->deadline = new_deadline; + return &p->tid; +} + +bool OutgoingTransferRegistry::exists(DataTypeID dtid, TransferType tt) const +{ + return NULL != map_.find(ExistenceCheckingPredicate(dtid, tt)); +} + +void OutgoingTransferRegistry::cleanup(MonotonicTime ts) +{ + map_.removeAllWhere(DeadlineExpiredPredicate(ts)); +} } diff --git a/libuavcan/src/transport/uc_transfer_sender.cpp b/libuavcan/src/transport/uc_transfer_sender.cpp index f0829976a6..0a9dc4a700 100644 --- a/libuavcan/src/transport/uc_transfer_sender.cpp +++ b/libuavcan/src/transport/uc_transfer_sender.cpp @@ -153,7 +153,7 @@ int TransferSender::send(const uint8_t* payload, unsigned payload_len, Monotonic UAVCAN_ASSERT(!tx_deadline.isZero()); const MonotonicTime otr_deadline = tx_deadline + max(max_transfer_interval_ * 2, - IOutgoingTransferRegistry::MinEntryLifetime); + OutgoingTransferRegistry::MinEntryLifetime); TransferID* const tid = dispatcher_.getOutgoingTransferRegistry().accessOrCreate(otr_key, otr_deadline); if (tid == NULL) diff --git a/libuavcan/test/node/test_node.hpp b/libuavcan/test/node/test_node.hpp index ea187c809f..bd71b89ee3 100644 --- a/libuavcan/test/node/test_node.hpp +++ b/libuavcan/test/node/test_node.hpp @@ -20,13 +20,11 @@ struct TestNode : public uavcan::INode { uavcan::PoolAllocator pool; - uavcan::OutgoingTransferRegistry otr; uavcan::Scheduler scheduler; uint64_t internal_failure_count; TestNode(uavcan::ICanDriver& can_driver, uavcan::ISystemClock& clock_driver, uavcan::NodeID self_node_id) - : otr(pool) - , scheduler(can_driver, pool, clock_driver, otr) + : scheduler(can_driver, pool, clock_driver) , internal_failure_count(0) { setNodeID(self_node_id); diff --git a/libuavcan/test/transport/dispatcher.cpp b/libuavcan/test/transport/dispatcher.cpp index 65724864ae..621ccf8972 100644 --- a/libuavcan/test/transport/dispatcher.cpp +++ b/libuavcan/test/transport/dispatcher.cpp @@ -57,9 +57,7 @@ TEST(Dispatcher, Reception) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(pool); - - uavcan::Dispatcher dispatcher(driver, pool, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, pool, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once ASSERT_FALSE(dispatcher.setNodeID(SELF_NODE_ID)); ASSERT_EQ(SELF_NODE_ID, dispatcher.getNodeID()); @@ -255,9 +253,7 @@ TEST(Dispatcher, Transmission) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(pool); - - uavcan::Dispatcher dispatcher(driver, pool, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, pool, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once ASSERT_FALSE(dispatcher.setNodeID(SELF_NODE_ID)); @@ -280,7 +276,8 @@ TEST(Dispatcher, Transmission) ASSERT_FALSE(dispatcher.hasPublisher(123)); ASSERT_FALSE(dispatcher.hasPublisher(456)); const uavcan::OutgoingTransferRegistryKey otr_key(123, uavcan::TransferTypeMessageBroadcast, 0); - ASSERT_TRUE(out_trans_reg.accessOrCreate(otr_key, uavcan::MonotonicTime::fromMSec(1000000))); + ASSERT_TRUE(dispatcher.getOutgoingTransferRegistry().accessOrCreate(otr_key, + uavcan::MonotonicTime::fromMSec(1000000))); ASSERT_TRUE(dispatcher.hasPublisher(123)); ASSERT_FALSE(dispatcher.hasPublisher(456)); @@ -319,9 +316,7 @@ TEST(Dispatcher, Spin) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); // Can be set only once ASSERT_FALSE(dispatcher.setNodeID(SELF_NODE_ID)); @@ -365,9 +360,7 @@ TEST(Dispatcher, Loopback) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher.setNodeID(SELF_NODE_ID)); { diff --git a/libuavcan/test/transport/transfer_sender.cpp b/libuavcan/test/transport/transfer_sender.cpp index a156b3039e..ff27b6fe98 100644 --- a/libuavcan/test/transport/transfer_sender.cpp +++ b/libuavcan/test/transport/transfer_sender.cpp @@ -34,12 +34,10 @@ TEST(TransferSender, Basic) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - static const uavcan::NodeID TX_NODE_ID(64); static const uavcan::NodeID RX_NODE_ID(65); - uavcan::Dispatcher dispatcher_tx(driver, poolmgr, clockmock, out_trans_reg); - uavcan::Dispatcher dispatcher_rx(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher_tx(driver, poolmgr, clockmock); + uavcan::Dispatcher dispatcher_rx(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher_tx.setNodeID(TX_NODE_ID)); ASSERT_TRUE(dispatcher_rx.setNodeID(RX_NODE_ID)); @@ -195,10 +193,8 @@ TEST(TransferSender, Loopback) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - static const uavcan::NodeID TX_NODE_ID(64); - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); ASSERT_TRUE(dispatcher.setNodeID(TX_NODE_ID)); uavcan::DataTypeDescriptor desc = makeDataType(uavcan::DataTypeKindMessage, 1, "Foobar"); @@ -236,9 +232,7 @@ TEST(TransferSender, PassiveMode) SystemClockMock clockmock(100); CanDriverMock driver(2, clockmock); - uavcan::OutgoingTransferRegistry out_trans_reg(poolmgr); - - uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock, out_trans_reg); + uavcan::Dispatcher dispatcher(driver, poolmgr, clockmock); uavcan::TransferSender sender(dispatcher, makeDataType(uavcan::DataTypeKindMessage, 123), uavcan::CanTxQueue::Volatile);