STM32WLXX SubGHZ Phy Radio Drivers: Confusing Definitions for LORA Bandwidths

[biobuilder]

When defining a LoRa application bandwidth, the function caller may write the following:

#define LORA_BANDWIDTH 0  // 125 kHz  /* [0: 125 kHz, 1: 250 kHz, 2: 500 kHz, 3: Reserved] ???*/

Which we then subsequently fill into the tx configuration here:

Radio.SetTxConfig(MODEM_LORA,                   // modem configuration
                    LORA_TX_OUTPUT_POWER,         // output power in dBm
                    0,                            // fdev (not used for LoRa)
                    LORA_BANDWIDTH,               // bandwidth index (0 -> 125 kHz)
                    LORA_SPREADING_FACTOR,        // spreading factor (SF7)
                    LORA_CODINGRATE,              // coding rate (4/5)
                    LORA_PREAMBLE_LENGTH,         // preamble length in symbols
                    LORA_FIX_LENGTH_PAYLOAD,      // fixed length payload (true / false)
                    LORA_USE_CRC,                 // crcOn (true / false)
                    0,                            // freqHopOn (true / false)             
                    0,                            // HopPeriod (not used for LoRa)
                    false,                        // IQ inversion (true / false)
                    LORA_TX_TIMEOUT_VALUE         // timeout value (ms)
                  );

The application LORA tx and rx configurations are defined by referencing an index in a global constant in "radio.c:"

const RadioLoRaBandwidths_t Bandwidths[] = { LORA_BW_125, LORA_BW_250, LORA_BW_500 };

However, in the radio_driver.h file, the configurations are defined as follows:

typedef enum
{
    LORA_BW_500                             = 6,
    LORA_BW_250                             = 5,
    LORA_BW_125                             = 4,
    LORA_BW_062                             = 3,
    LORA_BW_041                             = 10,
    LORA_BW_031                             = 2,
    LORA_BW_020                             = 9,
    LORA_BW_015                             = 1,
    LORA_BW_010                             = 8,
    LORA_BW_007                             = 0,
}RadioLoRaBandwidths_t;

Wouldn't it be more helpful and less memory for these enum definitions to be publicly exposed in the user application? 

For instance, the definitions for RadioLoRaCodingRates_t and RadioLoRaSpreadingFactors_t all properly match the function calls.  

The following mistake (among many other difficulties of just trying to understand how the Radio Driver works alongside FreeRTOS) cost us almost a week of painful debugging on a custom STM32WL5MOCH bare metal board:

#define LORA_BANDWIDTH LORA_BW_125

We had included the base "radio_driver.h" board to pull out those enums for defining our application. 

Our first attempts at integrating the SubGHZ Phy middleware was not working due to some configuration issues, which were not immediately apparent from the configuration software.  There were several modifications we did not discover until much later in the process.  We even re-wrote all of the driver functions to be a little more "rtos-friendly" with shorter blocking times and servicing routines outside of ISRs.  But we found out that clearing the radio spi ISRs requires blocking functions to work properly.  Getting the ISR to trigger some RTOS task just didn't cut it.  So we have to rely on those low level blocking HAL SPI functions.  I don't know why all the low level radio driver functions are blocking - aside from the fact that a second core could be babysitting these functions.  Seems like these functions could be interrupt driven and simply processed by the host task without soaking up huge amounts of time.  Since adding the second core and performing all functions as needed has proven to be extremely complicated to configure, we may just count this as a win for right now and move on to the MVP application.  

[STTwo-32]

Hello @biobuilder 

Do you have any question or recommendations about the current implementation of the SubGHZ Phy middleware. If so, we will be happy if you can write about that.

Best Regards.

STTwo-32

[biobuilder]

Hi STTwo-32,

Thanks for checking in.  Our objective was to reduce blocking time in the HAL SubGHZ ISR to make the program lighter weight and more RTOS-friendly in a single-core environment.  We wanted the IRQ handler to set some flags so our Radio Task could process the radio requests without blocking everything else in our program.

void HAL_SUBGHZ_IRQHandler(SUBGHZ_HandleTypeDef *hsubghz)

We discovered that the radio SPI peripheral had to be cleared immediately using the built-in blocking delays or else the system would get stuck indefinitely within this ISR.  Basically, we could not read/write to the radio SPI hardware outside of the ISR, or else the ISR would retrigger itself over and over again.

The ISR calls numerous blocking functions, which could all take many milliseconds to execute.  In the radio driver SPI communication functions, you'll see loads of these blocking delay loops:

  /* Initialize Timeout */
  count = SUBGHZ_DEFAULT_TIMEOUT * SUBGHZ_DEFAULT_LOOP_TIME;

  /* Wait until RXNE flag is set */
  do
  {
    if (count == 0U)
    {
      status = HAL_ERROR;
      hsubghz->ErrorCode = HAL_SUBGHZ_ERROR_TIMEOUT;
      break;
    }
    count--;
  } while (READ_BIT(SUBGHZSPI->SR, SPI_SR_RXNE) != (SPI_SR_RXNE));

These loops could in theory block for up to 100 milliseconds depending on the radio functions.  This means when the HAL ISR is called, it could sit in a blocked state for a very long time.

The radio timing is somewhat time-sensitive, yet being able to respond within 1 millisecond is possible with an RTOS.  All these "fast" blocking delays seem to be a violation of good ISR programming practice.  Why can't the calling ISR simply set some flags so a thread can process them?