From f1a5e2757e33d9000e5384d76a45ac0b46bcb37d Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Fri, 23 Feb 2024 12:08:16 +1100 Subject: [PATCH 01/45] Add in a new TimeTravel mesage to allow the system to change the NUClear clock and ensure that messages continue to work --- src/clock.hpp | 83 +++++++++++++++----- src/extension/ChronoController.hpp | 15 ++++ src/message/TimeTravel.hpp | 50 ++++++++++++ tests/individual/BaseClock.cpp | 17 ++-- tests/individual/CustomClock.cpp | 20 ++--- tests/network/NUClearNetwork.cpp | 121 +++++++++++++++++++++++++++++ 6 files changed, 265 insertions(+), 41 deletions(-) create mode 100644 src/message/TimeTravel.hpp create mode 100644 tests/network/NUClearNetwork.cpp diff --git a/src/clock.hpp b/src/clock.hpp index 8db78d417..edf3e300a 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -23,36 +23,81 @@ #ifndef NUCLEAR_CLOCK_HPP #define NUCLEAR_CLOCK_HPP +// Default to using the system clock but allow it to be overridden by the user +#ifndef NUCLEAR_CLOCK_TYPE + #define NUCLEAR_CLOCK_TYPE std::chrono::system_clock +#endif // NUCLEAR_CLOCK_TYPE + +#include +#include #include +#include namespace NUClear { -#ifdef NUCLEAR_CLOCK_TYPE -/// @brief The custom base clock that is used when defining the NUClear clock -using base_clock = NUCLEAR_CLOCK_TYPE; -#else -/// @brief The default base clock that is used when defining the NUClear clock -using base_clock = std::chrono::steady_clock; -#endif // NUCLEAR_CLOCK_TYPE +struct clock : public NUCLEAR_CLOCK_TYPE { + using base_clock = NUCLEAR_CLOCK_TYPE; + + static time_point now() { + ClockData current = data[active.load()]; // Take a copy in case it changes + return current.epoch + dc((base_clock::now() - current.base_from) * current.rtf); + } -#ifndef NUCLEAR_CUSTOM_CLOCK + static void adjust_clock(const duration& adjustment, const double& rtf = 1.0) { + std::lock_guard lock(mutex); + // Load the current state + auto& current = data[active.load()]; + int n = (active.load() + 1) % data.size(); + auto& next = data[n]; -/// @brief The clock that is used throughout the entire nuclear system -using clock = base_clock; + // Perform the update + auto base = base_clock::now(); + next.epoch = current.epoch + adjustment + dc((base - current.base_from) * current.rtf); + next.base_from = base; + next.rtf = rtf; + active = n; + } -#else + static void set_clock(const time_point& time, const double& rtf = 1.0) { + std::lock_guard lock(mutex); + // Load the current state + int n = (active.load() + 1) % data.size(); + auto& next = data[n]; -struct clock { - using rep = base_clock::rep; - using period = base_clock::period; - using duration = base_clock::duration; - using time_point = base_clock::time_point; - static constexpr bool is_steady = false; + // Perform the update + auto base = base_clock::now(); + next.epoch = time; + next.base_from = base; + next.rtf = rtf; + active = n; + } - static time_point now(); +private: + template + duration static dc(const T& t) { + return std::chrono::duration_cast(t); + } + + struct ClockData { + /// When the clock was last updated under the true time + time_point base_from; + /// Our calculated time when the clock was last updated in simulated time + time_point epoch = base_from; + /// The real time factor of the simulated clock + double rtf; + + ClockData() : base_from(base_clock::now()), epoch(base_from), rtf(1.0) {} + }; + + static std::mutex mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + static std::array data; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + static std::atomic active; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) }; -#endif +std::mutex clock::mutex; +std::array clock::data = std::array{}; +std::atomic clock::active{0}; + } // namespace NUClear diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index 50ff94fb8..1c7f5e157 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -25,6 +25,7 @@ #include "../PowerPlant.hpp" #include "../Reactor.hpp" +#include "../message/TimeTravel.hpp" #include "../util/precise_sleep.hpp" namespace NUClear { @@ -100,6 +101,20 @@ namespace extension { wait.notify_all(); }); + on>().then("Time Travel", [this](const message::TimeTravel& travel) { + const std::lock_guard lock(mutex); + + clock::adjust_clock(travel.adjustment, travel.rtf); + + // Adjust all the times of the tasks by the travel time + for (auto& task : tasks) { + task.time += travel.adjustment; + } + + // Poke the system + wait.notify_all(); + }); + on().then("Chrono Controller", [this] { // Run until we are told to stop while (running.load()) { diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp new file mode 100644 index 000000000..a3e0bfbf6 --- /dev/null +++ b/src/message/TimeTravel.hpp @@ -0,0 +1,50 @@ +/* + * MIT License + * + * Copyright (c) 2024 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_MESSAGE_TIME_TRAVEL_HPP +#define NUCLEAR_MESSAGE_TIME_TRAVEL_HPP + +#include "../clock.hpp" + +namespace NUClear { +namespace message { + + /** + * @brief This message is used to adjust the time of the system clock and the rate at which time passes. + * + * Using this message allows the NUClear system to adapt to the change by adjusting any time based operations + * to the new time and rate. + */ + struct TimeTravel { + /// @brief The amount of time to adjust the system clock by + clock::duration adjustment{0}; + /// @brief The rate at which time should pass + double rtf = 1.0; + + TimeTravel() = default; + TimeTravel(const clock::duration& adjustment, double rtf = 1.0) : adjustment(adjustment), rtf(rtf) {} + }; + +} // namespace message +} // namespace NUClear + +#endif // NUCLEAR_MESSAGE_TIME_TRAVEL_HPP diff --git a/tests/individual/BaseClock.cpp b/tests/individual/BaseClock.cpp index eae611fd6..95604568b 100644 --- a/tests/individual/BaseClock.cpp +++ b/tests/individual/BaseClock.cpp @@ -24,8 +24,8 @@ #include #include // for localtime_r/s -// This define declares that we are using system_clock as the base clock for NUClear -#define NUCLEAR_CLOCK_TYPE std::chrono::system_clock +// This define declares that we are using steady_clock as the base clock for NUClear +#define NUCLEAR_CLOCK_TYPE std::chrono::steady_clock #include #include @@ -38,7 +38,7 @@ namespace { // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -std::vector> times; +std::vector> times; // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) std::mutex times_mutex; constexpr int n_time = 100; @@ -56,7 +56,7 @@ class TestReactor : public NUClear::Reactor { on>().then( [this](const NUClear::message::ReactionStatistics& stats) { const std::lock_guard lock(times_mutex); - times.push_back(std::make_pair(stats.finished, std::chrono::system_clock::now())); + times.push_back(std::make_pair(stats.finished, std::chrono::steady_clock::now())); if (times.size() > n_time) { powerplant.shutdown(); } @@ -68,7 +68,7 @@ class TestReactor : public NUClear::Reactor { TEST_CASE("Testing base clock works correctly", "[api][base_clock]") { INFO("Ensure NUClear base_clock is the correct type"); - STATIC_REQUIRE(std::is_same::value); + STATIC_REQUIRE(std::is_base_of::value); // Construct the powerplant NUClear::Configuration config; @@ -105,8 +105,11 @@ TEST_CASE("Testing base clock works correctly", "[api][base_clock]") { // Compute the differences between the time pairs int match_count = 0; for (const auto& time_pairs : times) { - const std::time_t ntt = NUClear::clock::to_time_t(time_pairs.first); - const std::time_t stt = NUClear::clock::to_time_t(time_pairs.second); + using namespace std::chrono; + const std::time_t ntt = system_clock::to_time_t( + system_clock::time_point(duration_cast(time_pairs.first.time_since_epoch()))); + const std::time_t stt = system_clock::to_time_t( + system_clock::time_point(duration_cast(time_pairs.second.time_since_epoch()))); std::tm result{}; #ifdef WIN32 diff --git a/tests/individual/CustomClock.cpp b/tests/individual/CustomClock.cpp index 040b6edd6..3eda565b2 100644 --- a/tests/individual/CustomClock.cpp +++ b/tests/individual/CustomClock.cpp @@ -22,25 +22,10 @@ #define CATCH_CONFIG_MAIN #include - -// This define declares that we are using a custom clock and it should try to link -#define NUCLEAR_CUSTOM_CLOCK #include #include "test_util/TestBase.hpp" -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); - -namespace NUClear { -clock::time_point clock::now() { - - // Add half the time since we started (time moving at half speed) - auto now = std::chrono::steady_clock::now(); - return clock::time_point(start + (now - start) / 2); -} -} // namespace NUClear - // Anonymous namespace to keep everything file local namespace { @@ -63,6 +48,11 @@ class TestReactor : public test_util::TestBase { on>().then([this] { // powerplant.shutdown(); }); + + on().then([this] { + // Adjust the clock to run at half speed + emit(std::make_unique(NUClear::clock::duration(0), 0.5)); + }); } }; } // namespace diff --git a/tests/network/NUClearNetwork.cpp b/tests/network/NUClearNetwork.cpp new file mode 100644 index 000000000..82773a228 --- /dev/null +++ b/tests/network/NUClearNetwork.cpp @@ -0,0 +1,121 @@ +/* + * MIT License + * + * Copyright (c) 2023 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 +#include + +#include "test_util/TestBase.hpp" + +namespace { + +/// @brief Events that occur during the test +std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + +struct IPv4UnicastConfig { + static std::string hostname() {} +}; + +struct IPv6UnicastConfig { + static std::string hostname() {} +}; + +struct IPv4BroadcastConfig { + static std::string hostname() {} +}; + +struct IPv4MulticastConfig { + static std::string hostname() {} +}; + +struct IPv6MulticastConfig { + static std::string hostname() {} +}; + +template +class NetworkBase : public test_util::TestBase { + + // Set our function callback + network.set_packet_callback([this](const network::NUClearNetwork::NetworkTarget& remote, + const uint64_t& hash, + const bool& reliable, + std::vector&& payload) { + emit(std::make_unique(remote, hash, reliable, std::move(payload))); + }); + + // Set our join callback + network.set_join_callback([this](const network::NUClearNetwork::NetworkTarget& remote) { + // Join event + }); + + // Set our leave callback + network.set_leave_callback([this](const network::NUClearNetwork::NetworkTarget& remote) { + // Leave event + }); + + // Set our event timer callback + network.set_next_event_callback([this](std::chrono::steady_clock::time_point t) { + const std::chrono::steady_clock::duration emit_offset = t - std::chrono::steady_clock::now(); + emit(std::make_unique(), + std::chrono::duration_cast(emit_offset)); + }); + +public: + explicit TestReactor(std::unique_ptr environment) : Reactor(std::move(environment)) { + + // Reset both networks to connect them + network_a.reset(name, "127.0.0.1", 7447); + network_b.reset(name, "127.0.0.1", 7447); + + // Execution handle + process_handle = on>().then("Network processing", [this] { network.process(); }); + + for (auto& fd : network.listen_fds()) { + listen_handles.push_back(on(fd, IO::READ).then("Packet", [this] { network.process(); })); + } + } + + NUClear::extension::network::NUClearNetwork network; +}; +} // namespace + + +TEST_CASE("Testing that when an on statement does not have it's data satisfied it does not run", "[api][nodata]") { + + NUClear::Configuration config; + config.thread_count = 1; + NUClear::PowerPlant plant(config); + plant.install(); + plant.start(); + + const std::vector expected = { + "Emitting MessageA", + "MessageA triggered", + "Emitting MessageB", + "MessageB with MessageA triggered", + }; + + // Make an info print the diff in an easy to read way if we fail + INFO(test_util::diff_string(expected, events)); + + // Check the events fired in order and only those events + REQUIRE(events == expected); +} From 29927be21f89e3bf6547dfcd5dec08831ab6a6b4 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 6 Mar 2024 15:35:02 +1100 Subject: [PATCH 02/45] Delete NUClearNetwork.cpp --- tests/network/NUClearNetwork.cpp | 121 ------------------------------- 1 file changed, 121 deletions(-) delete mode 100644 tests/network/NUClearNetwork.cpp diff --git a/tests/network/NUClearNetwork.cpp b/tests/network/NUClearNetwork.cpp deleted file mode 100644 index 82773a228..000000000 --- a/tests/network/NUClearNetwork.cpp +++ /dev/null @@ -1,121 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2023 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 -#include - -#include "test_util/TestBase.hpp" - -namespace { - -/// @brief Events that occur during the test -std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) - -struct IPv4UnicastConfig { - static std::string hostname() {} -}; - -struct IPv6UnicastConfig { - static std::string hostname() {} -}; - -struct IPv4BroadcastConfig { - static std::string hostname() {} -}; - -struct IPv4MulticastConfig { - static std::string hostname() {} -}; - -struct IPv6MulticastConfig { - static std::string hostname() {} -}; - -template -class NetworkBase : public test_util::TestBase { - - // Set our function callback - network.set_packet_callback([this](const network::NUClearNetwork::NetworkTarget& remote, - const uint64_t& hash, - const bool& reliable, - std::vector&& payload) { - emit(std::make_unique(remote, hash, reliable, std::move(payload))); - }); - - // Set our join callback - network.set_join_callback([this](const network::NUClearNetwork::NetworkTarget& remote) { - // Join event - }); - - // Set our leave callback - network.set_leave_callback([this](const network::NUClearNetwork::NetworkTarget& remote) { - // Leave event - }); - - // Set our event timer callback - network.set_next_event_callback([this](std::chrono::steady_clock::time_point t) { - const std::chrono::steady_clock::duration emit_offset = t - std::chrono::steady_clock::now(); - emit(std::make_unique(), - std::chrono::duration_cast(emit_offset)); - }); - -public: - explicit TestReactor(std::unique_ptr environment) : Reactor(std::move(environment)) { - - // Reset both networks to connect them - network_a.reset(name, "127.0.0.1", 7447); - network_b.reset(name, "127.0.0.1", 7447); - - // Execution handle - process_handle = on>().then("Network processing", [this] { network.process(); }); - - for (auto& fd : network.listen_fds()) { - listen_handles.push_back(on(fd, IO::READ).then("Packet", [this] { network.process(); })); - } - } - - NUClear::extension::network::NUClearNetwork network; -}; -} // namespace - - -TEST_CASE("Testing that when an on statement does not have it's data satisfied it does not run", "[api][nodata]") { - - NUClear::Configuration config; - config.thread_count = 1; - NUClear::PowerPlant plant(config); - plant.install(); - plant.start(); - - const std::vector expected = { - "Emitting MessageA", - "MessageA triggered", - "Emitting MessageB", - "MessageB with MessageA triggered", - }; - - // Make an info print the diff in an easy to read way if we fail - INFO(test_util::diff_string(expected, events)); - - // Check the events fired in order and only those events - REQUIRE(events == expected); -} From 0786ca0c59688543152258eb1937356380c7e0af Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 6 Mar 2024 16:11:50 +1100 Subject: [PATCH 03/45] Fix linking --- src/clock.cpp | 29 +++++++++++++++++++++++++++++ src/clock.hpp | 5 ----- 2 files changed, 29 insertions(+), 5 deletions(-) create mode 100644 src/clock.cpp diff --git a/src/clock.cpp b/src/clock.cpp new file mode 100644 index 000000000..4e4bcb4dc --- /dev/null +++ b/src/clock.cpp @@ -0,0 +1,29 @@ +/* + * MIT License + * + * Copyright (c) 2013 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 "clock.hpp" + +namespace NUClear { +std::mutex clock::mutex; +std::array clock::data = std::array{}; +std::atomic clock::active{0}; +} // namespace NUClear diff --git a/src/clock.hpp b/src/clock.hpp index edf3e300a..3d05d8bb2 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -94,11 +94,6 @@ struct clock : public NUCLEAR_CLOCK_TYPE { static std::atomic active; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) }; -std::mutex clock::mutex; -std::array clock::data = std::array{}; -std::atomic clock::active{0}; - - } // namespace NUClear #endif // NUCLEAR_CLOCK_HPP From 621ea39edcbbd466693747a1c58432fcff8de8ef Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 20 Mar 2024 17:18:03 +1100 Subject: [PATCH 04/45] Add time travel types --- src/message/TimeTravel.hpp | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index a3e0bfbf6..67759e28d 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -27,7 +27,6 @@ namespace NUClear { namespace message { - /** * @brief This message is used to adjust the time of the system clock and the rate at which time passes. * @@ -35,13 +34,28 @@ namespace message { * to the new time and rate. */ struct TimeTravel { - /// @brief The amount of time to adjust the system clock by - clock::duration adjustment{0}; + enum class Action { + /// @brief Adjust clock and move all chrono tasks with it + RELATIVE, + + /// @brief Adjust clock to target time and leave chrono tasks where they are + JUMP, + + /// @brief Adjust clock to as close to target as possible without skipping any chrono tasks + NEAREST, + }; + + clock::time_point target; /// @brief The rate at which time should pass double rtf = 1.0; + /// @brief The type of time travel to perform + Action type = Action::RELATIVE; TimeTravel() = default; - TimeTravel(const clock::duration& adjustment, double rtf = 1.0) : adjustment(adjustment), rtf(rtf) {} + TimeTravel(const clock::duration& adjustment, double rtf = 1.0, Action type = Action::RELATIVE) + : target(clock::now() + adjustment), rtf(rtf), type(type) {} + TimeTravel(const clock::time_point& time, double rtf = 1.0, Action type = Action::RELATIVE) + : target(target), rtf(rtf), type(type) {} }; } // namespace message From b28856f8ae6cf95714d87ee6a61e0bad4f0b0f4e Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 14:27:50 +1100 Subject: [PATCH 05/45] Add time travel actions --- src/extension/ChronoController.hpp | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index 1c7f5e157..c9a51b6f6 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -104,11 +104,29 @@ namespace extension { on>().then("Time Travel", [this](const message::TimeTravel& travel) { const std::lock_guard lock(mutex); - clock::adjust_clock(travel.adjustment, travel.rtf); + // Adjust clock to target time and leave chrono tasks where they are + if (travel.type == message::TimeTravel::Action::JUMP) { + clock::set_clock(travel.target, travel.rtf); + } + + // Adjust clock to target time and leave chrono tasks where they are + if (travel.type == message::TimeTravel::Action::NEAREST) { + // Find the task in the list that is closest to be run + auto it = + std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { + return a.time < b.time; + }); + // Set the clock to the time closest of the nearest task and the target time + clock::set_clock(std::min(it->time, travel.target), travel.rtf); + } - // Adjust all the times of the tasks by the travel time - for (auto& task : tasks) { - task.time += travel.adjustment; + // Adjust clock and move all chrono tasks with it + if (travel.type == message::TimeTravel::Action::RELATIVE) { + auto adjustment = travel.target - NUClear::clock::now(); + for (auto& task : tasks) { + task.time += adjustment; + } + clock::set_clock(travel.target, travel.rtf); } // Poke the system From c73ad64462ea50719343f817d54dc766201a41e0 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 14:27:58 +1100 Subject: [PATCH 06/45] Update TimeTravel.hpp --- src/message/TimeTravel.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index 67759e28d..4b59263b8 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -45,6 +45,7 @@ namespace message { NEAREST, }; + /// @brief The target time to set the clock to clock::time_point target; /// @brief The rate at which time should pass double rtf = 1.0; @@ -54,7 +55,7 @@ namespace message { TimeTravel() = default; TimeTravel(const clock::duration& adjustment, double rtf = 1.0, Action type = Action::RELATIVE) : target(clock::now() + adjustment), rtf(rtf), type(type) {} - TimeTravel(const clock::time_point& time, double rtf = 1.0, Action type = Action::RELATIVE) + TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE) : target(target), rtf(rtf), type(type) {} }; From 36d022cd1faec682e8e2e457fb8013f6049c59fd Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 16:08:53 +1100 Subject: [PATCH 07/45] Change to adjustment --- src/extension/ChronoController.hpp | 49 +++++++++++++++--------------- src/message/TimeTravel.hpp | 8 ++--- 2 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index c9a51b6f6..df3c2a24f 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -102,35 +102,36 @@ namespace extension { }); on>().then("Time Travel", [this](const message::TimeTravel& travel) { - const std::lock_guard lock(mutex); + { + const std::lock_guard lock(mutex); - // Adjust clock to target time and leave chrono tasks where they are - if (travel.type == message::TimeTravel::Action::JUMP) { - clock::set_clock(travel.target, travel.rtf); - } + // Adjust clock to target time and leave chrono tasks where they are + if (travel.type == message::TimeTravel::Action::JUMP) { + clock::adjust_clock(travel.adjustment, travel.rtf); + } - // Adjust clock to target time and leave chrono tasks where they are - if (travel.type == message::TimeTravel::Action::NEAREST) { - // Find the task in the list that is closest to be run - auto it = - std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { - return a.time < b.time; - }); - // Set the clock to the time closest of the nearest task and the target time - clock::set_clock(std::min(it->time, travel.target), travel.rtf); - } + // Adjust clock to target time and leave chrono tasks where they are + if (travel.type == message::TimeTravel::Action::NEAREST) { + // Find the task in the list that is closest to be run + auto it = + std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { + return a.time < b.time; + }); + // Set the clock to the time closest of the nearest task and the target time + clock::adjust_clock(std::min(it->time - clock::now(), travel.adjustment), travel.rtf); + } - // Adjust clock and move all chrono tasks with it - if (travel.type == message::TimeTravel::Action::RELATIVE) { - auto adjustment = travel.target - NUClear::clock::now(); - for (auto& task : tasks) { - task.time += adjustment; + // Adjust clock and move all chrono tasks with it + if (travel.type == message::TimeTravel::Action::RELATIVE) { + clock::adjust_clock(travel.adjustment, travel.rtf); + for (auto& task : tasks) { + task.time += travel.adjustment; + } } - clock::set_clock(travel.target, travel.rtf); - } - // Poke the system - wait.notify_all(); + // Poke the system + wait.notify_all(); + } }); on().then("Chrono Controller", [this] { diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index 4b59263b8..9158cea62 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -45,8 +45,8 @@ namespace message { NEAREST, }; - /// @brief The target time to set the clock to - clock::time_point target; + /// @brief The duration to adjust the clock by + clock::duration adjustment; /// @brief The rate at which time should pass double rtf = 1.0; /// @brief The type of time travel to perform @@ -54,9 +54,9 @@ namespace message { TimeTravel() = default; TimeTravel(const clock::duration& adjustment, double rtf = 1.0, Action type = Action::RELATIVE) - : target(clock::now() + adjustment), rtf(rtf), type(type) {} + : adjustment(adjustment), rtf(rtf), type(type) {} TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE) - : target(target), rtf(rtf), type(type) {} + : adjustment(target - clock::now()), rtf(rtf), type(type) {} }; } // namespace message From 26f56055a6582a36e4625df1a2ec47009c3e6deb Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 16:09:00 +1100 Subject: [PATCH 08/45] Add first test --- tests/individual/TimeTravel.cpp | 136 ++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tests/individual/TimeTravel.cpp diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp new file mode 100644 index 000000000..3cb34b954 --- /dev/null +++ b/tests/individual/TimeTravel.cpp @@ -0,0 +1,136 @@ +#define CATCH_CONFIG_MAIN +#include +#include + +#include "test_util/TestBase.hpp" + +namespace { + +// Helper struct for testing chrono tasks +struct ChronoTaskTest { + NUClear::clock::time_point time; // Time the task is supposed to run + bool ran = false; // Whether the task has been executed + std::chrono::seconds steady_delta; + std::chrono::seconds system_delta; + NUClear::id_t id; // Unique identifier for the task + ChronoTaskTest(NUClear::clock::time_point time_, bool ran_, NUClear::id_t id_) : time(time_), ran(ran_), id(id_) {} +}; + +ChronoTaskTest test_task(NUClear::clock::time_point(), false, 0); + +NUClear::clock::duration time_travel_adjustment; +NUClear::message::TimeTravel::Action time_travel_action; +std::chrono::seconds chrono_task_delay; + +std::chrono::steady_clock::time_point steady_start_time; +NUClear::clock::time_point system_start_time; + +class TestReactor : public test_util::TestBase { +public: + TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { + + on().then([this] { + test_task = ChronoTaskTest(NUClear::clock::now() + chrono_task_delay, false, 99); + + auto chrono_task = std::make_unique( + [this, &test_task](NUClear::clock::time_point&) { + test_task.ran = true; + auto steady_end_time = std::chrono::steady_clock::now(); + test_task.steady_delta = + std::chrono::duration_cast(steady_end_time - steady_start_time); + auto system_end_time = NUClear::clock::now(); + test_task.system_delta = + std::chrono::duration_cast(system_end_time - system_start_time); + powerplant.shutdown(); + return false; // Do not repeat the test_task + }, + test_task.time, + test_task.id); + emit(std::move(chrono_task)); + + emit( + std::make_unique(time_travel_adjustment, 1.0, time_travel_action)); + }); + } +}; + +void perform_time_travel(const NUClear::message::TimeTravel::Action action, + const NUClear::clock::duration adjustment, + const std::chrono::seconds& shutdown_delay, + const std::chrono::seconds& chrono_task_delay_) { + NUClear::Configuration config; + NUClear::PowerPlant plant(config); + time_travel_action = action; + time_travel_adjustment = adjustment; + chrono_task_delay = chrono_task_delay_; + + // Reset the clock + NUClear::clock::set_clock(NUClear::clock::time_point()); + plant.install(); + + // Record the start time for NUClear::clock and std::chrono::steady_clock + steady_start_time = std::chrono::steady_clock::now(); + system_start_time = NUClear::clock::now(); + + // Spin off a separate thread to shutdown the powerplant after a steady clock delay + std::thread([shutdown_delay, &plant] { + std::this_thread::sleep_for(shutdown_delay); + plant.shutdown(); + }).detach(); + plant.start(); +} + +} // anonymous namespace + +TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { + SECTION("Action::RELATIVE with shutdown before task time") { + perform_time_travel(NUClear::message::TimeTravel::Action::RELATIVE, + std::chrono::seconds(3), // Time travel adjustment + std::chrono::seconds(2), // Shutdown delay + std::chrono::seconds(3)); // Chrono task delay + INFO("Task expected ran: " << 0 << ", actual ran: " << test_task.ran); + REQUIRE_FALSE(test_task.ran); + } + + SECTION("Action::RELATIVE with shutdown after task time") { + perform_time_travel(NUClear::message::TimeTravel::Action::RELATIVE, + std::chrono::seconds(3), // Time travel adjustment + std::chrono::seconds(3), // Shutdown delay + std::chrono::seconds(2)); // Chrono task delay + INFO("Task expected ran: " << 1 << ", actual ran: " << test_task.ran); + REQUIRE(test_task.ran); + } + + SECTION("Action::JUMP with task time before jump time") { + perform_time_travel(NUClear::message::TimeTravel::Action::JUMP, + std::chrono::seconds(3), // Time travel adjustment + std::chrono::seconds(4), // Shutdown delay + std::chrono::seconds(3)); // Chrono task delay + INFO("Task expected ran: " << 1 << ", actual ran: " << test_task.ran); + REQUIRE(test_task.ran); + REQUIRE(test_task.steady_delta.count() == 0); + REQUIRE(test_task.system_delta.count() == 3); + } + + SECTION("Action::JUMP with task time after jump time") { + perform_time_travel(NUClear::message::TimeTravel::Action::JUMP, + std::chrono::seconds(3), // Time travel adjustment + std::chrono::seconds(2), // Shutdown delay + std::chrono::seconds(5)); // Chrono task delay + INFO("Task expected ran: " << 0 << ", actual ran: " << test_task.ran); + REQUIRE_FALSE(test_task.ran); + REQUIRE(test_task.steady_delta.count() == 0); + REQUIRE(test_task.system_delta.count() == 3); + } + + SECTION("Action::NEAREST") { + perform_time_travel(NUClear::message::TimeTravel::Action::NEAREST, + std::chrono::seconds(5), // Time travel adjustment + std::chrono::seconds(10), // Shutdown delay + std::chrono::seconds(1)); // Chrono task delay + INFO("Task expected ran: " << 1 << ", actual ran: " << test_task.ran); + REQUIRE(test_task.ran); + REQUIRE(test_task.steady_delta.count() == 0); + REQUIRE(test_task.system_delta.count() == 1); + } +} From d71d315696397298b1fcb54fcb9a073af56d3efe Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 16:28:10 +1100 Subject: [PATCH 09/45] Use a pointer --- tests/individual/TimeTravel.cpp | 60 ++++++++++++++++----------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 3cb34b954..9169dcb3a 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -8,15 +8,16 @@ namespace { // Helper struct for testing chrono tasks struct ChronoTaskTest { - NUClear::clock::time_point time; // Time the task is supposed to run - bool ran = false; // Whether the task has been executed - std::chrono::seconds steady_delta; - std::chrono::seconds system_delta; - NUClear::id_t id; // Unique identifier for the task + NUClear::clock::time_point time = NUClear::clock::time_point(); // Time to execute the task + bool ran = false; // Whether the task has been executed + std::chrono::seconds steady_delta = std::chrono::seconds(0); // Real time delta + std::chrono::seconds system_delta = std::chrono::seconds(0); // System time delta + NUClear::id_t id = 0; // Unique identifier for the task + ChronoTaskTest() {} ChronoTaskTest(NUClear::clock::time_point time_, bool ran_, NUClear::id_t id_) : time(time_), ran(ran_), id(id_) {} }; -ChronoTaskTest test_task(NUClear::clock::time_point(), false, 0); +std::unique_ptr test_task; NUClear::clock::duration time_travel_adjustment; NUClear::message::TimeTravel::Action time_travel_action; @@ -30,24 +31,25 @@ class TestReactor : public test_util::TestBase { TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { - test_task = ChronoTaskTest(NUClear::clock::now() + chrono_task_delay, false, 99); - + // Emit a chrono task to run after a delay + test_task = std::make_unique(NUClear::clock::now() + chrono_task_delay, false, 1); auto chrono_task = std::make_unique( - [this, &test_task](NUClear::clock::time_point&) { - test_task.ran = true; + [this](NUClear::clock::time_point&) { + test_task->ran = true; auto steady_end_time = std::chrono::steady_clock::now(); - test_task.steady_delta = + test_task->steady_delta = std::chrono::duration_cast(steady_end_time - steady_start_time); auto system_end_time = NUClear::clock::now(); - test_task.system_delta = + test_task->system_delta = std::chrono::duration_cast(system_end_time - system_start_time); powerplant.shutdown(); return false; // Do not repeat the test_task }, - test_task.time, - test_task.id); + test_task->time, + test_task->id); emit(std::move(chrono_task)); + // Time travel! emit( std::make_unique(time_travel_adjustment, 1.0, time_travel_action)); }); @@ -88,8 +90,8 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::seconds(3), // Time travel adjustment std::chrono::seconds(2), // Shutdown delay std::chrono::seconds(3)); // Chrono task delay - INFO("Task expected ran: " << 0 << ", actual ran: " << test_task.ran); - REQUIRE_FALSE(test_task.ran); + INFO("Task expected ran: " << 0 << ", actual ran: " << test_task->ran); + REQUIRE_FALSE(test_task->ran); } SECTION("Action::RELATIVE with shutdown after task time") { @@ -97,8 +99,8 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::seconds(3), // Time travel adjustment std::chrono::seconds(3), // Shutdown delay std::chrono::seconds(2)); // Chrono task delay - INFO("Task expected ran: " << 1 << ", actual ran: " << test_task.ran); - REQUIRE(test_task.ran); + INFO("Task expected ran: " << 1 << ", actual ran: " << test_task->ran); + REQUIRE(test_task->ran); } SECTION("Action::JUMP with task time before jump time") { @@ -106,10 +108,10 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::seconds(3), // Time travel adjustment std::chrono::seconds(4), // Shutdown delay std::chrono::seconds(3)); // Chrono task delay - INFO("Task expected ran: " << 1 << ", actual ran: " << test_task.ran); - REQUIRE(test_task.ran); - REQUIRE(test_task.steady_delta.count() == 0); - REQUIRE(test_task.system_delta.count() == 3); + INFO("Task expected ran: " << 1 << ", actual ran: " << test_task->ran); + REQUIRE(test_task->ran); + REQUIRE(test_task->steady_delta.count() == 0); + REQUIRE(test_task->system_delta.count() == 3); } SECTION("Action::JUMP with task time after jump time") { @@ -117,10 +119,8 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::seconds(3), // Time travel adjustment std::chrono::seconds(2), // Shutdown delay std::chrono::seconds(5)); // Chrono task delay - INFO("Task expected ran: " << 0 << ", actual ran: " << test_task.ran); - REQUIRE_FALSE(test_task.ran); - REQUIRE(test_task.steady_delta.count() == 0); - REQUIRE(test_task.system_delta.count() == 3); + INFO("Task expected ran: " << 0 << ", actual ran: " << test_task->ran); + REQUIRE_FALSE(test_task->ran); } SECTION("Action::NEAREST") { @@ -128,9 +128,9 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::seconds(5), // Time travel adjustment std::chrono::seconds(10), // Shutdown delay std::chrono::seconds(1)); // Chrono task delay - INFO("Task expected ran: " << 1 << ", actual ran: " << test_task.ran); - REQUIRE(test_task.ran); - REQUIRE(test_task.steady_delta.count() == 0); - REQUIRE(test_task.system_delta.count() == 1); + INFO("Task expected ran: " << 1 << ", actual ran: " << test_task->ran); + REQUIRE(test_task->ran); + REQUIRE(test_task->steady_delta.count() == 0); + REQUIRE(test_task->system_delta.count() == 1); } } From cf73079dffd156b8746f4d6b84df8b476dc20101 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 16:29:49 +1100 Subject: [PATCH 10/45] chrono_task_delay_ -> task_delay --- tests/individual/TimeTravel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 9169dcb3a..c75751794 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -59,12 +59,12 @@ class TestReactor : public test_util::TestBase { void perform_time_travel(const NUClear::message::TimeTravel::Action action, const NUClear::clock::duration adjustment, const std::chrono::seconds& shutdown_delay, - const std::chrono::seconds& chrono_task_delay_) { + const std::chrono::seconds& task_delay) { NUClear::Configuration config; NUClear::PowerPlant plant(config); time_travel_action = action; time_travel_adjustment = adjustment; - chrono_task_delay = chrono_task_delay_; + chrono_task_delay = task_delay; // Reset the clock NUClear::clock::set_clock(NUClear::clock::time_point()); From 86f93ee7f9a16595898a62699e5e43b23456eb1a Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 27 Mar 2024 16:53:02 +1100 Subject: [PATCH 11/45] Move global vars to reactor --- tests/individual/TimeTravel.cpp | 146 +++++++++++++++++--------------- 1 file changed, 78 insertions(+), 68 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index c75751794..334cc455b 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -8,30 +8,38 @@ namespace { // Helper struct for testing chrono tasks struct ChronoTaskTest { - NUClear::clock::time_point time = NUClear::clock::time_point(); // Time to execute the task - bool ran = false; // Whether the task has been executed - std::chrono::seconds steady_delta = std::chrono::seconds(0); // Real time delta - std::chrono::seconds system_delta = std::chrono::seconds(0); // System time delta - NUClear::id_t id = 0; // Unique identifier for the task - ChronoTaskTest() {} + NUClear::clock::time_point time = NUClear::clock::time_point(); + bool ran = false; + std::chrono::seconds steady_delta = std::chrono::seconds(0); + std::chrono::seconds system_delta = std::chrono::seconds(0); + NUClear::id_t id = 0; + ChronoTaskTest() = default; ChronoTaskTest(NUClear::clock::time_point time_, bool ran_, NUClear::id_t id_) : time(time_), ran(ran_), id(id_) {} }; -std::unique_ptr test_task; - -NUClear::clock::duration time_travel_adjustment; -NUClear::message::TimeTravel::Action time_travel_action; -std::chrono::seconds chrono_task_delay; - -std::chrono::steady_clock::time_point steady_start_time; -NUClear::clock::time_point system_start_time; +// Struct to hold test results +struct TestResults { + bool task_ran = false; + std::chrono::seconds steady_delta = std::chrono::seconds(0); + std::chrono::seconds system_delta = std::chrono::seconds(0); +}; class TestReactor : public test_util::TestBase { public: + std::unique_ptr test_task; + NUClear::clock::duration time_travel_adjustment; + NUClear::message::TimeTravel::Action time_travel_action; + std::chrono::seconds chrono_task_delay; + + std::chrono::steady_clock::time_point steady_start_time; + NUClear::clock::time_point system_start_time; + TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { - // Emit a chrono task to run after a delay + steady_start_time = std::chrono::steady_clock::now(); + system_start_time = NUClear::clock::now(); + test_task = std::make_unique(NUClear::clock::now() + chrono_task_delay, false, 1); auto chrono_task = std::make_unique( [this](NUClear::clock::time_point&) { @@ -43,94 +51,96 @@ class TestReactor : public test_util::TestBase { test_task->system_delta = std::chrono::duration_cast(system_end_time - system_start_time); powerplant.shutdown(); - return false; // Do not repeat the test_task + return false; }, test_task->time, test_task->id); emit(std::move(chrono_task)); - // Time travel! emit( std::make_unique(time_travel_adjustment, 1.0, time_travel_action)); }); } }; -void perform_time_travel(const NUClear::message::TimeTravel::Action action, - const NUClear::clock::duration adjustment, +TestResults perform_test(NUClear::message::TimeTravel::Action action, + const NUClear::clock::duration& adjustment, const std::chrono::seconds& shutdown_delay, const std::chrono::seconds& task_delay) { NUClear::Configuration config; - NUClear::PowerPlant plant(config); - time_travel_action = action; - time_travel_adjustment = adjustment; - chrono_task_delay = task_delay; + auto plant = std::make_shared(config); + auto& reactor = plant->install(); - // Reset the clock - NUClear::clock::set_clock(NUClear::clock::time_point()); - plant.install(); + reactor.time_travel_action = action; + reactor.time_travel_adjustment = adjustment; + reactor.chrono_task_delay = task_delay; - // Record the start time for NUClear::clock and std::chrono::steady_clock - steady_start_time = std::chrono::steady_clock::now(); - system_start_time = NUClear::clock::now(); + NUClear::clock::set_clock(NUClear::clock::time_point()); - // Spin off a separate thread to shutdown the powerplant after a steady clock delay - std::thread([shutdown_delay, &plant] { + std::thread shutdown_thread([shutdown_delay, plant]() { std::this_thread::sleep_for(shutdown_delay); - plant.shutdown(); - }).detach(); - plant.start(); + plant->shutdown(); + }); + + plant->start(); + shutdown_thread.join(); + + // After shutdown, collect results from the reactor + TestResults results; + if (reactor.test_task) { + results.task_ran = reactor.test_task->ran; + results.steady_delta = reactor.test_task->steady_delta; + results.system_delta = reactor.test_task->system_delta; + } + return results; } } // anonymous namespace TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { SECTION("Action::RELATIVE with shutdown before task time") { - perform_time_travel(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::seconds(3), // Time travel adjustment - std::chrono::seconds(2), // Shutdown delay - std::chrono::seconds(3)); // Chrono task delay - INFO("Task expected ran: " << 0 << ", actual ran: " << test_task->ran); - REQUIRE_FALSE(test_task->ran); + auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, + std::chrono::seconds(2), // Time travel adjustment + std::chrono::seconds(1), // Shutdown delay + std::chrono::seconds(2)); // Chrono task delay + REQUIRE_FALSE(results.task_ran); } SECTION("Action::RELATIVE with shutdown after task time") { - perform_time_travel(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::seconds(3), // Time travel adjustment - std::chrono::seconds(3), // Shutdown delay - std::chrono::seconds(2)); // Chrono task delay - INFO("Task expected ran: " << 1 << ", actual ran: " << test_task->ran); - REQUIRE(test_task->ran); + auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, + std::chrono::seconds(2), // Time travel adjustment + std::chrono::seconds(3), // Shutdown delay + std::chrono::seconds(1)); // Chrono task delay + REQUIRE(results.task_ran); + REQUIRE(results.steady_delta.count() == 1); + REQUIRE(results.system_delta.count() == 3); } SECTION("Action::JUMP with task time before jump time") { - perform_time_travel(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::seconds(3), // Time travel adjustment - std::chrono::seconds(4), // Shutdown delay - std::chrono::seconds(3)); // Chrono task delay - INFO("Task expected ran: " << 1 << ", actual ran: " << test_task->ran); - REQUIRE(test_task->ran); - REQUIRE(test_task->steady_delta.count() == 0); - REQUIRE(test_task->system_delta.count() == 3); + auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, + std::chrono::seconds(2), // Time travel adjustment + std::chrono::seconds(3), // Shutdown delay + std::chrono::seconds(1)); // Chrono task delay + REQUIRE(results.task_ran); + REQUIRE(results.steady_delta.count() == 0); + REQUIRE(results.system_delta.count() == 2); } SECTION("Action::JUMP with task time after jump time") { - perform_time_travel(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::seconds(3), // Time travel adjustment - std::chrono::seconds(2), // Shutdown delay - std::chrono::seconds(5)); // Chrono task delay - INFO("Task expected ran: " << 0 << ", actual ran: " << test_task->ran); - REQUIRE_FALSE(test_task->ran); + auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, + std::chrono::seconds(2), // Time travel adjustment + std::chrono::seconds(1), // Shutdown delay + std::chrono::seconds(4)); // Chrono task delay + REQUIRE_FALSE(results.task_ran); } SECTION("Action::NEAREST") { - perform_time_travel(NUClear::message::TimeTravel::Action::NEAREST, - std::chrono::seconds(5), // Time travel adjustment - std::chrono::seconds(10), // Shutdown delay - std::chrono::seconds(1)); // Chrono task delay - INFO("Task expected ran: " << 1 << ", actual ran: " << test_task->ran); - REQUIRE(test_task->ran); - REQUIRE(test_task->steady_delta.count() == 0); - REQUIRE(test_task->system_delta.count() == 1); + auto results = perform_test(NUClear::message::TimeTravel::Action::NEAREST, + std::chrono::seconds(2), // Time travel adjustment + std::chrono::seconds(3), // Shutdown delay + std::chrono::seconds(1)); // Chrono task delay + REQUIRE(results.task_ran); + REQUIRE(results.steady_delta.count() == 0); + REQUIRE(results.system_delta.count() == 1); } } From 1efec1ad6ffd93f29f280cb3c359dd0db6014c2d Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 09:06:20 +1100 Subject: [PATCH 12/45] Const --- src/clock.hpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/clock.hpp b/src/clock.hpp index 3d05d8bb2..b18b0a9f6 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -46,9 +46,9 @@ struct clock : public NUCLEAR_CLOCK_TYPE { static void adjust_clock(const duration& adjustment, const double& rtf = 1.0) { std::lock_guard lock(mutex); // Load the current state - auto& current = data[active.load()]; - int n = (active.load() + 1) % data.size(); - auto& next = data[n]; + const auto& current = data[active.load()]; + const int n = (active.load() + 1) % data.size(); + auto& next = data[n]; // Perform the update auto base = base_clock::now(); @@ -84,7 +84,7 @@ struct clock : public NUCLEAR_CLOCK_TYPE { /// Our calculated time when the clock was last updated in simulated time time_point epoch = base_from; /// The real time factor of the simulated clock - double rtf; + double rtf = 1.0; ClockData() : base_from(base_clock::now()), epoch(base_from), rtf(1.0) {} }; From 6291c5d9424cace9291ef6fb5afb68a46940d0cc Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 10:59:30 +1100 Subject: [PATCH 13/45] Update test to use milliseconds --- tests/individual/TimeTravel.cpp | 66 ++++++++++++++++----------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 334cc455b..c1cfe25af 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -8,20 +8,20 @@ namespace { // Helper struct for testing chrono tasks struct ChronoTaskTest { - NUClear::clock::time_point time = NUClear::clock::time_point(); - bool ran = false; - std::chrono::seconds steady_delta = std::chrono::seconds(0); - std::chrono::seconds system_delta = std::chrono::seconds(0); - NUClear::id_t id = 0; - ChronoTaskTest() = default; + NUClear::clock::time_point time = NUClear::clock::time_point(); + bool ran = false; + std::chrono::milliseconds steady_delta = std::chrono::milliseconds(0); + std::chrono::milliseconds system_delta = std::chrono::milliseconds(0); + NUClear::id_t id = 0; + ChronoTaskTest() = default; ChronoTaskTest(NUClear::clock::time_point time_, bool ran_, NUClear::id_t id_) : time(time_), ran(ran_), id(id_) {} }; // Struct to hold test results struct TestResults { - bool task_ran = false; - std::chrono::seconds steady_delta = std::chrono::seconds(0); - std::chrono::seconds system_delta = std::chrono::seconds(0); + bool task_ran = false; + std::chrono::milliseconds steady_delta = std::chrono::milliseconds(0); + std::chrono::milliseconds system_delta = std::chrono::milliseconds(0); }; class TestReactor : public test_util::TestBase { @@ -29,7 +29,7 @@ class TestReactor : public test_util::TestBase { std::unique_ptr test_task; NUClear::clock::duration time_travel_adjustment; NUClear::message::TimeTravel::Action time_travel_action; - std::chrono::seconds chrono_task_delay; + std::chrono::milliseconds chrono_task_delay; std::chrono::steady_clock::time_point steady_start_time; NUClear::clock::time_point system_start_time; @@ -46,10 +46,10 @@ class TestReactor : public test_util::TestBase { test_task->ran = true; auto steady_end_time = std::chrono::steady_clock::now(); test_task->steady_delta = - std::chrono::duration_cast(steady_end_time - steady_start_time); + std::chrono::duration_cast(steady_end_time - steady_start_time); auto system_end_time = NUClear::clock::now(); test_task->system_delta = - std::chrono::duration_cast(system_end_time - system_start_time); + std::chrono::duration_cast(system_end_time - system_start_time); powerplant.shutdown(); return false; }, @@ -65,8 +65,8 @@ class TestReactor : public test_util::TestBase { TestResults perform_test(NUClear::message::TimeTravel::Action action, const NUClear::clock::duration& adjustment, - const std::chrono::seconds& shutdown_delay, - const std::chrono::seconds& task_delay) { + const std::chrono::milliseconds& shutdown_delay, + const std::chrono::milliseconds& task_delay) { NUClear::Configuration config; auto plant = std::make_shared(config); auto& reactor = plant->install(); @@ -100,47 +100,47 @@ TestResults perform_test(NUClear::message::TimeTravel::Action action, TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { SECTION("Action::RELATIVE with shutdown before task time") { auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::seconds(2), // Time travel adjustment - std::chrono::seconds(1), // Shutdown delay - std::chrono::seconds(2)); // Chrono task delay + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(10), // Shutdown delay + std::chrono::milliseconds(20)); // Chrono task delay REQUIRE_FALSE(results.task_ran); } SECTION("Action::RELATIVE with shutdown after task time") { auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::seconds(2), // Time travel adjustment - std::chrono::seconds(3), // Shutdown delay - std::chrono::seconds(1)); // Chrono task delay + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(30), // Shutdown delay + std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); - REQUIRE(results.steady_delta.count() == 1); - REQUIRE(results.system_delta.count() == 3); + REQUIRE(results.steady_delta.count() == 10); + REQUIRE(results.system_delta.count() == 30); } SECTION("Action::JUMP with task time before jump time") { auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::seconds(2), // Time travel adjustment - std::chrono::seconds(3), // Shutdown delay - std::chrono::seconds(1)); // Chrono task delay + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(30), // Shutdown delay + std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); REQUIRE(results.steady_delta.count() == 0); - REQUIRE(results.system_delta.count() == 2); + REQUIRE(results.system_delta.count() == 20); } SECTION("Action::JUMP with task time after jump time") { auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::seconds(2), // Time travel adjustment - std::chrono::seconds(1), // Shutdown delay - std::chrono::seconds(4)); // Chrono task delay + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(10), // Shutdown delay + std::chrono::milliseconds(40)); // Chrono task delay REQUIRE_FALSE(results.task_ran); } SECTION("Action::NEAREST") { auto results = perform_test(NUClear::message::TimeTravel::Action::NEAREST, - std::chrono::seconds(2), // Time travel adjustment - std::chrono::seconds(3), // Shutdown delay - std::chrono::seconds(1)); // Chrono task delay + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(30), // Shutdown delay + std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); REQUIRE(results.steady_delta.count() == 0); - REQUIRE(results.system_delta.count() == 1); + REQUIRE(results.system_delta.count() == 10); } } From fab29bf4c4953a6e172488f5ac57655e76a03037 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 11:39:43 +1100 Subject: [PATCH 14/45] Add tolerance --- tests/individual/TimeTravel.cpp | 45 ++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index c1cfe25af..f7cbd4619 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -98,49 +98,52 @@ TestResults perform_test(NUClear::message::TimeTravel::Action action, } // anonymous namespace TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { + // Tolerance (milliseconds) + const int tolerance = 1; + SECTION("Action::RELATIVE with shutdown before task time") { auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(10), // Shutdown delay - std::chrono::milliseconds(20)); // Chrono task delay + std::chrono::milliseconds(20), + std::chrono::milliseconds(10), + std::chrono::milliseconds(20)); REQUIRE_FALSE(results.task_ran); } SECTION("Action::RELATIVE with shutdown after task time") { auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(30), // Shutdown delay - std::chrono::milliseconds(10)); // Chrono task delay + std::chrono::milliseconds(20), + std::chrono::milliseconds(30), + std::chrono::milliseconds(10)); REQUIRE(results.task_ran); - REQUIRE(results.steady_delta.count() == 10); - REQUIRE(results.system_delta.count() == 30); + REQUIRE(abs(results.steady_delta.count() - 10) <= tolerance); + REQUIRE(abs(results.system_delta.count() - 30) <= tolerance); } SECTION("Action::JUMP with task time before jump time") { auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(30), // Shutdown delay - std::chrono::milliseconds(10)); // Chrono task delay + std::chrono::milliseconds(20), + std::chrono::milliseconds(30), + std::chrono::milliseconds(10)); REQUIRE(results.task_ran); - REQUIRE(results.steady_delta.count() == 0); - REQUIRE(results.system_delta.count() == 20); + REQUIRE(abs(results.steady_delta.count() - 0) <= tolerance); + REQUIRE(abs(results.system_delta.count() - 20) <= tolerance); } SECTION("Action::JUMP with task time after jump time") { auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(10), // Shutdown delay - std::chrono::milliseconds(40)); // Chrono task delay + std::chrono::milliseconds(20), + std::chrono::milliseconds(10), + std::chrono::milliseconds(40)); REQUIRE_FALSE(results.task_ran); } SECTION("Action::NEAREST") { auto results = perform_test(NUClear::message::TimeTravel::Action::NEAREST, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(30), // Shutdown delay - std::chrono::milliseconds(10)); // Chrono task delay + std::chrono::milliseconds(20), + std::chrono::milliseconds(30), + std::chrono::milliseconds(10)); REQUIRE(results.task_ran); - REQUIRE(results.steady_delta.count() == 0); - REQUIRE(results.system_delta.count() == 10); + REQUIRE(abs(results.steady_delta.count() - 0) <= tolerance); + REQUIRE(abs(results.system_delta.count() - 10) <= tolerance); } } From b746d53a7fa94b1b94661fd5d6afd0f54f527aa7 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 11:45:07 +1100 Subject: [PATCH 15/45] Add comments --- tests/individual/TimeTravel.cpp | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index f7cbd4619..9dac5bf72 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -103,17 +103,17 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { SECTION("Action::RELATIVE with shutdown before task time") { auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::milliseconds(20), - std::chrono::milliseconds(10), - std::chrono::milliseconds(20)); + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(10), // Steady clock shutdown delay + std::chrono::milliseconds(20)); // Chrono task delay REQUIRE_FALSE(results.task_ran); } SECTION("Action::RELATIVE with shutdown after task time") { auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::milliseconds(20), - std::chrono::milliseconds(30), - std::chrono::milliseconds(10)); + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(30), // Steady clock shutdown delay + std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); REQUIRE(abs(results.steady_delta.count() - 10) <= tolerance); REQUIRE(abs(results.system_delta.count() - 30) <= tolerance); @@ -121,9 +121,9 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { SECTION("Action::JUMP with task time before jump time") { auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::milliseconds(20), - std::chrono::milliseconds(30), - std::chrono::milliseconds(10)); + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(30), // Steady clock shutdown delay + std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); REQUIRE(abs(results.steady_delta.count() - 0) <= tolerance); REQUIRE(abs(results.system_delta.count() - 20) <= tolerance); @@ -131,17 +131,17 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { SECTION("Action::JUMP with task time after jump time") { auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, - std::chrono::milliseconds(20), - std::chrono::milliseconds(10), - std::chrono::milliseconds(40)); + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(10), // Steady clock shutdown delay + std::chrono::milliseconds(40)); // Chrono task delay REQUIRE_FALSE(results.task_ran); } SECTION("Action::NEAREST") { auto results = perform_test(NUClear::message::TimeTravel::Action::NEAREST, - std::chrono::milliseconds(20), - std::chrono::milliseconds(30), - std::chrono::milliseconds(10)); + std::chrono::milliseconds(20), // Time travel adjustment + std::chrono::milliseconds(30), // Steady clock shutdown delay + std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); REQUIRE(abs(results.steady_delta.count() - 0) <= tolerance); REQUIRE(abs(results.system_delta.count() - 10) <= tolerance); From 80136afea12b0a991d23cc52261e9282e3e20e67 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 16:36:46 +1100 Subject: [PATCH 16/45] Clang tidy things --- src/clock.hpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/clock.hpp b/src/clock.hpp index b18b0a9f6..9fbe75b11 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -39,12 +39,12 @@ struct clock : public NUCLEAR_CLOCK_TYPE { using base_clock = NUCLEAR_CLOCK_TYPE; static time_point now() { - ClockData current = data[active.load()]; // Take a copy in case it changes + const ClockData current = data[active.load()]; // Take a copy in case it changes return current.epoch + dc((base_clock::now() - current.base_from) * current.rtf); } static void adjust_clock(const duration& adjustment, const double& rtf = 1.0) { - std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); // Load the current state const auto& current = data[active.load()]; const int n = (active.load() + 1) % data.size(); @@ -59,10 +59,10 @@ struct clock : public NUCLEAR_CLOCK_TYPE { } static void set_clock(const time_point& time, const double& rtf = 1.0) { - std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); // Load the current state - int n = (active.load() + 1) % data.size(); - auto& next = data[n]; + const int n = (active.load() + 1) % data.size(); + auto& next = data[n]; // Perform the update auto base = base_clock::now(); @@ -80,13 +80,13 @@ struct clock : public NUCLEAR_CLOCK_TYPE { struct ClockData { /// When the clock was last updated under the true time - time_point base_from; + time_point base_from = base_clock::now(); /// Our calculated time when the clock was last updated in simulated time time_point epoch = base_from; /// The real time factor of the simulated clock double rtf = 1.0; - ClockData() : base_from(base_clock::now()), epoch(base_from), rtf(1.0) {} + ClockData() = default; }; static std::mutex mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) From 6e71474d1f39da35f21ab05b15abfbc27152ce5b Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 16:45:54 +1100 Subject: [PATCH 17/45] Clang tidy fixes --- src/clock.cpp | 7 ++++--- src/clock.hpp | 4 ++-- src/message/TimeTravel.hpp | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/clock.cpp b/src/clock.cpp index 4e4bcb4dc..0f9e795fb 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -23,7 +23,8 @@ #include "clock.hpp" namespace NUClear { -std::mutex clock::mutex; -std::array clock::data = std::array{}; -std::atomic clock::active{0}; +std::mutex clock::mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +std::array clock::data = + std::array{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +std::atomic clock::active{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) } // namespace NUClear diff --git a/src/clock.hpp b/src/clock.hpp index 9fbe75b11..b25d4d6f5 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -47,7 +47,7 @@ struct clock : public NUCLEAR_CLOCK_TYPE { const std::lock_guard lock(mutex); // Load the current state const auto& current = data[active.load()]; - const int n = (active.load() + 1) % data.size(); + const int n = static_cast((active.load() + 1) % data.size()); auto& next = data[n]; // Perform the update @@ -61,7 +61,7 @@ struct clock : public NUCLEAR_CLOCK_TYPE { static void set_clock(const time_point& time, const double& rtf = 1.0) { const std::lock_guard lock(mutex); // Load the current state - const int n = (active.load() + 1) % data.size(); + const int n = static_cast((active.load() + 1) % data.size()); auto& next = data[n]; // Perform the update diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index 9158cea62..1d66af6af 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -46,7 +46,7 @@ namespace message { }; /// @brief The duration to adjust the clock by - clock::duration adjustment; + clock::duration adjustment = clock::duration(0); /// @brief The rate at which time should pass double rtf = 1.0; /// @brief The type of time travel to perform From f339b0f96107e040c90d0d62366decb54e337ad4 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 16:50:04 +1100 Subject: [PATCH 18/45] Clang tidy --- src/clock.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/clock.cpp b/src/clock.cpp index 0f9e795fb..9c580d361 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -23,8 +23,8 @@ #include "clock.hpp" namespace NUClear { -std::mutex clock::mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -std::array clock::data = - std::array{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -std::atomic clock::active{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +std::mutex clock::mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +std::array clock::data = // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + std::array{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) +std::atomic clock::active{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) } // namespace NUClear From 51011beea3ad00da04c2eb3d264dacea3bdcac24 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 3 Apr 2024 16:55:13 +1100 Subject: [PATCH 19/45] Clang tidy --- tests/individual/BaseClock.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/individual/BaseClock.cpp b/tests/individual/BaseClock.cpp index 95604568b..788cdd2c6 100644 --- a/tests/individual/BaseClock.cpp +++ b/tests/individual/BaseClock.cpp @@ -105,7 +105,7 @@ TEST_CASE("Testing base clock works correctly", "[api][base_clock]") { // Compute the differences between the time pairs int match_count = 0; for (const auto& time_pairs : times) { - using namespace std::chrono; + using namespace std::chrono; // NOLINT(google-build-using-namespace) const std::time_t ntt = system_clock::to_time_t( system_clock::time_point(duration_cast(time_pairs.first.time_since_epoch()))); const std::time_t stt = system_clock::to_time_t( From 8ef05acee463684e635d9d977396d01b898a52d0 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 09:13:39 +1000 Subject: [PATCH 20/45] abs -> std::abs --- tests/individual/TimeTravel.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 9dac5bf72..bd243a636 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -67,7 +67,7 @@ TestResults perform_test(NUClear::message::TimeTravel::Action action, const NUClear::clock::duration& adjustment, const std::chrono::milliseconds& shutdown_delay, const std::chrono::milliseconds& task_delay) { - NUClear::Configuration config; + const NUClear::Configuration config; auto plant = std::make_shared(config); auto& reactor = plant->install(); @@ -115,8 +115,8 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::milliseconds(30), // Steady clock shutdown delay std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); - REQUIRE(abs(results.steady_delta.count() - 10) <= tolerance); - REQUIRE(abs(results.system_delta.count() - 30) <= tolerance); + REQUIRE(std::abs(results.steady_delta.count() - 10) <= tolerance); + REQUIRE(std::abs(results.system_delta.count() - 30) <= tolerance); } SECTION("Action::JUMP with task time before jump time") { @@ -125,8 +125,8 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::milliseconds(30), // Steady clock shutdown delay std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); - REQUIRE(abs(results.steady_delta.count() - 0) <= tolerance); - REQUIRE(abs(results.system_delta.count() - 20) <= tolerance); + REQUIRE(std::abs(results.steady_delta.count() - 0) <= tolerance); + REQUIRE(std::abs(results.system_delta.count() - 20) <= tolerance); } SECTION("Action::JUMP with task time after jump time") { @@ -143,7 +143,7 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { std::chrono::milliseconds(30), // Steady clock shutdown delay std::chrono::milliseconds(10)); // Chrono task delay REQUIRE(results.task_ran); - REQUIRE(abs(results.steady_delta.count() - 0) <= tolerance); - REQUIRE(abs(results.system_delta.count() - 10) <= tolerance); + REQUIRE(std::abs(results.steady_delta.count() - 0) <= tolerance); + REQUIRE(std::abs(results.system_delta.count() - 10) <= tolerance); } } From 78c05723ff85967c5217b538ce1dc026962276ed Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 09:21:41 +1000 Subject: [PATCH 21/45] Init fields --- tests/individual/TimeTravel.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index bd243a636..38d5d010f 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -26,13 +26,12 @@ struct TestResults { class TestReactor : public test_util::TestBase { public: - std::unique_ptr test_task; - NUClear::clock::duration time_travel_adjustment; - NUClear::message::TimeTravel::Action time_travel_action; - std::chrono::milliseconds chrono_task_delay; - - std::chrono::steady_clock::time_point steady_start_time; - NUClear::clock::time_point system_start_time; + std::unique_ptr test_task = nullptr; + NUClear::clock::duration time_travel_adjustment = std::chrono::milliseconds(0); + NUClear::message::TimeTravel::Action time_travel_action = NUClear::message::TimeTravel::Action::RELATIVE; + std::chrono::milliseconds chrono_task_delay = std::chrono::milliseconds(0); + std::chrono::steady_clock::time_point steady_start_time = std::chrono::steady_clock::now(); + NUClear::clock::time_point system_start_time = NUClear::clock::now(); TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { From 192f533e4dc60dfb2ce37963d8a4c9f015768d3e Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 09:43:48 +1000 Subject: [PATCH 22/45] Tidy up --- src/extension/ChronoController.hpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index df3c2a24f..8eb51f4c4 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -110,15 +110,13 @@ namespace extension { clock::adjust_clock(travel.adjustment, travel.rtf); } - // Adjust clock to target time and leave chrono tasks where they are + // Adjust clock to as close to target as possible without skipping any chrono tasks if (travel.type == message::TimeTravel::Action::NEAREST) { - // Find the task in the list that is closest to be run - auto it = + auto next_task = std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { return a.time < b.time; }); - // Set the clock to the time closest of the nearest task and the target time - clock::adjust_clock(std::min(it->time - clock::now(), travel.adjustment), travel.rtf); + clock::adjust_clock(std::min(next_task->time - clock::now(), travel.adjustment), travel.rtf); } // Adjust clock and move all chrono tasks with it From 2f62c1760f24dbebe511237a706683bd590b3e76 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 09:46:01 +1000 Subject: [PATCH 23/45] Move implementation to Cpp and add comments --- src/clock.cpp | 36 +++++++++++++++++++++++++ src/clock.hpp | 73 +++++++++++++++++++++++++++------------------------ 2 files changed, 74 insertions(+), 35 deletions(-) diff --git a/src/clock.cpp b/src/clock.cpp index 9c580d361..90b5a7bd5 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -23,8 +23,44 @@ #include "clock.hpp" namespace NUClear { + std::mutex clock::mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) std::array clock::data = // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) std::array{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) std::atomic clock::active{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + +clock::time_point clock::now() { + const ClockData current = data[active.load()]; // Take a copy in case it changes + return current.epoch + dc((base_clock::now() - current.base_from) * current.rtf); +} + +void clock::adjust_clock(const duration& adjustment, const double& rtf) { + const std::lock_guard lock(mutex); + // Load the current state + const auto& current = data[active.load()]; + const int n = static_cast((active.load() + 1) % data.size()); + auto& next = data[n]; + + // Perform the update + auto base = base_clock::now(); + next.epoch = current.epoch + adjustment + dc((base - current.base_from) * current.rtf); + next.base_from = base; + next.rtf = rtf; + active = n; +} + +void clock::set_clock(const time_point& time, const double& rtf) { + const std::lock_guard lock(mutex); + // Load the current state + const int n = static_cast((active.load() + 1) % data.size()); + auto& next = data[n]; + + // Perform the update + auto base = base_clock::now(); + next.epoch = time; + next.base_from = base; + next.rtf = rtf; + active = n; +} + } // namespace NUClear diff --git a/src/clock.hpp b/src/clock.hpp index b25d4d6f5..414e8a3db 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -35,49 +35,47 @@ namespace NUClear { +/** + * @brief A clock class that extends a base clock type and allows for clock adjustment and setting. + */ struct clock : public NUCLEAR_CLOCK_TYPE { using base_clock = NUCLEAR_CLOCK_TYPE; - static time_point now() { - const ClockData current = data[active.load()]; // Take a copy in case it changes - return current.epoch + dc((base_clock::now() - current.base_from) * current.rtf); - } - - static void adjust_clock(const duration& adjustment, const double& rtf = 1.0) { - const std::lock_guard lock(mutex); - // Load the current state - const auto& current = data[active.load()]; - const int n = static_cast((active.load() + 1) % data.size()); - auto& next = data[n]; - - // Perform the update - auto base = base_clock::now(); - next.epoch = current.epoch + adjustment + dc((base - current.base_from) * current.rtf); - next.base_from = base; - next.rtf = rtf; - active = n; - } - - static void set_clock(const time_point& time, const double& rtf = 1.0) { - const std::lock_guard lock(mutex); - // Load the current state - const int n = static_cast((active.load() + 1) % data.size()); - auto& next = data[n]; - - // Perform the update - auto base = base_clock::now(); - next.epoch = time; - next.base_from = base; - next.rtf = rtf; - active = n; - } + /** + * @brief Get the current time of the clock. + * @return The current time of the clock. + */ + static time_point now(); + + /** + * @brief Adjust the clock by a specified duration and real-time factor. + * @param adjustment The duration by which to adjust the clock. + * @param rtf The real-time factor to apply to the clock. + */ + static void adjust_clock(const duration& adjustment, const double& rtf = 1.0); + + /** + * @brief Set the clock to a specified time and real-time factor. + * @param time The time to set the clock to. + * @param rtf The real-time factor to apply to the clock. + */ + static void set_clock(const time_point& time, const double& rtf = 1.0); private: + /** + * @brief Convert a duration to the clock's duration type. + * @tparam T The type of the duration. + * @param t The duration to convert. + * @return The converted duration. + */ template duration static dc(const T& t) { return std::chrono::duration_cast(t); } + /** + * @brief Data structure to hold clock information. + */ struct ClockData { /// When the clock was last updated under the true time time_point base_from = base_clock::now(); @@ -89,9 +87,14 @@ struct clock : public NUCLEAR_CLOCK_TYPE { ClockData() = default; }; - static std::mutex mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + /// @brief The mutex to protect the clock data. + static std::mutex mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + + /// @brief The clock data for the system. static std::array data; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) - static std::atomic active; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + + /// @brief The active clock data index. + static std::atomic active; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) }; } // namespace NUClear From 70a15f7da34a9b7a0e0823f8dc4b1de03f9c11ff Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 09:52:06 +1000 Subject: [PATCH 24/45] Rename JUMP to ABSOLUTE --- src/extension/ChronoController.hpp | 2 +- src/message/TimeTravel.hpp | 2 +- tests/individual/TimeTravel.cpp | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index 8eb51f4c4..a95ca7ee6 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -106,7 +106,7 @@ namespace extension { const std::lock_guard lock(mutex); // Adjust clock to target time and leave chrono tasks where they are - if (travel.type == message::TimeTravel::Action::JUMP) { + if (travel.type == message::TimeTravel::Action::ABSOLUTE) { clock::adjust_clock(travel.adjustment, travel.rtf); } diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index 1d66af6af..b0c568d9e 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -39,7 +39,7 @@ namespace message { RELATIVE, /// @brief Adjust clock to target time and leave chrono tasks where they are - JUMP, + ABSOLUTE, /// @brief Adjust clock to as close to target as possible without skipping any chrono tasks NEAREST, diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 38d5d010f..9242c48f2 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -118,8 +118,8 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { REQUIRE(std::abs(results.system_delta.count() - 30) <= tolerance); } - SECTION("Action::JUMP with task time before jump time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, + SECTION("Action::ABSOLUTE with task time before adjustment time") { + auto results = perform_test(NUClear::message::TimeTravel::Action::ABSOLUTE, std::chrono::milliseconds(20), // Time travel adjustment std::chrono::milliseconds(30), // Steady clock shutdown delay std::chrono::milliseconds(10)); // Chrono task delay @@ -128,15 +128,15 @@ TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { REQUIRE(std::abs(results.system_delta.count() - 20) <= tolerance); } - SECTION("Action::JUMP with task time after jump time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::JUMP, + SECTION("Action::ABSOLUTE with task time after adjustment time") { + auto results = perform_test(NUClear::message::TimeTravel::Action::ABSOLUTE, std::chrono::milliseconds(20), // Time travel adjustment std::chrono::milliseconds(10), // Steady clock shutdown delay std::chrono::milliseconds(40)); // Chrono task delay REQUIRE_FALSE(results.task_ran); } - SECTION("Action::NEAREST") { + SECTION("Action::NEAREST with task time before adjustment time") { auto results = perform_test(NUClear::message::TimeTravel::Action::NEAREST, std::chrono::milliseconds(20), // Time travel adjustment std::chrono::milliseconds(30), // Steady clock shutdown delay From 96827e51bc7ce1a58ae6b10a486d7590d3701efc Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 12:32:49 +1000 Subject: [PATCH 25/45] Clean up test --- tests/individual/TimeTravel.cpp | 190 ++++++++++++++------------------ 1 file changed, 82 insertions(+), 108 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 9242c48f2..ae75031e4 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -6,143 +6,117 @@ namespace { -// Helper struct for testing chrono tasks -struct ChronoTaskTest { - NUClear::clock::time_point time = NUClear::clock::time_point(); - bool ran = false; - std::chrono::milliseconds steady_delta = std::chrono::milliseconds(0); - std::chrono::milliseconds system_delta = std::chrono::milliseconds(0); - NUClear::id_t id = 0; - ChronoTaskTest() = default; - ChronoTaskTest(NUClear::clock::time_point time_, bool ran_, NUClear::id_t id_) : time(time_), ran(ran_), id(id_) {} -}; - -// Struct to hold test results -struct TestResults { - bool task_ran = false; - std::chrono::milliseconds steady_delta = std::chrono::milliseconds(0); - std::chrono::milliseconds system_delta = std::chrono::milliseconds(0); -}; +constexpr NUClear::clock::duration time_unit = std::chrono::milliseconds(10); class TestReactor : public test_util::TestBase { public: - std::unique_ptr test_task = nullptr; - NUClear::clock::duration time_travel_adjustment = std::chrono::milliseconds(0); - NUClear::message::TimeTravel::Action time_travel_action = NUClear::message::TimeTravel::Action::RELATIVE; - std::chrono::milliseconds chrono_task_delay = std::chrono::milliseconds(0); + // Start time of steady clock std::chrono::steady_clock::time_point steady_start_time = std::chrono::steady_clock::now(); - NUClear::clock::time_point system_start_time = NUClear::clock::now(); + + // Time travel action + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; + + // Time adjustment + NUClear::clock::duration adjustment = std::chrono::milliseconds(0); + + // Real-time factor + double rtf = 1.0; + + // Expected results = {steady_delta_task_1, steady_delta_task_2} + std::array results = {std::chrono::milliseconds(0), std::chrono::milliseconds(0)}; TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { steady_start_time = std::chrono::steady_clock::now(); - system_start_time = NUClear::clock::now(); - test_task = std::make_unique(NUClear::clock::now() + chrono_task_delay, false, 1); - auto chrono_task = std::make_unique( + // Emit a chrono task to run at time 2 + auto chrono_task_1 = std::make_unique( [this](NUClear::clock::time_point&) { - test_task->ran = true; - auto steady_end_time = std::chrono::steady_clock::now(); - test_task->steady_delta = - std::chrono::duration_cast(steady_end_time - steady_start_time); - auto system_end_time = NUClear::clock::now(); - test_task->system_delta = - std::chrono::duration_cast(system_end_time - system_start_time); + results[0] = std::chrono::duration_cast(std::chrono::steady_clock::now() + - steady_start_time); + return false; + }, + NUClear::clock::now() + 2 * time_unit, + 1); + emit(std::move(chrono_task_1)); + + // Emit a chrono task to run at time 4, and shutdown + auto chrono_task_2 = std::make_unique( + [this](NUClear::clock::time_point&) { + results[1] = std::chrono::duration_cast(std::chrono::steady_clock::now() + - steady_start_time); powerplant.shutdown(); return false; }, - test_task->time, - test_task->id); - emit(std::move(chrono_task)); + NUClear::clock::now() + 4 * time_unit, + 1); + emit(std::move(chrono_task_2)); - emit( - std::make_unique(time_travel_adjustment, 1.0, time_travel_action)); + // Time travel! + emit(std::make_unique(adjustment, rtf, action)); }); } }; -TestResults perform_test(NUClear::message::TimeTravel::Action action, - const NUClear::clock::duration& adjustment, - const std::chrono::milliseconds& shutdown_delay, - const std::chrono::milliseconds& task_delay) { - const NUClear::Configuration config; - auto plant = std::make_shared(config); - auto& reactor = plant->install(); +} // anonymous namespace - reactor.time_travel_action = action; - reactor.time_travel_adjustment = adjustment; - reactor.chrono_task_delay = task_delay; - NUClear::clock::set_clock(NUClear::clock::time_point()); +// TEST_CASE("Test time travel correctly changes the time for zero rtf", "[time_travel][chrono_controller]") { - std::thread shutdown_thread([shutdown_delay, plant]() { - std::this_thread::sleep_for(shutdown_delay); - plant->shutdown(); - }); +// } - plant->start(); - shutdown_thread.join(); - - // After shutdown, collect results from the reactor - TestResults results; - if (reactor.test_task) { - results.task_ran = reactor.test_task->ran; - results.steady_delta = reactor.test_task->steady_delta; - results.system_delta = reactor.test_task->system_delta; - } - return results; -} +TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time_travel][chrono_controller]") { -} // anonymous namespace + using Action = NUClear::message::TimeTravel::Action; -TEST_CASE("TimeTravel Actions Test", "[time_travel][chrono_controller]") { - // Tolerance (milliseconds) - const int tolerance = 1; + const NUClear::Configuration config; + auto plant = std::make_shared(config); + auto& reactor = plant->install(); - SECTION("Action::RELATIVE with shutdown before task time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(10), // Steady clock shutdown delay - std::chrono::milliseconds(20)); // Chrono task delay - REQUIRE_FALSE(results.task_ran); - } + // Set the reactor fields to the values we want to test + auto action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + auto adjustment = GENERATE(-2, -1, 0, 1, 2, 3, 4, 5); + auto rtf = GENERATE(0.5, 1.0, 2.0); + reactor.action = action; + reactor.adjustment = adjustment * time_unit; + reactor.rtf = rtf; - SECTION("Action::RELATIVE with shutdown after task time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::RELATIVE, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(30), // Steady clock shutdown delay - std::chrono::milliseconds(10)); // Chrono task delay - REQUIRE(results.task_ran); - REQUIRE(std::abs(results.steady_delta.count() - 10) <= tolerance); - REQUIRE(std::abs(results.system_delta.count() - 30) <= tolerance); - } + // Reset clock to zero + NUClear::clock::set_clock(NUClear::clock::time_point()); - SECTION("Action::ABSOLUTE with task time before adjustment time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::ABSOLUTE, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(30), // Steady clock shutdown delay - std::chrono::milliseconds(10)); // Chrono task delay - REQUIRE(results.task_ran); - REQUIRE(std::abs(results.steady_delta.count() - 0) <= tolerance); - REQUIRE(std::abs(results.system_delta.count() - 20) <= tolerance); - } + // Start the powerplant + plant->start(); - SECTION("Action::ABSOLUTE with task time after adjustment time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::ABSOLUTE, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(10), // Steady clock shutdown delay - std::chrono::milliseconds(40)); // Chrono task delay - REQUIRE_FALSE(results.task_ran); + // Expected results = {steady_delta_task_1, steady_delta_task_2} + std::array expected_results; + switch (action) { + case Action::RELATIVE: + expected_results = {std::chrono::duration_cast((2 * time_unit) / rtf), + std::chrono::duration_cast((4 * time_unit) / rtf)}; + break; + case Action::ABSOLUTE: + expected_results = { + std::chrono::duration_cast((std::max(0, 2 - adjustment) * time_unit) / rtf), + std::chrono::duration_cast((std::max(0, 4 - adjustment) * time_unit) / rtf)}; + break; + case Action::NEAREST: + if (adjustment < 2) { + expected_results = { + std::chrono::duration_cast((2 - adjustment) * time_unit / rtf), + std::chrono::duration_cast((4 - adjustment) * time_unit / rtf)}; + } + else if (adjustment >= 2) { + expected_results = {std::chrono::milliseconds(0), + std::chrono::duration_cast((2 * time_unit) / rtf)}; + } + break; } - SECTION("Action::NEAREST with task time before adjustment time") { - auto results = perform_test(NUClear::message::TimeTravel::Action::NEAREST, - std::chrono::milliseconds(20), // Time travel adjustment - std::chrono::milliseconds(30), // Steady clock shutdown delay - std::chrono::milliseconds(10)); // Chrono task delay - REQUIRE(results.task_ran); - REQUIRE(std::abs(results.steady_delta.count() - 0) <= tolerance); - REQUIRE(std::abs(results.system_delta.count() - 10) <= tolerance); - } + // Check the results + INFO("action: " << static_cast(action) << ", adjustment: " << adjustment << ", rtf: " << rtf); + INFO("results: " << reactor.results[0].count() << ", " << reactor.results[1].count()); + INFO("expected: " << expected_results[0].count() << ", " << expected_results[1].count()); + REQUIRE(std::abs(reactor.results[0].count() - expected_results[0].count()) < time_unit.count()); + REQUIRE(std::abs(reactor.results[1].count() - expected_results[1].count()) < time_unit.count()); } From 118db7024fde0d0a08ee7602381fd8f63c40f9a8 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 12:34:51 +1000 Subject: [PATCH 26/45] Consider RTF in Chrono sleep --- src/clock.cpp | 4 ++++ src/clock.hpp | 6 ++++++ src/extension/ChronoController.hpp | 11 +++++++---- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/clock.cpp b/src/clock.cpp index 90b5a7bd5..1a7fdab48 100644 --- a/src/clock.cpp +++ b/src/clock.cpp @@ -63,4 +63,8 @@ void clock::set_clock(const time_point& time, const double& rtf) { active = n; } +double clock::rtf() { + return data[active.load()].rtf; +} + } // namespace NUClear diff --git a/src/clock.hpp b/src/clock.hpp index 414e8a3db..3dc3b514b 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -61,6 +61,12 @@ struct clock : public NUCLEAR_CLOCK_TYPE { */ static void set_clock(const time_point& time, const double& rtf = 1.0); + /** + * @brief Get the real-time factor of the clock. + * @return The real-time factor of the clock. + */ + static double rtf(); + private: /** * @brief Convert a duration to the clock's duration type. diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index a95ca7ee6..3254c556d 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -146,10 +146,12 @@ namespace extension { else { auto start = NUClear::clock::now(); auto target = tasks.front().time; + NUClear::clock::duration time_until_task = + std::chrono::duration_cast((target - start) / clock::rtf()); - if (target - start > cv_accuracy) { + if (time_until_task > cv_accuracy) { // A long time in the future // Wait on the cv - wait.wait_until(lock, target - cv_accuracy); + wait.wait_for(lock, time_until_task - cv_accuracy); // Update the accuracy of our cv wait const auto end = NUClear::clock::now(); @@ -158,9 +160,10 @@ namespace extension { cv_accuracy = error > cv_accuracy ? error : ((cv_accuracy * 99 + error) / 100); } } - else if (target - start > ns_accuracy) { + else if (time_until_task > ns_accuracy) { // Somewhat close in time // Wait on nanosleep - util::precise_sleep(target - start - ns_accuracy); + NUClear::clock::duration sleep_time = time_until_task - ns_accuracy; + util::precise_sleep(sleep_time); // Update the accuracy of our precise sleep const auto end = NUClear::clock::now(); From 64158d7a8f510514f66c4a7b3eec51c49a9adaa2 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 15:24:38 +1000 Subject: [PATCH 27/45] Clean up test --- tests/individual/TimeTravel.cpp | 122 +++++++++++++++++++------------- 1 file changed, 73 insertions(+), 49 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index ae75031e4..67aface45 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -6,7 +6,20 @@ namespace { -constexpr NUClear::clock::duration time_unit = std::chrono::milliseconds(10); +using TestUnits = std::chrono::duration>; +static constexpr int64_t EVENT_1_TIME = 4; +static constexpr int64_t EVENT_2_TIME = 8; + +struct Results { + struct TimePair { + NUClear::clock::time_point nuclear; + std::chrono::steady_clock::time_point steady; + }; + + TimePair start; + TimePair zero; + std::array events; +}; class TestReactor : public test_util::TestBase { public: @@ -22,39 +35,37 @@ class TestReactor : public test_util::TestBase { // Real-time factor double rtf = 1.0; - // Expected results = {steady_delta_task_1, steady_delta_task_2} - std::array results = {std::chrono::milliseconds(0), std::chrono::milliseconds(0)}; + // Results struct + Results results; TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { - steady_start_time = std::chrono::steady_clock::now(); + results.zero = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; - // Emit a chrono task to run at time 2 - auto chrono_task_1 = std::make_unique( + // Emit a chrono task to run at time EVENT_1_TIME + emit(std::make_unique( [this](NUClear::clock::time_point&) { - results[0] = std::chrono::duration_cast(std::chrono::steady_clock::now() - - steady_start_time); + results.events[0] = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; return false; }, - NUClear::clock::now() + 2 * time_unit, - 1); - emit(std::move(chrono_task_1)); + NUClear::clock::now() + TestUnits(EVENT_1_TIME), + 1)); - // Emit a chrono task to run at time 4, and shutdown - auto chrono_task_2 = std::make_unique( + // Emit a chrono task to run at time EVENT_2_TIME, and shutdown + emit(std::make_unique( [this](NUClear::clock::time_point&) { - results[1] = std::chrono::duration_cast(std::chrono::steady_clock::now() - - steady_start_time); + results.events[1] = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; powerplant.shutdown(); return false; }, - NUClear::clock::now() + 4 * time_unit, - 1); - emit(std::move(chrono_task_2)); + NUClear::clock::now() + TestUnits(EVENT_2_TIME), + 2)); // Time travel! emit(std::make_unique(adjustment, rtf, action)); + + results.start = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; }); } }; @@ -62,9 +73,7 @@ class TestReactor : public test_util::TestBase { } // anonymous namespace -// TEST_CASE("Test time travel correctly changes the time for zero rtf", "[time_travel][chrono_controller]") { - -// } +TEST_CASE("Test time travel correctly changes the time for zero rtf", "[time_travel][chrono_controller]") {} TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time_travel][chrono_controller]") { @@ -75,11 +84,12 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - auto action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); - auto adjustment = GENERATE(-2, -1, 0, 1, 2, 3, 4, 5); - auto rtf = GENERATE(0.5, 1.0, 2.0); + Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); + double rtf = GENERATE(0.5, 1.0, 2.0); + CAPTURE(action, adjustment, rtf); reactor.action = action; - reactor.adjustment = adjustment * time_unit; + reactor.adjustment = TestUnits(adjustment); reactor.rtf = rtf; // Reset clock to zero @@ -88,35 +98,49 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time // Start the powerplant plant->start(); - // Expected results = {steady_delta_task_1, steady_delta_task_2} - std::array expected_results; + // Expected results + std::array expected; switch (action) { - case Action::RELATIVE: - expected_results = {std::chrono::duration_cast((2 * time_unit) / rtf), - std::chrono::duration_cast((4 * time_unit) / rtf)}; - break; + case Action::RELATIVE: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; case Action::ABSOLUTE: - expected_results = { - std::chrono::duration_cast((std::max(0, 2 - adjustment) * time_unit) / rtf), - std::chrono::duration_cast((std::max(0, 4 - adjustment) * time_unit) / rtf)}; + expected = {std::max(0l, EVENT_1_TIME - adjustment), std::max(0l, EVENT_2_TIME - adjustment)}; break; case Action::NEAREST: - if (adjustment < 2) { - expected_results = { - std::chrono::duration_cast((2 - adjustment) * time_unit / rtf), - std::chrono::duration_cast((4 - adjustment) * time_unit / rtf)}; - } - else if (adjustment >= 2) { - expected_results = {std::chrono::milliseconds(0), - std::chrono::duration_cast((2 * time_unit) / rtf)}; - } + expected = adjustment < EVENT_1_TIME + ? std::array{EVENT_1_TIME - adjustment, EVENT_2_TIME - adjustment} + : std::array{0, EVENT_2_TIME - EVENT_1_TIME}; break; + default: throw std::runtime_error("Unknown action"); } - // Check the results - INFO("action: " << static_cast(action) << ", adjustment: " << adjustment << ", rtf: " << rtf); - INFO("results: " << reactor.results[0].count() << ", " << reactor.results[1].count()); - INFO("expected: " << expected_results[0].count() << ", " << expected_results[1].count()); - REQUIRE(std::abs(reactor.results[0].count() - expected_results[0].count()) < time_unit.count()); - REQUIRE(std::abs(reactor.results[1].count() - expected_results[1].count()) < time_unit.count()); + std::array expected_nuclear = {TestUnits(expected[0]), TestUnits(expected[1])}; + std::array expected_steady = {TestUnits(int64_t(expected[0] / rtf)), + TestUnits(int64_t(expected[1] / rtf))}; + + const auto& r = reactor.results; + const auto& n_start = reactor.results.start.nuclear; + const auto& s_start = reactor.results.start.steady; + + auto round_to_test_units = [](const auto& duration) { + double d = std::chrono::duration_cast>(duration).count(); + double t = (TestUnits::period::den * d) / TestUnits::period::num; + return TestUnits(std::lround(t)); + }; + + std::array actual_nuclear = { + round_to_test_units(r.events[0].nuclear - n_start), + round_to_test_units(r.events[1].nuclear - n_start), + }; + std::array actual_steady = { + round_to_test_units(r.events[0].steady - s_start), + round_to_test_units(r.events[1].steady - s_start), + }; + + TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); + TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); + CHECK(expected_nuclear[0] == actual_nuclear[0]); + CHECK(expected_nuclear[1] == actual_nuclear[1]); + CHECK(expected_steady[0] == actual_steady[0]); + CHECK(expected_steady[1] == actual_steady[1]); + CHECK(expected_adjustment == actual_adjustment); } From 27940ee5649c4d2cf74c39a31ce6428e8ed4deb5 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 16:40:02 +1000 Subject: [PATCH 28/45] Account for rtf = 0.0 in chrono controller --- src/extension/ChronoController.hpp | 114 +++++++++++++++-------------- 1 file changed, 61 insertions(+), 53 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index 3254c556d..11d2b2c5b 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -102,34 +102,32 @@ namespace extension { }); on>().then("Time Travel", [this](const message::TimeTravel& travel) { - { - const std::lock_guard lock(mutex); + const std::lock_guard lock(mutex); - // Adjust clock to target time and leave chrono tasks where they are - if (travel.type == message::TimeTravel::Action::ABSOLUTE) { - clock::adjust_clock(travel.adjustment, travel.rtf); - } + // Adjust clock to target time and leave chrono tasks where they are + if (travel.type == message::TimeTravel::Action::ABSOLUTE) { + clock::adjust_clock(travel.adjustment, travel.rtf); + } - // Adjust clock to as close to target as possible without skipping any chrono tasks - if (travel.type == message::TimeTravel::Action::NEAREST) { - auto next_task = - std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { - return a.time < b.time; - }); - clock::adjust_clock(std::min(next_task->time - clock::now(), travel.adjustment), travel.rtf); - } + // Adjust clock to as close to target as possible without skipping any chrono tasks + if (travel.type == message::TimeTravel::Action::NEAREST) { + auto next_task = + std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { + return a.time < b.time; + }); + clock::adjust_clock(std::min(next_task->time - clock::now(), travel.adjustment), travel.rtf); + } - // Adjust clock and move all chrono tasks with it - if (travel.type == message::TimeTravel::Action::RELATIVE) { - clock::adjust_clock(travel.adjustment, travel.rtf); - for (auto& task : tasks) { - task.time += travel.adjustment; - } + // Adjust clock and move all chrono tasks with it + if (travel.type == message::TimeTravel::Action::RELATIVE) { + clock::adjust_clock(travel.adjustment, travel.rtf); + for (auto& task : tasks) { + task.time += travel.adjustment; } - - // Poke the system - wait.notify_all(); } + + // Poke the system + wait.notify_all(); }); on().then("Chrono Controller", [this] { @@ -146,37 +144,8 @@ namespace extension { else { auto start = NUClear::clock::now(); auto target = tasks.front().time; - NUClear::clock::duration time_until_task = - std::chrono::duration_cast((target - start) / clock::rtf()); - - if (time_until_task > cv_accuracy) { // A long time in the future - // Wait on the cv - wait.wait_for(lock, time_until_task - cv_accuracy); - - // Update the accuracy of our cv wait - const auto end = NUClear::clock::now(); - const auto error = end - (target - cv_accuracy); // when ended - when wanted to end - if (error.count() > 0) { // only if we were late - cv_accuracy = error > cv_accuracy ? error : ((cv_accuracy * 99 + error) / 100); - } - } - else if (time_until_task > ns_accuracy) { // Somewhat close in time - // Wait on nanosleep - NUClear::clock::duration sleep_time = time_until_task - ns_accuracy; - util::precise_sleep(sleep_time); - - // Update the accuracy of our precise sleep - const auto end = NUClear::clock::now(); - const auto error = end - (target - ns_accuracy); // when ended - when wanted to end - if (error.count() > 0) { // only if we were late - ns_accuracy = error > ns_accuracy ? error : ((ns_accuracy * 99 + error) / 100); - } - } - else { - while (NUClear::clock::now() < tasks.front().time) { - // Spinlock until we get to the time - } + if (target <= start) { // Run our task and if it returns false remove it const bool renew = tasks.front()(); @@ -192,6 +161,45 @@ namespace extension { tasks.pop_back(); } } + else { + NUClear::clock::duration time_until_task = + std::chrono::duration_cast((target - start) / clock::rtf()); + + if (clock::rtf() == 0.0) { + // If we are paused then just wait until we are unpaused + wait.wait(lock, [&] { + return !running.load() || clock::rtf() != 0.0 || NUClear::clock::now() != start; + }); + } + else if (time_until_task > cv_accuracy) { // A long time in the future + // Wait on the cv + wait.wait_for(lock, time_until_task - cv_accuracy); + + // Update the accuracy of our cv wait + const auto end = NUClear::clock::now(); + const auto error = end - (target - cv_accuracy); // when ended - when wanted to end + if (error.count() > 0) { // only if we were late + cv_accuracy = error > cv_accuracy ? error : ((cv_accuracy * 99 + error) / 100); + } + } + else if (time_until_task > ns_accuracy) { // Somewhat close in time + // Wait on nanosleep + NUClear::clock::duration sleep_time = time_until_task - ns_accuracy; + util::precise_sleep(sleep_time); + + // Update the accuracy of our precise sleep + const auto end = NUClear::clock::now(); + const auto error = end - (target - ns_accuracy); // when ended - when wanted to end + if (error.count() > 0) { // only if we were late + ns_accuracy = error > ns_accuracy ? error : ((ns_accuracy * 99 + error) / 100); + } + } + else { + while (NUClear::clock::now() < tasks.front().time) { + // Spinlock until we get to the time + } + } + } } } }); From d86a6af6182a6f91534057a6682e3b654e40a949 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 16:42:09 +1000 Subject: [PATCH 29/45] Add test for TimeTravel when rtf = 0.0 --- tests/individual/TimeTravel.cpp | 3 - tests/individual/TimeTravelFrozen.cpp | 117 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 tests/individual/TimeTravelFrozen.cpp diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 67aface45..7088cc84d 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -72,9 +72,6 @@ class TestReactor : public test_util::TestBase { } // anonymous namespace - -TEST_CASE("Test time travel correctly changes the time for zero rtf", "[time_travel][chrono_controller]") {} - TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time_travel][chrono_controller]") { using Action = NUClear::message::TimeTravel::Action; diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp new file mode 100644 index 000000000..7e5e52f95 --- /dev/null +++ b/tests/individual/TimeTravelFrozen.cpp @@ -0,0 +1,117 @@ +#define CATCH_CONFIG_MAIN +#include +#include +#include + +#include "test_util/TestBase.hpp" + +namespace { + +static constexpr std::chrono::milliseconds EVENT_1_TIME = std::chrono::milliseconds(4); +static constexpr std::chrono::milliseconds EVENT_2_TIME = std::chrono::milliseconds(8); + +struct WaitForShutdown {}; + +class TestReactor : public test_util::TestBase { +public: + // Start time of steady clock + std::chrono::steady_clock::time_point steady_start_time = std::chrono::steady_clock::now(); + + // Time travel action + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; + + // Time adjustment + NUClear::clock::duration adjustment = std::chrono::milliseconds(0); + + // Real-time factor + double rtf = 1.0; + + // Events + std::vector events; + + TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { + + on().then([this] { + // Emit a chrono task to run at time EVENT_1_TIME + emit(std::make_unique( + [this](NUClear::clock::time_point&) { + events.push_back("Event 1"); + return false; + }, + NUClear::clock::now() + EVENT_1_TIME, + 1)); + + // Emit a chrono task to run at time EVENT_2_TIME + emit(std::make_unique( + [this](NUClear::clock::time_point&) { + events.push_back("Event 2"); + return false; + }, + NUClear::clock::now() + EVENT_2_TIME, + 2)); + + // Time travel + emit(std::make_unique(adjustment, rtf, action)); + + // Shutdown after steady clock amount of time + emit(std::make_unique()); + }); + + on>().then([this] { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + events.push_back("Finished"); + powerplant.shutdown(); + }); + } +}; + +} // anonymous namespace + +TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time_travel][chrono_controller]") { + + using Action = NUClear::message::TimeTravel::Action; + + const NUClear::Configuration config; + auto plant = std::make_shared(config); + auto& reactor = plant->install(); + + // Set the reactor fields to the values we want to test + Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); + CAPTURE(action, adjustment); + reactor.action = action; + reactor.adjustment = std::chrono::milliseconds(adjustment); + reactor.rtf = 0.0; + + // Reset clock to zero + NUClear::clock::set_clock(NUClear::clock::time_point()); + + // Start the powerplant + plant->start(); + + // Expected results + std::vector expected_events; + switch (action) { + case Action::RELATIVE: expected_events = {"Finished"}; break; + case Action::ABSOLUTE: + if (std::chrono::milliseconds(adjustment) < EVENT_1_TIME) { + expected_events = {"Finished"}; + } + else if (std::chrono::milliseconds(adjustment) < EVENT_2_TIME) { + expected_events = {"Event 1", "Finished"}; + } + else { + expected_events = {"Event 1", "Event 2", "Finished"}; + } + break; + case Action::NEAREST: + expected_events = std::chrono::milliseconds(adjustment) < EVENT_1_TIME + ? std::vector{"Finished"} + : std::vector{"Event 1", "Finished"}; + break; + default: throw std::runtime_error("Unknown action"); + } + + const auto& actual_events = reactor.events; + CHECK(expected_events == actual_events); +} From c66361abb0f0974ff053fbfd44c48587c17c31a5 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 16:42:36 +1000 Subject: [PATCH 30/45] Init events field --- tests/individual/TimeTravelFrozen.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index 7e5e52f95..71809bfdf 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -27,7 +27,7 @@ class TestReactor : public test_util::TestBase { double rtf = 1.0; // Events - std::vector events; + std::vector events = {}; TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { From e92af34939169dcea0545a496a87a1ac5e05c411 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 17:13:56 +1000 Subject: [PATCH 31/45] Delete CustomClock.cpp --- tests/individual/CustomClock.cpp | 83 -------------------------------- 1 file changed, 83 deletions(-) delete mode 100644 tests/individual/CustomClock.cpp diff --git a/tests/individual/CustomClock.cpp b/tests/individual/CustomClock.cpp deleted file mode 100644 index 3eda565b2..000000000 --- a/tests/individual/CustomClock.cpp +++ /dev/null @@ -1,83 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2018 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. - */ - -#define CATCH_CONFIG_MAIN -#include -#include - -#include "test_util/TestBase.hpp" - -// Anonymous namespace to keep everything file local -namespace { - -template -struct Message {}; - -// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) -std::vector> times; - -class TestReactor : public test_util::TestBase { -public: - TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { - - // Collect steady clock times as well as NUClear clock times - on>().then([] { // - times.emplace_back(std::chrono::steady_clock::now(), NUClear::clock::now()); - }); - - // Collect until the watchdog times out - on>().then([this] { // - powerplant.shutdown(); - }); - - on().then([this] { - // Adjust the clock to run at half speed - emit(std::make_unique(NUClear::clock::duration(0), 0.5)); - }); - } -}; -} // namespace - -TEST_CASE("Testing custom clock works correctly", "[api][custom_clock]") { - - NUClear::Configuration config; - config.thread_count = 1; - NUClear::PowerPlant plant(config); - plant.install(); - plant.start(); - - // Calculate the average ratio delta time for steady and custom clocks - double steady_total = 0; - double custom_total = 0; - - for (int i = 0; i + 1 < int(times.size()); ++i) { - using namespace std::chrono; // NOLINT(google-build-using-namespace) fine in function scope - steady_total += duration_cast>(times[i + 1].first - times[i].first).count(); - custom_total += duration_cast>(times[i + 1].second - times[i].second).count(); - } - - // The ratio should be about 0.5 - REQUIRE((custom_total / steady_total) == Approx(0.5)); - - // The amount of time that passed should be (n - 1) * 2 * 10ms - REQUIRE(steady_total == Approx(2.0 * (times.size() - 1) * 1e-2).margin(1e-3)); -} From 09d71e736fa8a6303d052b99837460a70fbc4997 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 17:16:30 +1000 Subject: [PATCH 32/45] Change time travel from adjustment to target --- src/clock.cpp | 70 --------------------------- src/clock.hpp | 56 +++++++++++++++++++-- src/extension/ChronoController.hpp | 34 ++++++------- src/message/TimeTravel.hpp | 8 ++- tests/individual/TimeTravel.cpp | 3 +- tests/individual/TimeTravelFrozen.cpp | 5 +- 6 files changed, 75 insertions(+), 101 deletions(-) delete mode 100644 src/clock.cpp diff --git a/src/clock.cpp b/src/clock.cpp deleted file mode 100644 index 1a7fdab48..000000000 --- a/src/clock.cpp +++ /dev/null @@ -1,70 +0,0 @@ -/* - * MIT License - * - * Copyright (c) 2013 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 "clock.hpp" - -namespace NUClear { - -std::mutex clock::mutex; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -std::array clock::data = // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) - std::array{}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) -std::atomic clock::active{0}; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) - -clock::time_point clock::now() { - const ClockData current = data[active.load()]; // Take a copy in case it changes - return current.epoch + dc((base_clock::now() - current.base_from) * current.rtf); -} - -void clock::adjust_clock(const duration& adjustment, const double& rtf) { - const std::lock_guard lock(mutex); - // Load the current state - const auto& current = data[active.load()]; - const int n = static_cast((active.load() + 1) % data.size()); - auto& next = data[n]; - - // Perform the update - auto base = base_clock::now(); - next.epoch = current.epoch + adjustment + dc((base - current.base_from) * current.rtf); - next.base_from = base; - next.rtf = rtf; - active = n; -} - -void clock::set_clock(const time_point& time, const double& rtf) { - const std::lock_guard lock(mutex); - // Load the current state - const int n = static_cast((active.load() + 1) % data.size()); - auto& next = data[n]; - - // Perform the update - auto base = base_clock::now(); - next.epoch = time; - next.base_from = base; - next.rtf = rtf; - active = n; -} - -double clock::rtf() { - return data[active.load()].rtf; -} - -} // namespace NUClear diff --git a/src/clock.hpp b/src/clock.hpp index 3dc3b514b..9526f6450 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -38,34 +38,66 @@ namespace NUClear { /** * @brief A clock class that extends a base clock type and allows for clock adjustment and setting. */ -struct clock : public NUCLEAR_CLOCK_TYPE { +template +struct nuclear_clock : public NUCLEAR_CLOCK_TYPE { using base_clock = NUCLEAR_CLOCK_TYPE; /** * @brief Get the current time of the clock. * @return The current time of the clock. */ - static time_point now(); + static time_point now() { + const ClockData current = data[active.load()]; // Take a copy in case it changes + return current.epoch + dc((base_clock::now() - current.base_from) * current.rtf); + } /** * @brief Adjust the clock by a specified duration and real-time factor. * @param adjustment The duration by which to adjust the clock. * @param rtf The real-time factor to apply to the clock. */ - static void adjust_clock(const duration& adjustment, const double& rtf = 1.0); + static void adjust_clock(const duration& adjustment, const double& rtf = 1.0) { + const std::lock_guard lock(mutex); + // Load the current state + const auto& current = data[active.load()]; + const int n = static_cast((active.load() + 1) % data.size()); + auto& next = data[n]; + + // Perform the update + auto base = base_clock::now(); + next.epoch = current.epoch + adjustment + dc((base - current.base_from) * current.rtf); + next.base_from = base; + next.rtf = rtf; + active = n; + } /** * @brief Set the clock to a specified time and real-time factor. * @param time The time to set the clock to. * @param rtf The real-time factor to apply to the clock. */ - static void set_clock(const time_point& time, const double& rtf = 1.0); + static void set_clock(const time_point& time, const double& rtf = 1.0) { + const std::lock_guard lock(mutex); + // Load the current state + const int n = static_cast((active.load() + 1) % data.size()); + auto& next = data[n]; + + // Perform the update + auto base = base_clock::now(); + next.epoch = time; + next.base_from = base; + next.rtf = rtf; + active = n; + } + /** * @brief Get the real-time factor of the clock. * @return The real-time factor of the clock. */ - static double rtf(); + static double rtf() { + return data[active.load()].rtf; + } private: /** @@ -103,6 +135,20 @@ struct clock : public NUCLEAR_CLOCK_TYPE { static std::atomic active; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) }; +template +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::mutex nuclear_clock::mutex; +template +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::array::ClockData, 3> nuclear_clock::data = + std::array::ClockData, 3>{}; +template +// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) +std::atomic nuclear_clock::active{0}; + +using clock = nuclear_clock<>; + + } // namespace NUClear #endif // NUCLEAR_CLOCK_HPP diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index 11d2b2c5b..e5b726a74 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -105,25 +105,23 @@ namespace extension { const std::lock_guard lock(mutex); // Adjust clock to target time and leave chrono tasks where they are - if (travel.type == message::TimeTravel::Action::ABSOLUTE) { - clock::adjust_clock(travel.adjustment, travel.rtf); - } - - // Adjust clock to as close to target as possible without skipping any chrono tasks - if (travel.type == message::TimeTravel::Action::NEAREST) { - auto next_task = - std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { - return a.time < b.time; - }); - clock::adjust_clock(std::min(next_task->time - clock::now(), travel.adjustment), travel.rtf); - } + switch (travel.type) { + case message::TimeTravel::Action::ABSOLUTE: clock::set_clock(travel.target, travel.rtf); break; + case message::TimeTravel::Action::RELATIVE: { + auto adjustment = travel.target - NUClear::clock::now(); + clock::set_clock(travel.target, travel.rtf); + for (auto& task : tasks) { + task.time += adjustment; + } - // Adjust clock and move all chrono tasks with it - if (travel.type == message::TimeTravel::Action::RELATIVE) { - clock::adjust_clock(travel.adjustment, travel.rtf); - for (auto& task : tasks) { - task.time += travel.adjustment; - } + } break; + case message::TimeTravel::Action::NEAREST: { + auto next_task = + std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { + return a.time < b.time; + }); + clock::set_clock(std::min(next_task->time, travel.target), travel.rtf); + } break; } // Poke the system diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index b0c568d9e..c2010b8ec 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -45,18 +45,16 @@ namespace message { NEAREST, }; - /// @brief The duration to adjust the clock by - clock::duration adjustment = clock::duration(0); + /// @brief The target time to set the clock to + clock::time_point target = clock::now(); /// @brief The rate at which time should pass double rtf = 1.0; /// @brief The type of time travel to perform Action type = Action::RELATIVE; TimeTravel() = default; - TimeTravel(const clock::duration& adjustment, double rtf = 1.0, Action type = Action::RELATIVE) - : adjustment(adjustment), rtf(rtf), type(type) {} TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE) - : adjustment(target - clock::now()), rtf(rtf), type(type) {} + : target(target), rtf(rtf), type(type) {} }; } // namespace message diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 7088cc84d..9ae12b077 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -63,7 +63,8 @@ class TestReactor : public test_util::TestBase { 2)); // Time travel! - emit(std::make_unique(adjustment, rtf, action)); + emit( + std::make_unique(NUClear::clock::time_point(adjustment), rtf, action)); results.start = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; }); diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index 71809bfdf..43cefa650 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -51,7 +51,8 @@ class TestReactor : public test_util::TestBase { 2)); // Time travel - emit(std::make_unique(adjustment, rtf, action)); + emit( + std::make_unique(NUClear::clock::now() + adjustment, rtf, action)); // Shutdown after steady clock amount of time emit(std::make_unique()); @@ -84,7 +85,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time reactor.rtf = 0.0; // Reset clock to zero - NUClear::clock::set_clock(NUClear::clock::time_point()); + NUClear::clock::set_clock(NUClear::clock::time_point(), 0.0); // Start the powerplant plant->start(); From dfae9226aaaf07a0c19f263cf9ea7e71fc1f713d Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 17:25:07 +1000 Subject: [PATCH 33/45] const things --- src/extension/ChronoController.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index e5b726a74..d39ea32fe 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -160,7 +160,7 @@ namespace extension { } } else { - NUClear::clock::duration time_until_task = + const NUClear::clock::duration time_until_task = std::chrono::duration_cast((target - start) / clock::rtf()); if (clock::rtf() == 0.0) { @@ -182,7 +182,7 @@ namespace extension { } else if (time_until_task > ns_accuracy) { // Somewhat close in time // Wait on nanosleep - NUClear::clock::duration sleep_time = time_until_task - ns_accuracy; + const NUClear::clock::duration sleep_time = time_until_task - ns_accuracy; util::precise_sleep(sleep_time); // Update the accuracy of our precise sleep From ceb92f64d12d6e4b6100d2113ec0b316a5e08552 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 17:32:21 +1000 Subject: [PATCH 34/45] Linting --- tests/individual/TimeTravel.cpp | 29 ++++++++++++++------------- tests/individual/TimeTravelFrozen.cpp | 4 ++-- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 9ae12b077..4a9321ffe 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -6,9 +6,9 @@ namespace { -using TestUnits = std::chrono::duration>; -static constexpr int64_t EVENT_1_TIME = 4; -static constexpr int64_t EVENT_2_TIME = 8; +using TestUnits = std::chrono::duration>; +constexpr int64_t EVENT_1_TIME = 4; +constexpr int64_t EVENT_2_TIME = 8; struct Results { struct TimePair { @@ -82,9 +82,9 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); - int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); - double rtf = GENERATE(0.5, 1.0, 2.0); + const Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + const int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); + const double rtf = GENERATE(0.5, 1.0, 2.0); CAPTURE(action, adjustment, rtf); reactor.action = action; reactor.adjustment = TestUnits(adjustment); @@ -97,11 +97,11 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time plant->start(); // Expected results - std::array expected; + std::array expected{}; switch (action) { case Action::RELATIVE: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; case Action::ABSOLUTE: - expected = {std::max(0l, EVENT_1_TIME - adjustment), std::max(0l, EVENT_2_TIME - adjustment)}; + expected = {std::max(0L, EVENT_1_TIME - adjustment), std::max(0L, EVENT_2_TIME - adjustment)}; break; case Action::NEAREST: expected = adjustment < EVENT_1_TIME @@ -112,16 +112,17 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time } std::array expected_nuclear = {TestUnits(expected[0]), TestUnits(expected[1])}; - std::array expected_steady = {TestUnits(int64_t(expected[0] / rtf)), - TestUnits(int64_t(expected[1] / rtf))}; + // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) + std::array expected_steady = {TestUnits(int64_t(expected[0] / rtf)), + TestUnits(int64_t(expected[1] / rtf))}; const auto& r = reactor.results; const auto& n_start = reactor.results.start.nuclear; const auto& s_start = reactor.results.start.steady; auto round_to_test_units = [](const auto& duration) { - double d = std::chrono::duration_cast>(duration).count(); - double t = (TestUnits::period::den * d) / TestUnits::period::num; + const double d = std::chrono::duration_cast>(duration).count(); + const double t = (TestUnits::period::den * d) / TestUnits::period::num; return TestUnits(std::lround(t)); }; @@ -134,8 +135,8 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time round_to_test_units(r.events[1].steady - s_start), }; - TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); - TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); + const TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); + const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); CHECK(expected_nuclear[0] == actual_nuclear[0]); CHECK(expected_nuclear[1] == actual_nuclear[1]); CHECK(expected_steady[0] == actual_steady[0]); diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index 43cefa650..563dd589f 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -7,8 +7,8 @@ namespace { -static constexpr std::chrono::milliseconds EVENT_1_TIME = std::chrono::milliseconds(4); -static constexpr std::chrono::milliseconds EVENT_2_TIME = std::chrono::milliseconds(8); +constexpr std::chrono::milliseconds EVENT_1_TIME = std::chrono::milliseconds(4); +constexpr std::chrono::milliseconds EVENT_2_TIME = std::chrono::milliseconds(8); struct WaitForShutdown {}; From 355e96de18e82942b646d86f4977bb3a79155350 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 10 Apr 2024 17:40:13 +1000 Subject: [PATCH 35/45] Lint --- tests/individual/TimeTravel.cpp | 4 ++-- tests/individual/TimeTravelFrozen.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 4a9321ffe..b0c2a764a 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -113,8 +113,8 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time std::array expected_nuclear = {TestUnits(expected[0]), TestUnits(expected[1])}; // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) - std::array expected_steady = {TestUnits(int64_t(expected[0] / rtf)), - TestUnits(int64_t(expected[1] / rtf))}; + std::array expected_steady = {TestUnits(std::lround(double(expected[0]) / rtf)), + TestUnits(std::lround(double(expected[1]) / rtf))}; const auto& r = reactor.results; const auto& n_start = reactor.results.start.nuclear; diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index 563dd589f..af41a9b5b 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -77,8 +77,8 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); - int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); + const Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + const int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); CAPTURE(action, adjustment); reactor.action = action; reactor.adjustment = std::chrono::milliseconds(adjustment); From 1ab2a872a8db34602107fd7c32cd1076f06c3e0a Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 08:56:40 +1000 Subject: [PATCH 36/45] Move clock reset --- tests/individual/TimeTravel.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index b0c2a764a..186abf8e5 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -41,6 +41,8 @@ class TestReactor : public test_util::TestBase { TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { + // Reset clock to zero + NUClear::clock::set_clock(NUClear::clock::time_point()); results.zero = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; // Emit a chrono task to run at time EVENT_1_TIME @@ -90,9 +92,6 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time reactor.adjustment = TestUnits(adjustment); reactor.rtf = rtf; - // Reset clock to zero - NUClear::clock::set_clock(NUClear::clock::time_point()); - // Start the powerplant plant->start(); From 452a1fa8b1a973f1041ebd4bf851e1a114f01a4c Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 09:29:32 +1000 Subject: [PATCH 37/45] Apply suggestions --- tests/individual/TimeTravel.cpp | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 186abf8e5..b3c0de85a 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -23,21 +23,6 @@ struct Results { class TestReactor : public test_util::TestBase { public: - // Start time of steady clock - std::chrono::steady_clock::time_point steady_start_time = std::chrono::steady_clock::now(); - - // Time travel action - NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; - - // Time adjustment - NUClear::clock::duration adjustment = std::chrono::milliseconds(0); - - // Real-time factor - double rtf = 1.0; - - // Results struct - Results results; - TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { @@ -51,7 +36,7 @@ class TestReactor : public test_util::TestBase { results.events[0] = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; return false; }, - NUClear::clock::now() + TestUnits(EVENT_1_TIME), + NUClear::clock::time_point(TestUnits(EVENT_1_TIME)), 1)); // Emit a chrono task to run at time EVENT_2_TIME, and shutdown @@ -61,7 +46,7 @@ class TestReactor : public test_util::TestBase { powerplant.shutdown(); return false; }, - NUClear::clock::now() + TestUnits(EVENT_2_TIME), + NUClear::clock::time_point(TestUnits(EVENT_2_TIME)), 2)); // Time travel! @@ -71,6 +56,18 @@ class TestReactor : public test_util::TestBase { results.start = Results::TimePair{NUClear::clock::now(), std::chrono::steady_clock::now()}; }); } + + // Time travel action + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; + + // Time adjustment + NUClear::clock::duration adjustment = std::chrono::milliseconds(0); + + // Real-time factor + double rtf = 1.0; + + // Results struct + Results results; }; } // anonymous namespace From 1d15fddecff2f8fd4aa4d4b423ee6325c60736eb Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 09:37:56 +1000 Subject: [PATCH 38/45] Apply suggestions --- tests/individual/TimeTravelFrozen.cpp | 67 +++++++++++++-------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index af41a9b5b..cba2920a5 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -7,38 +7,27 @@ namespace { -constexpr std::chrono::milliseconds EVENT_1_TIME = std::chrono::milliseconds(4); -constexpr std::chrono::milliseconds EVENT_2_TIME = std::chrono::milliseconds(8); +constexpr std::chrono::milliseconds EVENT_1_TIME = std::chrono::milliseconds(4); +constexpr std::chrono::milliseconds EVENT_2_TIME = std::chrono::milliseconds(8); +constexpr std::chrono::milliseconds SHUTDOWN_TIME = std::chrono::milliseconds(12); struct WaitForShutdown {}; class TestReactor : public test_util::TestBase { public: - // Start time of steady clock - std::chrono::steady_clock::time_point steady_start_time = std::chrono::steady_clock::now(); - - // Time travel action - NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; - - // Time adjustment - NUClear::clock::duration adjustment = std::chrono::milliseconds(0); - - // Real-time factor - double rtf = 1.0; - - // Events - std::vector events = {}; - TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { on().then([this] { + // Reset clock to zero + NUClear::clock::set_clock(NUClear::clock::time_point(), 0.0); + // Emit a chrono task to run at time EVENT_1_TIME emit(std::make_unique( [this](NUClear::clock::time_point&) { events.push_back("Event 1"); return false; }, - NUClear::clock::now() + EVENT_1_TIME, + NUClear::clock::time_point(EVENT_1_TIME), 1)); // Emit a chrono task to run at time EVENT_2_TIME @@ -47,23 +36,35 @@ class TestReactor : public test_util::TestBase { events.push_back("Event 2"); return false; }, - NUClear::clock::now() + EVENT_2_TIME, + NUClear::clock::time_point(EVENT_2_TIME), 2)); // Time travel emit( - std::make_unique(NUClear::clock::now() + adjustment, rtf, action)); + std::make_unique(NUClear::clock::time_point(adjustment), rtf, action)); // Shutdown after steady clock amount of time emit(std::make_unique()); }); on>().then([this] { - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + std::this_thread::sleep_for(SHUTDOWN_TIME); events.push_back("Finished"); powerplant.shutdown(); }); } + + // Time travel action + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; + + // Time adjustment + NUClear::clock::duration adjustment = std::chrono::milliseconds(0); + + // Real-time factor + double rtf = 1.0; + + // Events + std::vector events = {}; }; } // anonymous namespace @@ -84,35 +85,33 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time reactor.adjustment = std::chrono::milliseconds(adjustment); reactor.rtf = 0.0; - // Reset clock to zero - NUClear::clock::set_clock(NUClear::clock::time_point(), 0.0); - // Start the powerplant plant->start(); // Expected results - std::vector expected_events; + std::vector expected; switch (action) { - case Action::RELATIVE: expected_events = {"Finished"}; break; + case Action::RELATIVE: expected = {"Finished"}; break; case Action::ABSOLUTE: if (std::chrono::milliseconds(adjustment) < EVENT_1_TIME) { - expected_events = {"Finished"}; + expected = {"Finished"}; } else if (std::chrono::milliseconds(adjustment) < EVENT_2_TIME) { - expected_events = {"Event 1", "Finished"}; + expected = {"Event 1", "Finished"}; } else { - expected_events = {"Event 1", "Event 2", "Finished"}; + expected = {"Event 1", "Event 2", "Finished"}; } break; case Action::NEAREST: - expected_events = std::chrono::milliseconds(adjustment) < EVENT_1_TIME - ? std::vector{"Finished"} - : std::vector{"Event 1", "Finished"}; + expected = std::chrono::milliseconds(adjustment) < EVENT_1_TIME + ? std::vector{"Finished"} + : std::vector{"Event 1", "Finished"}; break; default: throw std::runtime_error("Unknown action"); } - const auto& actual_events = reactor.events; - CHECK(expected_events == actual_events); + INFO("Expected: " << expected); + INFO("Actual: " << reactor.events); + CHECK(expected == reactor.events); } From f5440b6dbb3178f5dff8576760d6c3ac7da9407d Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 09:47:37 +1000 Subject: [PATCH 39/45] Add diff string --- tests/CMakeLists.txt | 23 ++++++++++------------- tests/individual/TimeTravelFrozen.cpp | 3 +-- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cc7004de7..2a1158efc 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -42,7 +42,13 @@ if(CATCH_FOUND) "util/*.cpp" "util/network/*.cpp" "util/serialise/*.cpp" - "test_util/*.cpp" + ) + + file(GLOB test_util_src "test_util/*.cpp") + add_library(test_util STATIC ${test_util_src}) + target_link_libraries(test_util PUBLIC NUClear::nuclear) + target_include_directories( + test_util SYSTEM PUBLIC ${CATCH_INCLUDE_DIRS} ${PROJECT_BINARY_DIR}/include "${PROJECT_SOURCE_DIR}/src" ) # Some tests must be executed as individual binaries @@ -52,12 +58,9 @@ if(CATCH_FOUND) get_filename_component(test_name ${test_src} NAME_WE) add_executable(${test_name} ${test_src}) - target_link_libraries(${test_name} NUClear::nuclear) + target_link_libraries(${test_name} NUClear::nuclear test_util) set_target_properties(${test_name} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/individual") target_include_directories(${test_name} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - target_include_directories( - ${test_name} SYSTEM PRIVATE ${CATCH_INCLUDE_DIRS} ${PROJECT_BINARY_DIR}/include "${PROJECT_SOURCE_DIR}/src" - ) # Enable warnings, and all warnings are errors add_test(${test_name} test_nuclear) @@ -66,18 +69,12 @@ if(CATCH_FOUND) add_executable(test_nuclear ${test_src}) target_include_directories(test_nuclear PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - target_link_libraries(test_nuclear NUClear::nuclear) - target_include_directories( - test_nuclear SYSTEM PRIVATE ${CATCH_INCLUDE_DIRS} ${PROJECT_BINARY_DIR}/include "${PROJECT_SOURCE_DIR}/src" - ) + target_link_libraries(test_nuclear NUClear::nuclear test_util) add_test(test_nuclear test_nuclear) add_executable(test_network networktest.cpp) - target_link_libraries(test_network NUClear::nuclear) + target_link_libraries(test_network NUClear::nuclear test_util) target_include_directories(test_network PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - target_include_directories( - test_network SYSTEM PRIVATE ${CATCH_INCLUDE_DIRS} ${PROJECT_BINARY_DIR}/include "${PROJECT_SOURCE_DIR}/src" - ) endif(BUILD_TESTS) endif(CATCH_FOUND) diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index cba2920a5..5773a5f7f 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -111,7 +111,6 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time default: throw std::runtime_error("Unknown action"); } - INFO("Expected: " << expected); - INFO("Actual: " << reactor.events); + INFO(test_util::diff_string(expected, reactor.events)); CHECK(expected == reactor.events); } From 6ad8b298af6e944936119f0dce1c6e6020b78d7f Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 10:02:01 +1000 Subject: [PATCH 40/45] Cast type to int64_t --- tests/individual/TimeTravel.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index b3c0de85a..cd12f6e6d 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -97,7 +97,10 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time switch (action) { case Action::RELATIVE: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; case Action::ABSOLUTE: - expected = {std::max(0L, EVENT_1_TIME - adjustment), std::max(0L, EVENT_2_TIME - adjustment)}; + expected = { + std::max(int64_t(0), int64_t(EVENT_1_TIME - adjustment)), + std::max(int64_t(0), int64_t(EVENT_2_TIME - adjustment)), + }; break; case Action::NEAREST: expected = adjustment < EVENT_1_TIME From 9203c6e67b0c5c958b101b845e5a6d2d28533802 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 10:03:25 +1000 Subject: [PATCH 41/45] Test change enum for windows --- src/extension/ChronoController.hpp | 6 +++--- src/message/TimeTravel.hpp | 10 +++++----- tests/individual/TimeTravel.cpp | 12 ++++++------ tests/individual/TimeTravelFrozen.cpp | 10 +++++----- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index d39ea32fe..bb2f219cd 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -106,8 +106,8 @@ namespace extension { // Adjust clock to target time and leave chrono tasks where they are switch (travel.type) { - case message::TimeTravel::Action::ABSOLUTE: clock::set_clock(travel.target, travel.rtf); break; - case message::TimeTravel::Action::RELATIVE: { + case message::TimeTravel::Action::ABSOLUTE_: clock::set_clock(travel.target, travel.rtf); break; + case message::TimeTravel::Action::RELATIVE_: { auto adjustment = travel.target - NUClear::clock::now(); clock::set_clock(travel.target, travel.rtf); for (auto& task : tasks) { @@ -115,7 +115,7 @@ namespace extension { } } break; - case message::TimeTravel::Action::NEAREST: { + case message::TimeTravel::Action::NEAREST_: { auto next_task = std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { return a.time < b.time; diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index c2010b8ec..6489846eb 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -36,13 +36,13 @@ namespace message { struct TimeTravel { enum class Action { /// @brief Adjust clock and move all chrono tasks with it - RELATIVE, + RELATIVE_, /// @brief Adjust clock to target time and leave chrono tasks where they are - ABSOLUTE, + ABSOLUTE_, /// @brief Adjust clock to as close to target as possible without skipping any chrono tasks - NEAREST, + NEAREST_, }; /// @brief The target time to set the clock to @@ -50,10 +50,10 @@ namespace message { /// @brief The rate at which time should pass double rtf = 1.0; /// @brief The type of time travel to perform - Action type = Action::RELATIVE; + Action type = Action::RELATIVE_; TimeTravel() = default; - TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE) + TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE_) : target(target), rtf(rtf), type(type) {} }; diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index cd12f6e6d..0cfc0d358 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -58,7 +58,7 @@ class TestReactor : public test_util::TestBase { } // Time travel action - NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE_; // Time adjustment NUClear::clock::duration adjustment = std::chrono::milliseconds(0); @@ -81,7 +81,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - const Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + const Action action = GENERATE(Action::RELATIVE_, Action::ABSOLUTE_, Action::NEAREST_); const int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); const double rtf = GENERATE(0.5, 1.0, 2.0); CAPTURE(action, adjustment, rtf); @@ -95,14 +95,14 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time // Expected results std::array expected{}; switch (action) { - case Action::RELATIVE: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; - case Action::ABSOLUTE: + case Action::RELATIVE_: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; + case Action::ABSOLUTE_: expected = { std::max(int64_t(0), int64_t(EVENT_1_TIME - adjustment)), std::max(int64_t(0), int64_t(EVENT_2_TIME - adjustment)), }; break; - case Action::NEAREST: + case Action::NEAREST_: expected = adjustment < EVENT_1_TIME ? std::array{EVENT_1_TIME - adjustment, EVENT_2_TIME - adjustment} : std::array{0, EVENT_2_TIME - EVENT_1_TIME}; @@ -135,7 +135,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time }; const TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); - const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); + const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST_ ? EVENT_1_TIME : adjustment)); CHECK(expected_nuclear[0] == actual_nuclear[0]); CHECK(expected_nuclear[1] == actual_nuclear[1]); CHECK(expected_steady[0] == actual_steady[0]); diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index 5773a5f7f..40591aff2 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -55,7 +55,7 @@ class TestReactor : public test_util::TestBase { } // Time travel action - NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE_; // Time adjustment NUClear::clock::duration adjustment = std::chrono::milliseconds(0); @@ -78,7 +78,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - const Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); + const Action action = GENERATE(Action::RELATIVE_, Action::ABSOLUTE_, Action::NEAREST_); const int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); CAPTURE(action, adjustment); reactor.action = action; @@ -91,8 +91,8 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time // Expected results std::vector expected; switch (action) { - case Action::RELATIVE: expected = {"Finished"}; break; - case Action::ABSOLUTE: + case Action::RELATIVE_: expected = {"Finished"}; break; + case Action::ABSOLUTE_: if (std::chrono::milliseconds(adjustment) < EVENT_1_TIME) { expected = {"Finished"}; } @@ -103,7 +103,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time expected = {"Event 1", "Event 2", "Finished"}; } break; - case Action::NEAREST: + case Action::NEAREST_: expected = std::chrono::milliseconds(adjustment) < EVENT_1_TIME ? std::vector{"Finished"} : std::vector{"Event 1", "Finished"}; From a168f86a4f73338647e0692917452d8c8dd9e6c3 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 10:10:36 +1000 Subject: [PATCH 42/45] Revert "Test change enum for windows" This reverts commit 9203c6e67b0c5c958b101b845e5a6d2d28533802. --- src/extension/ChronoController.hpp | 6 +++--- src/message/TimeTravel.hpp | 10 +++++----- tests/individual/TimeTravel.cpp | 12 ++++++------ tests/individual/TimeTravelFrozen.cpp | 10 +++++----- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index bb2f219cd..d39ea32fe 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -106,8 +106,8 @@ namespace extension { // Adjust clock to target time and leave chrono tasks where they are switch (travel.type) { - case message::TimeTravel::Action::ABSOLUTE_: clock::set_clock(travel.target, travel.rtf); break; - case message::TimeTravel::Action::RELATIVE_: { + case message::TimeTravel::Action::ABSOLUTE: clock::set_clock(travel.target, travel.rtf); break; + case message::TimeTravel::Action::RELATIVE: { auto adjustment = travel.target - NUClear::clock::now(); clock::set_clock(travel.target, travel.rtf); for (auto& task : tasks) { @@ -115,7 +115,7 @@ namespace extension { } } break; - case message::TimeTravel::Action::NEAREST_: { + case message::TimeTravel::Action::NEAREST: { auto next_task = std::min_element(tasks.begin(), tasks.end(), [](const ChronoTask& a, const ChronoTask& b) { return a.time < b.time; diff --git a/src/message/TimeTravel.hpp b/src/message/TimeTravel.hpp index 6489846eb..c2010b8ec 100644 --- a/src/message/TimeTravel.hpp +++ b/src/message/TimeTravel.hpp @@ -36,13 +36,13 @@ namespace message { struct TimeTravel { enum class Action { /// @brief Adjust clock and move all chrono tasks with it - RELATIVE_, + RELATIVE, /// @brief Adjust clock to target time and leave chrono tasks where they are - ABSOLUTE_, + ABSOLUTE, /// @brief Adjust clock to as close to target as possible without skipping any chrono tasks - NEAREST_, + NEAREST, }; /// @brief The target time to set the clock to @@ -50,10 +50,10 @@ namespace message { /// @brief The rate at which time should pass double rtf = 1.0; /// @brief The type of time travel to perform - Action type = Action::RELATIVE_; + Action type = Action::RELATIVE; TimeTravel() = default; - TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE_) + TimeTravel(const clock::time_point& target, double rtf = 1.0, Action type = Action::RELATIVE) : target(target), rtf(rtf), type(type) {} }; diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 0cfc0d358..cd12f6e6d 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -58,7 +58,7 @@ class TestReactor : public test_util::TestBase { } // Time travel action - NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE_; + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; // Time adjustment NUClear::clock::duration adjustment = std::chrono::milliseconds(0); @@ -81,7 +81,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - const Action action = GENERATE(Action::RELATIVE_, Action::ABSOLUTE_, Action::NEAREST_); + const Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); const int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); const double rtf = GENERATE(0.5, 1.0, 2.0); CAPTURE(action, adjustment, rtf); @@ -95,14 +95,14 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time // Expected results std::array expected{}; switch (action) { - case Action::RELATIVE_: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; - case Action::ABSOLUTE_: + case Action::RELATIVE: expected = {EVENT_1_TIME, EVENT_2_TIME}; break; + case Action::ABSOLUTE: expected = { std::max(int64_t(0), int64_t(EVENT_1_TIME - adjustment)), std::max(int64_t(0), int64_t(EVENT_2_TIME - adjustment)), }; break; - case Action::NEAREST_: + case Action::NEAREST: expected = adjustment < EVENT_1_TIME ? std::array{EVENT_1_TIME - adjustment, EVENT_2_TIME - adjustment} : std::array{0, EVENT_2_TIME - EVENT_1_TIME}; @@ -135,7 +135,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time }; const TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); - const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST_ ? EVENT_1_TIME : adjustment)); + const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); CHECK(expected_nuclear[0] == actual_nuclear[0]); CHECK(expected_nuclear[1] == actual_nuclear[1]); CHECK(expected_steady[0] == actual_steady[0]); diff --git a/tests/individual/TimeTravelFrozen.cpp b/tests/individual/TimeTravelFrozen.cpp index 40591aff2..5773a5f7f 100644 --- a/tests/individual/TimeTravelFrozen.cpp +++ b/tests/individual/TimeTravelFrozen.cpp @@ -55,7 +55,7 @@ class TestReactor : public test_util::TestBase { } // Time travel action - NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE_; + NUClear::message::TimeTravel::Action action = NUClear::message::TimeTravel::Action::RELATIVE; // Time adjustment NUClear::clock::duration adjustment = std::chrono::milliseconds(0); @@ -78,7 +78,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time auto& reactor = plant->install(); // Set the reactor fields to the values we want to test - const Action action = GENERATE(Action::RELATIVE_, Action::ABSOLUTE_, Action::NEAREST_); + const Action action = GENERATE(Action::RELATIVE, Action::ABSOLUTE, Action::NEAREST); const int64_t adjustment = GENERATE(-4, -2, 0, 2, 4, 6, 8, 10); CAPTURE(action, adjustment); reactor.action = action; @@ -91,8 +91,8 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time // Expected results std::vector expected; switch (action) { - case Action::RELATIVE_: expected = {"Finished"}; break; - case Action::ABSOLUTE_: + case Action::RELATIVE: expected = {"Finished"}; break; + case Action::ABSOLUTE: if (std::chrono::milliseconds(adjustment) < EVENT_1_TIME) { expected = {"Finished"}; } @@ -103,7 +103,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time expected = {"Event 1", "Event 2", "Finished"}; } break; - case Action::NEAREST_: + case Action::NEAREST: expected = std::chrono::milliseconds(adjustment) < EVENT_1_TIME ? std::vector{"Finished"} : std::vector{"Event 1", "Finished"}; From a0ad58b7e8a3d3c0a78fa8c4fe8d248d54153180 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 10:12:14 +1000 Subject: [PATCH 43/45] undef RELATIVE and ABSOLUTE --- src/util/platform.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/platform.hpp b/src/util/platform.hpp index 52048ee6f..6cc6d9abd 100644 --- a/src/util/platform.hpp +++ b/src/util/platform.hpp @@ -81,6 +81,8 @@ // Whoever thought this was a good idea was a terrible person #undef ERROR + #undef RELATIVE + #undef ABSOLUTE // Make the windows shutdown functions look like the posix ones #define SHUT_RD SD_RECEIVE From eb73f3f599204cdf7d5c6ea4b3647ff71ab0f231 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 10:31:49 +1000 Subject: [PATCH 44/45] Apply suggestions --- tests/individual/TimeTravel.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index cd12f6e6d..15a90f551 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -111,9 +111,8 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time } std::array expected_nuclear = {TestUnits(expected[0]), TestUnits(expected[1])}; - // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) - std::array expected_steady = {TestUnits(std::lround(double(expected[0]) / rtf)), - TestUnits(std::lround(double(expected[1]) / rtf))}; + std::array expected_steady = {TestUnits(std::lround(double(expected[0]) / rtf)), + TestUnits(std::lround(double(expected[1]) / rtf))}; const auto& r = reactor.results; const auto& n_start = reactor.results.start.nuclear; @@ -136,6 +135,7 @@ TEST_CASE("Test time travel correctly changes the time for non zero rtf", "[time const TestUnits actual_adjustment(round_to_test_units(r.start.nuclear - r.zero.nuclear)); const TestUnits expected_adjustment(std::min(adjustment, action == Action::NEAREST ? EVENT_1_TIME : adjustment)); + CHECK(round_to_test_units(r.zero.nuclear.time_since_epoch()) == TestUnits(0)); CHECK(expected_nuclear[0] == actual_nuclear[0]); CHECK(expected_nuclear[1] == actual_nuclear[1]); CHECK(expected_steady[0] == actual_steady[0]); From ab6cdf73fff0844d7ed0bcfe3c4bf054ece0f651 Mon Sep 17 00:00:00 2001 From: Tom0Brien Date: Wed, 17 Apr 2024 10:44:30 +1000 Subject: [PATCH 45/45] Change TestUnits to help CI --- tests/individual/TimeTravel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/individual/TimeTravel.cpp b/tests/individual/TimeTravel.cpp index 15a90f551..11a70640d 100644 --- a/tests/individual/TimeTravel.cpp +++ b/tests/individual/TimeTravel.cpp @@ -6,7 +6,7 @@ namespace { -using TestUnits = std::chrono::duration>; +using TestUnits = std::chrono::duration>; constexpr int64_t EVENT_1_TIME = 4; constexpr int64_t EVENT_2_TIME = 8;