Properly defined timeouts and max cluster size

dsdl/uavcan/protocol/dynamic_node_id/server/1011.Discovery.uavcan

@@ -17,4 +17,4 @@ uint8 configured_cluster_size
 # Node ID of servers that are known to the publishing server, including the publishing server itself.
 # Capacity of this array defines maximum size of the server cluster.
 #
-uint8[<=7] known_nodes
+uint8[<=5] known_nodes

dsdl/uavcan/protocol/dynamic_node_id/server/220.AppendEntries.uavcan

@@ -3,6 +3,10 @@
 # Please refer to the specification for details.
 #
 
+uint16 DEFAULT_REQUEST_TIMEOUT_MS = 100
+
+uint16 DEFAULT_BASE_ELECTION_TIMEOUT_MS = 1000 # request timeout * max cluster size * 2 requests
+
 uint32 term
 uint8 prev_log_index
 uint8 prev_log_term

libuavcan/include/uavcan/protocol/dynamic_node_id_allocation_server.hpp

@@ -475,6 +475,12 @@ class RaftCore : private TimerBase
         { }
     };
 
+    /*
+     * Constants
+     */
+    const MonotonicDuration update_interval_;           ///< AE requests will be issued at this rate
+    const MonotonicDuration base_activity_timeout_;
+
     IDynamicNodeIDAllocationServerEventTracer& tracer_;
 
     /*
@@ -501,21 +507,17 @@ class RaftCore : private TimerBase
     ServiceServer<RequestVote, RequestVoteCallback>         request_vote_srv_;
     ServiceClient<RequestVote, RequestVoteResponseCallback> request_vote_client_;
 
-    /**
-     * This constant defines the rate at which internal state updates happen.
-     * It also defines timeouts for AppendEntries and RequestVote RPCs.
-     */
-    static MonotonicDuration getUpdateInterval() { return MonotonicDuration::fromMSec(100); }
-
     void trace(TraceEvent event, int64_t argument) { tracer_.onEvent(event, argument); }
 
-    INode& getNode() { return append_entries_srv_.getNode(); }
+    INode&       getNode()       { return append_entries_srv_.getNode(); }
+    const INode& getNode() const { return append_entries_srv_.getNode(); }
 
     void registerActivity() { last_activity_timestamp_ = getNode().getMonotonicTime(); }
+    bool isActivityTimedOut() const;
 
-    void updateFollower(const MonotonicTime& current_time);
-    void updateCandidate(const MonotonicTime& current_time);
-    void updateLeader(const MonotonicTime& current_time);
+    void updateFollower();
+    void updateCandidate();
+    void updateLeader();
 
     void switchState(ServerState new_state);
     void setActiveMode(bool new_active);
@@ -537,8 +539,14 @@ class RaftCore : private TimerBase
     virtual void handleTimerEvent(const TimerEvent& event);
 
 public:
-    RaftCore(INode& node, IDynamicNodeIDStorageBackend& storage, IDynamicNodeIDAllocationServerEventTracer& tracer)
+    RaftCore(INode& node, IDynamicNodeIDStorageBackend& storage, IDynamicNodeIDAllocationServerEventTracer& tracer,
+             MonotonicDuration update_interval =
+                 MonotonicDuration::fromMSec(AppendEntries::Request::DEFAULT_REQUEST_TIMEOUT_MS),
+             MonotonicDuration base_activity_timeout =
+                 MonotonicDuration::fromMSec(AppendEntries::Request::DEFAULT_BASE_ELECTION_TIMEOUT_MS))
         : TimerBase(node)
+        , update_interval_(update_interval)
+        , base_activity_timeout_(base_activity_timeout)
         , tracer_(tracer)
         , persistent_state_(storage, tracer)
         , cluster_(node, storage, persistent_state_.getLog(), tracer)

libuavcan/src/protocol/uc_dynamic_node_id_allocation_server.cpp

@@ -870,19 +870,26 @@ void ClusterManager::resetAllServerIndices()
 /*
  * RaftCore
  */
