Move ComputeMajorityVote() to a separate file

SpaceTeam · Aug 25, 2024 · 3c573d7 · 3c573d7
1 parent efbe584
commit 3c573d7
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 49 deletions.
diff --git a/Sts1CobcSw/FramSections/PersistentVariables.hpp b/Sts1CobcSw/FramSections/PersistentVariables.hpp
@@ -59,12 +59,6 @@ class PersistentVariables
 
     static RODOS::Semaphore semaphore;
 };
-
-
-// TODO: This should probably be defined somewhere else
-template<typename T>
-[[nodiscard]] static auto ComputeMajorityVote(T const & value0, T const & value1, T const & value2)
-    -> std::optional<T>;
 }
 
 

diff --git a/Sts1CobcSw/FramSections/PersistentVariables.ipp b/Sts1CobcSw/FramSections/PersistentVariables.ipp
@@ -2,6 +2,7 @@
 
 
 #include <Sts1CobcSw/FramSections/PersistentVariables.hpp>
+#include <Sts1CobcSw/Utility/ErrorDetectionAndCorrection.hpp>
 #include <Sts1CobcSw/Utility/Span.hpp>
 
 
@@ -191,19 +192,4 @@ std::tuple<typename PersistentVariableInfos::ValueType...> PersistentVariables<
     parentSection2,
     PersistentVariableInfos...>::cache2 =
     std::tuple(typename PersistentVariableInfos::ValueType{}...);
-
-
-template<typename T>
-auto ComputeMajorityVote(T const & value0, T const & value1, T const & value2) -> std::optional<T>
-{
-    if(value0 == value1 or value0 == value2)
-    {
-        return value0;
-    }
-    if(value1 == value2)
-    {
-        return value1;
-    }
-    return std::nullopt;
-}
 }
diff --git a/Sts1CobcSw/Utility/ErrorDetectionAndCorrection.hpp b/Sts1CobcSw/Utility/ErrorDetectionAndCorrection.hpp
@@ -0,0 +1,25 @@
+#pragma once
+
+
+#include <optional>
+
+
+namespace sts1cobcsw
+{
+// I won't make a separate .ipp file just for this simple function
+template<typename T>
+[[nodiscard]] constexpr auto ComputeMajorityVote(T const & value0,
+                                                 T const & value1,
+                                                 T const & value2) -> std::optional<T>
+{
+    if(value0 == value1 or value0 == value2)
+    {
+        return value0;
+    }
+    if(value1 == value2)
+    {
+        return value1;
+    }
+    return std::nullopt;
+}
+}
diff --git a/Tests/UnitTests/CMakeLists.txt b/Tests/UnitTests/CMakeLists.txt
@@ -1,9 +1,15 @@
 # --- Tests with Catch2 ---
 
 # TODO: Implement something like add_test_program() which adds the suffix "Test" to the output name
+
 add_program(Dummy Dummy.test.cpp)
 target_link_libraries(Sts1CobcSwTests_Dummy PRIVATE Catch2::Catch2WithMain Sts1CobcSw_Dummy)
 
+add_program(ErrorDetectionAndCorrection ErrorDetectionAndCorrection.test.cpp)
+target_link_libraries(
+    Sts1CobcSwTests_ErrorDetectionAndCorrection PRIVATE Catch2::Catch2WithMain Sts1CobcSw_Utility
+)
+
 add_program(FlatArray FlatArray.test.cpp)
 target_link_libraries(Sts1CobcSwTests_FlatArray PRIVATE Catch2::Catch2WithMain Sts1CobcSw_Utility)
 

diff --git a/Tests/UnitTests/ErrorDetectionAndCorrection.test.cpp b/Tests/UnitTests/ErrorDetectionAndCorrection.test.cpp
@@ -0,0 +1,30 @@
+#include <Sts1CobcSw/Utility/ErrorDetectionAndCorrection.hpp>
+
+#include <catch2/catch_test_macros.hpp>
+
+#include <optional>
+
+
+TEST_CASE("Majority vote")
+{
+    using sts1cobcsw::ComputeMajorityVote;
+
+    auto voteResult = ComputeMajorityVote(173, 173, 173);
+    CHECK(voteResult.has_value());
+    CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
+
+    voteResult = ComputeMajorityVote(-2, 173, 173);
+    CHECK(voteResult.has_value());
+    CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
+
+    voteResult = ComputeMajorityVote(173, -2, 173);
+    CHECK(voteResult.has_value());
+    CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
+
+    voteResult = ComputeMajorityVote(173, 173, -2);
+    CHECK(voteResult.has_value());
+    CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
+
+    voteResult = ComputeMajorityVote(17, 173, -2);
+    CHECK(not voteResult.has_value());
+}
diff --git a/Tests/UnitTests/PersistentVariables.test.cpp b/Tests/UnitTests/PersistentVariables.test.cpp
@@ -44,7 +44,7 @@ static_assert(std::is_same_v<decltype(pvs.Load<"activeFwImage">()), std::uint8_t
 static_assert(std::is_same_v<decltype(pvs.Load<"somethingElse">()), std::int16_t>);
 
 
-// UnitTestWithRodos.hpp
+// TODO: Move this to UnitTestWithRodos.hpp
 
 std::uint32_t printfMask = 0;
 
@@ -77,7 +77,7 @@ class UnitTestThread : public RODOS::StaticThread<>
 } unitTestThread;
 
 
-// UnitTestWithRodos.cpp
+// TODO: Move this to UnitTestWithRodos.cpp
 
 auto Require(bool condition, std::source_location location) -> void
 {
@@ -90,7 +90,7 @@ auto Require(bool condition, std::source_location location) -> void
 }
 
 
-// PersistentVariables.test.cpp
+// TODO: This should stay here in PersistentVariables.test.cpp
 
 auto RunUnitTest() -> void
 {
@@ -215,28 +215,3 @@ auto RunUnitTest() -> void
         }
     }
 }
-
-
-// TEST_CASE("Majority vote")
-// {
-//     using sts1cobcsw::ComputeMajorityVote;
-
-//     auto voteResult = ComputeMajorityVote(173, 173, 173);
-//     CHECK(voteResult.has_value());
-//     CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
-
-//     voteResult = ComputeMajorityVote(-2, 173, 173);
-//     CHECK(voteResult.has_value());
-//     CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
-
-//     voteResult = ComputeMajorityVote(173, -2, 173);
-//     CHECK(voteResult.has_value());
-//     CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
-
-//     voteResult = ComputeMajorityVote(173, 173, -2);
-//     CHECK(voteResult.has_value());
-//     CHECK(voteResult.value() == 173);  // NOLINT(*unchecked-optional-access)
-
-//     voteResult = ComputeMajorityVote(17, 173, -2);
-//     CHECK(not voteResult.has_value());
-// }