From 68e154a7beb2648aa3d7f69ff45ff620b79447d4 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Mon, 25 Sep 2023 00:17:55 +1000 Subject: [PATCH] Address some issues found by sonarcloud (#86) --- .github/workflows/sonarcloud.yaml | 1 + docs/startup.rst | 2 +- src/CMakeLists.txt | 2 +- src/Configuration.hpp | 41 +++++++ src/PowerPlant.hpp | 24 +--- src/PowerPlant.ipp | 24 ++-- src/Reactor.hpp | 2 +- src/clock.hpp | 11 +- src/dsl/fusion/GroupFusion.hpp | 5 +- src/dsl/fusion/NoOp.hpp | 32 ++--- src/dsl/fusion/PoolFusion.hpp | 5 +- src/dsl/operation/CacheGet.hpp | 2 +- src/dsl/operation/ChronoTask.hpp | 2 +- src/dsl/operation/TypeBind.hpp | 18 +-- src/dsl/operation/Unbind.hpp | 2 +- src/dsl/word/Always.hpp | 8 +- src/dsl/word/Buffer.hpp | 2 +- src/dsl/word/Every.hpp | 2 +- src/dsl/word/Group.hpp | 2 +- src/dsl/word/IO.hpp | 6 +- src/dsl/word/Last.hpp | 54 ++++++-- src/dsl/word/MainThread.hpp | 2 +- src/dsl/word/Network.hpp | 9 +- src/dsl/word/Optional.hpp | 2 +- src/dsl/word/Pool.hpp | 2 +- src/dsl/word/Priority.hpp | 10 +- src/dsl/word/TCP.hpp | 4 +- src/dsl/word/UDP.hpp | 28 +++-- src/dsl/word/Watchdog.hpp | 12 +- src/dsl/word/emit/Delay.hpp | 4 +- src/dsl/word/emit/UDP.hpp | 8 +- src/dsl/word/emit/Watchdog.hpp | 40 +++--- src/extension/ChronoController.hpp | 2 +- src/extension/IOController_Posix.hpp | 4 +- src/extension/IOController_Windows.hpp | 2 - src/extension/NetworkController.hpp | 10 +- src/extension/network/NUClearNetwork.cpp | 33 ++--- src/extension/network/NUClearNetwork.hpp | 23 ++-- src/extension/network/wire_protocol.hpp | 129 ++++++++++++-------- src/threading/Reaction.cpp | 2 +- src/threading/Reaction.hpp | 2 +- src/threading/ReactionHandle.hpp | 2 +- src/threading/ReactionTask.hpp | 5 +- src/threading/TaskScheduler.cpp | 5 +- src/threading/TaskScheduler.hpp | 9 +- src/util/CallbackGenerator.hpp | 23 ++-- src/util/Dereferencer.hpp | 2 +- src/util/Sequence.hpp | 48 ++++---- src/util/network/get_interfaces.hpp | 4 +- src/util/network/if_number_from_address.cpp | 2 +- src/util/network/resolve.cpp | 3 +- src/util/platform.hpp | 2 +- src/util/serialise/Serialise.hpp | 2 +- src/util/unpack.hpp | 4 +- tests/api/ReactionHandle.cpp | 2 +- tests/api/ReactionStatistics.cpp | 2 +- tests/dsl/Always.cpp | 2 +- tests/dsl/ArgumentFission.cpp | 2 +- tests/dsl/BlockNoData.cpp | 2 +- tests/dsl/Buffer.cpp | 2 +- tests/dsl/CommandLineArguments.cpp | 2 +- tests/dsl/CustomGet.cpp | 4 +- tests/dsl/DSLOrdering.cpp | 2 +- tests/dsl/DSLProxy.cpp | 2 +- tests/dsl/Every.cpp | 2 +- tests/dsl/FlagMessage.cpp | 2 +- tests/dsl/FusionInOrder.cpp | 2 +- tests/dsl/IO.cpp | 2 +- tests/dsl/Last.cpp | 2 +- tests/dsl/MainThread.cpp | 2 +- tests/dsl/MissingArguments.cpp | 2 +- tests/dsl/Once.cpp | 2 +- tests/dsl/Optional.cpp | 2 +- tests/dsl/Priority.cpp | 2 +- tests/dsl/Raw.cpp | 2 +- tests/dsl/RawFunction.cpp | 2 +- tests/dsl/Shutdown.cpp | 2 +- tests/dsl/Startup.cpp | 2 +- tests/dsl/Sync.cpp | 2 +- tests/dsl/SyncOrder.cpp | 2 +- tests/dsl/TCP.cpp | 10 +- tests/dsl/Transient.cpp | 2 +- tests/dsl/Trigger.cpp | 2 +- tests/dsl/UDP.cpp | 2 +- tests/dsl/Watchdog.cpp | 2 +- tests/dsl/With.cpp | 2 +- tests/dsl/emit/Delay.cpp | 2 +- tests/dsl/emit/EmitFusion.cpp | 16 ++- tests/dsl/emit/Initialise.cpp | 2 +- tests/individual/BaseClock.cpp | 2 +- tests/individual/CustomClock.cpp | 2 +- tests/log/Log.cpp | 2 +- tests/networktest.cpp | 2 +- 93 files changed, 455 insertions(+), 333 deletions(-) create mode 100644 src/Configuration.hpp diff --git a/.github/workflows/sonarcloud.yaml b/.github/workflows/sonarcloud.yaml index 10f1f3c51..9f0a1d843 100644 --- a/.github/workflows/sonarcloud.yaml +++ b/.github/workflows/sonarcloud.yaml @@ -3,6 +3,7 @@ on: push: branches: - main + - sonar pull_request: types: [opened, synchronize, reopened] diff --git a/docs/startup.rst b/docs/startup.rst index 72eec4bf2..01cc0554b 100644 --- a/docs/startup.rst +++ b/docs/startup.rst @@ -22,7 +22,7 @@ file for the process. .. code-block:: C++ - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9747987ff..911a429bd 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -35,7 +35,7 @@ add_library(NUClear::nuclear ALIAS nuclear) # Set compile options for NUClear target_link_libraries(nuclear ${CMAKE_THREAD_LIBS_INIT}) set_target_properties(nuclear PROPERTIES POSITION_INDEPENDENT_CODE ON) -target_compile_features(nuclear PUBLIC c_std_99 cxx_std_14) +target_compile_features(nuclear PUBLIC cxx_std_14) # Enable warnings, and all warnings are errors if(MSVC) diff --git a/src/Configuration.hpp b/src/Configuration.hpp new file mode 100644 index 000000000..c51d678ad --- /dev/null +++ b/src/Configuration.hpp @@ -0,0 +1,41 @@ +/* + * 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. + */ + +#ifndef NUCLEAR_CONFIGURATION_HPP +#define NUCLEAR_CONFIGURATION_HPP + +#include +#include + +namespace NUClear { + +/** + * @brief This class holds the configuration for a PowerPlant. + */ +struct Configuration { + /// @brief The number of threads the system will use + size_t thread_count = std::thread::hardware_concurrency() == 0 ? 2 : std::thread::hardware_concurrency(); +}; + +} // namespace NUClear + +#endif // NUCLEAR_CONFIGURATION_HPP diff --git a/src/PowerPlant.hpp b/src/PowerPlant.hpp index 10be06dbe..0a76090c3 100644 --- a/src/PowerPlant.hpp +++ b/src/PowerPlant.hpp @@ -37,6 +37,7 @@ #include // Utilities +#include "Configuration.hpp" #include "LogLevel.hpp" #include "message/LogMessage.hpp" #include "threading/ReactionTask.hpp" @@ -64,25 +65,6 @@ class PowerPlant { friend class Reactor; public: - /** - * @brief This class holds the configuration for a PowerPlant. - * - * @details - * It configures the number of threads that will be in the PowerPlants thread pool - */ - struct Configuration { - /// @brief default to the amount of hardware concurrency (or 2) threads - Configuration() - : thread_count(std::thread::hardware_concurrency() == 0 ? 2 : std::thread::hardware_concurrency()) {} - - /// @brief The number of threads the system will use - size_t thread_count; - }; - - /// @brief Holds the configuration information for this PowerPlant (such as number of pool threads) - const Configuration configuration; - - // There can only be one powerplant, so this is it static PowerPlant* powerplant; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) @@ -91,9 +73,7 @@ class PowerPlant { * Constructs a PowerPlant with the given configuration and provides access * to argv for all reactors. * - * @details - * If PowerPlant is constructed with argc and argv then a CommandLineArguments - * message will be emitted and available to all reactors. + * @details Arguments passed to this function will be emitted as a CommandLineArguments message. * * @param config The PowerPlant's configuration * @param argc The number of command line arguments diff --git a/src/PowerPlant.ipp b/src/PowerPlant.ipp index 539a34d25..dfd328a6b 100644 --- a/src/PowerPlant.ipp +++ b/src/PowerPlant.ipp @@ -27,8 +27,7 @@ namespace NUClear { // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) -inline PowerPlant::PowerPlant(Configuration config, int argc, const char* argv[]) - : configuration(config), scheduler(config.thread_count) { +inline PowerPlant::PowerPlant(Configuration config, int argc, const char* argv[]) : scheduler(config.thread_count) { // Stop people from making more then one powerplant if (powerplant != nullptr) { @@ -151,19 +150,16 @@ void PowerPlant::emit(Arguments&&... args) { emit_shared(std::forward(args)...); } -// Anonymous metafunction that concatenates everything into a single string -namespace { - template - inline void log_impl(std::stringstream& output, T&& first) { - output << first; - } +template +inline void log_impl(std::stringstream& output, T&& first) { + output << std::forward(first); +} - template - inline void log_impl(std::stringstream& output, First&& first, Remainder&&... args) { - output << first << " "; - log_impl(output, std::forward(args)...); - } -} // namespace +template +inline void log_impl(std::stringstream& output, First&& first, Remainder&&... args) { + output << std::forward(first) << " "; + log_impl(output, std::forward(args)...); +} template void PowerPlant::log(Arguments&&... args) { diff --git a/src/Reactor.hpp b/src/Reactor.hpp index fac7f76c2..bf1b6f04e 100644 --- a/src/Reactor.hpp +++ b/src/Reactor.hpp @@ -142,7 +142,7 @@ class Reactor { public: friend class PowerPlant; - Reactor(std::unique_ptr environment) + explicit Reactor(std::unique_ptr environment) : powerplant(environment->powerplant) , reactor_name(environment->reactor_name) , log_level(environment->log_level) {} diff --git a/src/clock.hpp b/src/clock.hpp index fef0b837d..8db78d417 100644 --- a/src/clock.hpp +++ b/src/clock.hpp @@ -27,12 +27,13 @@ namespace NUClear { -#ifndef NUCLEAR_CLOCK_TYPE - #define NUCLEAR_CLOCK_TYPE std::chrono::steady_clock -#endif // NUCLEAR_CLOCK_TYPE - -/// @brief The base clock that is used when defining the NUClear clock +#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 #ifndef NUCLEAR_CUSTOM_CLOCK diff --git a/src/dsl/fusion/GroupFusion.hpp b/src/dsl/fusion/GroupFusion.hpp index eb993b9b2..fb71fa987 100644 --- a/src/dsl/fusion/GroupFusion.hpp +++ b/src/dsl/fusion/GroupFusion.hpp @@ -24,6 +24,7 @@ #define NUCLEAR_DSL_FUSION_GROUPFUSION_HPP #include +#include #include "../../threading/Reaction.hpp" #include "../operation/DSLProxy.hpp" @@ -88,8 +89,8 @@ namespace dsl { struct GroupFuser> { template - static inline void group(threading::Reaction& /*reaction*/) { - throw std::runtime_error("Can not be a member of more than one group"); + static inline void group(const threading::Reaction& /*reaction*/) { + throw std::invalid_argument("Can not be a member of more than one group"); } }; diff --git a/src/dsl/fusion/NoOp.hpp b/src/dsl/fusion/NoOp.hpp index 455f03bcb..f33661b9e 100644 --- a/src/dsl/fusion/NoOp.hpp +++ b/src/dsl/fusion/NoOp.hpp @@ -42,35 +42,39 @@ namespace dsl { struct NoOp { template - static inline void bind(const std::shared_ptr& /*reaction*/, Args... /*args*/) {} + static inline void bind(const std::shared_ptr& /*reaction*/, Args... /*args*/) { + // Empty as this is a no-op placeholder + } template - static inline std::tuple<> get(threading::Reaction& /*reaction*/) { + static inline std::tuple<> get(const threading::Reaction& /*reaction*/) { return {}; } template - static inline bool precondition(threading::Reaction& /*reaction*/) { + static inline bool precondition(const threading::Reaction& /*reaction*/) { return true; } template - static inline int priority(threading::Reaction& /*reaction*/) { + static inline int priority(const threading::Reaction& /*reaction*/) { return word::Priority::NORMAL::value; } template - static inline util::GroupDescriptor group(threading::Reaction& /*reaction*/) { + static inline util::GroupDescriptor group(const threading::Reaction& /*reaction*/) { return util::GroupDescriptor{}; } template - static inline util::ThreadPoolDescriptor pool(threading::Reaction& /*reaction*/) { + static inline util::ThreadPoolDescriptor pool(const threading::Reaction& /*reaction*/) { return util::ThreadPoolDescriptor{}; } template - static inline void postcondition(threading::ReactionTask& /*task*/) {} + static inline void postcondition(const threading::ReactionTask& /*task*/) { + // Empty as this is a no-op placeholder + } }; /** @@ -80,19 +84,19 @@ namespace dsl { struct ParsedNoOp { struct DSL {}; - static inline std::tuple<> bind(const std::shared_ptr&); + static std::tuple<> bind(const std::shared_ptr&); - static inline std::tuple<> get(threading::Reaction&); + static std::tuple<> get(threading::Reaction&); - static inline bool precondition(threading::Reaction&); + static bool precondition(threading::Reaction&); - static inline int priority(threading::Reaction&); + static int priority(threading::Reaction&); - static inline util::GroupDescriptor group(threading::Reaction&); + static util::GroupDescriptor group(threading::Reaction&); - static inline util::ThreadPoolDescriptor pool(threading::Reaction&); + static util::ThreadPoolDescriptor pool(threading::Reaction&); - static inline void postcondition(threading::ReactionTask&); + static void postcondition(threading::ReactionTask&); }; } // namespace fusion diff --git a/src/dsl/fusion/PoolFusion.hpp b/src/dsl/fusion/PoolFusion.hpp index c13b839bf..795dcb3f9 100644 --- a/src/dsl/fusion/PoolFusion.hpp +++ b/src/dsl/fusion/PoolFusion.hpp @@ -24,6 +24,7 @@ #define NUCLEAR_DSL_FUSION_POOLFUSION_HPP #include +#include #include "../../threading/Reaction.hpp" #include "../operation/DSLProxy.hpp" @@ -88,8 +89,8 @@ namespace dsl { struct PoolFuser> { template - static inline util::ThreadPoolDescriptor pool(threading::Reaction& /*reaction*/) { - throw std::runtime_error("Can not be a member of more than one pool"); + static inline util::ThreadPoolDescriptor pool(const threading::Reaction& /*reaction*/) { + throw std::invalid_argument("Can not be a member of more than one pool"); } }; diff --git a/src/dsl/operation/CacheGet.hpp b/src/dsl/operation/CacheGet.hpp index 120381d04..4796cd28c 100644 --- a/src/dsl/operation/CacheGet.hpp +++ b/src/dsl/operation/CacheGet.hpp @@ -42,7 +42,7 @@ namespace dsl { struct CacheGet { template - static inline std::shared_ptr get(threading::Reaction& /*reaction*/) { + static inline std::shared_ptr get(const threading::Reaction& /*reaction*/) { return store::ThreadStore>::value == nullptr ? store::DataStore::get() diff --git a/src/dsl/operation/ChronoTask.hpp b/src/dsl/operation/ChronoTask.hpp index fece5299a..c88ce1c0e 100644 --- a/src/dsl/operation/ChronoTask.hpp +++ b/src/dsl/operation/ChronoTask.hpp @@ -51,7 +51,7 @@ namespace dsl { ChronoTask(std::function&& task, NUClear::clock::time_point time, uint64_t id) - : task(task), time(time), id(id) {} + : task(std::move(task)), time(time), id(id) {} /** * @brief Run the task and return true if the time has been updated to run again diff --git a/src/dsl/operation/TypeBind.hpp b/src/dsl/operation/TypeBind.hpp index 3694a82ab..aa3adeed1 100644 --- a/src/dsl/operation/TypeBind.hpp +++ b/src/dsl/operation/TypeBind.hpp @@ -46,17 +46,17 @@ namespace dsl { static inline void bind(const std::shared_ptr& reaction) { // Our unbinder to remove this reaction - reaction->unbinders.push_back([](threading::Reaction& reaction) { + reaction->unbinders.push_back([](const threading::Reaction& r) { auto& vec = store::TypeCallbackStore::get(); - auto item = std::find_if( + auto it = std::find_if( std::begin(vec), std::end(vec), - [&reaction](const std::shared_ptr& r) { return r->id == reaction.id; }); + [&r](const std::shared_ptr& item) { return item->id == r.id; }); // If the item is in the list erase the item - if (item != std::end(vec)) { - vec.erase(item); + if (it != std::end(vec)) { + vec.erase(it); } }); @@ -75,17 +75,17 @@ namespace dsl { reaction->emit_stats = false; // Our unbinder to remove this reaction - reaction->unbinders.push_back([](threading::Reaction& r) { + reaction->unbinders.push_back([](const threading::Reaction& r) { auto& vec = store::TypeCallbackStore::get(); - auto item = std::find_if( + auto it = std::find_if( std::begin(vec), std::end(vec), [&r](const std::shared_ptr& item) { return item->id == r.id; }); // If the item is in the list erase the item - if (item != std::end(vec)) { - vec.erase(item); + if (it != std::end(vec)) { + vec.erase(it); } }); diff --git a/src/dsl/operation/Unbind.hpp b/src/dsl/operation/Unbind.hpp index 39025cda6..956e65686 100644 --- a/src/dsl/operation/Unbind.hpp +++ b/src/dsl/operation/Unbind.hpp @@ -38,7 +38,7 @@ namespace dsl { */ template struct Unbind { - Unbind(uint64_t id) : id(id){}; + explicit Unbind(const uint64_t& id) : id(id){}; /// The id of the task to unbind const uint64_t id{0}; diff --git a/src/dsl/word/Always.hpp b/src/dsl/word/Always.hpp index 39b77e72d..30d3a4e3c 100644 --- a/src/dsl/word/Always.hpp +++ b/src/dsl/word/Always.hpp @@ -72,7 +72,7 @@ namespace dsl { struct Always { template - static inline util::ThreadPoolDescriptor pool(threading::Reaction& reaction) { + static inline util::ThreadPoolDescriptor pool(const threading::Reaction& reaction) { static std::map pool_id; static std::mutex mutex; @@ -112,13 +112,13 @@ namespace dsl { always_reaction->identifiers.reactor, always_reaction->identifiers.dsl, always_reaction->identifiers.function}, - [always_reaction](threading::Reaction& idle_reaction) -> util::GeneratedCallback { - auto callback = [&idle_reaction, always_reaction](threading::ReactionTask& /*task*/) { + [always_reaction](threading::Reaction& ir) -> util::GeneratedCallback { + auto callback = [&ir, always_reaction](const threading::ReactionTask& /*task*/) { // Get a task for the always reaction and submit it to the scheduler always_reaction->reactor.powerplant.submit(always_reaction->get_task()); // Get a task for the idle reaction and submit it to the scheduler - idle_reaction.reactor.powerplant.submit(idle_reaction.get_task()); + ir.reactor.powerplant.submit(ir.get_task()); }; // Make sure that idle reaction always has lower priority than the always reaction diff --git a/src/dsl/word/Buffer.hpp b/src/dsl/word/Buffer.hpp index f233fccb9..9d6a1d19e 100644 --- a/src/dsl/word/Buffer.hpp +++ b/src/dsl/word/Buffer.hpp @@ -49,7 +49,7 @@ namespace dsl { struct Buffer { template - static inline bool precondition(threading::Reaction& reaction) { + static inline bool precondition(const threading::Reaction& reaction) { // We only run if there are less than the target number of active tasks return reaction.active_tasks < (n + 1); } diff --git a/src/dsl/word/Every.hpp b/src/dsl/word/Every.hpp index ba0b10619..14b4cf1f7 100644 --- a/src/dsl/word/Every.hpp +++ b/src/dsl/word/Every.hpp @@ -43,7 +43,7 @@ namespace dsl { template struct Per>> : public clock::duration { - Per(int ticks) + explicit Per(int ticks) : clock::duration(std::lround((double(num) / double(ticks * den)) * (double(clock::period::den) / double(clock::period::num)))) {} }; diff --git a/src/dsl/word/Group.hpp b/src/dsl/word/Group.hpp index f8ea8341a..d8eaa7211 100644 --- a/src/dsl/word/Group.hpp +++ b/src/dsl/word/Group.hpp @@ -72,7 +72,7 @@ namespace dsl { static const util::GroupDescriptor group_descriptor; template - static inline util::GroupDescriptor group(threading::Reaction& /*reaction*/) { + static inline util::GroupDescriptor group(const threading::Reaction& /*reaction*/) { return group_descriptor; } }; diff --git a/src/dsl/word/IO.hpp b/src/dsl/word/IO.hpp index d1c732dd5..e5d45a79f 100644 --- a/src/dsl/word/IO.hpp +++ b/src/dsl/word/IO.hpp @@ -59,7 +59,7 @@ namespace dsl { * @brief This is emitted when an IO operation has finished. */ struct IOFinished { - IOFinished(const uint64_t& id) : id(id) {} + explicit IOFinished(const uint64_t& id) : id(id) {} /// @brief The id of the reaction that has finished uint64_t id; }; @@ -137,14 +137,14 @@ namespace dsl { r.reactor.emit(std::make_unique>(r.id)); }); - auto io_config = std::make_unique(IOConfiguration{fd, watch_set, reaction}); + auto io_config = std::make_unique(fd, watch_set, reaction); // Send our configuration out reaction->reactor.emit(io_config); } template - static inline Event get(threading::Reaction& /*reaction*/) { + static inline Event get(const threading::Reaction& /*reaction*/) { // If our thread store has a value if (ThreadEventStore::value) { diff --git a/src/dsl/word/Last.hpp b/src/dsl/word/Last.hpp index 19d6309d9..1bdb192b6 100644 --- a/src/dsl/word/Last.hpp +++ b/src/dsl/word/Last.hpp @@ -33,15 +33,39 @@ namespace NUClear { namespace dsl { namespace word { + /** + * @brief A class that stores the last received items from a reaction + * + * This class stores the received items and provides conversion operators so that when passed into the reaction + * it can be converted to an appropriate type. + * + * @tparam n The number of items to store. + * @tparam T The type of the items to store. + */ template struct LastItemStorage { - // The items we are storing - std::list list; - - LastItemStorage() : list() {} - - LastItemStorage(T&& data) : list({data}) {} - + LastItemStorage() = default; + + /** + * @brief Constructs a LastItemStorage object with the given data. + * + * @param data The data to store. + */ + explicit LastItemStorage(T&& data) : list({std::move(data)}) {} + /** + * @brief Constructs a LastItemStorage object with the given data. + * + * @param data The data to store. + */ + explicit LastItemStorage(const T& data) : list({data}) {} + + /** + * @brief Converts the stored list to a list of the given type. + * + * @tparam Output The type of the output list. + * + * @return The output list. + */ template operator std::list() const { @@ -54,6 +78,13 @@ namespace dsl { return out; } + /** + * @brief Converts the stored list to a vector of the given type. + * + * @tparam Output The type of the output vector. + * + * @return The output vector. + */ template operator std::vector() const { @@ -66,9 +97,18 @@ namespace dsl { return out; } + /** + * @brief Bool operator to allow the reaction to decide not to run if there is no data. + * + * @return true If the list is not empty. + * @return false If the list is empty. + */ operator bool() const { return !list.empty(); } + + /// The items we are storing + std::list list; }; /** diff --git a/src/dsl/word/MainThread.hpp b/src/dsl/word/MainThread.hpp index 720d974d7..29c91b795 100644 --- a/src/dsl/word/MainThread.hpp +++ b/src/dsl/word/MainThread.hpp @@ -41,7 +41,7 @@ namespace dsl { struct MainThread { template - static inline util::ThreadPoolDescriptor pool(threading::Reaction& /*reaction*/) { + static inline util::ThreadPoolDescriptor pool(const threading::Reaction& /*reaction*/) { return util::ThreadPoolDescriptor{util::ThreadPoolDescriptor::MAIN_THREAD_POOL_ID, 1}; } }; diff --git a/src/dsl/word/Network.hpp b/src/dsl/word/Network.hpp index 29fa9bb17..d796b0699 100644 --- a/src/dsl/word/Network.hpp +++ b/src/dsl/word/Network.hpp @@ -36,22 +36,17 @@ namespace dsl { template struct NetworkData : public std::shared_ptr { NetworkData() : std::shared_ptr() {} - NetworkData(T* ptr) : std::shared_ptr(ptr) {} + explicit NetworkData(T* ptr) : std::shared_ptr(ptr) {} NetworkData(const std::shared_ptr& ptr) : std::shared_ptr(ptr) {} - NetworkData(std::shared_ptr&& ptr) : std::shared_ptr(ptr) {} }; struct NetworkSource { - NetworkSource() = default; - std::string name{}; util::network::sock_t address{}; bool reliable{false}; }; struct NetworkListen { - NetworkListen() = default; - uint64_t hash{0}; std::shared_ptr reaction{nullptr}; }; @@ -99,7 +94,7 @@ namespace dsl { template static inline std::tuple, NetworkData> get( - threading::Reaction& /*reaction*/) { + const threading::Reaction& /*reaction*/) { auto* data = store::ThreadStore>::value; auto* source = store::ThreadStore::value; diff --git a/src/dsl/word/Optional.hpp b/src/dsl/word/Optional.hpp index a80e5f674..8831b6457 100644 --- a/src/dsl/word/Optional.hpp +++ b/src/dsl/word/Optional.hpp @@ -31,7 +31,7 @@ namespace dsl { template struct OptionalWrapper { - OptionalWrapper(T&& d) : d(std::move(d)) {} + explicit OptionalWrapper(T&& d) : d(std::move(d)) {} T operator*() const { return std::move(d); diff --git a/src/dsl/word/Pool.hpp b/src/dsl/word/Pool.hpp index 48fcab3f7..63c539199 100644 --- a/src/dsl/word/Pool.hpp +++ b/src/dsl/word/Pool.hpp @@ -79,7 +79,7 @@ namespace dsl { * @tparam DSL the DSL used for this reaction */ template - static inline util::ThreadPoolDescriptor pool(threading::Reaction& /*reaction*/) { + static inline util::ThreadPoolDescriptor pool(const threading::Reaction& /*reaction*/) { return pool_descriptor; } }; diff --git a/src/dsl/word/Priority.hpp b/src/dsl/word/Priority.hpp index 9efa92143..d30f93340 100644 --- a/src/dsl/word/Priority.hpp +++ b/src/dsl/word/Priority.hpp @@ -72,7 +72,7 @@ namespace dsl { static constexpr int value = 1000; template - static inline int priority(threading::Reaction& /*reaction*/) { + static inline int priority(const threading::Reaction& /*reaction*/) { return value; } }; @@ -82,7 +82,7 @@ namespace dsl { static constexpr int value = 750; template - static inline int priority(threading::Reaction& /*reaction*/) { + static inline int priority(const threading::Reaction& /*reaction*/) { return value; } }; @@ -92,7 +92,7 @@ namespace dsl { static constexpr int value = 500; template - static inline int priority(threading::Reaction& /*reaction*/) { + static inline int priority(const threading::Reaction& /*reaction*/) { return value; } }; @@ -102,7 +102,7 @@ namespace dsl { static constexpr int value = 250; template - static inline int priority(threading::Reaction& /*reaction*/) { + static inline int priority(const threading::Reaction& /*reaction*/) { return value; } }; @@ -112,7 +112,7 @@ namespace dsl { static constexpr int value = 0; template - static inline int priority(threading::Reaction& /*reaction*/) { + static inline int priority(const threading::Reaction& /*reaction*/) { return value; } }; diff --git a/src/dsl/word/TCP.hpp b/src/dsl/word/TCP.hpp index 606ef8a0c..7d01fcaa9 100644 --- a/src/dsl/word/TCP.hpp +++ b/src/dsl/word/TCP.hpp @@ -112,7 +112,7 @@ namespace dsl { // Make our socket util::FileDescriptor fd(::socket(address.sock.sa_family, SOCK_STREAM, IPPROTO_TCP), - [](fd_t fd) { ::shutdown(fd, SHUT_RDWR); }); + [](const fd_t& f) { ::shutdown(f, SHUT_RDWR); }); if (!fd.valid()) { throw std::system_error(network_errno, std::system_category(), "Unable to open the TCP socket"); @@ -179,7 +179,7 @@ namespace dsl { // Accept the remote connection socklen_t remote_size = sizeof(util::network::sock_t); util::FileDescriptor fd(::accept(event.fd, &remote.sock, &remote_size), - [](fd_t fd) { ::shutdown(fd, SHUT_RDWR); }); + [](const fd_t& f) { ::shutdown(f, SHUT_RDWR); }); // Get our local address socklen_t local_size = sizeof(util::network::sock_t); diff --git a/src/dsl/word/UDP.hpp b/src/dsl/word/UDP.hpp index 21c982879..5c5d72ec9 100644 --- a/src/dsl/word/UDP.hpp +++ b/src/dsl/word/UDP.hpp @@ -24,6 +24,7 @@ #define NUCLEAR_DSL_WORD_UDP_HPP #include +#include #include "../../PowerPlant.hpp" #include "../../threading/Reaction.hpp" @@ -72,7 +73,9 @@ namespace dsl { */ struct ConnectOptions { /// @brief The type of connection we are making - enum Type { UNICAST, BROADCAST, MULTICAST } type{}; + enum class Type { UNICAST, BROADCAST, MULTICAST }; + /// @brief The type of connection we are making + Type type{}; /// @brief The address we are binding to or empty for any std::string bind_address{}; /// @brief The port we are binding to or 0 for any @@ -144,7 +147,7 @@ namespace dsl { // Resolve the addresses util::network::sock_t bind_address{}; util::network::sock_t multicast_target{}; - if (options.type == ConnectOptions::MULTICAST) { + if (options.type == ConnectOptions::Type::MULTICAST) { multicast_target = util::network::resolve(options.target_address, options.port); // If there is no bind address, make sure we bind to an address of the same family @@ -159,13 +162,13 @@ namespace dsl { bind_address.ipv6.sin6_port = htons(options.port); bind_address.ipv6.sin6_addr = in6addr_any; } break; - default: throw std::runtime_error("Unknown socket family"); + default: throw std::invalid_argument("Unknown socket family"); } } else { bind_address = util::network::resolve(options.bind_address, options.port); if (multicast_target.sock.sa_family != bind_address.sock.sa_family) { - throw std::runtime_error("Multicast address family does not match bind address family"); + throw std::invalid_argument("Multicast address family does not match bind address family"); } } } @@ -210,7 +213,8 @@ namespace dsl { } // Broadcast and multicast reuse address and port - if (options.type == ConnectOptions::BROADCAST || options.type == ConnectOptions::MULTICAST) { + if (options.type == ConnectOptions::Type::BROADCAST + || options.type == ConnectOptions::Type::MULTICAST) { if (::setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, reinterpret_cast(&yes), sizeof(yes)) < 0) { throw std::system_error(network_errno, @@ -241,7 +245,7 @@ namespace dsl { } // If we have a multicast address, then we need to join the multicast groups - if (options.type == ConnectOptions::MULTICAST) { + if (options.type == ConnectOptions::Type::MULTICAST) { // Our multicast join request will depend on protocol version if (multicast_target.sock.sa_family == AF_INET) { @@ -323,7 +327,7 @@ namespace dsl { port = ntohs(bind_address.ipv6.sin6_port); } else { - throw std::runtime_error("Unknown socket family"); + throw std::invalid_argument("Unknown socket family"); } // Generate a reaction for the IO system that closes on death @@ -389,7 +393,7 @@ namespace dsl { && cmsg->cmsg_type == IP_PKTINFO) { // Access the packet header information - auto* pi = reinterpret_cast(reinterpret_cast(cmsg) + sizeof(*cmsg)); + const auto* pi = reinterpret_cast(reinterpret_cast(cmsg) + sizeof(*cmsg)); local.ipv4.sin_family = AF_INET; local.ipv4.sin_addr = pi->ipi_addr; @@ -402,7 +406,7 @@ namespace dsl { && cmsg->cmsg_type == IPV6_PKTINFO) { // Access the packet header information - auto* pi = reinterpret_cast(reinterpret_cast(cmsg) + sizeof(*cmsg)); + const auto* pi = reinterpret_cast(reinterpret_cast(cmsg) + sizeof(*cmsg)); local.ipv6.sin6_addr = pi->ipi6_addr; // We are done @@ -417,7 +421,7 @@ namespace dsl { static inline std::tuple bind(const std::shared_ptr& reaction, const in_port_t& port = 0, const std::string& bind_address = "") { - return connect(reaction, ConnectOptions{ConnectOptions::UNICAST, bind_address, port, ""}); + return connect(reaction, ConnectOptions{ConnectOptions::Type::UNICAST, bind_address, port, ""}); } template @@ -462,7 +466,7 @@ namespace dsl { const in_port_t& port = 0, const std::string& bind_address = "") { return UDP::connect(reaction, - ConnectOptions{ConnectOptions::BROADCAST, bind_address, port, ""}); + ConnectOptions{ConnectOptions::Type::BROADCAST, bind_address, port, ""}); } template @@ -509,7 +513,7 @@ namespace dsl { const std::string& bind_address = "") { return UDP::connect( reaction, - ConnectOptions{ConnectOptions::MULTICAST, bind_address, port, multicast_group}); + ConnectOptions{ConnectOptions::Type::MULTICAST, bind_address, port, multicast_group}); } template diff --git a/src/dsl/word/Watchdog.hpp b/src/dsl/word/Watchdog.hpp index e5d4f39a6..0987018e0 100644 --- a/src/dsl/word/Watchdog.hpp +++ b/src/dsl/word/Watchdog.hpp @@ -23,6 +23,8 @@ #ifndef NUCLEAR_DSL_WORD_WATCHDOG_HPP #define NUCLEAR_DSL_WORD_WATCHDOG_HPP +#include + #include "../../threading/Reaction.hpp" #include "../../util/demangle.hpp" #include "../operation/Unbind.hpp" @@ -71,9 +73,9 @@ namespace dsl { */ static const NUClear::clock::time_point& get(const RuntimeType& data) { if (WatchdogStore::get() == nullptr || WatchdogStore::get()->count(data) == 0) { - throw std::runtime_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) + ", " - + util::demangle(typeid(MapType).name()) - + "> is trying to field a service call for an unknown data type"); + throw std::domain_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) + ", " + + util::demangle(typeid(MapType).name()) + + "> is trying to field a service call for an unknown data type"); } return WatchdogStore::get()->at(data); } @@ -118,8 +120,8 @@ namespace dsl { */ static const NUClear::clock::time_point& get() { if (WatchdogStore::get() == nullptr) { - throw std::runtime_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) - + "> is trying to field a service call for an unknown data type"); + throw std::domain_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) + + "> is trying to field a service call for an unknown data type"); } return *WatchdogStore::get(); } diff --git a/src/dsl/word/emit/Delay.hpp b/src/dsl/word/emit/Delay.hpp index b84b0e3f7..e683910e2 100644 --- a/src/dsl/word/emit/Delay.hpp +++ b/src/dsl/word/emit/Delay.hpp @@ -60,7 +60,7 @@ namespace dsl { // Our chrono task is just to do a normal emit in the amount of time auto msg = std::make_shared( - [&powerplant, data](NUClear::clock::time_point&) { + [&powerplant, data](const NUClear::clock::time_point&) { // Do the emit emit::Local::emit(powerplant, data); @@ -80,7 +80,7 @@ namespace dsl { // Our chrono task is just to do a normal emit in the amount of time auto msg = std::make_shared( - [&powerplant, data](NUClear::clock::time_point&) { + [&powerplant, data](const NUClear::clock::time_point&) { // Do the emit emit::Local::emit(powerplant, data); diff --git a/src/dsl/word/emit/UDP.hpp b/src/dsl/word/emit/UDP.hpp index 640bfacf3..d6b116369 100644 --- a/src/dsl/word/emit/UDP.hpp +++ b/src/dsl/word/emit/UDP.hpp @@ -23,6 +23,8 @@ #ifndef NUCLEAR_DSL_WORD_EMIT_UDP_HPP #define NUCLEAR_DSL_WORD_EMIT_UDP_HPP +#include + #include "../../../PowerPlant.hpp" #include "../../../util/FileDescriptor.hpp" #include "../../../util/network/if_number_from_address.hpp" @@ -60,7 +62,7 @@ namespace dsl { template struct UDP { - static inline void emit(PowerPlant& /*powerplant*/, + static inline void emit(const PowerPlant& /*powerplant*/, std::shared_ptr data, const std::string& to_addr, in_port_t to_port, @@ -88,13 +90,13 @@ namespace dsl { local.ipv6.sin6_port = htons(from_port); local.ipv6.sin6_addr = IN6ADDR_ANY_INIT; } break; - default: throw std::runtime_error("Unknown socket family"); + default: throw std::invalid_argument("Unknown socket family"); } } else { local = util::network::resolve(from_addr, from_port); if (local.sock.sa_family != remote.sock.sa_family) { - throw std::runtime_error("to and from addresses are not the same family"); + throw std::invalid_argument("to and from addresses are not the same family"); } } diff --git a/src/dsl/word/emit/Watchdog.hpp b/src/dsl/word/emit/Watchdog.hpp index 9974f28a2..e94dad020 100644 --- a/src/dsl/word/emit/Watchdog.hpp +++ b/src/dsl/word/emit/Watchdog.hpp @@ -23,6 +23,8 @@ #ifndef NUCLEAR_DSL_WORD_EMIT_WATCHDOG_HPP #define NUCLEAR_DSL_WORD_EMIT_WATCHDOG_HPP +#include + #include "../../../PowerPlant.hpp" #include "../../../util/TypeMap.hpp" #include "../../../util/demangle.hpp" @@ -50,11 +52,20 @@ namespace dsl { using WatchdogStore = util::TypeMap>; - WatchdogServicer() : when(NUClear::clock::now()), data() {} - WatchdogServicer(const RuntimeType& data) : when(NUClear::clock::now()), data(data) {} + /** + * @brief Construct a new Watchdog Servicer object + */ + WatchdogServicer() = default; /** - * @brief * Services the watchdog + * @brief Construct a new Watchdog Servicer object + * + * @param data The runtime argument that was passed to on>() + */ + explicit WatchdogServicer(const RuntimeType& data) : data(data) {} + + /** + * @brief Services the watchdog * * @details * The watchdog timer that is specified by the WatchdogGroup/RuntimeType/data @@ -62,16 +73,18 @@ namespace dsl { */ void service() { if (WatchdogStore::get() == nullptr || WatchdogStore::get()->count(data) == 0) { - throw std::runtime_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) + ", " - + util::demangle(typeid(RuntimeType).name()) - + "> has not been created yet or no watchdog has been set up"); + throw std::domain_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) + ", " + + util::demangle(typeid(RuntimeType).name()) + + "> has not been created yet or no watchdog has been set up"); } WatchdogStore::get()->at(data) = when; } private: - NUClear::clock::time_point when; - RuntimeType data; + /// @brief The time when the watchdog was serviced + NUClear::clock::time_point when{NUClear::clock::now()}; + /// @brief The runtime argument that was passed to on>() + RuntimeType data{}; }; /** @@ -88,8 +101,6 @@ namespace dsl { struct WatchdogServicer { using WatchdogStore = util::TypeMap; - WatchdogServicer() : when(NUClear::clock::now()) {} - /** * @brief * Services the watchdog * @@ -99,14 +110,14 @@ namespace dsl { */ void service() { if (WatchdogStore::get() == nullptr) { - throw std::runtime_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) - + "> has not been created yet or no watchdog has been set up"); + throw std::domain_error("Store for <" + util::demangle(typeid(WatchdogGroup).name()) + + "> has not been created yet or no watchdog has been set up"); } WatchdogStore::set(std::make_shared(when)); } private: - NUClear::clock::time_point when; + NUClear::clock::time_point when{NUClear::clock::now()}; }; /** @@ -157,7 +168,8 @@ namespace dsl { struct Watchdog { template - static void emit(PowerPlant& /*powerplant*/, WatchdogServicer& servicer) { + static void emit(const PowerPlant& /*powerplant*/, + WatchdogServicer& servicer) { // Update our service time servicer.service(); } diff --git a/src/extension/ChronoController.hpp b/src/extension/ChronoController.hpp index 47d1a7fc7..50ff94fb8 100644 --- a/src/extension/ChronoController.hpp +++ b/src/extension/ChronoController.hpp @@ -138,8 +138,8 @@ namespace extension { } } else { - // Spinlock until we get to the time while (NUClear::clock::now() < tasks.front().time) { + // Spinlock until we get to the time } // Run our task and if it returns false remove it diff --git a/src/extension/IOController_Posix.hpp b/src/extension/IOController_Posix.hpp index 870f9d2d1..fdd5bc1b0 100644 --- a/src/extension/IOController_Posix.hpp +++ b/src/extension/IOController_Posix.hpp @@ -114,8 +114,6 @@ namespace extension { * @brief Fires the event for the task if it is ready * * @param task the task to try to fire the event for - * - * @return the iterator to the next task in the list */ void fire_event(Task& task) { if (task.processing_events == 0 && task.waiting_events != 0) { @@ -165,7 +163,7 @@ namespace extension { throw std::system_error(network_errno, std::system_category(), "There was an error reading our notification pipe?"); - }; + } } // It's a regular handle else { diff --git a/src/extension/IOController_Windows.hpp b/src/extension/IOController_Windows.hpp index 734a40f6c..78a7a90e1 100644 --- a/src/extension/IOController_Windows.hpp +++ b/src/extension/IOController_Windows.hpp @@ -84,8 +84,6 @@ namespace extension { * @brief Fires the event for the task if it is ready * * @param task the task to try to fire the event for - * - * @return the iterator to the next task in the list */ void fire_event(Task& task) { if (task.processing_events == 0 && task.waiting_events != 0) { diff --git a/src/extension/NetworkController.hpp b/src/extension/NetworkController.hpp index 1a29bef82..32b3213b4 100644 --- a/src/extension/NetworkController.hpp +++ b/src/extension/NetworkController.hpp @@ -53,13 +53,13 @@ namespace extension { const bool& reliable, std::vector&& payload) { // Construct our NetworkSource information - dsl::word::NetworkSource src; - src.name = remote.name; - src.address = remote.target; - src.reliable = reliable; + dsl::word::NetworkSource src{remote.name, remote.target, reliable}; + + // Move the payload in as we are stealing it + std::vector p(std::move(payload)); // Store in our thread local cache - dsl::store::ThreadStore>::value = &payload; + dsl::store::ThreadStore>::value = &p; dsl::store::ThreadStore::value = &src; /* Mutex Scope */ { diff --git a/src/extension/network/NUClearNetwork.cpp b/src/extension/network/NUClearNetwork.cpp index ed1c41570..5aa6796a7 100644 --- a/src/extension/network/NUClearNetwork.cpp +++ b/src/extension/network/NUClearNetwork.cpp @@ -330,8 +330,9 @@ namespace extension { shutdown(); // Lock all mutexes - const std::lock_guard target_lock(target_mutex); - const std::lock_guard send_lock(send_queue_mutex); + std::lock(target_mutex, send_queue_mutex); + const std::lock_guard target_lock(target_mutex, std::adopt_lock); + const std::lock_guard send_lock(send_queue_mutex, std::adopt_lock); // Clear all our data structures send_queue.clear(); @@ -353,14 +354,14 @@ namespace extension { bind_target.ipv6.sin6_addr = IN6ADDR_ANY_INIT; } else { - throw std::runtime_error("Unknown address family"); + throw std::invalid_argument("Unknown address family"); } } else { bind_target = util::network::resolve(bind_address, port); // If the family doesn't match, throw an error if (bind_target.sock.sa_family != announce_target.sock.sa_family) { - throw std::runtime_error("Bind address family does not match announce address family"); + throw std::invalid_argument("Bind address family does not match announce address family"); } } @@ -439,7 +440,7 @@ namespace extension { } // Run the callback for anyone that left - for (auto& l : leavers) { + for (const auto& l : leavers) { leave_callback(*l); } @@ -471,8 +472,9 @@ namespace extension { void NUClearNetwork::retransmit() { // Locking send_queue_mutex second after target_mutex - const std::lock_guard target_lock(target_mutex); - const std::lock_guard send_lock(send_queue_mutex); + std::lock(target_mutex, send_queue_mutex); + const std::lock_guard target_lock(target_mutex, std::adopt_lock); + const std::lock_guard send_lock(send_queue_mutex, std::adopt_lock); for (auto qit = send_queue.begin(); qit != send_queue.end();) { for (auto it = qit->second.targets.begin(); it != qit->second.targets.end();) { @@ -756,7 +758,7 @@ namespace extension { response.packet_count = packet.packet_count; // Set the bits for the packets we thought we received - for (auto& p : assembler.second) { + for (const auto& p : assembler.second) { (&response.packets)[p.first / 8] |= uint8_t(1 << (p.first % 8)); } @@ -796,7 +798,7 @@ namespace extension { response.packet_count = packet.packet_count; // Set the bits for the packets we have received - for (auto& p : assembler.second) { + for (const auto& p : assembler.second) { (&response.packets)[p.first / 8] |= uint8_t(1 << (p.first % 8)); } @@ -818,7 +820,7 @@ namespace extension { // Work out exactly how much data we will need first so we only need one // allocation size_t payload_size = 0; - for (auto& p : assembler.second) { + for (const auto& p : assembler.second) { payload_size += p.second.size() - sizeof(DataPacket) + 1; } @@ -1001,8 +1003,8 @@ namespace extension { // Our packet we are sending msghdr message{}; - iovec data[2]; // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) - message.msg_iov = static_cast(data); + std::array data{}; + message.msg_iov = data.data(); message.msg_iovlen = 2; // Update our headers packet number and set it in the message @@ -1035,7 +1037,6 @@ namespace extension { throw std::runtime_error("Cannot send messages as the network is not connected"); } - // The header for our packet DataPacket header; @@ -1054,9 +1055,9 @@ namespace extension { // If this was a reliable packet we need to cache it in case it needs to be resent if (reliable) { - - const std::lock_guard lock_target(target_mutex); - const std::lock_guard lock_send(send_queue_mutex); + std::lock(target_mutex, send_queue_mutex); + const std::lock_guard lock_target(target_mutex, std::adopt_lock); + const std::lock_guard lock_send(send_queue_mutex, std::adopt_lock); auto& queue = send_queue[header.packet_id]; diff --git a/src/extension/network/NUClearNetwork.hpp b/src/extension/network/NUClearNetwork.hpp index c7b584c4e..0f46bfe21 100644 --- a/src/extension/network/NUClearNetwork.hpp +++ b/src/extension/network/NUClearNetwork.hpp @@ -51,9 +51,10 @@ namespace extension { public: struct NetworkTarget { - NetworkTarget(std::string name, - sock_t target, - std::chrono::steady_clock::time_point last_update = std::chrono::steady_clock::now()) + NetworkTarget( + std::string name, + const sock_t& target, + const std::chrono::steady_clock::time_point& last_update = std::chrono::steady_clock::now()) : name(std::move(name)), target(target), last_update(last_update) { // Set our recent packets to an invalid value @@ -77,13 +78,15 @@ namespace extension { std::pair>>> assemblers{}; - /// A little kalman filter for estimating round trip time + /// Struct storing the kalman filter for round trip time struct RoundTripKF { float process_noise = 1e-6f; float measurement_noise = 1e-1f; float variance = 1.0f; float mean = 1.0f; - } round_trip_kf; + }; + /// A little kalman filter for estimating round trip time + RoundTripKF round_trip_kf{}; std::chrono::steady_clock::duration round_trip_time{std::chrono::seconds(1)}; @@ -94,10 +97,10 @@ namespace extension { std::chrono::duration_cast>>(time); // Alias variables - auto& Q = round_trip_kf.process_noise; - auto& R = round_trip_kf.measurement_noise; - auto& P = round_trip_kf.variance; - auto& X = round_trip_kf.mean; + const auto& Q = round_trip_kf.process_noise; + const auto& R = round_trip_kf.measurement_noise; + auto& P = round_trip_kf.variance; + auto& X = round_trip_kf.mean; // Calculate our kalman gain const float K = (P + Q) / (P + Q + R); @@ -337,7 +340,7 @@ namespace extension { std::list> targets; /// A map of string names to targets with that name - std::multimap> name_target; + std::multimap, std::less<>> name_target; /// A map of ip/port pairs to the network target they belong to std::map, std::shared_ptr> udp_target; diff --git a/src/extension/network/wire_protocol.hpp b/src/extension/network/wire_protocol.hpp index 96b36b0e0..f9832de62 100644 --- a/src/extension/network/wire_protocol.hpp +++ b/src/extension/network/wire_protocol.hpp @@ -23,61 +23,94 @@ #ifndef NUCLEAR_EXTENSION_NETWORK_WIRE_PROTOCOL_HPP #define NUCLEAR_EXTENSION_NETWORK_WIRE_PROTOCOL_HPP +#include + +// These macros are used to pack the structs so that they are sent over the network in the correct format +#ifdef _MSC_VER + // NOLINTNEXTLINE(cppcoreguidelines-macro-usage) + #define PACK(...) __pragma(pack(push, 1)) __VA_ARGS__ __pragma(pack(pop)) +#else + // NOLINTNEXTLINE(cppcoreguidelines-macro-usage) + #define PACK(...) __VA_ARGS__ __attribute__((__packed__)) +#endif + namespace NUClear { namespace extension { namespace network { -#pragma pack(push, 1) + /** + * @brief A number that is used to represent the type of packet that is being sent/received + */ enum Type : uint8_t { ANNOUNCE = 1, LEAVE = 2, DATA = 3, DATA_RETRANSMISSION = 4, ACK = 5, NACK = 6 }; - struct PacketHeader { - PacketHeader(const Type& t) : type(t) {} - - // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) - uint8_t header[3] = {0xE2, 0x98, 0xA2}; // Radioactive symbol in UTF8 - uint8_t version = 0x02; // The NUClear networking version - Type type; // The type of packet - }; - - struct AnnouncePacket : public PacketHeader { - AnnouncePacket() : PacketHeader(ANNOUNCE) {} - - char name{0}; // A null terminated string name for this node (&name) - }; - - struct LeavePacket : public PacketHeader { - LeavePacket() : PacketHeader(LEAVE) {} - }; - struct DataPacket : public PacketHeader { - DataPacket() : PacketHeader(DATA) {} + /** + * @brief The header that is sent with every packet + */ + PACK(struct PacketHeader { + explicit PacketHeader(const Type& t) : type(t) {} - uint16_t packet_id{0}; // A semi-unique identifier for this packet group - uint16_t packet_no{0}; // What packet number this is within the group - uint16_t packet_count{1}; // How many packets there are in the group - bool reliable{false}; // If this packet is reliable and should be acked - uint64_t hash{0}; // The 64 bit hash to identify the data type - char data{0}; // The data (&data) - }; - - struct ACKPacket : public PacketHeader { - ACKPacket() : PacketHeader(ACK) {} - - uint16_t packet_id{0}; // The packet group identifier we are acknowledging - uint16_t packet_no{0}; // The index of the packet we are acknowledging - uint16_t packet_count{1}; // How many packets there are in the group - uint8_t packets{0}; // A bitset of which packets we have received (&packets) - }; - - struct NACKPacket : public PacketHeader { - - NACKPacket() : PacketHeader(NACK) {} - - uint16_t packet_id{0}; // The packet group identifier we are acknowledging - uint16_t packet_count{1}; // How many packets there are in the group - uint8_t packets{0}; // A bitset of which packets we have received (&packets) - }; - -#pragma pack(pop) + /// Radioactive symbol in UTF8 + // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) + uint8_t header[3] = {0xE2, 0x98, 0xA2}; + /// The NUClear networking version + uint8_t version = 0x02; + /// The type of packet + Type type; + }); + + PACK(struct AnnouncePacket + : public PacketHeader { + AnnouncePacket() : PacketHeader(ANNOUNCE) {} + + // A null terminated string name for this node (&name) + char name{0}; + }); + + PACK(struct LeavePacket : public PacketHeader{LeavePacket(): PacketHeader(LEAVE){}}); + + PACK(struct DataPacket + : public PacketHeader { + DataPacket() : PacketHeader(DATA) {} + + // A semi-unique identifier for this packet group + uint16_t packet_id{0}; + // What packet number this is within the group + uint16_t packet_no{0}; + // How many packets there are in the group + uint16_t packet_count{1}; + // If this packet is reliable and should be acked + bool reliable{false}; + // The 64 bit hash to identify the data type + uint64_t hash{0}; + // The data (access using &data) + char data{0}; + }); + + PACK(struct ACKPacket + : public PacketHeader { + ACKPacket() : PacketHeader(ACK) {} + + /// The packet group identifier we are acknowledging + uint16_t packet_id{0}; + /// The index of the packet we are acknowledging + uint16_t packet_no{0}; + /// How many packets there are in the group + uint16_t packet_count{1}; + /// A bitset of which packets we have received (access using &packets) + uint8_t packets{0}; + }); + + PACK(struct NACKPacket + : public PacketHeader { + NACKPacket() : PacketHeader(NACK) {} + + /// The packet group identifier we are acknowledging + uint16_t packet_id{0}; + /// How many packets there are in the group + uint16_t packet_count{1}; + /// A bitset of which packets we have received (access using &packets) + uint8_t packets{0}; + }); } // namespace network } // namespace extension diff --git a/src/threading/Reaction.cpp b/src/threading/Reaction.cpp index f1cbb4ab8..b165f40a4 100644 --- a/src/threading/Reaction.cpp +++ b/src/threading/Reaction.cpp @@ -29,7 +29,7 @@ namespace threading { std::atomic Reaction::reaction_id_source(0); // NOLINT Reaction::Reaction(Reactor& reactor, ReactionIdentifiers&& identifiers, TaskGenerator&& generator) - : reactor(reactor), identifiers(identifiers), id(++reaction_id_source), generator(generator) {} + : reactor(reactor), identifiers(std::move(identifiers)), generator(std::move(generator)) {} void Reaction::unbind() { // Unbind diff --git a/src/threading/Reaction.hpp b/src/threading/Reaction.hpp index 8908d2129..f5f651c0b 100644 --- a/src/threading/Reaction.hpp +++ b/src/threading/Reaction.hpp @@ -107,7 +107,7 @@ namespace threading { ReactionIdentifiers identifiers; /// @brief the unique identifier for this Reaction object - const uint64_t id; + const uint64_t id{++reaction_id_source}; /// @brief if this is false, we cannot emit ReactionStatistics from any reaction triggered by this one bool emit_stats{true}; diff --git a/src/threading/ReactionHandle.hpp b/src/threading/ReactionHandle.hpp index 837793b6c..fd3f7d6f9 100644 --- a/src/threading/ReactionHandle.hpp +++ b/src/threading/ReactionHandle.hpp @@ -54,7 +54,7 @@ namespace threading { * @param * context the reaction that we are interacting with. */ - ReactionHandle(const std::shared_ptr& context = nullptr) : context(context) {} + explicit ReactionHandle(const std::shared_ptr& context = nullptr) : context(context) {} /** * @brief diff --git a/src/threading/ReactionTask.hpp b/src/threading/ReactionTask.hpp index dc69826ba..27b0c3f15 100644 --- a/src/threading/ReactionTask.hpp +++ b/src/threading/ReactionTask.hpp @@ -83,7 +83,6 @@ namespace threading { const util::ThreadPoolDescriptor& thread_pool_descriptor, TaskFunction&& callback) : parent(parent) - , id(new_task_id()) , priority(priority) , stats(std::make_shared(parent.identifiers, parent.id, @@ -97,7 +96,7 @@ namespace threading { , emit_stats(parent.emit_stats && (current_task != nullptr ? current_task->emit_stats : true)) , group_descriptor(group_descriptor) , thread_pool_descriptor(thread_pool_descriptor) - , callback(callback) {} + , callback(std::move(callback)) {} /** @@ -129,7 +128,7 @@ namespace threading { /// @brief the parent Reaction object which spawned this ReactionType& parent; /// @brief the task id of this task (the sequence number of this particular task) - uint64_t id; + uint64_t id{new_task_id()}; /// @brief the priority to run this task at int priority; /// @brief the statistics object that persists after this for information and debugging diff --git a/src/threading/TaskScheduler.cpp b/src/threading/TaskScheduler.cpp index 79db69803..8cf1d3dd2 100644 --- a/src/threading/TaskScheduler.cpp +++ b/src/threading/TaskScheduler.cpp @@ -252,7 +252,7 @@ namespace threading { // So if we can't find a task to run we should just quit. if (!running.load()) { condition.notify_all(); - throw std::runtime_error("Task scheduler has shutdown"); + throw ShutdownThreadException(); } } @@ -263,7 +263,8 @@ namespace threading { condition.wait(lock); // NOSONAR } - throw std::runtime_error("Task scheduler has shutdown"); + // If we get out here then we are finished running. + throw ShutdownThreadException(); } // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) diff --git a/src/threading/TaskScheduler.hpp b/src/threading/TaskScheduler.hpp index 3c9c26dbb..e58e17197 100644 --- a/src/threading/TaskScheduler.hpp +++ b/src/threading/TaskScheduler.hpp @@ -83,6 +83,11 @@ namespace threading { */ class TaskScheduler { private: + /** + * @brief Exception thrown when a thread in the pool should shut down. + */ + class ShutdownThreadException : public std::exception {}; + /** * @brief A struct which contains all the information about an individual task */ @@ -113,7 +118,7 @@ namespace threading { * @brief A struct which contains all the information about an individual thread pool */ struct PoolQueue { - PoolQueue(const util::ThreadPoolDescriptor& pool_descriptor) : pool_descriptor(pool_descriptor) {} + explicit PoolQueue(const util::ThreadPoolDescriptor& pool_descriptor) : pool_descriptor(pool_descriptor) {} /// @brief The descriptor for this thread pool const util::ThreadPoolDescriptor pool_descriptor; /// @brief The threads which are running in this thread pool @@ -130,7 +135,7 @@ namespace threading { /** * @brief Constructs a new TaskScheduler instance, and builds the nullptr sync queue. */ - TaskScheduler(const size_t& default_thread_count); + explicit TaskScheduler(const size_t& default_thread_count); /** * @brief Starts the scheduler, and begins executing tasks. diff --git a/src/util/CallbackGenerator.hpp b/src/util/CallbackGenerator.hpp index 319d25bb2..7234d13e7 100644 --- a/src/util/CallbackGenerator.hpp +++ b/src/util/CallbackGenerator.hpp @@ -38,12 +38,12 @@ namespace NUClear { namespace util { template - inline typename std::enable_if::type check_data(const std::tuple& /*t*/) { + inline std::enable_if_t<(I == sizeof...(T)), bool> check_data(const std::tuple& /*t*/) { return true; } template - inline typename std::enable_if < I::type check_data(const std::tuple& t) { + inline std::enable_if_t<(I < sizeof...(T)), bool> check_data(const std::tuple& t) { return std::get(t) && check_data(t); } @@ -52,14 +52,11 @@ namespace util { struct CallbackGenerator { // Don't use this constructor if F is of type CallbackGenerator - template ::type>::type, - CallbackGenerator>::value, - bool>::type = true> - CallbackGenerator(F&& callback) - : callback(std::forward(callback)) - , transients(std::make_shared::type>()) {} + template < + typename F, + std::enable_if_t>, CallbackGenerator>::value, + bool> = true> + explicit CallbackGenerator(F&& callback) : callback(std::forward(callback)) {} template void merge_transients(std::tuple& data, @@ -108,7 +105,7 @@ namespace util { return GeneratedCallback(DSL::priority(r), DSL::group(r), DSL::pool(r), - [c, data](threading::ReactionTask& task) { + [c, data](threading::ReactionTask& task) noexcept { // Update our thread's priority to the correct level update_current_thread_priority(task.priority); @@ -121,7 +118,6 @@ namespace util { util::apply_relevant(c, std::move(data)); } catch (...) { - // Catch our exception if it happens task.stats->exception = std::current_exception(); } @@ -143,7 +139,8 @@ namespace util { } Function callback; - std::shared_ptr::type> transients; + std::shared_ptr::type> transients{ + std::make_shared::type>()}; }; } // namespace util diff --git a/src/util/Dereferencer.hpp b/src/util/Dereferencer.hpp index c314afd5b..84a084824 100644 --- a/src/util/Dereferencer.hpp +++ b/src/util/Dereferencer.hpp @@ -55,7 +55,7 @@ namespace util { T& value; public: - Dereferencer(T& value) : value(value) {} + explicit Dereferencer(T& value) : value(value) {} // We can return the type as normal operator T&() { diff --git a/src/util/Sequence.hpp b/src/util/Sequence.hpp index 55bb10865..070e1948d 100644 --- a/src/util/Sequence.hpp +++ b/src/util/Sequence.hpp @@ -36,34 +36,30 @@ namespace util { static constexpr int length = sizeof...(S); }; - // Anonymous NOLINTNEXTLINE(cert-dcl59-cpp,google-build-namespaces) - namespace { - - /** - * @brief Generate a sequence of numbers between a start and an end, this is the entry case - */ - template > - struct GenSequence; + /** + * @brief Generate a sequence of numbers between a start and an end, this is the entry case + */ + template > + struct GenSequence; - /** - * Generates sequence - */ - template - struct GenSequence> - : std::conditional_t<(Start > End), - GenSequence<0, - 0> // If we have been given an invalid sequence, just return an empty one - , - GenSequence>> {}; + /** + * Generates sequence + */ + template + struct GenSequence> + : std::conditional_t<(Start > End), + GenSequence<0, + 0> // If we have been given an invalid sequence, just return an empty one + , + GenSequence>> {}; - /** - * Runs when start and end are the same, terminates - */ - template - struct GenSequence> { - using type = Sequence; - }; - } // namespace + /** + * Runs when start and end are the same, terminates + */ + template + struct GenSequence> { + using type = Sequence; + }; /** * @brief Holds a generated integer sequence of numbers as a variadic pack. diff --git a/src/util/network/get_interfaces.hpp b/src/util/network/get_interfaces.hpp index 501696e73..6287810ff 100644 --- a/src/util/network/get_interfaces.hpp +++ b/src/util/network/get_interfaces.hpp @@ -55,7 +55,9 @@ namespace util { bool pointtopoint{false}; /// @brief True if the interface is a multicast interface bool multicast{false}; - } flags; + }; + /// @brief The flags that are set on the interface + Flags flags; }; /** diff --git a/src/util/network/if_number_from_address.cpp b/src/util/network/if_number_from_address.cpp index 9f09f5569..272aeea5e 100644 --- a/src/util/network/if_number_from_address.cpp +++ b/src/util/network/if_number_from_address.cpp @@ -42,7 +42,7 @@ namespace util { } // Find the correct interface to join on (the one that has our bind address) - for (auto& iface : get_interfaces()) { + for (const auto& iface : get_interfaces()) { // iface must be, ipv6, and have the same address as our bind address if (iface.ip.sock.sa_family == AF_INET6 && ::memcmp(iface.ip.ipv6.sin6_addr.s6_addr, ipv6.sin6_addr.s6_addr, sizeof(in6_addr)) == 0) { diff --git a/src/util/network/resolve.cpp b/src/util/network/resolve.cpp index 7a0f2d3d1..73435b41b 100644 --- a/src/util/network/resolve.cpp +++ b/src/util/network/resolve.cpp @@ -53,7 +53,8 @@ namespace util { throw std::runtime_error("Unable to find an address for " + address + ":" + std::to_string(port)); } - std::unique_ptr servinfo(servinfo_ptr, ::freeaddrinfo); + std::unique_ptr servinfo(servinfo_ptr, + [](addrinfo* ptr) { ::freeaddrinfo(ptr); }); // Empty sock_t struct NUClear::util::network::sock_t target{}; diff --git a/src/util/platform.hpp b/src/util/platform.hpp index 94fe691a4..52048ee6f 100644 --- a/src/util/platform.hpp +++ b/src/util/platform.hpp @@ -162,7 +162,7 @@ int sendmsg(fd_t fd, msghdr* msg, int flags); * @brief This struct is used to setup WSA on windows in a single place so we don't have to worry about it * * A single instance of this struct will be created statically at startup which will ensure that WSA is setup for the - * lifetime of the program and torn down as it exists. + * lifetime of the program and torn down as it exits. */ struct WSAHolder { static WSAHolder instance; diff --git a/src/util/serialise/Serialise.hpp b/src/util/serialise/Serialise.hpp index 3262d0241..16c74e0a0 100644 --- a/src/util/serialise/Serialise.hpp +++ b/src/util/serialise/Serialise.hpp @@ -53,7 +53,7 @@ namespace util { static inline std::vector serialise(const T& in) { // Copy the bytes into an array - const char* dataptr = reinterpret_cast(&in); + const auto* dataptr = reinterpret_cast(&in); return {dataptr, dataptr + sizeof(T)}; } diff --git a/src/util/unpack.hpp b/src/util/unpack.hpp index dc7534aab..182079738 100644 --- a/src/util/unpack.hpp +++ b/src/util/unpack.hpp @@ -48,7 +48,9 @@ namespace util { * @tparam Ts the types of the resulting objects from executing the functions, these are ignored */ template - void unpack(Ts... /*t*/) {} + void unpack(Ts... /*t*/) { + // This function is empty because it is only used for its template expansion + } } // namespace util } // namespace NUClear diff --git a/tests/api/ReactionHandle.cpp b/tests/api/ReactionHandle.cpp index 325c4ec3a..e39ac071b 100644 --- a/tests/api/ReactionHandle.cpp +++ b/tests/api/ReactionHandle.cpp @@ -69,7 +69,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing reaction handle functionality", "[api][reactionhandle]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/api/ReactionStatistics.cpp b/tests/api/ReactionStatistics.cpp index 766a0e4c6..b7ac15d7f 100644 --- a/tests/api/ReactionStatistics.cpp +++ b/tests/api/ReactionStatistics.cpp @@ -89,7 +89,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing reaction statistics functionality", "[api][reactionstatistics]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Always.cpp b/tests/dsl/Always.cpp index 18b86d6ea..a639e3ddb 100644 --- a/tests/dsl/Always.cpp +++ b/tests/dsl/Always.cpp @@ -61,7 +61,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("The Always DSL keyword runs continuously when it can", "[api][always]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/ArgumentFission.cpp b/tests/dsl/ArgumentFission.cpp index 6ffdb26fb..114455c8d 100644 --- a/tests/dsl/ArgumentFission.cpp +++ b/tests/dsl/ArgumentFission.cpp @@ -90,7 +90,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing distributing arguments to multiple bind functions (NUClear Fission)", "[api][dsl][fission]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/BlockNoData.cpp b/tests/dsl/BlockNoData.cpp index 6b92822c7..90f1005b7 100644 --- a/tests/dsl/BlockNoData.cpp +++ b/tests/dsl/BlockNoData.cpp @@ -67,7 +67,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing that when an on statement does not have it's data satisfied it does not run", "[api][nodata]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Buffer.cpp b/tests/dsl/Buffer.cpp index a218709d8..eccdcd7ed 100644 --- a/tests/dsl/Buffer.cpp +++ b/tests/dsl/Buffer.cpp @@ -99,7 +99,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Test that Buffer and Single limit the number of concurrent executions", "[api][precondition][single]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/CommandLineArguments.cpp b/tests/dsl/CommandLineArguments.cpp index 00dbed6a3..555ade400 100644 --- a/tests/dsl/CommandLineArguments.cpp +++ b/tests/dsl/CommandLineArguments.cpp @@ -52,7 +52,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing the Command Line argument capturing", "[api][command_line_arguments]") { const int argc = 2; const char* argv[] = {"Hello", "World"}; // NOLINT(cppcoreguidelines-avoid-c-arrays,modernize-avoid-c-arrays) - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config, argc, argv); plant.install(); diff --git a/tests/dsl/CustomGet.cpp b/tests/dsl/CustomGet.cpp index ec2ce60df..4fe966dbd 100644 --- a/tests/dsl/CustomGet.cpp +++ b/tests/dsl/CustomGet.cpp @@ -33,7 +33,7 @@ std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-gl struct CustomGet : public NUClear::dsl::operation::TypeBind { template - static inline std::shared_ptr get(NUClear::threading::Reaction& /*unused*/) { + static inline std::shared_ptr get(const NUClear::threading::Reaction& /*unused*/) { return std::make_shared("Data from a custom getter"); } }; @@ -58,7 +58,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Test a custom reactor that returns a type that needs dereferencing", "[api][custom_get]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/DSLOrdering.cpp b/tests/dsl/DSLOrdering.cpp index 527206acd..4631860e6 100644 --- a/tests/dsl/DSLOrdering.cpp +++ b/tests/dsl/DSLOrdering.cpp @@ -72,7 +72,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing poorly ordered on arguments", "[api][dsl][order][with]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/DSLProxy.cpp b/tests/dsl/DSLProxy.cpp index 0b50242c7..e9a5c763e 100644 --- a/tests/dsl/DSLProxy.cpp +++ b/tests/dsl/DSLProxy.cpp @@ -73,7 +73,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing that the DSL proxy works as expected for binding unmodifyable types", "[api][dsl][proxy]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Every.cpp b/tests/dsl/Every.cpp index 0092d2aff..58fbf7d16 100644 --- a/tests/dsl/Every.cpp +++ b/tests/dsl/Every.cpp @@ -75,7 +75,7 @@ void test_results(const std::vector& times) { TEST_CASE("Testing the Every<> DSL word", "[api][every][per]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/FlagMessage.cpp b/tests/dsl/FlagMessage.cpp index 6a8a876cf..3da36807c 100644 --- a/tests/dsl/FlagMessage.cpp +++ b/tests/dsl/FlagMessage.cpp @@ -72,7 +72,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing emitting types that are flag types (Have no contents)", "[api][flag]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/FusionInOrder.cpp b/tests/dsl/FusionInOrder.cpp index 2ff9f9324..c1953b567 100644 --- a/tests/dsl/FusionInOrder.cpp +++ b/tests/dsl/FusionInOrder.cpp @@ -48,7 +48,7 @@ class TestReactor : public NUClear::Reactor { TEST_CASE("Testing that the bind functions of extensions are executed in order", "[api][extension][bind]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/IO.cpp b/tests/dsl/IO.cpp index 1ad8e610e..bbc48a192 100644 --- a/tests/dsl/IO.cpp +++ b/tests/dsl/IO.cpp @@ -100,7 +100,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing the IO extension", "[api][io]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Last.cpp b/tests/dsl/Last.cpp index 04c975f5c..2b2c26031 100644 --- a/tests/dsl/Last.cpp +++ b/tests/dsl/Last.cpp @@ -61,7 +61,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing the last n feature", "[api][last]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/MainThread.cpp b/tests/dsl/MainThread.cpp index 8e1ca4988..084ddcae4 100644 --- a/tests/dsl/MainThread.cpp +++ b/tests/dsl/MainThread.cpp @@ -67,7 +67,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing that the MainThread keyword runs tasks on the main thread", "[api][dsl][main_thread]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/MissingArguments.cpp b/tests/dsl/MissingArguments.cpp index d362bec54..12d5df159 100644 --- a/tests/dsl/MissingArguments.cpp +++ b/tests/dsl/MissingArguments.cpp @@ -63,7 +63,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing that when arguments missing from the call it can still run", "[api][missing_arguments]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Once.cpp b/tests/dsl/Once.cpp index 38d5b7f7a..476bcb988 100644 --- a/tests/dsl/Once.cpp +++ b/tests/dsl/Once.cpp @@ -66,7 +66,7 @@ class TestReactor : public NUClear::Reactor { TEST_CASE("Reactions with the Once DSL keyword only execute once", "[api][once]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Optional.cpp b/tests/dsl/Optional.cpp index 69458de1e..9a56176c1 100644 --- a/tests/dsl/Optional.cpp +++ b/tests/dsl/Optional.cpp @@ -70,7 +70,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing that optional is able to let data through even if it's invalid", "[api][optional]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Priority.cpp b/tests/dsl/Priority.cpp index feeb85834..7ea837866 100644 --- a/tests/dsl/Priority.cpp +++ b/tests/dsl/Priority.cpp @@ -89,7 +89,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Tests that priority orders the tasks appropriately", "[api][priority]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Raw.cpp b/tests/dsl/Raw.cpp index af778da51..6300a2ce7 100644 --- a/tests/dsl/Raw.cpp +++ b/tests/dsl/Raw.cpp @@ -76,7 +76,7 @@ class TestReactor : public NUClear::Reactor { TEST_CASE("Testing the raw type conversions work properly", "[api][raw]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/RawFunction.cpp b/tests/dsl/RawFunction.cpp index 4d8d6e11d..e485995b5 100644 --- a/tests/dsl/RawFunction.cpp +++ b/tests/dsl/RawFunction.cpp @@ -106,7 +106,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Test reaction can take a raw function instead of just a lambda", "[api][raw_function]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Shutdown.cpp b/tests/dsl/Shutdown.cpp index 68ef1f5bf..0a7016e4a 100644 --- a/tests/dsl/Shutdown.cpp +++ b/tests/dsl/Shutdown.cpp @@ -54,7 +54,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("A test that a shutdown message is emitted when the system shuts down", "[api][shutdown]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Startup.cpp b/tests/dsl/Startup.cpp index 3b4f8bf77..099e4d9a4 100644 --- a/tests/dsl/Startup.cpp +++ b/tests/dsl/Startup.cpp @@ -53,7 +53,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing the startup event is emitted at the start of the program", "[api][startup]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Sync.cpp b/tests/dsl/Sync.cpp index 20746fe81..a63ad26ea 100644 --- a/tests/dsl/Sync.cpp +++ b/tests/dsl/Sync.cpp @@ -99,7 +99,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing that the Sync word works correctly", "[api][sync]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 4; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/SyncOrder.cpp b/tests/dsl/SyncOrder.cpp index 9b178d453..2eac5873a 100644 --- a/tests/dsl/SyncOrder.cpp +++ b/tests/dsl/SyncOrder.cpp @@ -60,7 +60,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Sync events execute in order", "[api][sync][priority]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 4; NUClear::PowerPlant plant(config); diff --git a/tests/dsl/TCP.cpp b/tests/dsl/TCP.cpp index ebd27e6c4..a8571f276 100644 --- a/tests/dsl/TCP.cpp +++ b/tests/dsl/TCP.cpp @@ -80,9 +80,9 @@ class TestReactor : public test_util::TestBase { TestReactor(std::unique_ptr environment) : TestBase(std::move(environment), false) { - // Bind to IPv4 and a known port for (const auto& t : active_tests) { switch (t) { + // Bind to IPv4 and a known port case V4_KNOWN: { on(KNOWN_V4_PORT).then([this](const TCP::Connection& connection) { on(connection.fd, IO::READ | IO::CLOSE).then([this](IO::Event event) { @@ -90,8 +90,8 @@ class TestReactor : public test_util::TestBase { }); }); } break; + // Bind to IPv4 an unknown port and get the port number case V4_EPHEMERAL: { - // Bind to IPv4 an unknown port and get the port number auto v4 = on().then([this](const TCP::Connection& connection) { on(connection.fd, IO::READ | IO::CLOSE).then([this](IO::Event event) { handle_data("v4 Ephemeral", event); @@ -100,7 +100,7 @@ class TestReactor : public test_util::TestBase { v4_port = std::get<1>(v4); } break; - // Bind to IPv6 and a known port + // Bind to IPv6 and a known port case V6_KNOWN: { on(KNOWN_V6_PORT, "::").then([this](const TCP::Connection& connection) { on(connection.fd, IO::READ | IO::CLOSE).then([this](IO::Event event) { @@ -109,7 +109,7 @@ class TestReactor : public test_util::TestBase { }); } break; - // Bind to IPv6 an unknown port and get the port number + // Bind to IPv6 an unknown port and get the port number case V6_EPHEMERAL: { auto v6 = on(0, "::").then([this](const TCP::Connection& connection) { on(connection.fd, IO::READ | IO::CLOSE).then([this](IO::Event event) { @@ -211,7 +211,7 @@ TEST_CASE("Testing listening for TCP connections and receiving data messages", " active_tests.push_back(V6_EPHEMERAL); } - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 2; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Transient.cpp b/tests/dsl/Transient.cpp index 31440f201..9c2ecdb9e 100644 --- a/tests/dsl/Transient.cpp +++ b/tests/dsl/Transient.cpp @@ -131,7 +131,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing whether getters that return transient data can cache between calls", "[api][transient]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Trigger.cpp b/tests/dsl/Trigger.cpp index f0ba2e553..b38b30454 100644 --- a/tests/dsl/Trigger.cpp +++ b/tests/dsl/Trigger.cpp @@ -56,7 +56,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Test that Trigger statements get the correct data", "[api][trigger]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/UDP.cpp b/tests/dsl/UDP.cpp index 5194ba936..8d2f6c8ac 100644 --- a/tests/dsl/UDP.cpp +++ b/tests/dsl/UDP.cpp @@ -368,7 +368,7 @@ TEST_CASE("Testing sending and receiving of UDP messages", "[api][network][udp]" active_tests.push_back(MULTICAST_V6_EPHEMERAL); } - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/Watchdog.cpp b/tests/dsl/Watchdog.cpp index ad359f73b..9023f7b0d 100644 --- a/tests/dsl/Watchdog.cpp +++ b/tests/dsl/Watchdog.cpp @@ -98,7 +98,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing the Watchdog Smart Type", "[api][watchdog]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/With.cpp b/tests/dsl/With.cpp index 9740f8c0a..567785537 100644 --- a/tests/dsl/With.cpp +++ b/tests/dsl/With.cpp @@ -85,7 +85,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing the with dsl keyword", "[api][with]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/emit/Delay.cpp b/tests/dsl/emit/Delay.cpp index 888c74e80..8e5fb5c1a 100644 --- a/tests/dsl/emit/Delay.cpp +++ b/tests/dsl/emit/Delay.cpp @@ -102,7 +102,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing the delay emit", "[api][emit][delay]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/emit/EmitFusion.cpp b/tests/dsl/emit/EmitFusion.cpp index e190c9f8d..5eca9aea6 100644 --- a/tests/dsl/emit/EmitFusion.cpp +++ b/tests/dsl/emit/EmitFusion.cpp @@ -34,22 +34,28 @@ std::vector events; // NOLINT(cppcoreguidelines-avoid-non-const-gl template struct E1 { - static inline void emit(NUClear::PowerPlant& /*unused*/, std::shared_ptr p, const int& a, const std::string& b) { + static inline void emit(const NUClear::PowerPlant& /*powerplant*/, + std::shared_ptr p, + const int& a, + const std::string& b) { events.push_back("E1a " + *p + " " + std::to_string(a) + " " + b); } - static inline void emit(NUClear::PowerPlant& /*unused*/, std::shared_ptr p, const std::string& c) { + static inline void emit(const NUClear::PowerPlant& /*powerplant*/, std::shared_ptr p, const std::string& c) { events.push_back("E1b " + *p + " " + c); } }; template struct E2 { - static inline void emit(NUClear::PowerPlant& /*unused*/, std::shared_ptr p, const bool& d) { + static inline void emit(const NUClear::PowerPlant& /*powerplant*/, std::shared_ptr p, const bool& d) { events.push_back("E2a " + *p + " " + (d ? "true" : "false")); } - static inline void emit(NUClear::PowerPlant& /*unused*/, std::shared_ptr p, const int& e, const std::string& f) { + static inline void emit(const NUClear::PowerPlant& /*powerplant*/, + std::shared_ptr p, + const int& e, + const std::string& f) { events.push_back("E2b " + *p + " " + std::to_string(e) + " " + f); } }; @@ -79,7 +85,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing emit function fusion", "[api][emit][fusion]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/dsl/emit/Initialise.cpp b/tests/dsl/emit/Initialise.cpp index 36e102f45..0af41a4c4 100644 --- a/tests/dsl/emit/Initialise.cpp +++ b/tests/dsl/emit/Initialise.cpp @@ -64,7 +64,7 @@ class TestReactor : public test_util::TestBase { } // namespace TEST_CASE("Testing the Initialize scope", "[api][emit][initialize]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/individual/BaseClock.cpp b/tests/individual/BaseClock.cpp index 40273a1a5..eae611fd6 100644 --- a/tests/individual/BaseClock.cpp +++ b/tests/individual/BaseClock.cpp @@ -71,7 +71,7 @@ TEST_CASE("Testing base clock works correctly", "[api][base_clock]") { STATIC_REQUIRE(std::is_same::value); // Construct the powerplant - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); diff --git a/tests/individual/CustomClock.cpp b/tests/individual/CustomClock.cpp index 62ca2331f..040b6edd6 100644 --- a/tests/individual/CustomClock.cpp +++ b/tests/individual/CustomClock.cpp @@ -69,7 +69,7 @@ class TestReactor : public test_util::TestBase { TEST_CASE("Testing custom clock works correctly", "[api][custom_clock]") { - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); plant.install(); diff --git a/tests/log/Log.cpp b/tests/log/Log.cpp index 7faca8e4a..ca2bbbcfd 100644 --- a/tests/log/Log.cpp +++ b/tests/log/Log.cpp @@ -147,7 +147,7 @@ TEST_CASE("Testing the Log<>() function", "[api][log]") { // Local scope to force powerplant destruction { // Build with one thread - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 1; NUClear::PowerPlant plant(config); diff --git a/tests/networktest.cpp b/tests/networktest.cpp index 7ed596f99..0bebf9190 100644 --- a/tests/networktest.cpp +++ b/tests/networktest.cpp @@ -174,7 +174,7 @@ int main(int argc, const char* argv[]) { return -1; } - NUClear::PowerPlant::Configuration config; + NUClear::Configuration config; config.thread_count = 4; NUClear::PowerPlant plant(config, argc, argv); plant.install();