[ntcore] Update transmit period on topic add/remove

- correctly handle the first subscription being a send all - don't restart outgoing timer if period didn't change
PeterJohnson · Dec 8, 2023 · ebffb79 · ebffb79
1 parent 9d11544
commit ebffb79
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 13 deletions.
diff --git a/ntcore/src/main/native/cpp/NetworkClient.cpp b/ntcore/src/main/native/cpp/NetworkClient.cpp
@@ -206,7 +206,9 @@ void NetworkClient3::TcpConnected(uv::Tcp& tcp) {
   auto clientImpl = std::make_shared<net3::ClientImpl3>(
       m_loop.Now().count(), m_inst, *wire, m_logger, [this](uint32_t repeatMs) {
         DEBUG4("Setting periodic timer to {}", repeatMs);
-        if (m_sendOutgoingTimer) {
+        if (m_sendOutgoingTimer &&
+            (!m_sendOutgoingTimer->IsActive() ||
+             uv::Timer::Time{repeatMs} != m_sendOutgoingTimer->GetRepeat())) {
           m_sendOutgoingTimer->Start(uv::Timer::Time{repeatMs},
                                      uv::Timer::Time{repeatMs});
         }
@@ -406,7 +408,9 @@ void NetworkClient::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp,
       m_loop.Now().count(), m_inst, *m_wire, m_logger, m_timeSyncUpdated,
       [this](uint32_t repeatMs) {
         DEBUG4("Setting periodic timer to {}", repeatMs);
-        if (m_sendOutgoingTimer) {
+        if (m_sendOutgoingTimer &&
+            (!m_sendOutgoingTimer->IsActive() ||
+             uv::Timer::Time{repeatMs} != m_sendOutgoingTimer->GetRepeat())) {
           m_sendOutgoingTimer->Start(uv::Timer::Time{repeatMs},
                                      uv::Timer::Time{repeatMs});
         }

diff --git a/ntcore/src/main/native/cpp/NetworkServer.cpp b/ntcore/src/main/native/cpp/NetworkServer.cpp
@@ -112,7 +112,8 @@ void NetworkServer::ServerConnection::UpdateOutgoingTimer(uint32_t repeatMs) {
   DEBUG4("Setting periodic timer to {}", repeatMs);
   if (repeatMs == UINT32_MAX) {
     m_outgoingTimer->Stop();
-  } else {
+  } else if (!m_outgoingTimer->IsActive() ||
+             uv::Timer::Time{repeatMs} != m_outgoingTimer->GetRepeat()) {
     m_outgoingTimer->Start(uv::Timer::Time{repeatMs},
                            uv::Timer::Time{repeatMs});
   }

diff --git a/ntcore/src/main/native/cpp/net/ServerImpl.cpp b/ntcore/src/main/native/cpp/net/ServerImpl.cpp
@@ -1663,8 +1663,12 @@ ServerImpl::TopicData* ServerImpl::CreateTopic(ClientData* client,
       }
 
       auto& tcd = topic->clients[aClient.get()];
+      bool added = false;
       for (auto subscriber : subscribers) {
-        tcd.AddSubscriber(subscriber);
+        added = added || tcd.AddSubscriber(subscriber);
+      }
+      if (added) {
+        aClient->UpdatePeriod(tcd, topic);
       }
 
       if (aClient.get() == client) {
@@ -1707,6 +1711,7 @@ void ServerImpl::DeleteTopic(TopicData* topic) {
   // unannounce to all subscribers
   for (auto&& tcd : topic->clients) {
     if (!tcd.second.subscribers.empty()) {
+      tcd.first->UpdatePeriod(tcd.second, topic);
       tcd.first->SendUnannounce(topic);
     }
   }

diff --git a/ntcore/src/main/native/cpp/net/ServerImpl.h b/ntcore/src/main/native/cpp/net/ServerImpl.h
@@ -148,10 +148,12 @@ class ServerImpl final {
 
       bool AddSubscriber(SubscriberData* sub) {
         bool added = subscribers.insert(sub).second;
-        if (!sub->options.topicsOnly && sendMode == ValueSendMode::kDisabled) {
-          sendMode = ValueSendMode::kNormal;
-        } else if (sub->options.sendAll) {
-          sendMode = ValueSendMode::kAll;
+        if (!sub->options.topicsOnly) {
+          if (sub->options.sendAll) {
+            sendMode = ValueSendMode::kAll;
+          } else if (sendMode == ValueSendMode::kDisabled) {
+            sendMode = ValueSendMode::kNormal;
+          }
         }
         return added;
       }
@@ -200,9 +202,10 @@ class ServerImpl final {
     std::string_view GetName() const { return m_name; }
     int GetId() const { return m_id; }
 
-   protected:
-    virtual void UpdatePeriodic(TopicData* topic) {}
+    virtual void UpdatePeriod(TopicData::TopicClientData& tcd,
+                              TopicData* topic) {}
 
+   protected:
     std::string m_name;
     std::string m_connInfo;
     bool m_local;  // local to machine
@@ -245,9 +248,6 @@ class ServerImpl final {
 
     void ClientSetValue(int64_t pubuid, const Value& value);
 
-    virtual void UpdatePeriod(TopicData::TopicClientData& tcd,
-                              TopicData* topic) {}
-
     wpi::DenseMap<TopicData*, bool> m_announceSent;
   };