Swap to using the same timeout estimator as TCP

TrentHouliston · TrentHouliston · commit 118cd9f0310e · 2025-05-18T20:54:26.000+10:00
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -28,7 +28,15 @@ configure_file(nuclear.in ${PROJECT_BINARY_DIR}/nuclear)
 
 # Build the library
 find_package(Threads REQUIRED)
-file(GLOB_RECURSE src "*.c" "*.cpp" "*.hpp" "*.ipp")
+file(
+  GLOB_RECURSE
+  src
+  CONFIGURE_DEPENDS
+  "*.c"
+  "*.cpp"
+  "*.hpp"
+  "*.ipp"
+)
 add_library(nuclear STATIC ${src})
 add_library(NUClear::nuclear ALIAS nuclear)
 
diff --git a/src/extension/network/NUClearNetwork.cpp b/src/extension/network/NUClearNetwork.cpp
@@ -501,7 +501,7 @@ namespace extension {
                     if (ptr) {
 
                         auto now     = std::chrono::steady_clock::now();
-                        auto timeout = it->last_send + ptr->round_trip_time;
+                        auto timeout = it->last_send + ptr->rtt.timeout();
 
                         // Check if we should have expected an ack by now for some packets
                         if (timeout < now) {
@@ -510,7 +510,7 @@ namespace extension {
                             it->last_send = now;
 
                             // The next time we should check for a timeout
-                            auto next_timeout = now + ptr->round_trip_time;
+                            auto next_timeout = now + ptr->rtt.timeout();
                             if (next_timeout < next_event) {
                                 next_event = next_timeout;
                                 next_event_callback(next_event);
@@ -869,7 +869,7 @@ namespace extension {
                                 // Check for and delete any timed out packets
                                 for (auto it = assemblers.begin(); it != assemblers.end();) {
                                     const auto now              = std::chrono::steady_clock::now();
-                                    const auto timeout          = remote->round_trip_time * 10.0;
+                                    const auto timeout          = remote->rtt.timeout() * 10.0;
                                     const auto& last_chunk_time = it->second.first;
 
                                     it = now > last_chunk_time + timeout ? assemblers.erase(it) : std::next(it);
@@ -919,8 +919,7 @@ namespace extension {
 
                                     // Approximate how long the round trip is to this remote so we can work out how
                                     // long before retransmitting
-                                    // We use a baby kalman filter to help smooth out jitter
-                                    remote->measure_round_trip(round_trip);
+                                    remote->rtt.measure(round_trip);
 
                                     // Update our acks
                                     bool all_acked = true;
@@ -987,7 +986,7 @@ namespace extension {
                                     s->last_send = std::chrono::steady_clock::now();
 
                                     // The next time we should check for a timeout
-                                    auto next_timeout = s->last_send + remote->round_trip_time;
+                                    auto next_timeout = s->last_send + remote->rtt.timeout();
                                     if (next_timeout < next_event) {
                                         next_event = next_timeout;
                                         next_event_callback(next_event);
@@ -1108,7 +1107,7 @@ namespace extension {
                         queue.targets.emplace_back(it->second, acks);
 
                         // The next time we should check for a timeout
-                        auto next_timeout = std::chrono::steady_clock::now() + it->second->round_trip_time;
+                        auto next_timeout = std::chrono::steady_clock::now() + it->second->rtt.timeout();
                         if (next_timeout < next_event) {
                             next_event = next_timeout;
                             next_event_callback(next_event);
diff --git a/src/extension/network/NUClearNetwork.hpp b/src/extension/network/NUClearNetwork.hpp
@@ -39,6 +39,7 @@
 
 #include "../../util/network/sock_t.hpp"
 #include "../../util/platform.hpp"
+#include "RTTEstimator.hpp"
 #include "wire_protocol.hpp"
 
 namespace NUClear {
@@ -79,41 +80,8 @@ namespace extension {
                          std::pair<std::chrono::steady_clock::time_point, std::map<uint16_t, std::vector<uint8_t>>>>
                     assemblers;
 
-                /// Struct storing the kalman filter for round trip time
-                struct RoundTripKF {
-                    float process_noise     = 1e-6f;
-                    float measurement_noise = 1e-1f;
-                    float variance          = 1.0f;
-                    float mean              = 1.0f;
-                };
-                /// A little kalman filter for estimating round trip time
-                RoundTripKF round_trip_kf{};
-
-                std::chrono::steady_clock::duration round_trip_time{std::chrono::seconds(1)};
-
-                void measure_round_trip(std::chrono::steady_clock::duration time) {
-
-                    // Make our measurement into a float seconds type
-                    const std::chrono::duration<float> m =
-                        std::chrono::duration_cast<std::chrono::duration<float>>(time);
-
-                    // Alias variables
-                    const auto& Q = round_trip_kf.process_noise;
-                    const auto& R = round_trip_kf.measurement_noise;
-                    auto& P       = round_trip_kf.variance;
-                    auto& X       = round_trip_kf.mean;
-
-                    // Calculate our kalman gain
-                    const float K = (P + Q) / (P + Q + R);
-
-                    // Do filter
-                    P = R * (P + Q) / (R + P + Q);
-                    X = X + (m.count() - X) * K;
-
-                    // Put result into our variable
-                    round_trip_time = std::chrono::duration_cast<std::chrono::steady_clock::duration>(
-                        std::chrono::duration<float>(X));
-                }
+                /// RTT estimator for this network target
+                RTTEstimator rtt;
             };
 
             NUClearNetwork() = default;
diff --git a/src/extension/network/RTTEstimator.cpp b/src/extension/network/RTTEstimator.cpp
@@ -0,0 +1,52 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2025 NUClear Contributors
+ *
+ * This file is part of the NUClear codebase.
+ * See https://github.com/Fastcode/NUClear for further info.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+ * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "RTTEstimator.hpp"
+
+#include <cmath>
+
+namespace NUClear {
+namespace extension {
+    namespace network {
+
+        void RTTEstimator::measure(std::chrono::steady_clock::duration time) {
+            // Convert measurement to float seconds
+            const std::chrono::duration<float> m = std::chrono::duration_cast<std::chrono::duration<float>>(time);
+            const float sample_rtt               = m.count();
+
+            // Calculate RTT variation
+            const float err = sample_rtt - smoothed_rtt;
+            rtt_var         = (1 - beta) * rtt_var + beta * std::abs(err);
+
+            // Update smoothed RTT
+            smoothed_rtt = (1 - alpha) * smoothed_rtt + alpha * sample_rtt;
+
+            // Calculate RTO (smoothed RTT + 4 * RTT variation) and bound to limits
+            rto = std::min(std::max(smoothed_rtt + 4 * rtt_var, min_rto), max_rto);
+        }
+
+        std::chrono::steady_clock::duration RTTEstimator::timeout() const {
+            return std::chrono::duration_cast<std::chrono::steady_clock::duration>(std::chrono::duration<float>(rto));
+        }
+
+    }  // namespace network
+}  // namespace extension
+}  // namespace NUClear
diff --git a/src/extension/network/RTTEstimator.hpp b/src/extension/network/RTTEstimator.hpp
@@ -0,0 +1,125 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2025 NUClear Contributors
+ *
+ * This file is part of the NUClear codebase.
+ * See https://github.com/Fastcode/NUClear for further info.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+ * WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#ifndef NUCLEAR_EXTENSION_NETWORK_RTT_ESTIMATOR_HPP
+#define NUCLEAR_EXTENSION_NETWORK_RTT_ESTIMATOR_HPP
+
+#include <chrono>
+#include <cstdint>
+#include <stdexcept>
+
+namespace NUClear {
+namespace extension {
+    namespace network {
+
+        /**
+         * Implements TCP-style Round Trip Time (RTT) estimation using Jacobson/Karels algorithm.
+         *
+         * This class provides RTT estimation functionality similar to TCP's RTT estimation mechanism.
+         * It uses an Exponentially Weighted Moving Average (EWMA) to smooth RTT measurements and
+         * calculate a retransmission timeout (RTO) value. The implementation follows the TCP
+         * Jacobson/Karels algorithm which provides robust RTT estimation that:
+         * - Smoothly tracks the mean RTT
+         * - Adapts to RTT variations
+         * - Handles network jitter
+         * - Provides conservative timeout values
+         */
+        class RTTEstimator {
+        public:
+            /**
+             * Construct a new RTT Estimator
+             *
+             * @param alpha           Weight for RTT smoothing (default: 0.125, TCP standard)
+             * @param beta            Weight for RTT variation (default: 0.25, TCP standard)
+             * @param initial_rtt     Initial RTT estimate in seconds (default: 1.0)
+             * @param initial_rtt_var Initial RTT variation in seconds (default: 0.0)
+             * @param min_rto         Minimum RTO value in seconds (default: 0.1)
+             * @param max_rto         Maximum RTO value in seconds (default: 60.0)
+             *
+             * The alpha and beta parameters control how quickly the estimator adapts to changes:
+             * - alpha: Lower values (e.g. 0.125) make the smoothed RTT more stable but slower to adapt
+             * - beta:  Lower values (e.g. 0.25) make the RTT variation more stable but slower to adapt
+             *
+             * @throws std::invalid_argument if alpha or beta are not in range [0,1]
+             * @throws std::invalid_argument if min_rto >= max_rto
+             */
+            RTTEstimator(float alpha           = 0.125f,
+                         float beta            = 0.25f,
+                         float initial_rtt     = 1.0f,
+                         float initial_rtt_var = 0.0f,
+                         float min_rto         = 0.1f,
+                         float max_rto         = 60.0f)
+                : alpha(alpha)
+                , beta(beta)
+                , min_rto(min_rto)
+                , max_rto(max_rto)
+                , smoothed_rtt(initial_rtt)
+                , rtt_var(initial_rtt_var)
+                , rto(std::min(std::max(initial_rtt + 4 * initial_rtt_var, min_rto), max_rto)) {
+
+                if (alpha < 0.0f || alpha > 1.0f) {
+                    throw std::invalid_argument("alpha must be in range [0,1]");
+                }
+                if (beta < 0.0f || beta > 1.0f) {
+                    throw std::invalid_argument("beta must be in range [0,1]");
+                }
+                if (min_rto >= max_rto) {
+                    throw std::invalid_argument("min_rto must be less than max_rto");
+                }
+            }
+
+            /**
+             * Update the RTT estimate with a new measurement
+             *
+             * Updates the smoothed RTT, RTT variation, and RTO using the Jacobson/Karels algorithm:
+             * 1. RTT variation = (1 - beta) * old_variation + beta * |smoothed_rtt - new_rtt|
+             * 2. Smoothed RTT = (1 - alpha) * old_rtt + alpha * new_rtt
+             * 3. RTO = smoothed_rtt + 4 * rtt_var
+             *
+             * The RTO is bounded between min_rto and max_rto to prevent extreme values.
+             *
+             * @param time The measured round trip time
+             */
+            void measure(std::chrono::steady_clock::duration time);
+
+            /**
+             * Get the current retransmission timeout
+             *
+             * @return The RTO as a duration. This value represents the recommended timeout
+             *         for network operations based on the current RTT estimates.
+             */
+            std::chrono::steady_clock::duration timeout() const;
+
+        private:
+            float alpha;         ///< Weight for RTT smoothing (typically 0.125)
+            float beta;          ///< Weight for RTT variation (typically 0.25)
+            float min_rto;       ///< Minimum RTO value in seconds
+            float max_rto;       ///< Maximum RTO value in seconds
+            float smoothed_rtt;  ///< Smoothed RTT estimate in seconds
+            float rtt_var;       ///< RTT variation in seconds
+            float rto;           ///< Retransmission timeout in seconds
+        };
+
+    }  // namespace network
+}  // namespace extension
+}  // namespace NUClear
+
+#endif  // NUCLEAR_EXTENSION_NETWORK_RTT_ESTIMATOR_HPP
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -40,7 +40,7 @@ set_target_properties(${catch2_targets} PROPERTIES CXX_CLANG_TIDY "")
 set_target_properties(${catch2_target} PROPERTIES CMAKE_CXX_FLAGS "")
 
 # Create a test_util library that is used by all tests
-file(GLOB_RECURSE test_util_src "test_util/*.cpp")
+file(GLOB_RECURSE test_util_src CONFIGURE_DEPENDS "test_util/*.cpp")
 add_library(test_util OBJECT ${test_util_src})
 # This is linking WHOLE_ARCHIVE as otherwise the linker will remove the WSAHolder from the final binary
 # As a result the WSA initialisation code won't run and the network tests will fail
@@ -50,7 +50,7 @@ target_include_directories(test_util PUBLIC ${PROJECT_BINARY_DIR}/include ${PROJ
 target_include_directories(test_util PUBLIC ${CMAKE_CURRENT_SOURCE_DIR})
 
 # Create a test binary for each test file
-file(GLOB_RECURSE test_sources "tests/*.cpp")
+file(GLOB_RECURSE test_sources CONFIGURE_DEPENDS "tests/*.cpp")
 foreach(test_file ${test_sources})
   get_filename_component(test_name ${test_file} NAME_WE)
   get_filename_component(test_dir ${test_file} DIRECTORY)
diff --git a/tests/tests/network/RTTEstimator.cpp b/tests/tests/network/RTTEstimator.cpp
