From 203961a8c457ca714e27d0194285bd2e0b68926c Mon Sep 17 00:00:00 2001 From: Foniks Date: Sat, 14 Feb 2026 00:13:56 +0100 Subject: [PATCH 1/8] feat(SPI): Add error recovery mechanism to SPI --- Inc/HALAL/Models/SPI/SPI2.hpp | 54 +++++++++++++++++++++++++++++++++++ Src/HALAL/Models/SPI/SPI2.cpp | 15 +++++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index dbcd23507..e481a25d5 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -469,6 +469,21 @@ struct SPIDomain { SPI_TypeDef* instance; volatile bool* operation_flag = nullptr; + uint32_t error_count = 0; + bool was_aborted = false; + + void recover() { + // Abort any ongoing SPI operation + HAL_SPI_Abort(&hspi); + + error_count++; + was_aborted = true; + operation_flag = nullptr; + + // Reset SPI state + HAL_SPI_DeInit(&hspi); + HAL_SPI_Init(&hspi); + } }; @@ -493,6 +508,31 @@ struct SPIDomain { SPIWrapper(Instance &instance) : spi_instance{instance} {} + /** + * @brief Aborts any ongoing SPI operation and recovers the peripheral. + */ + void abort_and_recover() { + spi_instance.recover(); + } + + /** + * @brief Checks if the SPI instance was aborted since the last reset. + */ + bool was_aborted() const { + return spi_instance.was_aborted; + } + + void clear_abort_flag() { + spi_instance.was_aborted = false; + } + + /** + * @brief Gets the error count for this SPI instance. + */ + uint32_t get_error_count() const { + return spi_instance.error_count; + } + /** * @brief Sends data over SPI in blocking mode. */ @@ -749,6 +789,20 @@ struct SPIDomain { } } + /** + * @brief Aborts any ongoing SPI operation and recovers the peripheral. + */ + void abort_and_recover() { + spi_instance.recover(); + } + + /** + * @brief Gets the error count for this SPI instance. + */ + uint32_t get_error_count() const { + return spi_instance.error_count; + } + /** * @brief Listens for data over SPI using DMA, uses an optional operation flag to signal completion. */ diff --git a/Src/HALAL/Models/SPI/SPI2.cpp b/Src/HALAL/Models/SPI/SPI2.cpp index f83ea553c..f7540336f 100644 --- a/Src/HALAL/Models/SPI/SPI2.cpp +++ b/Src/HALAL/Models/SPI/SPI2.cpp @@ -100,6 +100,7 @@ void HAL_SPI_TxCpltCallback(SPI_HandleTypeDef *hspi) { if (inst->operation_flag != nullptr) { *(inst->operation_flag) = true; + inst->operation_flag = nullptr; // Clear pointer after setting flag } } @@ -115,25 +116,37 @@ void HAL_SPI_ErrorCallback(SPI_HandleTypeDef *hspi) { auto& spi_instances = ST_LIB::SPIDomain::spi_instances; uint32_t error_code = hspi->ErrorCode; + ST_LIB::SPIDomain::Instance* inst = nullptr; uint32_t inst_idx = 0; + if (spi_instances[0] != nullptr && hspi == &spi_instances[0]->hspi) { + inst = spi_instances[0]; inst_idx = 0; } else if (spi_instances[1] != nullptr && hspi == &spi_instances[1]->hspi) { + inst = spi_instances[1]; inst_idx = 1; } else if (spi_instances[2] != nullptr && hspi == &spi_instances[2]->hspi) { + inst = spi_instances[2]; inst_idx = 2; } else if (spi_instances[3] != nullptr && hspi == &spi_instances[3]->hspi) { + inst = spi_instances[3]; inst_idx = 3; } else if (spi_instances[4] != nullptr && hspi == &spi_instances[4]->hspi) { + inst = spi_instances[4]; inst_idx = 4; } else if (spi_instances[5] != nullptr && hspi == &spi_instances[5]->hspi) { + inst = spi_instances[5]; inst_idx = 5; } else { ErrorHandler("SPI IRQ Callback called but instance is null"); return; } - ErrorHandler("SPI%i failed with error number %u", inst_idx + 1, error_code); + // Attempt recovery before failing + inst->recover(); + + ErrorHandler("SPI%i failed with error number %u (recovered, error count: %u)", + inst_idx + 1, error_code, inst->error_count); } } \ No newline at end of file From f2cb8dde61484a79f086f3acb4d447f449aaf4b9 Mon Sep 17 00:00:00 2001 From: Foniks Date: Sat, 14 Feb 2026 00:31:29 +0100 Subject: [PATCH 2/8] fix(SPI): Add DeInit to mock --- Inc/HALAL/Models/SPI/SPI2.hpp | 1 + Inc/MockedDrivers/stm32h7xx_hal_mock.h | 1 + 2 files changed, 2 insertions(+) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index e481a25d5..82b68ff5a 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -8,6 +8,7 @@ #ifndef SPI2_HPP #define SPI2_HPP +#include "hal_wrapper.h" #include "C++Utilities/CppImports.hpp" #include "HALAL/Models/GPIO.hpp" #include "HALAL/Models/Pin.hpp" diff --git a/Inc/MockedDrivers/stm32h7xx_hal_mock.h b/Inc/MockedDrivers/stm32h7xx_hal_mock.h index 704e581d2..23d5af346 100644 --- a/Inc/MockedDrivers/stm32h7xx_hal_mock.h +++ b/Inc/MockedDrivers/stm32h7xx_hal_mock.h @@ -816,6 +816,7 @@ uint32_t HAL_ADC_GetState(const ADC_HandleTypeDef *hadc); uint32_t HAL_ADC_GetError(const ADC_HandleTypeDef *hadc); HAL_StatusTypeDef HAL_SPI_Init(SPI_HandleTypeDef *hspi); +HAL_StatusTypeDef HAL_SPI_DeInit(SPI_HandleTypeDef *hspi); HAL_StatusTypeDef HAL_SPI_Abort(SPI_HandleTypeDef *hspi); HAL_StatusTypeDef HAL_SPI_Transmit(SPI_HandleTypeDef *hspi, uint8_t *pData, uint16_t Size, uint32_t Timeout); From 64cca20eb9a0e94b828e1c2d7a4ccddf38e22690 Mon Sep 17 00:00:00 2001 From: Foniks Date: Sat, 14 Feb 2026 00:37:03 +0100 Subject: [PATCH 3/8] feat(SPI): Some nitpicks and improvements --- Inc/HALAL/Models/SPI/SPI2.hpp | 17 ++++++++++++----- Src/HALAL/Models/SPI/SPI2.cpp | 10 ++++++---- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index 82b68ff5a..c73282e81 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -470,10 +470,10 @@ struct SPIDomain { SPI_TypeDef* instance; volatile bool* operation_flag = nullptr; - uint32_t error_count = 0; - bool was_aborted = false; + volatile uint32_t error_count = 0; + volatile bool was_aborted = false; - void recover() { + bool recover() { // Abort any ongoing SPI operation HAL_SPI_Abort(&hspi); @@ -482,8 +482,15 @@ struct SPIDomain { operation_flag = nullptr; // Reset SPI state - HAL_SPI_DeInit(&hspi); - HAL_SPI_Init(&hspi); + auto status = HAL_SPI_DeInit(&hspi); + if (status != HAL_OK) { + return false; + } + status = HAL_SPI_Init(&hspi); + if (status != HAL_OK) { + return false; + } + return true; } }; diff --git a/Src/HALAL/Models/SPI/SPI2.cpp b/Src/HALAL/Models/SPI/SPI2.cpp index f7540336f..bca7d315e 100644 --- a/Src/HALAL/Models/SPI/SPI2.cpp +++ b/Src/HALAL/Models/SPI/SPI2.cpp @@ -142,11 +142,13 @@ void HAL_SPI_ErrorCallback(SPI_HandleTypeDef *hspi) { return; } - // Attempt recovery before failing - inst->recover(); + if (!inst->recover()) { + ErrorHandler("SPI%i failed with error number %u (recovery failed, error count: %u)", + inst_idx + 1, error_code, inst->error_count); + } - ErrorHandler("SPI%i failed with error number %u (recovered, error count: %u)", - inst_idx + 1, error_code, inst->error_count); + (void)error_code; + (void)inst_idx; } } \ No newline at end of file From 4267ef43d60d777f083aada160176b2cc80f5f3e Mon Sep 17 00:00:00 2001 From: Boris Mladenov Beslimov Date: Sat, 14 Feb 2026 00:38:22 +0100 Subject: [PATCH 4/8] fix(SPI): Make slave interface consistent with master Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- Inc/HALAL/Models/SPI/SPI2.hpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index c73282e81..7394c14d6 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -811,6 +811,20 @@ struct SPIDomain { return spi_instance.error_count; } + /** + * @brief Indicates whether the last operation was aborted. + */ + bool was_aborted() const { + return spi_instance.was_aborted; + } + + /** + * @brief Clears the aborted state flag for this SPI instance. + */ + void clear_abort_flag() { + spi_instance.was_aborted = false; + } + /** * @brief Listens for data over SPI using DMA, uses an optional operation flag to signal completion. */ From c17d00c74f5f5f85e1137ea4553c1cc69d827e99 Mon Sep 17 00:00:00 2001 From: Foniks Date: Sat, 14 Feb 2026 00:42:29 +0100 Subject: [PATCH 5/8] fix(mocks): include stdint for uint32_t in common.hpp --- Inc/MockedDrivers/common.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Inc/MockedDrivers/common.hpp b/Inc/MockedDrivers/common.hpp index 57a62719d..1645ad1e7 100644 --- a/Inc/MockedDrivers/common.hpp +++ b/Inc/MockedDrivers/common.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #ifndef glue #define glue_(a,b) a##b #define glue(a,b) glue_(a,b) From ef0a32190e3a287171d34d0553b838cb1e3c2096 Mon Sep 17 00:00:00 2001 From: Foniks Date: Sat, 14 Feb 2026 00:45:07 +0100 Subject: [PATCH 6/8] fix(SPI): volatile ++ doesn't work --- Inc/HALAL/Models/SPI/SPI2.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index 7394c14d6..f01d9dcbc 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -477,7 +477,8 @@ struct SPIDomain { // Abort any ongoing SPI operation HAL_SPI_Abort(&hspi); - error_count++; + uint32_t newErrorCount = error_count + 1; // Cause volatile in c++ freacking sucks + error_count = newErrorCount; was_aborted = true; operation_flag = nullptr; From 884f793d5602f581324a30cf66534c5c29886687 Mon Sep 17 00:00:00 2001 From: Foniks Date: Sat, 14 Feb 2026 00:53:50 +0100 Subject: [PATCH 7/8] fix(SPI): Small things to make it compile in more compiler versions --- Inc/HALAL/Models/SPI/SPI2.hpp | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index f01d9dcbc..ab6125f15 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -240,18 +240,18 @@ struct SPIDomain { nss_idx = nss_gpio.value().inscribe(ctx); } - Entry e{ - .peripheral = peripheral, - .mode = mode, - .sck_gpio_idx = sck_gpio.inscribe(ctx), - .miso_gpio_idx = miso_gpio.inscribe(ctx), - .mosi_gpio_idx = mosi_gpio.inscribe(ctx), - .nss_gpio_idx = nss_idx, - .dma_rx_idx = dma_indices[0], - .dma_tx_idx = dma_indices[1], - .max_baudrate = max_baudrate, - .config = config - }; + Entry e; + + e.peripheral = peripheral; + e.mode = mode; + e.sck_gpio_idx = sck_gpio.inscribe(ctx); + e.miso_gpio_idx = miso_gpio.inscribe(ctx); + e.mosi_gpio_idx = mosi_gpio.inscribe(ctx); + e.nss_gpio_idx = nss_idx; + e.dma_rx_idx = dma_indices[0]; + e.dma_tx_idx = dma_indices[1]; + e.max_baudrate = max_baudrate; + e.config = config; return ctx.template add(e, this); } From 76c0126fe76d72ae5947879ebf51f4ce29c08410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20S=C3=A1ez?= Date: Sat, 14 Feb 2026 12:52:03 +0100 Subject: [PATCH 8/8] updated tests to match new functionality --- Inc/HALAL/Models/SPI/SPI2.hpp | 36 +++++++++--------------------- Src/HALAL/Models/SPI/SPI2.cpp | 20 ++++++++++------- Tests/spi2_test.cpp | 42 ++++++++++++++++++++++++++++++++--- 3 files changed, 61 insertions(+), 37 deletions(-) diff --git a/Inc/HALAL/Models/SPI/SPI2.hpp b/Inc/HALAL/Models/SPI/SPI2.hpp index c22daf61d..bdf7a2e1c 100644 --- a/Inc/HALAL/Models/SPI/SPI2.hpp +++ b/Inc/HALAL/Models/SPI/SPI2.hpp @@ -527,12 +527,12 @@ struct SPIDomain { bool recover() { // Abort any ongoing SPI operation HAL_SPI_Abort(&hspi); - + uint32_t newErrorCount = error_count + 1; // Cause volatile in c++ freacking sucks error_count = newErrorCount; was_aborted = true; operation_flag = nullptr; - + // Reset SPI state auto status = HAL_SPI_DeInit(&hspi); if (status != HAL_OK) { @@ -570,27 +570,19 @@ struct SPIDomain { /** * @brief Aborts any ongoing SPI operation and recovers the peripheral. */ - void abort_and_recover() { - spi_instance.recover(); - } + void abort_and_recover() { spi_instance.recover(); } /** * @brief Checks if the SPI instance was aborted since the last reset. */ - bool was_aborted() const { - return spi_instance.was_aborted; - } + bool was_aborted() const { return spi_instance.was_aborted; } - void clear_abort_flag() { - spi_instance.was_aborted = false; - } + void clear_abort_flag() { spi_instance.was_aborted = false; } /** * @brief Gets the error count for this SPI instance. */ - uint32_t get_error_count() const { - return spi_instance.error_count; - } + uint32_t get_error_count() const { return spi_instance.error_count; } /** * @brief Sends data over SPI in blocking mode. @@ -1031,30 +1023,22 @@ struct SPIDomain { /** * @brief Aborts any ongoing SPI operation and recovers the peripheral. */ - void abort_and_recover() { - spi_instance.recover(); - } + void abort_and_recover() { spi_instance.recover(); } /** * @brief Gets the error count for this SPI instance. */ - uint32_t get_error_count() const { - return spi_instance.error_count; - } + uint32_t get_error_count() const { return spi_instance.error_count; } /** * @brief Indicates whether the last operation was aborted. */ - bool was_aborted() const { - return spi_instance.was_aborted; - } + bool was_aborted() const { return spi_instance.was_aborted; } /** * @brief Clears the aborted state flag for this SPI instance. */ - void clear_abort_flag() { - spi_instance.was_aborted = false; - } + void clear_abort_flag() { spi_instance.was_aborted = false; } /** * @brief Listens for data over SPI using DMA, uses an optional operation flag to signal diff --git a/Src/HALAL/Models/SPI/SPI2.cpp b/Src/HALAL/Models/SPI/SPI2.cpp index 3d138a40a..16f8dbbea 100644 --- a/Src/HALAL/Models/SPI/SPI2.cpp +++ b/Src/HALAL/Models/SPI/SPI2.cpp @@ -99,7 +99,7 @@ void HAL_SPI_TxCpltCallback(SPI_HandleTypeDef* hspi) { if (inst->operation_flag != nullptr) { *(inst->operation_flag) = true; - inst->operation_flag = nullptr; // Clear pointer after setting flag + inst->operation_flag = nullptr; // Clear pointer after setting flag } } @@ -117,7 +117,7 @@ void HAL_SPI_ErrorCallback(SPI_HandleTypeDef* hspi) { uint32_t error_code = hspi->ErrorCode; ST_LIB::SPIDomain::Instance* inst = nullptr; uint32_t inst_idx = 0; - + if (spi_instances[0] != nullptr && hspi == &spi_instances[0]->hspi) { inst = spi_instances[0]; inst_idx = 0; @@ -141,12 +141,16 @@ void HAL_SPI_ErrorCallback(SPI_HandleTypeDef* hspi) { return; } - if (!inst->recover()) { - ErrorHandler("SPI%i failed with error number %u (recovery failed, error count: %u)", - inst_idx + 1, error_code, inst->error_count); + if (!inst->recover()) { + ErrorHandler( + "SPI%i failed with error number %u (recovery failed, error count: %u)", + inst_idx + 1, + error_code, + inst->error_count + ); } - - (void)error_code; - (void)inst_idx; + + (void)error_code; + (void)inst_idx; } } \ No newline at end of file diff --git a/Tests/spi2_test.cpp b/Tests/spi2_test.cpp index 46f13dbfa..e0d50fb6b 100644 --- a/Tests/spi2_test.cpp +++ b/Tests/spi2_test.cpp @@ -299,18 +299,54 @@ TEST_F(SPI2Test, CallbacksWithUnknownHandleTriggerErrorPath) { EXPECT_EQ(ST_LIB::TestErrorHandler::call_count, 1); } -TEST_F(SPI2Test, ErrorCallbackOnKnownHandleTriggersErrorPath) { +TEST_F(SPI2Test, ErrorCallbackOnKnownHandleRecoversWithoutErrorPath) { ST_LIB::TestErrorHandler::set_fail_on_error(false); - init_spi( - 20'000'000U + auto& instance = + init_spi( + 20'000'000U + ); + ST_LIB::SPIDomain::SPIWrapper spi(instance); + + const auto before_abort = + ST_LIB::MockedHAL::spi_get_call_count(ST_LIB::MockedHAL::SPIOperation::Abort); + const auto before_init = + ST_LIB::MockedHAL::spi_get_call_count(ST_LIB::MockedHAL::SPIOperation::Init); + + auto* hspi = ST_LIB::MockedHAL::spi_get_last_handle(); + ASSERT_NE(hspi, nullptr); + hspi->ErrorCode = 0x55U; + HAL_SPI_ErrorCallback(hspi); + + EXPECT_EQ(ST_LIB::TestErrorHandler::call_count, 0); + EXPECT_EQ( + ST_LIB::MockedHAL::spi_get_call_count(ST_LIB::MockedHAL::SPIOperation::Abort), + before_abort + 1U + ); + EXPECT_EQ( + ST_LIB::MockedHAL::spi_get_call_count(ST_LIB::MockedHAL::SPIOperation::Init), + before_init + 1U ); + EXPECT_EQ(spi.get_error_count(), 1U); + EXPECT_TRUE(spi.was_aborted()); +} + +TEST_F(SPI2Test, ErrorCallbackOnKnownHandleTriggersErrorPathWhenRecoveryFails) { + ST_LIB::TestErrorHandler::set_fail_on_error(false); + auto& instance = + init_spi( + 20'000'000U + ); + ST_LIB::SPIDomain::SPIWrapper spi(instance); auto* hspi = ST_LIB::MockedHAL::spi_get_last_handle(); ASSERT_NE(hspi, nullptr); hspi->ErrorCode = 0x55U; + ST_LIB::MockedHAL::spi_set_status(HAL_ERROR); HAL_SPI_ErrorCallback(hspi); EXPECT_EQ(ST_LIB::TestErrorHandler::call_count, 1); + EXPECT_EQ(spi.get_error_count(), 1U); + EXPECT_TRUE(spi.was_aborted()); } TEST_F(SPI2Test, SlaveWrapperDMAOperations) {