-void RaftCore::updateFollower(const MonotonicTime& current_time)
+bool RaftCore::isActivityTimedOut() const
 {
-    (void)current_time;
+    const int multiplier = static_cast<int>(getNode().getNodeID().get()) - 1;
+
+    const MonotonicDuration activity_timeout =
+        MonotonicDuration::fromUSec(base_activity_timeout_.toUSec() + update_interval_.toUSec() * multiplier);
+
+    return getNode().getMonotonicTime() > (last_activity_timestamp_ + activity_timeout);
 }
 
-void RaftCore::updateCandidate(const MonotonicTime& current_time)
+void RaftCore::updateFollower()
 {
-    (void)current_time;
 }
 
-void RaftCore::updateLeader(const MonotonicTime& current_time)
+void RaftCore::updateCandidate()
+{
+}
+
+void RaftCore::updateLeader()
 {
-    (void)current_time;
     propagateCommitIndex();
 }
 
@@ -1244,7 +1251,7 @@ void RaftCore::handleRequestVoteResponse(const ServiceCallResult<RequestVote>& r
     // I'm no fan of asynchronous programming. At all.
 }
 
-void RaftCore::handleTimerEvent(const TimerEvent& event)
+void RaftCore::handleTimerEvent(const TimerEvent&)
 {
     if (cluster_.hadDiscoveryActivity())
     {
@@ -1255,17 +1262,17 @@ void RaftCore::handleTimerEvent(const TimerEvent& event)
     {
     case ServerStateFollower:
     {
-        updateFollower(event.real_time);
+        updateFollower();
         break;
     }
     case ServerStateCandidate:
     {
-        updateCandidate(event.real_time);
+        updateCandidate();
         break;
     }
     case ServerStateLeader:
     {
-        updateLeader(event.real_time);
+        updateLeader();
         break;
     }
     default:
@@ -1320,18 +1327,18 @@ int RaftCore::init(uint8_t cluster_size)
     {
         return res;
     }
-    append_entries_client_.setRequestTimeout(getUpdateInterval());
+    append_entries_client_.setRequestTimeout(update_interval_);
 
     res = request_vote_client_.init();
     if (res < 0)
     {
         return res;
     }
-    request_vote_client_.setRequestTimeout(getUpdateInterval());
+    request_vote_client_.setRequestTimeout(update_interval_);
 
-    startPeriodic(getUpdateInterval());
+    startPeriodic(update_interval_);
 
-    trace(TraceRaftCoreInited, getUpdateInterval().toUSec());
+    trace(TraceRaftCoreInited, update_interval_.toUSec());
 
     UAVCAN_ASSERT(res >= 0);
     return 0;

libuavcan/test/protocol/dynamic_node_id_allocation_server.cpp

@@ -580,14 +580,14 @@ TEST(DynamicNodeIDAllocationServer, ClusterManagerInitialization)
         ASSERT_EQ(0, storage.getNumKeys());
 
         // OK
-        ASSERT_LE(0, mgr.init(7));
+        ASSERT_LE(0, mgr.init(5));
         ASSERT_EQ(1, storage.getNumKeys());
-        ASSERT_EQ("7", storage.get("cluster_size"));
+        ASSERT_EQ("5", storage.get("cluster_size"));
 
         // Testing other states
         ASSERT_EQ(0, mgr.getNumKnownServers());
-        ASSERT_EQ(7, mgr.getClusterSize());
-        ASSERT_EQ(4, mgr.getQuorumSize());
+        ASSERT_EQ(5, mgr.getClusterSize());
+        ASSERT_EQ(3, mgr.getQuorumSize());
         ASSERT_FALSE(mgr.getRemoteServerNodeIDAtIndex(0).isValid());
     }
     /*
@@ -605,7 +605,7 @@ TEST(DynamicNodeIDAllocationServer, ClusterManagerInitialization)
         ASSERT_EQ(0, storage.getNumKeys());
 
         // OK
-        storage.set("cluster_size", "7");
+        storage.set("cluster_size", "5");
         ASSERT_LE(0, mgr.init());
         ASSERT_EQ(1, storage.getNumKeys());
     }