nopnop2002 / Arduino-LoRa-Ra01S

An Arduino Library for LoRa Communication using SX1262/1268
MIT License
33 stars 11 forks source link

Support threaded environments with RTOS semaphores #16

Closed kazetsukaimiko closed 3 months ago

kazetsukaimiko commented 3 months ago

Hello there, I have successfully integrated a customized version of this library into an ESP32-S3 / FreeRTOS project in PIO. We addressed threading concerns by adding RTOS semaphores to all of the SPI transactions, and I'd like to get this mainlined if possible.

Change overview

First we define a status code for SPI request timeouts:

Ra01S.h

  // Status code define to handle SPI semaphore timeouts
  #define SX126X_STATUS_SEMAPHORE_TIMEOUT               0b10101010  //  7     0     RTOS semaphore timeout without checkout

Then we define methods on the class to take and release the semaphore.

Ra01S.h

class SX126x {
  public:
    // ...
    bool     ReserveSPI();
    bool     FreeSPIReservation();
    // ...
}

Then we wrap all transactions in ReserveSPI and FreeSPIReservation.

Ra01S.cpp

  // Take semaphore or fail the operation. Return nothing for void methods.
  if (!ReserveSPI()) {
    return SX126X_STATUS_SEMAPHORE_TIMEOUT;
  } 

  // SPI op
  digitalWrite(SX126x_SPI_SELECT, LOW);
  SPI.beginTransaction(SPISettings(2000000, MSBFIRST, SPI_MODE0));
  // ...
  SPI.endTransaction();
  digitalWrite(SX126x_SPI_SELECT, HIGH); 

  // Release Semaphore
  FreeSPIReservation();

Caveat: Some transactions occur in void methods, which do not indicate success or failure. Examining these usages, this addition may necessitate a return value. I did not add this by default to maintain backwards API compatibility.

Finally to enable define-based integration, we have one flag and two overrides:

// Enable RA01S to use semaphores.
#define RA01S_USE_SPI_SEMAPHORES 1
// Optional: Define our own semaphore instance
extern SemaphoreHandle_t rtos_Semaphore_SPI;
#define RA01S_SEMAPHORE_HANDLE rtos_Semaphore_SPI
// Optional: Define our own timeout.
#define RA01S_SEMAPHORE_TIMEOUT 100

Alternatively, override the programmatic methods.

SomeKindOfLock mySPILock;
class SX126x_WithSomeKindOfLock : public SX126x {
public:
    SX126x_ResourceLocked(int spiSelect, int reset, int busy, int txen = -1, int rxen = -1, bool eager = false) :
            SX126x(spiSelect, reset, busy, txen, rxen, eager) {}
    bool ReserveSPI() {
        return mySPILock.take();
    }

    bool FreeSPIReservation() {
        return mySPILock.give();
    }
};
nopnop2002 commented 3 months ago

Please use this for ESP32.

Arduino for ESP32 has too many limitations. And it has too few features.

https://github.com/nopnop2002/esp-idf-sx126x

kazetsukaimiko commented 3 months ago

Please use this for ESP32.

https://github.com/nopnop2002/esp-idf-sx126x

Can you elaborate? Should I reimplement these changes there, or do you already have a mechanism to lock the SPI bus?

kazetsukaimiko commented 3 months ago

Please use this for ESP32.

Arduino for ESP32 has too many limitations. And it has too few features.

I agree. PLEASE update the readme here to note that.

nopnop2002 commented 3 months ago

do you already have a mechanism to lock the SPI bus?

By using ESP-IDF multitasking, simultaneous SPI accesses can be completely prevented.