Merge pull request #1443 from jonny5532/feature/can-log-based-testing-rebase

CAN log based testing

Software/src/battery/MG-HS-PHEV-BATTERY.cpp

@@ -6,6 +6,7 @@
 #include "../datalayer/datalayer.h"
 #include "../devboard/utils/events.h"
 #include "../devboard/utils/logging.h"
+
 /*
 MG HS PHEV 16.6kWh battery integration
 

Software/src/battery/RANGE-ROVER-PHEV-BATTERY.cpp

@@ -3,6 +3,7 @@
 #include "../communication/can/comm_can.h"
 #include "../datalayer/datalayer.h"
 #include "../devboard/utils/events.h"
+
 /* TODO
 - LOG files from vehicle needed to determine CAN content needed to send towards battery!
   - BCCM_PMZ_A (0x18B 50ms)

Software/src/battery/SANTA-FE-PHEV-BATTERY.cpp

@@ -3,6 +3,7 @@
 #include "../communication/can/comm_can.h"
 #include "../datalayer/datalayer.h"
 #include "../devboard/utils/events.h"
+
 /* Credits go to maciek16c for these findings!
 https://github.com/maciek16c/hyundai-santa-fe-phev-battery
 https://openinverter.org/forum/viewtopic.php?p=62256

Software/src/battery/VOLVO-SPA-BATTERY.cpp

@@ -5,6 +5,7 @@
 #include "../datalayer/datalayer_extended.h"  //For "More battery info" webpage
 #include "../devboard/utils/events.h"
 #include "../devboard/utils/logging.h"
+
 void VolvoSpaBattery::
     update_values() {  //This function maps all the values fetched via CAN to the correct parameters used for the inverter
 

Software/src/battery/VOLVO-SPA-HYBRID-BATTERY.cpp

@@ -5,6 +5,7 @@
 #include "../datalayer/datalayer_extended.h"  //For "More battery info" webpage
 #include "../devboard/utils/events.h"
 #include "../devboard/utils/logging.h"
+
 void VolvoSpaHybridBattery::
     update_values() {  //This function maps all the values fetched via CAN to the correct parameters used for the inverter
   uint8_t cnt = 0;

test/CMakeLists.txt

@@ -63,13 +63,18 @@ include_directories("${source_dir}/googletest/include"
 include_directories(emul)
 
 # For eModBus
-add_compile_definitions(ESP32 HW_LILYGO NISSAN_LEAF_BATTERY)
+add_compile_definitions(ESP32 HW_LILYGO COMMON_IMAGE)
 
 # add the executable
 add_executable(tests 
     tests.cpp 
     safety_tests.cpp 
     battery/NissanLeafTest.cpp 
+    can_log_based/canlog_safety_tests.cpp
+    can_log_based/utils.cpp
+    ../Software/src/communication/can/obd.cpp
+    ../Software/src/communication/contactorcontrol/comm_contactorcontrol.cpp
+    ../Software/src/communication/rs485/comm_rs485.cpp
     ../Software/src/devboard/safety/safety.cpp
     ../Software/src/devboard/hal/hal.cpp
     ../Software/src/devboard/utils/events.cpp

test/can_log_based/can_logs/24_RjxzsBms_ov_cov.txt

@@ -0,0 +1,5 @@
+# 65278V pack (overvoltage)
+(123.893) RX0 f5 [8] 03 fe fe 00 00 00 00 00
+
+# 5V max cell (cell overvoltage)
+(124.893) RX0 f5 [8] 51 01 01 01 01 13 88 00

test/can_log_based/can_logs/37_MgHsPhev_base.txt

@@ -0,0 +1,6 @@
+(84215.921) RX0 295 [8] d5 1d 00 01 a6 4e 20 01
+(84215.922) RX0 171 [8] 00 34 b5 75 4d 03 78 c2
+(84215.923) RX0 172 [8] 3b e0 d2 3f 20 dd 00 00
+(84216.021) RX0 173 [8] 00 00 c8 c8 0e 2d 0e 29
+(84216.027) RX0 2a2 [8] 78 77 04 c7 40 76 00 6a
+(84216.028) RX0 3ac [8] 01 a6 01 2f 05 1d 4e 20

test/can_log_based/can_logs/37_MgHsPhev_ov_cov.txt

@@ -0,0 +1,5 @@
+# 450V pack (overvoltage)
+(13148.893) RX0 3ac [8] 01 d4 01 6f 07 08 4d d0
+
+# 5V max cell (cell overvoltage)
+(38245.429) RX0 173 [8] 00 00 c8 c3 13 88 0e 18

test/can_log_based/can_logs/5_BydAtto3_base.txt

@@ -0,0 +1,8 @@
+# voltage
+(0.001) RX0 7ef [8] 00 00 00 08 81 34 ee ee
+# lowest cell
+(0.002) RX0 7ef [8] 00 00 00 2f 30 ee ee ee
+# highest cell
+(0.002) RX0 7ef [8] 00 00 00 31 40 ee ee ee
+
+# this is enough to pass (all the other params have defaults)

test/can_log_based/canlog_safety_tests.cpp

@@ -0,0 +1,194 @@
+#include <gtest/gtest.h>
+
+#include "utils.h"
+
+#include "../../Software/src/battery/BATTERIES.h"
+#include "../../Software/src/devboard/utils/events.h"
+
+#include <fstream>
+
+namespace fs = std::filesystem;
+
+// These tests replay CAN logs against individual batteries to check that they
+// are correctly parsed, and that safety mechanisms work.
+
+// The base class for our tests
+class CanLogTestFixture : public testing::Test {
+ public:
+  CanLogTestFixture(fs::path path) : path_(std::move(path)) {}
+  // Optional:
+  //   static void SetUpTestSuite() { ... }
+  //   static void TearDownTestSuite() { ... }
+
+  void SetUp() override {
+    // Reset the datalayer and events before each test
+    datalayer = DataLayer();
+    reset_all_events();
+    if (battery) {
+      delete battery;
+      battery = nullptr;
+    }
+
+    // Assume a 90s NMC pack for custom-BMS batteries
+    user_selected_max_pack_voltage_dV = 378 + 10;
+    user_selected_min_pack_voltage_dV = 261 - 10;
+    user_selected_max_cell_voltage_mV = 4200 + 20;
+    user_selected_min_cell_voltage_mV = 2900 - 20;
+
+    // Extract battery type from log filename
+    std::string filename = path_.filename().string();
+    std::string batteryId = filename.substr(0, filename.find('_'));
+    user_selected_battery_type = (BatteryType)std::stoi(batteryId);
+    setup_battery();
+
+    // Initialize datalayer to invalid values
+    datalayer.battery.status.voltage_dV = 0;
+    datalayer.battery.status.current_dA = INT16_MIN;
+    datalayer.battery.status.cell_min_voltage_mV = 0;
+    datalayer.battery.status.cell_max_voltage_mV = 0;
+    datalayer.battery.status.real_soc = UINT16_MAX;
+    datalayer.battery.status.temperature_max_dC = INT16_MIN;
+    datalayer.battery.status.temperature_min_dC = INT16_MIN;
+  }
+
+  void TearDown() override {
+    if (battery) {
+      delete battery;
+      battery = nullptr;
+    }
+  }
+
+  void ProcessLog() {
+    std::vector<CAN_frame> parsedMessages = parse_can_log_file(path_);
+
+    for (const auto& msg : parsedMessages) {
+      dynamic_cast<CanBattery*>(battery)->handle_incoming_can_frame(msg);
+      dynamic_cast<CanBattery*>(battery)->update_values();
+    }
+
+    update_machineryprotection();
+
+    // When debugging, uncomment this to see the parsed values
+    // PrintValues();
+  }
+
+  void PrintValues() {
+    std::cout << "Battery voltage: " << (datalayer.battery.status.voltage_dV / 10.0) << " V" << std::endl;
+    std::cout << "Battery current: " << (datalayer.battery.status.current_dA / 10.0) << " A" << std::endl;
+    std::cout << "Battery cell min voltage: " << datalayer.battery.status.cell_min_voltage_mV << " mV" << std::endl;
+    std::cout << "Battery cell max voltage: " << datalayer.battery.status.cell_max_voltage_mV << " mV" << std::endl;
+    std::cout << "Battery real SoC: " << (datalayer.battery.status.real_soc / 100.0) << " %" << std::endl;
+    std::cout << "Battery temperature max: " << (datalayer.battery.status.temperature_max_dC / 10.0) << " C"
+              << std::endl;
+    std::cout << "Battery temperature min: " << (datalayer.battery.status.temperature_min_dC / 10.0) << " C"
+              << std::endl;
+  }
+
+ private:
+  fs::path path_;
+};
+
+// Check that the parsed logs populate the minimum required datalayer values for
+// Battery Emulator to function.
+class BaseValuesPresentTest : public CanLogTestFixture {
+ public:
+  explicit BaseValuesPresentTest(fs::path path) : CanLogTestFixture(path) {}
+  void TestBody() override {
+    ProcessLog();
+
+    EXPECT_NE(datalayer.battery.status.voltage_dV, 0);
+    // TODO: Current isn't actually a requirement? check power instead?
+    //EXPECT_NE(datalayer.battery.status.current_dA, INT16_MIN);
+    EXPECT_NE(datalayer.battery.status.cell_min_voltage_mV, 0);
+    EXPECT_NE(datalayer.battery.status.cell_max_voltage_mV, 0);
+    EXPECT_NE(datalayer.battery.status.real_soc, UINT16_MAX);
+    EXPECT_NE(datalayer.battery.status.temperature_max_dC, INT16_MIN);
+    EXPECT_NE(datalayer.battery.status.temperature_min_dC, INT16_MIN);
+
+    EXPECT_EQ(get_event_pointer(EVENT_BATTERY_OVERVOLTAGE)->occurences, 0);
+    EXPECT_EQ(get_event_pointer(EVENT_BATTERY_UNDERVOLTAGE)->occurences, 0);
+  }
+};
+
+// Check that the parsed logs correctly trigger an overvoltage event.
+class OverVoltageTest : public CanLogTestFixture {
+ public:
+  explicit OverVoltageTest(fs::path path) : CanLogTestFixture(path) {}
+  void TestBody() override {
+    ProcessLog();
+
+    EXPECT_EQ(get_event_pointer(EVENT_BATTERY_OVERVOLTAGE)->occurences, 1);
+  }
+};
+
+// Check that the parsed logs correctly trigger a cell overvoltage event.
+class CellOverVoltageTest : public CanLogTestFixture {
+ public:
+  explicit CellOverVoltageTest(fs::path path) : CanLogTestFixture(path) {}
+  void TestBody() override {
+    ProcessLog();
+
+    EXPECT_EQ(get_event_pointer(EVENT_CELL_OVER_VOLTAGE)->occurences, 1);
+    EXPECT_EQ(get_event_pointer(EVENT_CELL_CRITICAL_OVER_VOLTAGE)->occurences, 1);
+  }
+};
+
+// Check that the parsed logs correctly trigger a cell undervoltage event.
+class CellUnderVoltageTest : public CanLogTestFixture {
+ public:
+  explicit CellUnderVoltageTest(fs::path path) : CanLogTestFixture(path) {}
+  void TestBody() override {
+    ProcessLog();
+
+    EXPECT_EQ(get_event_pointer(EVENT_CELL_UNDER_VOLTAGE)->occurences, 1);
+    EXPECT_EQ(get_event_pointer(EVENT_CELL_CRITICAL_UNDER_VOLTAGE)->occurences, 1);
+  }
+};
+
+void RegisterCanLogTests() {
+  // The logs should be named as follows:
+  //
+  // <battery_type>_<battery class name>_<flag1>_<flag2...>.txt
+  //
+  // where:
+  //   battery_type is the integer in the BatteryType enum
+  //   flag1/flag2... are flags that indicate which tests to run:
+  //     base: test that the minimmum required values are populated (and no events triggered)
+  //     ov:   test that an overvoltage event is triggered
+  //     cov:  test that normal and critical cell overvoltage events are triggered
+  //     cuv:  test that normal and critical cell undervoltage events are triggered
+
+  std::string directoryPath = "../can_log_based/can_logs";
+
+  for (const auto& entry : fs::directory_iterator(directoryPath)) {
+    if (!entry.is_regular_file() || entry.path().extension().string() != ".txt") {
+      continue;
+    }
+
+    auto bits = split(entry.path().stem(), '_');
+    auto has_flag = [&bits](const std::string& flag) -> bool {
+      return std::find(bits.begin() + 2, bits.end(), flag) != bits.end();
+    };
+
+    if (has_flag("base")) {
+      testing::RegisterTest("CanLogTestFixture", ("TestBaseValuesPresent_" + entry.path().filename().string()).c_str(),
+                            nullptr, entry.path().filename().string().c_str(), __FILE__, __LINE__,
+                            [=]() -> CanLogTestFixture* { return new BaseValuesPresentTest(entry.path()); });
+    }
+    if (has_flag("ov")) {
+      testing::RegisterTest("CanLogTestFixture", ("TestOverVoltage_" + entry.path().filename().string()).c_str(),
+                            nullptr, entry.path().filename().string().c_str(), __FILE__, __LINE__,
+                            [=]() -> CanLogTestFixture* { return new OverVoltageTest(entry.path()); });
+    }
+    if (has_flag("cov")) {
+      testing::RegisterTest("CanLogTestFixture", ("TestCellOverVoltage_" + entry.path().filename().string()).c_str(),
+                            nullptr, entry.path().filename().string().c_str(), __FILE__, __LINE__,
+                            [=]() -> CanLogTestFixture* { return new CellOverVoltageTest(entry.path()); });
+    }
+    if (has_flag("cuv")) {
+      testing::RegisterTest("CanLogTestFixture", ("TestCellUnderVoltage_" + entry.path().filename().string()).c_str(),
+                            nullptr, entry.path().filename().string().c_str(), __FILE__, __LINE__,
+                            [=]() -> CanLogTestFixture* { return new CellUnderVoltageTest(entry.path()); });
+    }
+  }
+}

test/can_log_based/utils.cpp

@@ -0,0 +1,109 @@
+#include "utils.h"
+
+#include <fstream>
+
+namespace fs = std::filesystem;
+
+bool ends_with(const std::string& str, const std::string& suffix) {
+  return str.size() >= suffix.size() && str.compare(str.size() - suffix.size(), suffix.size(), suffix) == 0;
+}
+
+std::vector<std::string> split(const std::string& text, char sep) {
+  std::vector<std::string> tokens;
+  std::size_t start = 0, end = 0;
+  while ((end = text.find(sep, start)) != std::string::npos) {
+    tokens.push_back(text.substr(start, end - start));
+    start = end + 1;
+  }
+  tokens.push_back(text.substr(start));
+  return tokens;
+}
+
+void print_frame(const CAN_frame& frame) {
+  std::cout << "ID: " << std::hex << frame.ID << ", DLC: " << (int)frame.DLC << ", Data: ";
+  for (int i = 0; i < frame.DLC; ++i) {
+    std::cout << std::hex << (int)frame.data.u8[i] << " ";
+  }
+  std::cout << std::dec << "\n";
+}
+
+CAN_frame parse_can_log_line(const std::string& logLine) {
+  std::stringstream ss(logLine);
+  CAN_frame frame = {};
+  char dummy;
+
+  double timestamp;
+  std::string interfaceName;
+
+  // timestamp and interface name are parsed but not used
+  ss >> dummy >> timestamp >> dummy;
+  ss >> interfaceName;
+
+  // parse hexadecimal CAN ID
+  ss >> std::hex >> frame.ID;
+  if (ss.fail()) {
+    throw std::runtime_error("Invalid format: Failed to parse CAN ID.");
+  }
+  // check whether the ID is in the extended range
+  frame.ext_ID = (frame.ID > 0x7FF);
+
+  // parse the data length
+  int dlc_val;
+  ss >> dummy;  // Consume '['
+  if (ss.fail() || dummy != '[') {
+    throw std::runtime_error("Invalid format: Missing opening bracket for data length.");
+  }
+  ss >> dlc_val;
+  frame.DLC = static_cast<uint8_t>(dlc_val);
+  ss >> dummy;  // Consume ']'
+  if (ss.fail() || dummy != ']') {
+    throw std::runtime_error("Invalid format: Missing closing bracket for data length.");
+  }
+  // crudely assume CAN FD if DLC > 8
+  frame.FD = (frame.DLC > 8);
+
+  // parse the actual data bytes
+  unsigned int byte;
+  for (int i = 0; i < frame.DLC; ++i) {
+    ss >> std::hex >> byte;
+    if (ss.fail()) {
+      throw std::runtime_error("Fewer data bytes than specified by data length.");
+    }
+    frame.data.u8[i] = static_cast<uint8_t>(byte);
+  }
+
+  return frame;
+}
+
+std::vector<CAN_frame> parse_can_log_file(const fs::path& filePath) {
+  std::ifstream logFile(filePath);
+  if (!logFile.is_open()) {
+    std::cerr << "Error: Could not open file " << filePath << std::endl;
+    return {};
+  }
+
+  std::vector<CAN_frame> frames;
+  std::string line;
+  int lineNumber = 0;
+
+  // read the file line by line
+  while (std::getline(logFile, line)) {
+    lineNumber++;
+    if (line.empty()) {
+      continue;
+    }
+
+    if (line[0] == '#' || line[0] == ';') {
+      continue;  // skip comment lines
+    }
+
+    try {
+      frames.push_back(parse_can_log_line(line));
+    } catch (const std::runtime_error& e) {
+      std::cerr << "Warning: Skipping malformed line " << lineNumber << " in " << filePath.filename()
+                << ". Reason: " << e.what() << std::endl;
+    }
+  }
+
+  return frames;
+}

test/can_log_based/utils.h

@@ -0,0 +1,11 @@
+#include "../../Software/src/devboard/utils/types.h"
+
+#include <filesystem>
+#include <iostream>
+
+namespace fs = std::filesystem;
+
+bool ends_with(const std::string& str, const std::string& suffix);
+std::vector<std::string> split(const std::string& text, char sep);
+
+std::vector<CAN_frame> parse_can_log_file(const fs::path& filePath);

test/emul/Arduino.cpp

@@ -15,6 +15,7 @@ int digitalRead(uint8_t pin) {
   return 0;
 }
 void digitalWrite(uint8_t pin, uint8_t val) {}
+
 unsigned long micros() {
   return 0;
 }
@@ -24,22 +25,11 @@ int max(int a, int b) {
   return (a > b) ? a : b;
 }
 
-// Mock implementation for OBD
-#include "../../Software/src/communication/can/obd.h"
-void handle_obd_frame(CAN_frame& frame) {
-  (void)frame;
+bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, int8_t channel) {
+  return true;
 }
-void transmit_obd_can_frame(unsigned int address, int interface, bool canFD) {
-  (void)interface;
-}
-
-void start_bms_reset() {}
-
-#include "../../Software/src/communication/rs485/comm_rs485.h"
-
-// Mock implementation
-void register_receiver(Rs485Receiver* receiver) {
-  (void)receiver;  // Silence unused parameter warning
+bool ledcWrite(uint8_t pin, uint32_t duty) {
+  return true;
 }
 
 ESPClass ESP;

test/emul/Arduino.h

@@ -121,6 +121,9 @@ void delay(unsigned long ms);
 void delayMicroseconds(unsigned long us);
 int max(int a, int b);
 
+bool ledcAttachChannel(uint8_t pin, uint32_t freq, uint8_t resolution, int8_t channel);
+bool ledcWrite(uint8_t pin, uint32_t duty);
+
 class ESPClass {
  public:
   size_t getFlashChipSize() {

test/emul/can.cpp

@@ -18,3 +18,5 @@ char const* getCANInterfaceName(CAN_Interface) {
 }
 
 void register_transmitter(Transmitter* transmitter) {}
+
+void dump_can_frame(CAN_frame& frame, frameDirection msgDir) {}

test/tests.cpp

@@ -5,8 +5,11 @@
 #include "../Software/src/devboard/safety/safety.h"
 #include "../Software/src/devboard/utils/events.h"
 
+void RegisterCanLogTests(void);
+
 int main(int argc, char** argv) {
   testing::InitGoogleTest(&argc, argv);
+  RegisterCanLogTests();
   return RUN_ALL_TESTS();
 }