From 1ddf460dd1bf1ca5deca00ad12f112d473c45111 Mon Sep 17 00:00:00 2001
From: Kilian Bracher <k.bracher@fasttube.de>
Date: Tue, 25 Feb 2025 20:49:00 +0100
Subject: [PATCH] refactor: enhance SPI error handling and logging

---
 .../Core/Src/ADBMS_HighLevel.c                |  5 --
 .../Core/Src/ADBMS_LL_Driver.c                | 55 ++++++++++++++++++-
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_HighLevel.c b/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_HighLevel.c
index 670f3ed..fc7ca1d 100644
--- a/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_HighLevel.c
+++ b/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_HighLevel.c
@@ -82,11 +82,6 @@ ADBMS_DetailedStatus AMS_Idle_Loop() {
 
     if (any(module.status.CS_FLT || module.status.SPIFLT || module.status.CMED || module.status.SMED ||
             module.status.VDE || module.status.VDEL || module.status.OSCCHK || module.status.TMODCHK)) {
-
-        const uint8_t* ptr = ((uint8_t*)&modules[0].status) + 4; // skip conversion counter
-        error_data[ADBMS_INTERNAL_BMS_FAULT].data[2] = ptr[1];
-        error_data[ADBMS_INTERNAL_BMS_FAULT].data[3] = ptr[0];
-
         // Fault bits are latched -- clear them so we can check again next
         // iteration.
         amsClearFlag();
diff --git a/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_LL_Driver.c b/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_LL_Driver.c
index 243a34c..201df0d 100644
--- a/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_LL_Driver.c
+++ b/AMS_Master_Code/Core/Lib/ADBMS6830B_Driver/Core/Src/ADBMS_LL_Driver.c
@@ -128,6 +128,44 @@ static uint8_t checkDataPEC(uint8_t* data, uint8_t len) {
     return (computeCRC10(data, len, false) == 0) ? 0 : 1;
 }
 
+static const char* const HAL_Statuses[] = {"HAL_OK", "HAL_ERROR", "HAL_BUSY", "HAL_TIMEOUT"};
+
+static void print_spi_details() {
+  uint32_t spi_error = HAL_SPI_GetError(adbmsspi);
+
+  typedef struct {
+    uint32_t mask;
+    const char *label;
+  } SPIError;
+
+  const SPIError errors[] = {
+    {HAL_SPI_ERROR_NONE,            "NONE"},
+    {HAL_SPI_ERROR_MODF,            "MODF"},
+    {HAL_SPI_ERROR_CRC,             "CRC"},
+    {HAL_SPI_ERROR_OVR,             "OVR"},
+    {HAL_SPI_ERROR_FRE,             "FRE"},
+    {HAL_SPI_ERROR_DMA,             "DMA"},
+    {HAL_SPI_ERROR_FLAG,            "FLAG"},
+    {HAL_SPI_ERROR_ABORT,           "ABORT"},
+    {HAL_SPI_ERROR_UDR,             "UDR"},
+    {HAL_SPI_ERROR_TIMEOUT,         "TIMEOUT"},
+    {HAL_SPI_ERROR_UNKNOW,          "UNKNOWN"},
+    {HAL_SPI_ERROR_NOT_SUPPORTED,   "NOT_SUPPORTED"},
+    {HAL_SPI_ERROR_RELOAD,          "RELOAD"},
+#ifdef HAL_SPI_ERROR_INVALID_CALLBACK
+    {HAL_SPI_ERROR_INVALID_CALLBACK,"INVALID_CALLBACK"},
+#endif
+  };
+
+  constexpr size_t numErrors = sizeof(errors) / sizeof(errors[0]);
+  for (size_t i = 0; i < numErrors; i++) {
+    if (spi_error & errors[i].mask) {
+      debug_log_cont(LOG_LEVEL_ERROR, "%s ", errors[i].label);
+    }
+  }
+  debug_log_cont(LOG_LEVEL_ERROR, "\n");
+}
+
 HAL_StatusTypeDef ___writeCMD(uint16_t command, uint8_t * args, size_t arglen) {
   HAL_StatusTypeDef ret;
   if (arglen > 0) {
@@ -167,6 +205,11 @@ HAL_StatusTypeDef ___writeCMD(uint16_t command, uint8_t * args, size_t arglen) {
     mcuAdbmsCSHigh();
   }
 
+  if (ret != HAL_OK) {
+    debug_log(LOG_LEVEL_ERROR, "STM32 SPI HAL returned error %s\n SPI error bits: ", HAL_Statuses[ret]);
+    print_spi_details();
+  }
+
   return ret;
 }
 
@@ -179,7 +222,11 @@ HAL_StatusTypeDef ___readCMD(uint16_t command, uint8_t * buffer, size_t arglen)
   HAL_StatusTypeDef status = mcuSPITransmitReceive(buffer, buffer, CMD_BUFFER_SIZE(arglen));
   mcuAdbmsCSHigh();
 
-  if (status != HAL_OK) return status;
+  if (status != HAL_OK) {
+    debug_log(LOG_LEVEL_ERROR, "STM32 SPI HAL returned error %s\n SPI error bits: ", HAL_Statuses[status]);
+    print_spi_details();
+    return status;
+  }
 
   //[[maybe_unused]] uint8_t commandCounter = buffer[sizeof(buffer) - 2] & 0xFC; //command counter is bits 7-2
                                                                                  //TODO: check command counter?
@@ -220,7 +267,11 @@ HAL_StatusTypeDef __pollCMD(uint16_t command, uint8_t waitTime) {
   HAL_StatusTypeDef status = mcuSPITransmitReceive(buffer, buffer, sizeof buffer);
   mcuAdbmsCSHigh();
 
-  if (status != HAL_OK) return status;
+  if (status != HAL_OK) {
+    debug_log(LOG_LEVEL_ERROR, "STM32 SPI HAL returned error %s\n SPI error bits: ", HAL_Statuses[status]);
+    print_spi_details();
+    return status;
+  }
 
   return ((buffer[4 + (N_BMS * 2)] & 0x0F) == 0x0) ? HAL_BUSY : HAL_OK; //SDO goes high when data is ready
 }
\ No newline at end of file