[ntcore] Networking improvements (#5659)

- Utilize TrySend to properly backpressure network traffic
- Split updates into reasonable sized frames using WS fragmentation
- Use WS pings for network aliveness (requires 4.1 protocol revision)
- Measure RTT only at start of connection, rather than periodically
  (this avoids them being affected by other network traffic)
- Refactor network queue
- Refactor network ping, ping from server as well
- Improve meta topic performance
- Implement unified approach for network value updates (currently client and server use very different approaches) that respects requested subscriber update frequency

This adds a new protocol version (4.1) due to WS bugs in prior versions.

ntcore/src/test/native/cpp/net/MockWireConnection.cpp

@@ -1,26 +0,0 @@
-// Copyright (c) FIRST and other WPILib contributors.
-// Open Source Software; you can modify and/or share it under the terms of
-// the WPILib BSD license file in the root directory of this project.
-
-#include "MockWireConnection.h"
-
-using namespace nt::net;
-
-void MockWireConnection::StartSendText() {
-  if (m_in_text) {
-    m_text_os << ',';
-  } else {
-    m_text_os << '[';
-    m_in_text = true;
-  }
-}
-
-void MockWireConnection::FinishSendText() {
-  if (m_in_text) {
-    m_text_os << ']';
-    m_in_text = false;
-  }
-  m_text_os.flush();
-  Text(m_text);
-  m_text.clear();
-}

ntcore/src/test/native/cpp/net/MockWireConnection.h

@@ -20,37 +20,52 @@ namespace nt::net {
 
 class MockWireConnection : public WireConnection {
  public:
-  MockWireConnection() : m_text_os{m_text}, m_binary_os{m_binary} {}
+  MOCK_METHOD(unsigned int, GetVersion, (), (const, override));
+
+  MOCK_METHOD(void, SendPing, (uint64_t time), (override));
 
   MOCK_METHOD(bool, Ready, (), (const, override));
 
-  TextWriter SendText() override { return {m_text_os, *this}; }
-  BinaryWriter SendBinary() override { return {m_binary_os, *this}; }
-
-  MOCK_METHOD(void, Text, (std::string_view contents));
-  MOCK_METHOD(void, Binary, (std::span<const uint8_t> contents));
-
-  MOCK_METHOD(void, Flush, (), (override));
-
-  MOCK_METHOD(uint64_t, GetLastFlushTime, (), (const, override));
-
-  MOCK_METHOD(void, Disconnect, (std::string_view reason), (override));
-
- protected:
-  void StartSendText() override;
-  void FinishSendText() override;
-  void StartSendBinary() override {}
-  void FinishSendBinary() override {
-    Binary(m_binary);
-    m_binary.resize(0);
+  int WriteText(wpi::function_ref<void(wpi::raw_ostream& os)> writer) override {
+    std::string text;
+    wpi::raw_string_ostream os{text};
+    writer(os);
+    return DoWriteText(text);
+  }
+  int WriteBinary(
+      wpi::function_ref<void(wpi::raw_ostream& os)> writer) override {
+    std::vector<uint8_t> binary;
+    wpi::raw_uvector_ostream os{binary};
+    writer(os);
+    return DoWriteBinary(binary);
   }
 
- private:
-  std::string m_text;
-  wpi::raw_string_ostream m_text_os;
-  std::vector<uint8_t> m_binary;
-  wpi::raw_uvector_ostream m_binary_os;
-  bool m_in_text{false};
+  void SendText(wpi::function_ref<void(wpi::raw_ostream& os)> writer) override {
+    std::string text;
+    wpi::raw_string_ostream os{text};
+    writer(os);
+    DoSendText(text);
+  }
+  void SendBinary(
+      wpi::function_ref<void(wpi::raw_ostream& os)> writer) override {
+    std::vector<uint8_t> binary;
+    wpi::raw_uvector_ostream os{binary};
+    writer(os);
+    DoSendBinary(binary);
+  }
+
+  MOCK_METHOD(int, DoWriteText, (std::string_view contents));
+  MOCK_METHOD(int, DoWriteBinary, (std::span<const uint8_t> contents));
+
+  MOCK_METHOD(void, DoSendText, (std::string_view contents));
+  MOCK_METHOD(void, DoSendBinary, (std::span<const uint8_t> contents));
+
+  MOCK_METHOD(int, Flush, (), (override));
+
+  MOCK_METHOD(uint64_t, GetLastFlushTime, (), (const, override));
+  MOCK_METHOD(uint64_t, GetLastPingResponse, (), (const, override));
+
+  MOCK_METHOD(void, Disconnect, (std::string_view reason), (override));
 };
 
 }  // namespace nt::net

ntcore/src/test/native/cpp/net/ServerImplTest.cpp

@@ -50,7 +50,7 @@ class ServerImplTest : public ::testing::Test {
 
 TEST_F(ServerImplTest, AddClient) {
   ::testing::StrictMock<net::MockWireConnection> wire;
-  EXPECT_CALL(wire, Flush());
+  // EXPECT_CALL(wire, Flush());
   MockSetPeriodicFunc setPeriodic;
   auto [name, id] = server.AddClient("test", "connInfo", false, wire,
                                      setPeriodic.AsStdFunction());
@@ -63,8 +63,8 @@ TEST_F(ServerImplTest, AddDuplicateClient) {
   ::testing::StrictMock<net::MockWireConnection> wire2;
   MockSetPeriodicFunc setPeriodic1;
   MockSetPeriodicFunc setPeriodic2;
-  EXPECT_CALL(wire1, Flush());
-  EXPECT_CALL(wire2, Flush());
+  // EXPECT_CALL(wire1, Flush());
+  // EXPECT_CALL(wire2, Flush());
 
   auto [name1, id1] = server.AddClient("test", "connInfo", false, wire1,
                                        setPeriodic1.AsStdFunction());
@@ -79,6 +79,14 @@ TEST_F(ServerImplTest, AddDuplicateClient) {
 
 TEST_F(ServerImplTest, AddClient3) {}
 
+template <typename T>
+static std::string EncodeText1(const T& msg) {
+  std::string data;
+  wpi::raw_string_ostream os{data};
+  net::WireEncodeText(os, msg);
+  return data;
+}
+
 template <typename T>
 static std::string EncodeText(const T& msgs) {
   std::string data;
@@ -97,6 +105,23 @@ static std::string EncodeText(const T& msgs) {
   return data;
 }
 
+template <typename T>
+static std::vector<uint8_t> EncodeServerBinary1(const T& msg) {
+  std::vector<uint8_t> data;
+  wpi::raw_uvector_ostream os{data};
+  if constexpr (std::same_as<T, net::ServerMessage>) {
+    if (auto m = std::get_if<net::ServerValueMsg>(&msg.contents)) {
+      net::WireEncodeBinary(os, m->topic, m->value.time(), m->value);
+    }
+  } else if constexpr (std::same_as<T, net::ClientMessage>) {
+    if (auto m = std::get_if<net::ClientValueMsg>(&msg.contents)) {
+      net::WireEncodeBinary(os, Handle{m->pubHandle}.GetIndex(),
+                            m->value.time(), m->value);
+    }
+  }
+  return data;
+}
+
 template <typename T>
 static std::vector<uint8_t> EncodeServerBinary(const T& msgs) {
   std::vector<uint8_t> data;
@@ -150,29 +175,30 @@ TEST_F(ServerImplTest, PublishLocal) {
   // subscribes
   ::testing::StrictMock<net::MockWireConnection> wire;
   MockSetPeriodicFunc setPeriodic;
+  EXPECT_CALL(wire, GetVersion()).WillRepeatedly(Return(0x0401));
   {
     ::testing::InSequence seq;
-    EXPECT_CALL(wire, Flush());                         // AddClient()
-    EXPECT_CALL(setPeriodic, Call(100));                // ClientSubscribe()
-    EXPECT_CALL(wire, Flush());                         // ClientSubscribe()
+    // EXPECT_CALL(wire, Flush()).WillOnce(Return(0));     // AddClient()
+    EXPECT_CALL(setPeriodic, Call(100));  // ClientSubscribe()
+    // EXPECT_CALL(wire, Flush()).WillOnce(Return(0));     // ClientSubscribe()
+    EXPECT_CALL(wire, GetLastPingResponse()).WillOnce(Return(0));
+    EXPECT_CALL(wire, SendPing(100));
     EXPECT_CALL(wire, Ready()).WillOnce(Return(true));  // SendControl()
-    {
-      std::vector<net::ServerMessage> smsgs;
-      smsgs.emplace_back(net::ServerMessage{net::AnnounceMsg{
-          "test", 3, "double", std::nullopt, wpi::json::object()}});
-      smsgs.emplace_back(net::ServerMessage{net::AnnounceMsg{
-          "test2", 8, "double", std::nullopt, wpi::json::object()}});
-      EXPECT_CALL(wire, Text(StrEq(EncodeText(smsgs))));  // SendControl()
-    }
-    EXPECT_CALL(wire, Flush());                         // SendControl()
+    EXPECT_CALL(
+        wire, DoWriteText(StrEq(EncodeText1(net::ServerMessage{net::AnnounceMsg{
+                  "test", 3, "double", std::nullopt, wpi::json::object()}}))))
+        .WillOnce(Return(0));
+    EXPECT_CALL(
+        wire, DoWriteText(StrEq(EncodeText1(net::ServerMessage{net::AnnounceMsg{
+                  "test2", 8, "double", std::nullopt, wpi::json::object()}}))))
+        .WillOnce(Return(0));
+    EXPECT_CALL(wire, Flush()).WillOnce(Return(0));     // SendControl()
     EXPECT_CALL(wire, Ready()).WillOnce(Return(true));  // SendControl()
-    {
-      std::vector<net::ServerMessage> smsgs;
-      smsgs.emplace_back(net::ServerMessage{net::AnnounceMsg{
-          "test3", 11, "double", std::nullopt, wpi::json::object()}});
-      EXPECT_CALL(wire, Text(StrEq(EncodeText(smsgs))));  // SendControl()
-    }
-    EXPECT_CALL(wire, Flush());  // SendControl()
+    EXPECT_CALL(
+        wire, DoWriteText(StrEq(EncodeText1(net::ServerMessage{net::AnnounceMsg{
+                  "test3", 11, "double", std::nullopt, wpi::json::object()}}))))
+        .WillOnce(Return(0));
+    EXPECT_CALL(wire, Flush()).WillOnce(Return(0));  // SendControl()
   }
   auto [name, id] = server.AddClient("test", "connInfo", false, wire,
                                      setPeriodic.AsStdFunction());
@@ -193,7 +219,7 @@ TEST_F(ServerImplTest, PublishLocal) {
     server.HandleLocal(msgs);
   }
 
-  server.SendControl(100);
+  server.SendAllOutgoing(100, false);
 
   // publish after send control
   {
@@ -203,7 +229,7 @@ TEST_F(ServerImplTest, PublishLocal) {
     server.HandleLocal(msgs);
   }
 
-  server.SendControl(200);
+  server.SendAllOutgoing(200, false);
 }
 
 TEST_F(ServerImplTest, ClientSubTopicOnlyThenValue) {
@@ -225,31 +251,28 @@ TEST_F(ServerImplTest, ClientSubTopicOnlyThenValue) {
   }
 
   ::testing::StrictMock<net::MockWireConnection> wire;
+  EXPECT_CALL(wire, GetVersion()).WillRepeatedly(Return(0x0401));
   MockSetPeriodicFunc setPeriodic;
   {
     ::testing::InSequence seq;
-    EXPECT_CALL(wire, Flush());                         // AddClient()
-    EXPECT_CALL(setPeriodic, Call(100));                // ClientSubscribe()
-    EXPECT_CALL(wire, Flush());                         // ClientSubscribe()
+    // EXPECT_CALL(wire, Flush()).WillOnce(Return(0));     // AddClient()
+    EXPECT_CALL(setPeriodic, Call(100));  // ClientSubscribe()
+    // EXPECT_CALL(wire, Flush()).WillOnce(Return(0));     // ClientSubscribe()
+    EXPECT_CALL(wire, GetLastPingResponse()).WillOnce(Return(0));
+    EXPECT_CALL(wire, SendPing(100));
     EXPECT_CALL(wire, Ready()).WillOnce(Return(true));  // SendValues()
-    {
-      std::vector<net::ServerMessage> smsgs;
-      smsgs.emplace_back(net::ServerMessage{net::AnnounceMsg{
-          "test", 3, "double", std::nullopt, wpi::json::object()}});
-      EXPECT_CALL(wire, Text(StrEq(EncodeText(smsgs))));  // SendValues()
-    }
-    EXPECT_CALL(wire, Flush());                         // SendValues()
-    EXPECT_CALL(setPeriodic, Call(100));                // ClientSubscribe()
-    EXPECT_CALL(wire, Flush());                         // ClientSubscribe()
+    EXPECT_CALL(
+        wire, DoWriteText(StrEq(EncodeText1(net::ServerMessage{net::AnnounceMsg{
+                  "test", 3, "double", std::nullopt, wpi::json::object()}}))))
+        .WillOnce(Return(0));
+    EXPECT_CALL(wire, Flush()).WillOnce(Return(0));  // SendValues()
+    EXPECT_CALL(setPeriodic, Call(100));             // ClientSubscribe()
+    // EXPECT_CALL(wire, Flush()).WillOnce(Return(0));     // ClientSubscribe()
     EXPECT_CALL(wire, Ready()).WillOnce(Return(true));  // SendValues()
-    {
-      std::vector<net::ServerMessage> smsgs;
-      smsgs.emplace_back(net::ServerMessage{
-          net::ServerValueMsg{3, Value::MakeDouble(1.0, 10)}});
-      EXPECT_CALL(
-          wire,
-          Binary(wpi::SpanEq(EncodeServerBinary(smsgs))));  // SendValues()
-    }
+    EXPECT_CALL(
+        wire, DoWriteBinary(wpi::SpanEq(EncodeServerBinary1(net::ServerMessage{
+                  net::ServerValueMsg{3, Value::MakeDouble(1.0, 10)}}))))
+        .WillOnce(Return(0));
     EXPECT_CALL(wire, Flush());  // SendValues()
   }
 
@@ -268,7 +291,7 @@ TEST_F(ServerImplTest, ClientSubTopicOnlyThenValue) {
     server.ProcessIncomingText(id, EncodeText(msgs));
   }
 
-  server.SendValues(id, 100);
+  server.SendOutgoing(id, 100);
 
   // subscribe normal; will not resend announcement, but will send value
   {
@@ -279,7 +302,7 @@ TEST_F(ServerImplTest, ClientSubTopicOnlyThenValue) {
     server.ProcessIncomingText(id, EncodeText(msgs));
   }
 
-  server.SendValues(id, 200);
+  server.SendOutgoing(id, 200);
 }
 
 TEST_F(ServerImplTest, ZeroTimestampNegativeTime) {
@@ -319,7 +342,7 @@ TEST_F(ServerImplTest, ZeroTimestampNegativeTime) {
   MockSetPeriodicFunc setPeriodic;
   {
     ::testing::InSequence seq;
-    EXPECT_CALL(wire, Flush());  // AddClient()
+    // EXPECT_CALL(wire, Flush());  // AddClient()
   }
   auto [name, id] = server.AddClient("test", "connInfo", false, wire,
                                      setPeriodic.AsStdFunction());