Fix/watchdog fix (#9)

* (fix) making test better and making clear singleton for watchdog impl

* (fix) ignoring linter issue and fixing typo

Author: RCMast3r

include/VCR_Constants.h

@@ -16,46 +16,49 @@ const int ADC1_CS = 11; // MCP3208. ADC1 in VCR schematic. Used for extra thermi
 /* -------------------------------------------------- */
 
 /* Channels on adc_0 */
-const int GLV_SENSE_CHANNEL       = 0;
-const int CURRENT_SENSE_CHANNEL   = 1;
-const int REFERENCE_SENSE_CHANNEL = 2;
-const int RL_LOADCELL_CHANNEL     = 3;
-const int RR_LOADCELL_CHANNEL     = 4;
-const int RL_SUS_POT_CHANNEL      = 5;
-const int RR_SUS_POT_CHANNEL      = 6;
+constexpr int GLV_SENSE_CHANNEL       = 0;
+constexpr int CURRENT_SENSE_CHANNEL   = 1;
+constexpr int REFERENCE_SENSE_CHANNEL = 2;
+constexpr int RL_LOADCELL_CHANNEL     = 3;
+constexpr int RR_LOADCELL_CHANNEL     = 4;
+constexpr int RL_SUS_POT_CHANNEL      = 5;
+constexpr int RR_SUS_POT_CHANNEL      = 6;
 // const int UNUSED_CHANNEL       = 7;
 
 /* Channels on ADC_1 */
-const int THERMISTOR_0 = 0;
-const int THERMISTOR_1 = 1;
-const int THERMISTOR_2 = 2;
-const int THERMISTOR_3 = 3;
-const int THERMISTOR_4 = 4;
-const int THERMISTOR_5 = 5;
-const int THERMISTOR_6 = 6;
-const int THERMISTOR_7 = 7;
+constexpr int THERMISTOR_0 = 0;
+constexpr int THERMISTOR_1 = 1;
+constexpr int THERMISTOR_2 = 2;
+constexpr int THERMISTOR_3 = 3;
+constexpr int THERMISTOR_4 = 4;
+constexpr int THERMISTOR_5 = 5;
+constexpr int THERMISTOR_6 = 6;
+constexpr int THERMISTOR_7 = 7;
 
 /* Scaling and offset */
-const float GLV_SENSE_SCALE = (float)(24.0/((2.77149877/3.3)*4096.0)); //unsure about the multiplication by 4.0865
-const int GLV_SENSE_OFFSET = 0; //No offset for GLV
-const float CURRENT_SENSE_SCALE = (float)(24/((2.77149877/3.3)*4096)); //unsure about the multiplication by 4.0865
-const int CURRENT_SENSE_OFFSET = 0; //No offset for CURRENT_SENSE
-const float REFERENCE_SENSE_SCALE = (float)(24/((2.77149877/3.3)*4096)); //unsure about the multiplication by 4.0865
-const int REFERENCE_SENSE_OFFSET = 0; //No offset for REFERENCE_SENSE
+constexpr float GLV_SENSE_SCALE = (float)(24.0/((2.77149877/3.3)*4096.0)); //unsure about the multiplication by 4.0865
+constexpr int GLV_SENSE_OFFSET = 0; //No offset for GLV
+constexpr float CURRENT_SENSE_SCALE = (float)(24/((2.77149877/3.3)*4096)); //unsure about the multiplication by 4.0865
+constexpr int CURRENT_SENSE_OFFSET = 0; //No offset for CURRENT_SENSE
+constexpr float REFERENCE_SENSE_SCALE = (float)(24/((2.77149877/3.3)*4096)); //unsure about the multiplication by 4.0865
+constexpr int REFERENCE_SENSE_OFFSET = 0; //No offset for REFERENCE_SENSE
 
 //Values are from the old MCU rev15
-const float RL_LOADCELL_SCALE = 0.1149f;
-const float RL_LOADCELL_OFFSET = 13.526f / RL_LOADCELL_SCALE;
-const float RR_LOADCELL_SCALE = 0.118f;
-const float RR_LOADCELL_OFFSET = 25.721f / RR_LOADCELL_SCALE;
+constexpr float RL_LOADCELL_SCALE = 0.1149f;
+constexpr float RL_LOADCELL_OFFSET = 13.526f / RL_LOADCELL_SCALE;
+constexpr float RR_LOADCELL_SCALE = 0.118f;
+constexpr float RR_LOADCELL_OFFSET = 25.721f / RR_LOADCELL_SCALE;
 
 //does not matter that much
-const float RL_SUS_POT_SCALE = 1;
-const int RL_SUS_POT_OFFSET = 1;
-const float RR_SUS_POT_SCALE = 1;
-const int RR_SUS_POT_OFFSET = 1;
-
-const int WATCHDOG_PIN = 36;
+constexpr float RL_SUS_POT_SCALE = 1;
+constexpr int RL_SUS_POT_OFFSET = 1;
+constexpr float RR_SUS_POT_SCALE = 1;
+constexpr int RR_SUS_POT_OFFSET = 1;
 
+constexpr int WATCHDOG_PIN = 36;
 
+namespace default_system_params
+{
+    constexpr unsigned long KICK_INTERVAL_MS = 10UL;
+}
 #endif /* VCR_CONSTANTS */

include/VCR_Globals.h

@@ -19,7 +19,5 @@ constexpr unsigned int channels_within_mcp_adc = 8;
 extern MCP_ADC<channels_within_mcp_adc> adc_0; // MCP3208. ADC0 in VCR schematic. Used for valuable telem data.
 extern MCP_ADC<channels_within_mcp_adc> adc_1; // MCP3208. ADC1 in VCR schematic. Used for extra thermistors or extra sensors while testing.
 
-/* Watchdog pin */
-extern const int watchdog_pin;
 
 #endif /* VCR_GLOBALS */

lib/systems/include/WatchdogSystem.h

@@ -1,51 +1,39 @@
 #ifndef WATCHDOG_SYSTEM_H
 #define WATCHDOG_SYSTEM_H
 
+#include <etl/singleton.h>
 
 /**
  * This class controls the boolean _watchdog_state, but does not directly control the watchdog.
  * WatchdogSystem provides functionality to initialize, monitor, and "kick" the watchdog to prevent system resets.
  * 
  * NOTE:  To ensure system responsiveness, WatchdogSystem requires periodic updates by calling the `get_watchdog_state()` method. 
- * This toggles the _watchdog_state (if the interval has passed) and returns the new state, which must then be sent to the watchdog. 
- * If this is not done within the `WATCHDOG_KICK_INTERVAL`, it assumes the system is unresponsive and may trigger a reset. The singleton design ensures 
- * only one instance manages the watchdog, which will help mantain consistency and control.
- * 
+ * This toggles the _watchdog_state (if the interval has passed) and returns the new state, which must then be sent to the watchdog.  
  */
 
-const unsigned long WATCHDOG_KICK_INTERVAL = 10;    // milliseconds
 
 class WatchdogSystem
 {
+public:
+    
+    WatchdogSystem(const unsigned long kick_interval_ms = 10UL) : _watchdog_time(0), _watchdog_state(false), _watchdog_kick_interval(kick_interval_ms) {};
 private:
 
     /*Private constructor*/
-    WatchdogSystem() : _watchdog_time(0), _watchdog_state(false) {};
 
     /* Watchdog last kicked time */
     unsigned long _watchdog_time;
-
-    /* Watchdog output state */
     bool _watchdog_state;
+    unsigned long _watchdog_kick_interval;
+    /* Watchdog output state */
 
 public:
 
-    /*Getter to return the instance and make a new one if it hasn't been made yet*/
-    static WatchdogSystem& getInstance() 
-    {
-        static WatchdogSystem instance;
-        return instance;
-    }
-
-    /*Prevent copying*/
-    WatchdogSystem(const WatchdogSystem&) = delete;
-
-    /*Prevent Assignment*/
-    WatchdogSystem& operator= (const WatchdogSystem&) = delete;
-
     /* Get/update watchdog state */
     bool get_watchdog_state(unsigned long curr_millis);
 
 };
 
+using WatchdogInstance = etl::singleton<WatchdogSystem>;
+
 #endif /* WATCHDOG_SYSTEM_H */

lib/systems/src/WatchdogSystem.cpp

@@ -5,7 +5,7 @@
 bool WatchdogSystem::get_watchdog_state(unsigned long curr_millis) 
 {
 
-    if ((curr_millis - _watchdog_time) > WATCHDOG_KICK_INTERVAL) {
+    if ((curr_millis - _watchdog_time) > _watchdog_kick_interval) {
         _watchdog_state = !_watchdog_state;
         _watchdog_time = curr_millis;
     }

platformio.ini

@@ -11,7 +11,7 @@ lib_deps_shared =
   https://github.com/hytech-racing/shared_firmware_types.git
   https://github.com/ssilverman/QNEthernet#v0.26.0
   https://github.com/hytech-racing/HT_SCHED
-
+  Embedded Template Library@^20.39.4
 ; Teensy41 Environment. This environment is the primary environment for uploading code to the car.
 ; * Build to verify the on-car software.
 ; * UPLOAD to compile and upload on-car software.
@@ -26,11 +26,8 @@ build_unflags = -std=gnu++11
 build_flags = 
   -std=c++17
   -D TEENSY_OPT_SMALLEST_CODE
-check_src_filters =
-  +<include/*>
-  +<lib/*>
-  +<src/*>
-  -<src/old_main.cpp>
+
+
 platform = teensy
 board = teensy41
 framework = arduino
@@ -50,9 +47,13 @@ lib_deps =
 [env:test_systems_env]
 platform = native
 test_framework = googletest
+; test_build_src = yes 
 build_src_filter =
-  -<**/*.c>
-  -<**/*.cpp>
+  -<VCR_Globals.cpp>
+  -<VCR_Tasks.cpp>
+  -<main.cpp>
+  +<VCR_Constants.cpp>
+  +<../test/test_systems/test_systems.cpp>
 build_unflags = -std=gnu++11
 build_flags = 
   -std=c++17
@@ -62,4 +63,5 @@ lib_ignore =
 test_ignore=
   test_interfaces*
 lib_deps =
+  google/googletest@^1.15.2
   ${common.lib_deps_shared}

prepush.sh

@@ -0,0 +1,5 @@
+#!/bin/bash
+set -o pipefail
+pio run -e teensy41
+pio test -e test_systems_env
+pio check -e teensy41 --fail-on-defect high

src/VCR_Globals.cpp

@@ -13,4 +13,4 @@ VCRSystemData_s system_data;
 
 /* ADC setup */
 MCP_ADC<channels_within_mcp_adc> adc_0 = MCP_ADC<8>(ADC0_CS); // MCP3208. ADC0 in VCR schematic. Used for valuable telem data.
-MCP_ADC<channels_within_mcp_adc> adc_1 = MCP_ADC<8>(ADC1_CS); // MCP3208. ADC1 in VCR schematic. Used for extra thermistors or extra sensors while testing.
+MCP_ADC<channels_within_mcp_adc> adc_1 = MCP_ADC<8>(ADC1_CS); // MCP3208. ADC1 in VCR schematic. Used for extra thermistors or extra sensors while testing.

src/VCR_Tasks.cpp

@@ -15,8 +15,6 @@
 #include "VCR_Globals.h"
 #include "VehicleStateMachine.h"
 
-
-
 bool init_read_adc0_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo& taskInfo)
 {
 
@@ -52,8 +50,6 @@ bool run_read_adc0_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo&
 
 HT_TASK::Task read_adc0_task = HT_TASK::Task(init_read_adc0_task, run_read_adc0_task, 10, 1000UL); // 1000us is 1kHz //NOLINT
 
-
-
 bool run_tick_state_machine_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo& taskInfo)
 {
     VehicleStateMachine::getInstance().tick_state_machine(sysMicros / 1000, system_data); // tick function requires millis //NOLINT
@@ -62,8 +58,6 @@ bool run_tick_state_machine_task(const unsigned long& sysMicros, const HT_TASK::
 
 HT_TASK::Task tick_state_machine_task = HT_TASK::Task(HT_TASK::DUMMY_FUNCTION, run_tick_state_machine_task, 2); // Idle (constant-update) task //NOLINT
 
-
-
 bool init_read_adc1_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo& taskInfo)
 {
     /* NOLINTBEGIN */ // Thermistor channels are for testing purposes only, the pin numbers 0-7 are acceptable "magic numbers".
@@ -94,8 +88,6 @@ bool run_read_adc1_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo&
 
 HT_TASK::Task read_adc1_task = HT_TASK::Task(init_read_adc1_task, run_read_adc1_task, 10, 40000UL); // 20000us is 25Hz //NOLINT
 
-
-
 bool run_update_buzzer_controller_task(const unsigned long& sysMicros, const HT_TASK::TaskInfo& taskInfo)
 {
 
@@ -107,10 +99,16 @@ bool run_update_buzzer_controller_task(const unsigned long& sysMicros, const HT_
 HT_TASK::Task update_buzzer_controller_task = HT_TASK::Task(HT_TASK::DUMMY_FUNCTION, run_update_buzzer_controller_task, 10, 20000UL); // 20000us is 50hz //NOLINT
 
 
+bool create_watchdog(const unsigned long & sysMicros, const HT_TASK::TaskInfo &task_info)
+{
+    WatchdogInstance::create(default_system_params::KICK_INTERVAL_MS); // this has issues for some reason with clang-tidy // NOLINT
+    return true;
+}
 
 bool run_kick_watchdog(const unsigned long& sysMicros, const HT_TASK::TaskInfo& taskInfo)
 {
-    digitalWrite(WATCHDOG_PIN, WatchdogSystem::getInstance().get_watchdog_state(sysMicros / 1000)); //NOLINT
+    digitalWrite(WATCHDOG_PIN, WatchdogInstance::instance().get_watchdog_state(sysMicros / 1000));
     return true;
 }
-HT_TASK::Task kick_watchdog_task = HT_TASK::Task(HT_TASK::DUMMY_FUNCTION, run_kick_watchdog, 3, 2000UL); // 2000us is 500hz //NOLINT
+
+HT_TASK::Task kick_watchdog_task = HT_TASK::Task(create_watchdog, run_kick_watchdog, 3, 2000UL); // 2000us is 500hz // NOLINT

test/test_systems/test_watchdog.h

@@ -2,71 +2,29 @@
 #include "gtest/gtest.h"
 #include "WatchdogSystem.h"
 
-WatchdogSystem &watchdog = WatchdogSystem::getInstance();
-int time_var = 0; // starting time
-int time_arb = 2500; // arbitrary number greater than 2000
+#include "VCR_Constants.h"
 
-//Test case where time is 0 millisecs
-TEST (WatchdogSystemTesting, initial_state) {
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
+TEST(WatchdogSystemTesting, test_watchdog) {
+    WatchdogInstance::create(10);
 
-//Test consecutive steps
-TEST (WatchdogSystemTesting, next_state1) {
-    time_var += 1; // time_var = 1
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state2) {
-    time_var += 1; // time_var = 2
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state3) {
-    time_var += 1; // time_var = 3
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state4) {
-    time_var += 1; // time_var = 4
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state5) {
-    time_var += 1; // time_var = 5
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state6) {
-    time_var += 1; // time_var = 6
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state7) {
-    time_var += 1; // time_var = 7
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state8) {
-    time_var += 1; // time_var = 8
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state9) {
-    time_var += 1; // time_var = 9
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state10) {
-    time_var += 1; // time_var = 10
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), false);
-}
-TEST (WatchdogSystemTesting, next_state11) {
-    time_var += 1; // time_var = 11
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), true);
-}
-TEST (WatchdogSystemTesting, next_state12) {
-    time_var += 1; // time_var = 12
-    ASSERT_EQ(watchdog.get_watchdog_state(time_var), true);
-}
+    WatchdogSystem &watchdog = WatchdogInstance::instance();
+    const int time_var = 0; // starting time
+    const int time_arb = 2500; // arbitrary number greater than 2000
 
-//Test an arbituary time above 10 millisecs from previous time
-TEST (WatchdogSystemTesting, instance_state1) {
-    ASSERT_EQ(watchdog.get_watchdog_state(time_arb), false);
-}
+    const int end_time = 13;
 
-//Test second arbituary time above 10 millisecs from previous time
-TEST (WatchdogSystemTesting, instance_state2) {
+    for(int time = 0; time < end_time; time++)
+    {
+        if(time <= 10)
+        {   
+            ASSERT_EQ(watchdog.get_watchdog_state(time), false);
+        } else {
+            ASSERT_EQ(watchdog.get_watchdog_state(time), true);
+        }
+    }
+    
+    ASSERT_EQ(watchdog.get_watchdog_state(time_arb), false);
+    
+    //Test an arbituary time above 1000 millisecs from previous time
     ASSERT_EQ(watchdog.get_watchdog_state(time_arb + 1000), true);
-}
+}