refactor(EvseManager): more generic error checking

modules/EvseManager/BUILD.bazel

@@ -16,6 +16,7 @@ cc_everest_module(
     impls = IMPLS,
     srcs = glob(
         [
+            "lib/*.hpp",
             "*.cpp",
             "*.hpp",
         ],

modules/EvseManager/ErrorHandling.cpp

@@ -3,19 +3,10 @@
 
 #include "ErrorHandling.hpp"
 
-namespace module {
+#include "lib/error_defs.hpp"
+#include "lib/util.hpp"
 
-using ErrorList = std::list<Everest::error::ErrorType>;
-static const struct IgnoreErrors {
-    // p_evse. We need to ignore Inoperative here as this is the result of this check.
-    ErrorList evse{"evse_manager/Inoperative"};
-    ErrorList bsp{"evse_board_support/MREC3HighTemperature", "evse_board_support/MREC18CableOverTempDerate",
-                  "evse_board_support/VendorWarning"};
-    ErrorList connector_lock{"connector_lock/VendorWarning"};
-    ErrorList ac_rcd{"ac_rcd/VendorWarning"};
-    ErrorList imd{"isolation_monitor/VendorWarning"};
-    ErrorList powersupply{"power_supply_DC/VendorWarning"};
-} ignore_errors;
+namespace module {
 
 ErrorHandling::ErrorHandling(const std::unique_ptr<evse_board_supportIntf>& _r_bsp,
                              const std::vector<std::unique_ptr<ISO15118_chargerIntf>>& _r_hlc,
@@ -109,57 +100,55 @@ void ErrorHandling::process_error() {
     }
 }
 
-// Check all errors from p_evse and all requirements to see if they block charging
-std::optional<std::string> ErrorHandling::errors_prevent_charging() {
-
-    auto is_fatal = [](auto errors, auto ignore_list) -> std::optional<std::string> {
-        for (const auto e : errors) {
-            if (std::none_of(ignore_list.begin(), ignore_list.end(), [e](const auto& ign) { return e->type == ign; })) {
-                return e->type;
-            }
+template <typename T> struct ErrorView {
+    const T& source;
+    const ErrorList& to_ignore;
+};
+template <typename T> ErrorView(T&, const ErrorList&) -> ErrorView<T>;
+
+std::optional<std::string> find_first_error(const std::list<Everest::error::ErrorPtr>& active_errors,
+                                            const ErrorList& to_ignore) {
+    for (const auto& error : active_errors) {
+        if (std::none_of(std::begin(to_ignore), std::end(to_ignore),
+                         [&error](const auto& error_to_ignore) { return error->type == error_to_ignore; })) {
+            return error->type;
         }
-        return std::nullopt;
-    };
-
-    auto fatal = is_fatal(p_evse->error_state_monitor->get_active_errors(), ignore_errors.evse);
-    if (fatal) {
-        return fatal;
     }
 
-    fatal = is_fatal(r_bsp->error_state_monitor->get_active_errors(), ignore_errors.bsp);
-    if (fatal) {
-        return fatal;
-    }
-
-    if (r_connector_lock.size() > 0) {
-        fatal = is_fatal(r_connector_lock[0]->error_state_monitor->get_active_errors(), ignore_errors.connector_lock);
-        if (fatal) {
-            return fatal;
-        }
-    }
+    return std::nullopt;
+}
 
-    if (r_ac_rcd.size() > 0) {
-        fatal = is_fatal(r_ac_rcd[0]->error_state_monitor->get_active_errors(), ignore_errors.ac_rcd);
-        if (fatal) {
-            return fatal;
-        }
-    }
+struct Matcher {
+    using return_type = std::optional<std::string>;
 
-    if (r_imd.size() > 0) {
-        fatal = is_fatal(r_imd[0]->error_state_monitor->get_active_errors(), ignore_errors.imd);
-        if (fatal) {
-            return fatal;
-        }
+    template <typename T> return_type operator()(const T& error_view) {
+        return find_first_error(error_view.source.error_state_monitor->get_active_errors(), error_view.to_ignore);
     }
 
-    if (r_powersupply.size() > 0) {
-        fatal = is_fatal(r_powersupply[0]->error_state_monitor->get_active_errors(), ignore_errors.powersupply);
-        if (fatal) {
-            return fatal;
+    // specialization for requirement lists
+    template <typename T> return_type operator()(const ErrorView<const std::vector<T>>& error_view) {
+        for (const auto& elem : error_view.source) {
+            auto match = find_first_error(elem->error_state_monitor->get_active_errors(), error_view.to_ignore);
+            if (match) {
+                return match;
+            }
         }
+        return std::nullopt;
     }
+};
 
-    return std::nullopt;
+// Check all errors from p_evse and all requirements to see if they block charging
+std::optional<std::string> ErrorHandling::errors_prevent_charging() {
+    /* clang-format off */
+    return ev::util::find_first_match(Matcher(),
+        ErrorView{*p_evse, errors_to_ignore::evse},
+        ErrorView{*r_bsp, errors_to_ignore::bsp},
+        ErrorView{r_connector_lock, errors_to_ignore::connector_lock},
+        ErrorView{r_ac_rcd, errors_to_ignore::ac_rcd},
+        ErrorView{r_imd, errors_to_ignore::imd},
+        ErrorView{r_powersupply, errors_to_ignore::powersupply}
+    );
+    /* clang-format on */
 }
 
 void ErrorHandling::raise_inoperative_error(const std::string& caused_by) {

modules/EvseManager/lib/error_defs.hpp

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Pionix GmbH and Contributors to EVerest
+#pragma once
+
+#include <initializer_list>
+
+// FIXME (aw): everest-framework imports are not properly namespaced
+#include <utils/error.hpp>
+
+namespace module {
+
+using ErrorList = std::initializer_list<Everest::error::ErrorType>;
+
+namespace errors_to_ignore {
+// FIXME (aw): these strings should be strongly typed (propably via autogeneration) instead of repeatedly hardcoded
+// p_evse. We need to ignore Inoperative here as this is the result of this check.
+inline const ErrorList evse{"evse_manager/Inoperative"};
+inline const ErrorList bsp{"evse_board_support/MREC3HighTemperature", "evse_board_support/MREC18CableOverTempDerate",
+                           "evse_board_support/VendorWarning"};
+inline const ErrorList connector_lock{"connector_lock/VendorWarning"};
+inline const ErrorList ac_rcd{"ac_rcd/VendorWarning"};
+inline const ErrorList imd{"isolation_monitor/VendorWarning"};
+inline const ErrorList powersupply{"power_supply_DC/VendorWarning"};
+
+} // namespace errors_to_ignore
+
+} // namespace module

modules/EvseManager/lib/util.hpp

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Pionix GmbH and Contributors to EVerest
+#pragma once
+
+namespace ev::util {
+
+template <typename Matcher, typename... Args> auto find_first_match(Matcher&& matcher, Args&&... sets) {
+    typename Matcher::return_type match;
+    // the following line makes use of short-circuiting in boolean expression
+    (((match = matcher(std::forward<Args>(sets))) || ...));
+    return match;
+}
+
+} // namespace ev::util

modules/EvseManager/tests/CMakeLists.txt

@@ -31,3 +31,15 @@ target_link_libraries(${TEST_TARGET_NAME} PRIVATE
 )
 
 add_test(${TEST_TARGET_NAME} ${TEST_TARGET_NAME})
+
+set(UTIL_TEST_TARGET ${MODULE_NAME}_util)
+add_executable(${UTIL_TEST_TARGET} util.cpp)
+
+target_compile_definitions(${UTIL_TEST_TARGET} PRIVATE
+    BUILD_TESTING_MODULE_EVSE_MANAGER
+)
+
+target_link_libraries(${UTIL_TEST_TARGET}
+    PRIVATE
+        Catch2::Catch2WithMain
+)

modules/EvseManager/tests/util.cpp

@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright Pionix GmbH and Contributors to EVerest
+#include <optional>
+
+#include <catch2/catch_test_macros.hpp>
+#include <catch2/matchers/catch_matchers.hpp>
+
+#include "../lib/util.hpp"
+
+using namespace ev::util;
+
+struct SenseOfLive {
+    int value;
+    static constexpr auto correct_value = 42;
+};
+
+struct LatchCheck {
+    bool valid;
+    bool got_checked;
+    static constexpr auto return_value = -42;
+};
+struct Matcher {
+    using return_type = std::optional<int>;
+
+    return_type operator()(const SenseOfLive& item) {
+        if (item.value == SenseOfLive::correct_value) {
+            return item.value;
+        }
+
+        return std::nullopt;
+    }
+
+    return_type operator()(LatchCheck& item) {
+        item.got_checked = true;
+        if (item.valid) {
+            return LatchCheck::return_value;
+        }
+
+        return std::nullopt;
+    }
+};
+
+SCENARIO("feeding find_first_match with various arguments", "[util::find_first_match]") {
+    WHEN("no matchables are supplied") {
+        const auto result = find_first_match(Matcher());
+
+        THEN("the result should be the default constructed return value") {
+            REQUIRE(result == Matcher::return_type());
+        }
+    }
+    WHEN("all matchables are false") {
+        const auto result = find_first_match(Matcher(), SenseOfLive{23}, SenseOfLive{12});
+
+        THEN("there should be no match") {
+            REQUIRE(not result);
+        }
+    }
+
+    WHEN("a matchable is correct") {
+        const auto result =
+            find_first_match(Matcher(), SenseOfLive{13}, SenseOfLive{SenseOfLive::correct_value}, SenseOfLive{12});
+
+        THEN("there should be a match") {
+            REQUIRE(result);
+        }
+
+        AND_THEN("it should have the correct value") {
+            REQUIRE(*result == SenseOfLive::correct_value);
+        }
+    }
+
+    WHEN("the first matchable is correct") {
+        LatchCheck latch_check{true};
+
+        const auto result = find_first_match(Matcher(), SenseOfLive{SenseOfLive::correct_value}, latch_check);
+
+        THEN("the second matchable shouldn't have been tested") {
+            REQUIRE(latch_check.got_checked == false);
+        }
+    }
+
+    WHEN("the first matchable is not correct") {
+        LatchCheck latch_check{true};
+
+        const auto result = find_first_match(Matcher(), SenseOfLive{0}, latch_check);
+
+        THEN("the second matchabe should be checked") {
+            REQUIRE(latch_check.got_checked == true);
+        }
+
+        AND_THEN("it should have the correct value") {
+            REQUIRE(result);
+            REQUIRE(*result == LatchCheck::return_value);
+        }
+    }
+}