From a58f6afa44e53ce3c5978bcea1b6fd5be9c9ef62 Mon Sep 17 00:00:00 2001 From: KerstinKeller Date: Wed, 10 Jul 2024 17:16:51 +0200 Subject: [PATCH] [core] Fix bug in frequency calculator that will reset the frequency to 0.0 even though data is still incoming. (#1650) --- ecal/core/src/util/frequency_calculator.h | 46 ++++++++++++------ ecal/tests/cpp/util_test/src/util_test.cpp | 55 +++++++++++++++++++++- 2 files changed, 85 insertions(+), 16 deletions(-) diff --git a/ecal/core/src/util/frequency_calculator.h b/ecal/core/src/util/frequency_calculator.h index 1ec85f4f04..05bda24a3f 100644 --- a/ecal/core/src/util/frequency_calculator.h +++ b/ecal/core/src/util/frequency_calculator.h @@ -1,6 +1,6 @@ -/* ========================= eCAL LICENSE ================================= +/* ========================= eCAL LICENSE ================================= * - * Copyright (C) 2016 - 2019 Continental Corporation + * Copyright (C) 2016 - 2024 Continental Corporation * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -52,8 +52,8 @@ namespace eCAL FrequencyCalculator(const time_point& now_) : counted_elements(0) - , first_tick(now_) - , last_tick(now_) + , first_tick_time(now_) + , last_tick_time(now_) , previous_frequency(0.0) { } @@ -61,7 +61,7 @@ namespace eCAL void addTick(const time_point& now) { counted_elements++; - last_tick = now; + last_tick_time = now; } double getFrequency() @@ -71,16 +71,16 @@ namespace eCAL return previous_frequency; } - previous_frequency = calculateFrequency(first_tick, last_tick, counted_elements); - first_tick = last_tick; + previous_frequency = calculateFrequency(first_tick_time, last_tick_time, counted_elements); + first_tick_time = last_tick_time; counted_elements = 0; return previous_frequency; } private: long long counted_elements; - time_point first_tick; - time_point last_tick; + time_point first_tick_time; + time_point last_tick_time; double previous_frequency; }; @@ -105,22 +105,37 @@ namespace eCAL { calculator->addTick(now); } - last_tick = now; + last_tick_time = now; + received_tick_since_get_frequency_called = true; } double getFrequency(const time_point& now) { double frequency = calculator ? calculator->getFrequency() : 0.0; + // if the frequency is 0.0, return it right away if (frequency == 0.0) { return 0.0; } - // calculate theoretical frequency to detect timeouts; - double theoretical_frequency = calculateFrequency(last_tick, now, 1); - // If the frequency is higher than reset_factor * theoretical_frequency, we reset. - if (frequency >= theoretical_frequency * reset_factor) + // if we have received ticks, this means we don't want to reset the calculation + // so we return the frequency that the calculator has calculated + if (received_tick_since_get_frequency_called) + { + received_tick_since_get_frequency_called = false; + return frequency; + } + + // if we have not received ticks since the last call of getFrequency + // then we calculate the ΔT, e.g. the timespan in seconds between last received and next incoming element + // Based on the last time we received a tick we could calculate when we expect the next tick + // We can also calculate when we want to time out, based on the reset factor + // timeout = last_tick + ΔT * reset_factor + // If the current time is greater than the timeout time, we reset the calculator and return 0 + auto expected_time_in_seconds_between_last_and_next_tick = std::chrono::duration(1 / frequency); + time_point timeout_time = last_tick_time + std::chrono::duration_cast(expected_time_in_seconds_between_last_and_next_tick * reset_factor); + if (now > timeout_time) { calculator.reset(); return 0.0; @@ -131,7 +146,8 @@ namespace eCAL private: float reset_factor; - time_point last_tick; + time_point last_tick_time; + bool received_tick_since_get_frequency_called = false; std::unique_ptr> calculator; }; } diff --git a/ecal/tests/cpp/util_test/src/util_test.cpp b/ecal/tests/cpp/util_test/src/util_test.cpp index abaf84d48f..5936dbf459 100644 --- a/ecal/tests/cpp/util_test/src/util_test.cpp +++ b/ecal/tests/cpp/util_test/src/util_test.cpp @@ -1,6 +1,6 @@ /* ========================= eCAL LICENSE ================================= * - * Copyright (C) 2016 - 2019 Continental Corporation + * Copyright (C) 2016 - 2024 Continental Corporation * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -194,3 +194,56 @@ TEST(core_cpp_util, Freq_ResettableFrequencyCalculator) } } } + + +struct command +{ + std::string command_type; + long long time_point; +}; + +std::vector commands = +{ +command{"getting frequency", 0 }, +command{"ticking", 500 }, +command{"getting frequency", 1000 }, +command{"ticking", 1500 }, +command{"getting frequency", 2000 }, +command{"ticking", 2500 }, +command{"getting frequency", 3000 }, +command{"ticking", 3998 }, +command{"getting frequency", 4000 }, +command{"ticking", 4002 }, +command{"getting frequency", 5000 }, +command{"ticking", 5998 }, +command{"getting frequency", 6000 }, +}; + +TEST(core_cpp_util, Freq_NoZeroFrequency) +{ + eCAL::ResettableFrequencyCalculator calculator(3.0f); + int i = 0; + + for (const auto& command : commands) + { + std::chrono::steady_clock::time_point time(std::chrono::milliseconds(command.time_point)); + + if (command.command_type == "ticking") + { + calculator.addTick(time); + } + else if (command.command_type == "getting frequency") + { + auto frequency = calculator.getFrequency(time); + if (i > 1) + { + ASSERT_GT(frequency, 0.0) << "Iteration " << i; + } + + ++i; + } + else + { + } + } +}