fixing beginTransaction() thread safety (#6425)

Author: smarq8

Diff for: libraries/SPI/src/SPI.cpp

@@ -20,6 +20,15 @@
  */
 
 #include "SPI.h"
+#include "esp32-hal-log.h"
+
+#if !CONFIG_DISABLE_HAL_LOCKS
+#define SPI_PARAM_LOCK()    do {} while (xSemaphoreTake(paramLock, portMAX_DELAY) != pdPASS)
+#define SPI_PARAM_UNLOCK()  xSemaphoreGive(paramLock)
+#else
+#define SPI_PARAM_LOCK()
+#define SPI_PARAM_UNLOCK()
+#endif
 
 SPIClass::SPIClass(uint8_t spi_bus)
     :_spi_num(spi_bus)
@@ -32,7 +41,31 @@ SPIClass::SPIClass(uint8_t spi_bus)
     ,_div(0)
     ,_freq(1000000)
     ,_inTransaction(false)
+#if !CONFIG_DISABLE_HAL_LOCKS
+    ,paramLock(NULL)
+{
+    if(paramLock==NULL){
+        paramLock = xSemaphoreCreateMutex();
+        if(paramLock==NULL){
+            log_e("xSemaphoreCreateMutex failed");
+            return;
+        }
+    }
+}
+#else
 {}
+#endif
+
+SPIClass::~SPIClass()
+{
+    end();
+#if !CONFIG_DISABLE_HAL_LOCKS
+    if(paramLock!=NULL){
+        vSemaphoreDelete(paramLock);
+        paramLock = NULL;
+    }
+#endif
+}
 
 void SPIClass::begin(int8_t sck, int8_t miso, int8_t mosi, int8_t ss)
 {
@@ -106,19 +139,23 @@ void SPIClass::setHwCs(bool use)
 
 void SPIClass::setFrequency(uint32_t freq)
 {
+    SPI_PARAM_LOCK();
     //check if last freq changed
     uint32_t cdiv = spiGetClockDiv(_spi);
     if(_freq != freq || _div != cdiv) {
         _freq = freq;
         _div = spiFrequencyToClockDiv(_freq);
         spiSetClockDiv(_spi, _div);
     }
+    SPI_PARAM_UNLOCK();
 }
 
 void SPIClass::setClockDivider(uint32_t clockDiv)
 {
+    SPI_PARAM_LOCK();
     _div = clockDiv;
     spiSetClockDiv(_spi, _div);
+    SPI_PARAM_UNLOCK();
 }
 
 uint32_t SPIClass::getClockDivider()
@@ -138,6 +175,7 @@ void SPIClass::setBitOrder(uint8_t bitOrder)
 
 void SPIClass::beginTransaction(SPISettings settings)
 {
+    SPI_PARAM_LOCK();
     //check if last freq changed
     uint32_t cdiv = spiGetClockDiv(_spi);
     if(_freq != settings._clock || _div != cdiv) {
@@ -153,6 +191,7 @@ void SPIClass::endTransaction()
     if(_inTransaction){
         _inTransaction = false;
         spiEndTransaction(_spi);
+        SPI_PARAM_UNLOCK(); // <-- Im not sure should it be here or right after spiTransaction()
     }
 }
 

Diff for: libraries/SPI/src/SPI.h

@@ -24,6 +24,8 @@
 #include <stdlib.h>
 #include "pins_arduino.h"
 #include "esp32-hal-spi.h"
+#include "freertos/FreeRTOS.h"
+#include "freertos/semphr.h"
 
 #define SPI_HAS_TRANSACTION
 
@@ -50,10 +52,14 @@ class SPIClass
     uint32_t _div;
     uint32_t _freq;
     bool _inTransaction;
+#if !CONFIG_DISABLE_HAL_LOCKS
+    SemaphoreHandle_t paramLock=NULL;
+#endif
     void writePattern_(const uint8_t * data, uint8_t size, uint8_t repeat);
 
 public:
     SPIClass(uint8_t spi_bus=HSPI);
+    ~SPIClass();
     void begin(int8_t sck=-1, int8_t miso=-1, int8_t mosi=-1, int8_t ss=-1);
     void end();