From 789af2ad2697bc36c4cf69e5262c2396c8e2a87e Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 19 Jan 2024 21:26:00 -0800 Subject: [PATCH] [ntcore] Use last received time instead of last ping response This relaxes the timeout constraint for long message transmissions. --- ntcore/src/main/native/cpp/NetworkClient.cpp | 1 - ntcore/src/main/native/cpp/NetworkServer.cpp | 1 - ntcore/src/main/native/cpp/net/NetworkPing.cpp | 11 ++++++----- .../src/main/native/cpp/net/WebSocketConnection.cpp | 12 ------------ ntcore/src/main/native/cpp/net/WebSocketConnection.h | 7 +++---- ntcore/src/main/native/cpp/net/WireConnection.h | 4 ++-- ntcore/src/test/native/cpp/net/MockWireConnection.h | 2 +- ntcore/src/test/native/cpp/net/ServerImplTest.cpp | 4 ++-- 8 files changed, 14 insertions(+), 28 deletions(-) diff --git a/ntcore/src/main/native/cpp/NetworkClient.cpp b/ntcore/src/main/native/cpp/NetworkClient.cpp index 149dcb690b..365c73b5c1 100644 --- a/ntcore/src/main/native/cpp/NetworkClient.cpp +++ b/ntcore/src/main/native/cpp/NetworkClient.cpp @@ -403,7 +403,6 @@ void NetworkClient::WsConnected(wpi::WebSocket& ws, uv::Tcp& tcp, m_wire = std::make_shared(ws, connInfo.protocol_version); - m_wire->Start(); m_clientImpl = std::make_unique( m_loop.Now().count(), m_inst, *m_wire, m_logger, m_timeSyncUpdated, [this](uint32_t repeatMs) { diff --git a/ntcore/src/main/native/cpp/NetworkServer.cpp b/ntcore/src/main/native/cpp/NetworkServer.cpp index 5b5e78d54b..06f794f975 100644 --- a/ntcore/src/main/native/cpp/NetworkServer.cpp +++ b/ntcore/src/main/native/cpp/NetworkServer.cpp @@ -281,7 +281,6 @@ void NetworkServer::ServerConnection4::ProcessWsUpgrade() { INFO("CONNECTED NT4 client '{}' (from {})", dedupName, m_connInfo); m_info.remote_id = dedupName; m_server.AddConnection(this, m_info); - m_wire->Start(); m_websocket->closed.connect([this](uint16_t, std::string_view reason) { auto realReason = m_wire->GetDisconnectReason(); INFO("DISCONNECTED NT4 client '{}' (from {}): {}", m_info.remote_id, diff --git a/ntcore/src/main/native/cpp/net/NetworkPing.cpp b/ntcore/src/main/native/cpp/net/NetworkPing.cpp index fdbd26c132..e3d55a2b31 100644 --- a/ntcore/src/main/native/cpp/net/NetworkPing.cpp +++ b/ntcore/src/main/native/cpp/net/NetworkPing.cpp @@ -12,14 +12,15 @@ bool NetworkPing::Send(uint64_t curTimeMs) { if (curTimeMs < m_nextPingTimeMs) { return true; } - // if we didn't receive a timely response to our last ping, disconnect - uint64_t lastPing = m_wire.GetLastPingResponse(); + // if we haven't received data in a while, disconnect + // (we should at least be getting PONG responses) + uint64_t lastData = m_wire.GetLastReceivedTime(); // DEBUG4("WS ping: lastPing={} curTime={} pongTimeMs={}\n", lastPing, // curTimeMs, m_pongTimeMs); - if (lastPing == 0) { - lastPing = m_pongTimeMs; + if (lastData == 0) { + lastData = m_pongTimeMs; } - if (m_pongTimeMs != 0 && curTimeMs > (lastPing + kPingTimeoutMs)) { + if (m_pongTimeMs != 0 && curTimeMs > (lastData + kPingTimeoutMs)) { m_wire.Disconnect("connection timed out"); return false; } diff --git a/ntcore/src/main/native/cpp/net/WebSocketConnection.cpp b/ntcore/src/main/native/cpp/net/WebSocketConnection.cpp index e97d2fd0c1..eb9a39e0b6 100644 --- a/ntcore/src/main/native/cpp/net/WebSocketConnection.cpp +++ b/ntcore/src/main/native/cpp/net/WebSocketConnection.cpp @@ -116,18 +116,6 @@ WebSocketConnection::~WebSocketConnection() { } } -void WebSocketConnection::Start() { - m_ws.pong.connect([selfweak = weak_from_this()](auto data) { - if (data.size() != 8) { - return; - } - if (auto self = selfweak.lock()) { - self->m_lastPingResponse = - wpi::support::endian::read64(data.data()); - } - }); -} - void WebSocketConnection::SendPing(uint64_t time) { auto buf = AllocBuf(); buf.len = 8; diff --git a/ntcore/src/main/native/cpp/net/WebSocketConnection.h b/ntcore/src/main/native/cpp/net/WebSocketConnection.h index 4398451532..60a07f6eef 100644 --- a/ntcore/src/main/native/cpp/net/WebSocketConnection.h +++ b/ntcore/src/main/native/cpp/net/WebSocketConnection.h @@ -26,8 +26,6 @@ class WebSocketConnection final WebSocketConnection(const WebSocketConnection&) = delete; WebSocketConnection& operator=(const WebSocketConnection&) = delete; - void Start(); - unsigned int GetVersion() const final { return m_version; } void SendPing(uint64_t time) final; @@ -51,7 +49,9 @@ class WebSocketConnection final uint64_t GetLastFlushTime() const final { return m_lastFlushTime; } - uint64_t GetLastPingResponse() const final { return m_lastPingResponse; } + uint64_t GetLastReceivedTime() const final { + return m_ws.GetLastReceivedTime(); + } void Disconnect(std::string_view reason) final; @@ -92,7 +92,6 @@ class WebSocketConnection final State m_state = kEmpty; std::string m_reason; uint64_t m_lastFlushTime = 0; - uint64_t m_lastPingResponse = 0; unsigned int m_version; }; diff --git a/ntcore/src/main/native/cpp/net/WireConnection.h b/ntcore/src/main/native/cpp/net/WireConnection.h index 2c876cc29f..c1efaff2f8 100644 --- a/ntcore/src/main/native/cpp/net/WireConnection.h +++ b/ntcore/src/main/native/cpp/net/WireConnection.h @@ -51,8 +51,8 @@ class WireConnection { virtual uint64_t GetLastFlushTime() const = 0; // in microseconds - // Gets the timestamp of the last ping we got a reply to - virtual uint64_t GetLastPingResponse() const = 0; // in microseconds + // Gets the timestamp of the last incoming data + virtual uint64_t GetLastReceivedTime() const = 0; // in microseconds virtual void Disconnect(std::string_view reason) = 0; }; diff --git a/ntcore/src/test/native/cpp/net/MockWireConnection.h b/ntcore/src/test/native/cpp/net/MockWireConnection.h index 797ca253b7..cc2d19b171 100644 --- a/ntcore/src/test/native/cpp/net/MockWireConnection.h +++ b/ntcore/src/test/native/cpp/net/MockWireConnection.h @@ -63,7 +63,7 @@ class MockWireConnection : public WireConnection { MOCK_METHOD(int, Flush, (), (override)); MOCK_METHOD(uint64_t, GetLastFlushTime, (), (const, override)); - MOCK_METHOD(uint64_t, GetLastPingResponse, (), (const, override)); + MOCK_METHOD(uint64_t, GetLastReceivedTime, (), (const, override)); MOCK_METHOD(void, Disconnect, (std::string_view reason), (override)); }; diff --git a/ntcore/src/test/native/cpp/net/ServerImplTest.cpp b/ntcore/src/test/native/cpp/net/ServerImplTest.cpp index 64238111ee..b1fac0518c 100644 --- a/ntcore/src/test/native/cpp/net/ServerImplTest.cpp +++ b/ntcore/src/test/native/cpp/net/ServerImplTest.cpp @@ -181,7 +181,7 @@ TEST_F(ServerImplTest, PublishLocal) { // 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, GetLastReceivedTime()).WillOnce(Return(0)); EXPECT_CALL(wire, SendPing(100)); EXPECT_CALL(wire, Ready()).WillOnce(Return(true)); // SendControl() EXPECT_CALL( @@ -258,7 +258,7 @@ TEST_F(ServerImplTest, ClientSubTopicOnlyThenValue) { // 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, GetLastReceivedTime()).WillOnce(Return(0)); EXPECT_CALL(wire, SendPing(100)); EXPECT_CALL(wire, Ready()).WillOnce(Return(true)); // SendValues() EXPECT_CALL(