From 97dbcdd25207ade6ac3592129be9e46bc157b16a Mon Sep 17 00:00:00 2001 From: Matt Morley Date: Mon, 24 Mar 2025 22:32:29 -0400 Subject: [PATCH] Paranoia test TSP client (#1844) ## Description Added paranoia checks to satisfy @Gold872 ## Meta Merge checklist: - [x] Pull Request title is [short, imperative summary](https://cbea.ms/git-commit/) of proposed changes - [x] The description documents the _what_ and _why_ - [ ] If this PR changes behavior or adds a feature, user documentation is updated - [ ] If this PR touches photon-serde, all messages have been regenerated and hashes have not changed unexpectedly - [ ] If this PR touches configuration, this is backwards compatible with settings back to v2024.3.1 - [x] If this PR addresses a bug, a regression test for it is added --- .../main/native/cpp/net/TimeSyncClient.cpp | 48 ++++---- .../main/native/include/net/TimeSyncClient.h | 4 + .../src/test/native/cpp/net/TimeSyncTest.cpp | 112 ++++++++++++++++++ 3 files changed, 143 insertions(+), 21 deletions(-) diff --git a/photon-targeting/src/main/native/cpp/net/TimeSyncClient.cpp b/photon-targeting/src/main/native/cpp/net/TimeSyncClient.cpp index 69197b563..8bafe3b38 100644 --- a/photon-targeting/src/main/native/cpp/net/TimeSyncClient.cpp +++ b/photon-targeting/src/main/native/cpp/net/TimeSyncClient.cpp @@ -57,6 +57,31 @@ static void ClientLoggerFunc(unsigned int level, const char* file, line); } +void wpi::tsp::TimeSyncClient::UpdateStatistics(uint64_t pong_local_time, + wpi::tsp::TspPing ping, + wpi::tsp::TspPong pong) { + // when time = send_time+rtt2/2, server time = server time + // server time = local time + offset + // offset = (server time - local time) = (server time) - (send_time + + // rtt2/2) + auto rtt2 = pong_local_time - ping.client_time; + int64_t serverTimeOffsetUs = pong.server_time - rtt2 / 2 - ping.client_time; + + auto filtered = m_lastOffsets.Calculate(serverTimeOffsetUs); + + // wpi::println("Ping-ponged! RTT2 {} uS, offset {}/filtered offset {} uS", + // rtt2, + // serverTimeOffsetUs, filtered); + + { + std::lock_guard lock{m_offsetMutex}; + m_metadata.offset = filtered; + m_metadata.rtt2 = rtt2; + m_metadata.pongsReceived++; + m_metadata.lastPongTime = pong_local_time; + } +} + void wpi::tsp::TimeSyncClient::Tick() { // wpi::println("wpi::tsp::TimeSyncClient::Tick"); // Regardless of if we've gotten a pong back yet, we'll ping again. this is @@ -122,28 +147,9 @@ void wpi::tsp::TimeSyncClient::UdpCallback(uv::Buffer& buf, size_t nbytes, return; } - // when time = send_time+rtt2/2, server time = server time - // server time = local time + offset - // offset = (server time - local time) = (server time) - (send_time + - // rtt2/2) - auto rtt2 = pong_local_time - ping.client_time; - int64_t serverTimeOffsetUs = pong.server_time - rtt2 / 2 - ping.client_time; + UpdateStatistics(pong_local_time, ping, pong); - auto filtered = m_lastOffsets.Calculate(serverTimeOffsetUs); - - // wpi::println("Ping-ponged! RTT2 {} uS, offset {}/filtered offset {} uS", - // rtt2, - // serverTimeOffsetUs, filtered); - - { - std::lock_guard lock{m_offsetMutex}; - m_metadata.offset = filtered; - m_metadata.rtt2 = rtt2; - m_metadata.pongsReceived++; - m_metadata.lastPongTime = pong_local_time; - } - - using std::cout; + // using std::cout; // wpi::println("Ping-ponged! RTT2 {} uS, offset {} uS", rtt2, // serverTimeOffsetUs); // wpi::println("Estimated server time {} s", diff --git a/photon-targeting/src/main/native/include/net/TimeSyncClient.h b/photon-targeting/src/main/native/include/net/TimeSyncClient.h index 82e5b0ebc..1c9a06d92 100644 --- a/photon-targeting/src/main/native/include/net/TimeSyncClient.h +++ b/photon-targeting/src/main/native/include/net/TimeSyncClient.h @@ -96,6 +96,10 @@ class TimeSyncClient { void Stop(); int64_t GetOffset(); Metadata GetMetadata(); + + // public for testability + void UpdateStatistics(uint64_t pong_local_time, wpi::tsp::TspPing ping, + wpi::tsp::TspPong pong); }; } // namespace tsp diff --git a/photon-targeting/src/test/native/cpp/net/TimeSyncTest.cpp b/photon-targeting/src/test/native/cpp/net/TimeSyncTest.cpp index 0c2364444..f735de7b4 100644 --- a/photon-targeting/src/test/native/cpp/net/TimeSyncTest.cpp +++ b/photon-targeting/src/test/native/cpp/net/TimeSyncTest.cpp @@ -38,3 +38,115 @@ TEST(TimeSyncProtocolTest, Smoketest) { server.Stop(); } + +TEST(TimeSyncClientTest, CalculateZero) { + using namespace wpi::tsp; + using namespace std::chrono_literals; + + // GIVEN a fresh client + TimeSyncClient client{"127.0.0.1", 5812, 100ms}; + + // AND a ping-pong sent with no delay + // client -> server -> client + uint64_t ping_client_time{100}; + uint64_t pong_server_time{100}; + uint64_t pong_client_time{100}; + + // setup our ping/pong packets + TspPing ping{.version = 1, .message_id = 1, .client_time = ping_client_time}; + TspPong pong{ping, pong_server_time}; + + // WHEN we update statistics + client.UpdateStatistics(pong_client_time, ping, pong); + + // THEN the statistics will reflect no delay + EXPECT_EQ(0, client.GetMetadata().offset); + EXPECT_EQ(0, client.GetMetadata().rtt2); + EXPECT_EQ(1u, client.GetMetadata().pongsReceived); + EXPECT_EQ(pong_client_time, client.GetMetadata().lastPongTime); +} + +TEST(TimeSyncClientTest, CalculateZeroOffset) { + using namespace wpi::tsp; + using namespace std::chrono_literals; + + // GIVEN a fresh client + TimeSyncClient client{"127.0.0.1", 5812, 100ms}; + + // AND a ping-pong sent with 10ms delay each way + // client -> server -> client + uint64_t ping_client_time{100}; + uint64_t pong_server_time{110}; + uint64_t pong_client_time{120}; + + // setup our ping/pong packets + TspPing ping{.version = 1, .message_id = 1, .client_time = ping_client_time}; + TspPong pong{ping, pong_server_time}; + + // WHEN we update statistics + client.UpdateStatistics(pong_client_time, ping, pong); + + // THEN the statistics will reflect no offset, and the expected rtt2 + // (client-to-client) latency + EXPECT_EQ(0, client.GetMetadata().offset); + EXPECT_EQ(20, client.GetMetadata().rtt2); + EXPECT_EQ(1u, client.GetMetadata().pongsReceived); + EXPECT_EQ(pong_client_time, client.GetMetadata().lastPongTime); +} + +TEST(TimeSyncClientTest, CalculateZeroRtt) { + using namespace wpi::tsp; + using namespace std::chrono_literals; + + // GIVEN a fresh client + TimeSyncClient client{"127.0.0.1", 5812, 100ms}; + + // AND a ping-pong sent with no delay + // client -> server -> client + uint64_t ping_client_time{100}; + uint64_t pong_server_time{123}; + uint64_t pong_client_time{100}; + + // setup our ping/pong packets + TspPing ping{.version = 1, .message_id = 1, .client_time = ping_client_time}; + TspPong pong{ping, pong_server_time}; + + // WHEN we update statistics + client.UpdateStatistics(pong_client_time, ping, pong); + + // THEN the statistics will reflect the expected 23ms offset + EXPECT_EQ(23, client.GetMetadata().offset); + EXPECT_EQ(0, client.GetMetadata().rtt2); + EXPECT_EQ(1u, client.GetMetadata().pongsReceived); + EXPECT_EQ(pong_client_time, client.GetMetadata().lastPongTime); +} + +TEST(TimeSyncClientTest, CalculateBoth) { + using namespace wpi::tsp; + using namespace std::chrono_literals; + + // GIVEN a fresh client + TimeSyncClient client{"127.0.0.1", 5812, 100ms}; + + // AND a ping-pong sent with no delay + // client -> server -> client + int64_t offset{-234}; + int64_t network_latency{23}; + + uint64_t ping_client_time{100}; + uint64_t pong_server_time{ping_client_time + offset + network_latency}; + uint64_t pong_client_time{ping_client_time + 2 * network_latency}; + + // setup our ping/pong packets + TspPing ping{.version = 1, .message_id = 1, .client_time = ping_client_time}; + TspPong pong{ping, pong_server_time}; + + // WHEN we update statistics + client.UpdateStatistics(pong_client_time, ping, pong); + + // THEN the statistics will reflect the expected latency and RTT + EXPECT_EQ(offset, client.GetMetadata().offset); + EXPECT_EQ(network_latency * 2, client.GetMetadata().rtt2); + EXPECT_EQ(1u, client.GetMetadata().pongsReceived); + EXPECT_EQ(pong_client_time, client.GetMetadata().lastPongTime); +}