Refactor tests to use an event style and fix some found bugs

.github/workflows/main.yaml

@@ -24,7 +24,8 @@ jobs:
 
     strategy:
       matrix:
-        container: ['gcc:5', 'gcc:7', 'gcc:9', 'gcc:10', 'gcc:11', 'gcc:12', 'gcc:13']
+        container:
+          ["gcc:5", "gcc:7", "gcc:9", "gcc:10", "gcc:11", "gcc:12", "gcc:13"]
 
     # The type of runner that the job will run on
     runs-on: ubuntu-latest
@@ -33,9 +34,7 @@ jobs:
     # Use the container for this specific version of gcc
     container: ${{ matrix.container }}
 
-    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-      # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - name: Checkout Code
         uses: actions/checkout@v3
 
@@ -57,7 +56,7 @@ jobs:
         run: cmake --build build --config Release --parallel 2
 
       - name: Test
-        timeout-minutes: 5
+        timeout-minutes: 10
         # Execute tests defined by the CMake configuration.
         # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
         run: |
@@ -66,13 +65,9 @@ jobs:
 
   build-osx:
     name: MacOS Clang
-
-    # The type of runner that the job will run on
     runs-on: macos-latest
 
-    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-      # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - name: Checkout Code
         uses: actions/checkout@v3
 
@@ -87,7 +82,7 @@ jobs:
         run: cmake --build build --config Release --parallel 2
 
       - name: Test
-        timeout-minutes: 5
+        timeout-minutes: 10
         # Execute tests defined by the CMake configuration.
         # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
         run: |
@@ -100,9 +95,7 @@ jobs:
     # The type of runner that the job will run on
     runs-on: windows-latest
 
-    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-      # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - name: Checkout Code
         uses: actions/checkout@v3
 
@@ -117,7 +110,7 @@ jobs:
         run: cmake --build build --config Release --parallel 2
 
       - name: Test
-        timeout-minutes: 5
+        timeout-minutes: 10
         # Execute tests defined by the CMake configuration.
         # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail
         run: |
@@ -127,21 +120,17 @@ jobs:
 
   check-clang-tidy-linux:
     name: Clang-Tidy Linux
-
-    # The type of runner that the job will run on
     runs-on: ubuntu-latest
 
-    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
       - name: Install clang-tidy-15
         run: |
-         wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
-         echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main" | sudo tee /etc/apt/sources.list.d/llvm-15
-         echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main" | sudo tee -a /etc/apt/sources.list.d/llvm-15
-         sudo apt-get update
-         sudo apt-get install -y clang-tidy-15
+          wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
+          echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main" | sudo tee /etc/apt/sources.list.d/llvm-15
+          echo "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-15 main" | sudo tee -a /etc/apt/sources.list.d/llvm-15
+          sudo apt-get update
+          sudo apt-get install -y clang-tidy-15
 
-      # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - name: Checkout Code
         uses: actions/checkout@v3
 
@@ -164,13 +153,9 @@ jobs:
 
   check-clang-tidy-msvc:
     name: Clang-Tidy MSVC
-
-    # The type of runner that the job will run on
     runs-on: windows-latest
 
-    # Steps represent a sequence of tasks that will be executed as part of the job
     steps:
-      # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - name: Checkout Code
         uses: actions/checkout@v3
 

CMakeLists.txt

@@ -13,8 +13,14 @@
 # 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.
+cmake_minimum_required(VERSION 3.15.0)
 
-cmake_minimum_required(VERSION 3.1.0)
+# Set the project after the build type as the Project command can change the build type
+project(
+  NUClear
+  VERSION 1.0.0
+  LANGUAGES C CXX
+)
 
 # We use additional modules that cmake needs to know about
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_SOURCE_DIR}/cmake/Modules/")
@@ -35,13 +41,6 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
   set_property(CACHE CMAKE_BUILD_TYPE PROPERTY STRINGS "Debug" "Release" "MinSizeRel" "RelWithDebInfo")
 endif()
 
