From e2ef4a4518a0064a9e86b3427b7f83c4cdb322f1 Mon Sep 17 00:00:00 2001 From: Pavel Kirienko Date: Wed, 13 May 2015 22:32:23 +0300 Subject: [PATCH] Restructured NodeStatusMonitor and NodeInfoRetriever - TimerBase is not inherited by the monitor now because that was a suboptimal solution for a class designed for inheritance --- .../uavcan/protocol/node_info_retriever.hpp | 62 +++++++++++++++---- .../uavcan/protocol/node_status_monitor.hpp | 20 +++--- 2 files changed, 62 insertions(+), 20 deletions(-) diff --git a/libuavcan/include/uavcan/protocol/node_info_retriever.hpp b/libuavcan/include/uavcan/protocol/node_info_retriever.hpp index 542210e113..4c4245b6c8 100644 --- a/libuavcan/include/uavcan/protocol/node_info_retriever.hpp +++ b/libuavcan/include/uavcan/protocol/node_info_retriever.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -17,7 +18,7 @@ namespace uavcan /** * Classes that need to receive GetNodeInfo responses should implement this interface. */ -class INodeInfoListener +class UAVCAN_EXPORT INodeInfoListener { public: /** @@ -63,7 +64,8 @@ public: * implement the GetNodeInfo service. * Events from this class can be routed to many subscribers. */ -class NodeInfoRetriever : protected NodeStatusMonitor +class UAVCAN_EXPORT NodeInfoRetriever : NodeStatusMonitor + , TimerBase { public: enum { MaxNumRequestAttempts = 254 }; @@ -150,6 +152,8 @@ private: /* * Methods */ + static MonotonicDuration getTimerPollInterval() { return MonotonicDuration::fromMSec(100); } + const Entry& getEntry(NodeID node_id) const { return const_cast(this)->getEntry(node_id); } Entry& getEntry(NodeID node_id) { @@ -160,6 +164,14 @@ private: return entries_[node_id.get() - 1]; } + void startTimerIfNotRunning() + { + if (!TimerBase::isRunning()) + { + TimerBase::startPeriodic(getTimerPollInterval()); + } + } + NodeID pickNextNodeToQuery() const { for (unsigned iter_cnt_ = 0; iter_cnt_ < (sizeof(entries_) / sizeof(entries_[0])); iter_cnt_++) // Round-robin @@ -182,22 +194,38 @@ private: return NodeID(); // No node could be found } - virtual void handleTimerEvent(const TimerEvent& event) // 2 Hz + virtual void handleTimerEvent(const TimerEvent&) { - NodeStatusMonitor::handleTimerEvent(event); - - if (!get_node_info_client_.isPending()) // If request is pending, this condition will fail every second time + if (get_node_info_client_.isPending()) // If request is pending, this condition will fail every second time { - const NodeID next = pickNextNodeToQuery(); // Typ. 1 Hz - if (next.isUnicast()) + return; + } + + const NodeID next = pickNextNodeToQuery(); + if (next.isUnicast()) + { + getEntry(next).updated_since_last_attempt = false; + const int res = get_node_info_client_.call(next, protocol::GetNodeInfo::Request()); + if (res < 0) { - getEntry(next).updated_since_last_attempt = false; - const int res = get_node_info_client_.call(next, protocol::GetNodeInfo::Request()); - if (res < 0) + get_node_info_client_.getNode().registerInternalFailure("NodeInfoRetriever GetNodeInfo call"); + } + } + else + { + bool requests_needed = false; + for (uint8_t i = 1; i <= NodeID::Max; i++) + { + if (getEntry(i).request_needed) { - get_node_info_client_.getNode().registerInternalFailure("NodeInfoRetriever GetNodeInfo call"); + requests_needed = true; + break; } } + if (!requests_needed) + { + TimerBase::stop(); + } } } @@ -218,6 +246,11 @@ private: UAVCAN_TRACE("NodeInfoRetriever", "Offline status change: node ID %d, request needed: %d", int(event.node_id.get()), int(entry.request_needed)); + + if (entry.request_needed) + { + startTimerIfNotRunning(); + } } listeners_.removeWhere( @@ -232,6 +265,8 @@ private: { entry.request_needed = true; entry.num_attempts_made = 0; + + startTimerIfNotRunning(); } entry.uptime_sec = msg.uptime_sec; entry.updated_since_last_attempt = true; @@ -272,6 +307,7 @@ private: public: NodeInfoRetriever(INode& node) : NodeStatusMonitor(node) + , TimerBase(node) , listeners_(node.getAllocator()) , get_node_info_client_(node) , last_picked_node_(1) @@ -298,7 +334,7 @@ public: } get_node_info_client_.setCallback(GetNodeInfoResponseCallback(this, &NodeInfoRetriever::handleGetNodeInfoResponse)); - + // Note: the timer will be started ad-hoc return 0; } diff --git a/libuavcan/include/uavcan/protocol/node_status_monitor.hpp b/libuavcan/include/uavcan/protocol/node_status_monitor.hpp index cf0dd7fd9b..06f7bb7605 100644 --- a/libuavcan/include/uavcan/protocol/node_status_monitor.hpp +++ b/libuavcan/include/uavcan/protocol/node_status_monitor.hpp @@ -19,7 +19,7 @@ namespace uavcan * This class implements the core functionality of a network monitor. * It can be extended by inheritance to add more complex logic, or used directly as is. */ -class UAVCAN_EXPORT NodeStatusMonitor : protected TimerBase +class UAVCAN_EXPORT NodeStatusMonitor { public: typedef typename StorageType::Type NodeStatusCode; @@ -43,12 +43,14 @@ public: }; private: - enum { TimerPeriodMs100 = 5 }; + enum { TimerPeriodMs100 = 4 }; typedef MethodBinder&)> NodeStatusCallback; + typedef MethodBinder TimerCallback; + /* * We'll be able to handle this many nodes in the network without any dynamic memory. */ @@ -56,6 +58,8 @@ private: Subscriber sub_; + TimerEventForwarder timer_; + struct Entry { NodeStatusCode status_code; @@ -110,7 +114,7 @@ private: handleNodeStatusMessage(msg); } - virtual void handleTimerEvent(const TimerEvent&) + void handleTimerEvent(const TimerEvent&) { const int OfflineTimeoutMs100 = protocol::NodeStatus::OFFLINE_TIMEOUT_MS / 100; @@ -122,7 +126,8 @@ private: if (entry.time_since_last_update_ms100 >= 0 && entry.status_code != protocol::NodeStatus::STATUS_OFFLINE) { - entry.time_since_last_update_ms100 = int8_t(entry.time_since_last_update_ms100 + int8_t(TimerPeriodMs100)); + entry.time_since_last_update_ms100 = + int8_t(entry.time_since_last_update_ms100 + int8_t(TimerPeriodMs100)); if (entry.time_since_last_update_ms100 >= OfflineTimeoutMs100) { Entry new_entry_value; @@ -157,8 +162,8 @@ protected: public: explicit NodeStatusMonitor(INode& node) - : TimerBase(node) - , sub_(node) + : sub_(node) + , timer_(node) { } virtual ~NodeStatusMonitor() { } @@ -173,7 +178,8 @@ public: const int res = sub_.start(NodeStatusCallback(this, &NodeStatusMonitor::handleNodeStatus)); if (res >= 0) { - TimerBase::startPeriodic(MonotonicDuration::fromMSec(TimerPeriodMs100 * 100)); + timer_.setCallback(TimerCallback(this, &NodeStatusMonitor::handleTimerEvent)); + timer_.startPeriodic(MonotonicDuration::fromMSec(TimerPeriodMs100 * 100)); } return res; }