Restructured NodeStatusMonitor and NodeInfoRetriever - TimerBase is not inherited by the monitor now because that was a suboptimal solution for a class designed for inheritance

libuavcan/include/uavcan/protocol/node_info_retriever.hpp

@@ -9,6 +9,7 @@
 #include <uavcan/debug.hpp>
 #include <uavcan/util/map.hpp>
 #include <uavcan/node/service_client.hpp>
+#include <uavcan/node/timer.hpp>
 #include <uavcan/protocol/node_status_monitor.hpp>
 #include <uavcan/protocol/GetNodeInfo.hpp>
 
@@ -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<NodeInfoRetriever*>(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;
     }
 

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<typename protocol::NodeStatus::FieldTypes::status_code>::Type NodeStatusCode;
@@ -43,12 +43,14 @@ public:
     };
 
 private:
-    enum { TimerPeriodMs100 = 5 };
+    enum { TimerPeriodMs100 = 4 };
 
     typedef MethodBinder<NodeStatusMonitor*,
                          void (NodeStatusMonitor::*)(const ReceivedDataStructure<protocol::NodeStatus>&)>
             NodeStatusCallback;
 
+    typedef MethodBinder<NodeStatusMonitor*, void (NodeStatusMonitor::*)(const TimerEvent&)> TimerCallback;
+
     /*
      * We'll be able to handle this many nodes in the network without any dynamic memory.
      */
@@ -56,6 +58,8 @@ private:
 
     Subscriber<protocol::NodeStatus, NodeStatusCallback, NumStaticReceivers, 0> sub_;
 
+    TimerEventForwarder<TimerCallback> 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;
     }