-# Set the project after the build type as the Project command can change the build type
-project(
-  NUClear
-  VERSION 1.0.0
-  LANGUAGES C CXX
-)
-
 # NUClear targets c++14
 set(CMAKE_CXX_STANDARD 14)
 
@@ -51,6 +50,12 @@ if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
   set(MASTER_PROJECT ON)
 endif()
 
+if(MSVC)
+  add_compile_options(/W4)
+else()
+  add_compile_options(-Wall -Wextra -pedantic)
+endif(MSVC)
+
 # If this option is set we are building using continous integration
 option(CI_BUILD "Enable build options for building in the CI server" OFF)
 

src/Environment.hpp

@@ -40,19 +40,19 @@ class PowerPlant;
  */
 class Environment {
 public:
-    Environment(PowerPlant& powerplant, std::string&& reactor_name, LogLevel log_level)
-        : powerplant(powerplant), log_level(log_level), reactor_name(reactor_name) {}
+    Environment(PowerPlant& powerplant, std::string reactor_name, const LogLevel& log_level)
+        : powerplant(powerplant), reactor_name(std::move(reactor_name)), log_level(log_level) {}
 
 private:
     friend class PowerPlant;
     friend class Reactor;
 
     /// @brief The PowerPlant to use in this reactor
     PowerPlant& powerplant;
-    /// @brief The log level for this reactor
-    LogLevel log_level;
     /// @brief The name of the reactor
     std::string reactor_name;
+    /// @brief The log level for this reactor
+    LogLevel log_level;
 };
 
 }  // namespace NUClear

src/PowerPlant.cpp

@@ -37,8 +37,9 @@ void PowerPlant::start() {
     // We are now running
     is_running.store(true);
 
-    // Direct emit startup event
+    // Direct emit startup event and command line arguments
     emit<dsl::word::emit::Direct>(std::make_unique<dsl::word::Startup>());
+    emit_shared<dsl::word::emit::Direct>(dsl::store::DataStore<message::CommandLineArguments>::get());
 
     // Start all of the threads
     scheduler.start();

src/PowerPlant.ipp

@@ -45,9 +45,8 @@ inline PowerPlant::PowerPlant(Configuration config, int argc, const char* argv[]
         args.emplace_back(argv[i]);
     }
 
-    // We emit this twice, so the data is available for extensions
+    // Emit our command line arguments
     emit(std::make_unique<message::CommandLineArguments>(args));
-    emit<dsl::word::emit::Initialise>(std::make_unique<message::CommandLineArguments>(args));
 }
 
 template <typename T, enum LogLevel level>

src/Reactor.hpp

@@ -159,7 +159,7 @@ class Reactor {
     std::vector<threading::ReactionHandle> reaction_handles{};
 
 public:
-    /// @brief TODO
+    /// @brief The powerplant that this reactor is running in
     PowerPlant& powerplant;
 
     /// @brief The demangled string name of this reactor

src/dsl/word/IO.hpp

@@ -31,12 +31,35 @@ namespace NUClear {
 namespace dsl {
     namespace word {
 
+#ifdef _WIN32
+        using event_t = long;  // NOLINT(google-runtime-int)
+#else
+        using event_t = short;  // NOLINT(google-runtime-int)
+#endif
+
+        /**
+         * @brief This message is sent to the IO controller to configure a new IO operation.
+         */
         struct IOConfiguration {
+            IOConfiguration(fd_t fd, event_t events, std::shared_ptr<threading::Reaction> reaction)
+                : fd(fd), events(events), reaction(std::move(reaction)) {}
+            /// @brief The file descriptor to watch
             fd_t fd;
-            int events;
+            /// @brief The events to watch for on this file descriptor
+            event_t events;
+            /// @brief The reaction to trigger when this file descriptor has an event
             std::shared_ptr<threading::Reaction> reaction;
         };
 
+        /**
+         * @brief This is emitted when an IO operation has finished.
+         */
+        struct IOFinished {
+            IOFinished(const uint64_t& id) : id(id) {}
+            /// @brief The id of the reaction that has finished
+            uint64_t id;
+        };
+
         /**
          * @brief
          *  This is used to trigger reactions based on standard I/O operations using file descriptors.
@@ -68,30 +91,43 @@ namespace dsl {
          * @par Implements
          *  Bind
          */
-        struct IO : public Single {
+        struct IO {
 
 // On windows we use different wait events
 #ifdef _WIN32
             // NOLINTNEXTLINE(google-runtime-int)
-            enum EventType : short{READ = FD_READ | FD_OOB | FD_ACCEPT, WRITE = FD_WRITE, CLOSE = FD_CLOSE, ERROR = 0};
+            enum EventType : event_t {
+                READ  = FD_READ | FD_OOB | FD_ACCEPT,
+                WRITE = FD_WRITE,
+                CLOSE = FD_CLOSE,
+                ERROR = 0,
+            };
 #else
             // NOLINTNEXTLINE(google-runtime-int)
-            enum EventType : short { READ = POLLIN, WRITE = POLLOUT, CLOSE = POLLHUP, ERROR = POLLNVAL | POLLERR };
+            enum EventType : event_t {
+                READ  = POLLIN,
+                WRITE = POLLOUT,
+                CLOSE = POLLHUP,
+                ERROR = POLLNVAL | POLLERR,
+            };
 #endif
 
             struct Event {
+                /// @brief The file descriptor that this event is for
                 fd_t fd;
-                int events;
+                /// @brief The events that have occurred on this file descriptor
+                event_t events;
 
+                /// @brief Returns true if the event is for the given event type
                 operator bool() const {
-                    return fd != -1;
+                    return fd != INVALID_SOCKET;
                 }
             };
 
             using ThreadEventStore = dsl::store::ThreadStore<Event>;
 
             template <typename DSL>
-            static inline void bind(const std::shared_ptr<threading::Reaction>& reaction, fd_t fd, int watch_set) {
+            static inline void bind(const std::shared_ptr<threading::Reaction>& reaction, fd_t fd, event_t watch_set) {
 
                 reaction->unbinders.push_back([](const threading::Reaction& r) {
                     r.reactor.emit<emit::Direct>(std::make_unique<operation::Unbind<IO>>(r.id));
@@ -114,6 +150,11 @@ namespace dsl {
                 // Otherwise return an invalid event
                 return Event{INVALID_SOCKET, 0};
             }
+
+            template <typename DSL>
+            static inline void postcondition(threading::ReactionTask& task) {
+                task.parent.reactor.emit<emit::Direct>(std::make_unique<IOFinished>(task.parent.id));
+            }
         };
 
     }  // namespace word

src/dsl/word/UDP.hpp

@@ -333,7 +333,6 @@ namespace dsl {
 
             template <typename DSL>
             static inline RecvResult read(threading::Reaction& reaction) {
-
                 // Get our file descriptor from the magic cache
                 auto event = IO::get<DSL>(reaction);
 
@@ -429,7 +428,7 @@ namespace dsl {
                 p.local       = Packet::Target{local_s.first, local_s.second};
                 p.remote      = Packet::Target{remote_s.first, remote_s.second};
 
-                // Confirm that this packet was sent to one of our broadcast addresses
+                // Confirm that this packet was sent to one of our local addresses
                 for (const auto& iface : util::network::get_interfaces()) {
                     if (iface.ip.sock.sa_family == result.local.sock.sa_family) {
                         // If the two are equal

src/dsl/word/Watchdog.hpp

@@ -266,11 +266,11 @@ namespace dsl {
 
                 // Check if our watchdog has timed out
                 if (NUClear::clock::now() > (service_time + period(ticks))) {
-                        // Submit the reaction to the thread pool
+                    // Submit the reaction to the thread pool
                     reaction->reactor.powerplant.submit(reaction->get_task());
 
                     // Now automatically service the watchdog
-                    time = NUClear::clock::now() + period(ticks);
+                    time += period(ticks);
                 }
                 // Change our wait time to our new watchdog time
                 else {