Request to restore SPI.transfer(bufout, bufin, count) removed from 2.7.0.

[greiman]

This version of transfer:
void transfer(void *_bufout, void *_bufin, size_t _count, SPITransferMode _mode = SPI_LAST)

Has been removed form 2.7.0.

The API is important for several reasons and should be restored in an improved form. See below for an API common in other board support packages.

The Arduino Standard array API, void transfer(void *buf, size_t count) is not suitable for many applications.

1. buf will be overwritten. This means you must copy buf to tmp array if it is must not be overwritten.

2. Some devices such as SD cards in SPI mode are full duplex examine the data stream so you must fill buf with a value. Typically this is 0XFF or 0X00.

3. The remaining transfer(buf, count) is not DMA. This can be very slow. See my logic analyzer trace of 2.7.0. It's about 1/4 as fast as DMA for this case.

4. Here is a version of transfer that is common with several vendors that solves the above problems and provides send only and receive only by using nullptr for either the send or receive buffer. An async callback mode would be nice but is optional.

SPI.transfer(const void* tx_buffer, void* rx_buffer, size_t length, callback_t myFunction = nullptr)

tx_buffer: array of Tx bytes that is filled by the user before starting the SPI transfer. If nullptr, default dummy 0xFF bytes will be clocked out.

rx_buffer: array of Rx bytes that will be filled by the slave during the SPI transfer. If nullptr, the received data will be discarded.

length: number of data bytes that are to be transferred

myFunction: user specified function callback to be called after completion of the SPI DMA transfer. It takes no argument and returns nothing, e.g.: void myHandler(). Can be nullptr if not using a callback.

NOTE: tx_buffer and rx_buffer sizes MUST be identical (of size length)

[fpistm]

Hi @greiman
Thanks for submitting the issue.
I've made a PR to solve and enhance the API as suggested.
If you can test and give me your feedback, it would be nice.

[greiman]

If you can test and give me your feedback, it would be nice.

It appears that transfer(tx_buf, rx_buf, count) works OK in the modes I used before 2.7.0.

For my test with a Nucleo L476RG it appears not to use DMA. I will look into this when I have time.

I also tested for the case where rx_buf is nullptr. It appears to fail in a way I have seen with other board packages.

The write with transfer(tx_buf, nullptr, count) seems to work OK but a single byte receive after this call gets the wrong byte.

I saw this in the Adafruit SAMD SPI library.

In the SAMD board package transfer(tx_buf, nullptr, count) did not clear the SPI controller of the last rx byte. After that single byte transfer was out of sync, the receive stream had an extra byte.

SdFat fails since the single byte read after the block write for SD write status is not correct.

I don't have time to pursue this case right now. This was a difficult problem to diagnose in the SAMD case.

[greiman]

I looked at SPI/src/utility/spi_com.c at about line 536.

Looks like DMA is not used and receive bytes are not read if rx_buffer is NULL.

      if (rx_buffer) {
#if defined(SPI_SR_RXP)
        while (!LL_SPI_IsActiveFlag_RXP(_SPI));
#else
        while (!LL_SPI_IsActiveFlag_RXNE(_SPI));
#endif
        *rx_buffer++ = LL_SPI_ReceiveData8(_SPI);   // <<<Here  - probably leaves one or more bytes in controller<<<<
      }

[greiman]

I modified two functions and all SdFat SPI options seem to work.

Here is the mod for SPI/src/utility/spi_com.c . Look at the section with #ifdef OLD_WAY //

/**
  * @brief This function is implemented by user to send/receive data over
  *         SPI interface
  * @param  obj : pointer to spi_t structure
  * @param  tx_buffer : tx data to send before reception
  * @param  rx_buffer : rx data to receive if not numm
  * @param  len : length in byte of the data to send and receive
  * @retval status of the send operation (0) in case of error
  */
spi_status_e spi_transfer(spi_t *obj, const uint8_t *tx_buffer, uint8_t *rx_buffer,
                          uint16_t len)
{
  spi_status_e ret = SPI_OK;
  uint32_t tickstart, size = len;
  SPI_TypeDef *_SPI = obj->handle.Instance;
  uint8_t *tx_buf = (uint8_t *)tx_buffer;

  if (len == 0) {
    ret = SPI_ERROR;
  } else {
    tickstart = HAL_GetTick();

#if defined(SPI_CR2_TSIZE)
    /* Start transfer */
    LL_SPI_SetTransferSize(_SPI, size);
    LL_SPI_Enable(_SPI);
    LL_SPI_StartMasterTransfer(_SPI);
#endif

    while (size--) {
#if defined(SPI_SR_TXP)
      while (!LL_SPI_IsActiveFlag_TXP(_SPI));
#else
      while (!LL_SPI_IsActiveFlag_TXE(_SPI));
#endif
#ifdef OLD_WAY  //=================================================
      LL_SPI_TransmitData8(_SPI, *tx_buf++);

      if (rx_buffer) {
#if defined(SPI_SR_RXP)
        while (!LL_SPI_IsActiveFlag_RXP(_SPI));
#else
        while (!LL_SPI_IsActiveFlag_RXNE(_SPI));
#endif
        *rx_buffer++ = LL_SPI_ReceiveData8(_SPI);
      }
#else  // OLD_WAY ================================================
      LL_SPI_TransmitData8(_SPI, tx_buf ? *tx_buf++ : 0XFF);
#if defined(SPI_SR_RXP)
        while (!LL_SPI_IsActiveFlag_RXP(_SPI));
#else
        while (!LL_SPI_IsActiveFlag_RXNE(_SPI));
#endif
        if (rx_buffer) {
          *rx_buffer++ = LL_SPI_ReceiveData8(_SPI);
        } else {
          LL_SPI_ReceiveData8(_SPI);
        }
#endif // OLD_WAY    ==============================================
      
      if ((SPI_TRANSFER_TIMEOUT != HAL_MAX_DELAY) &&
          (HAL_GetTick() - tickstart >= SPI_TRANSFER_TIMEOUT)) {
        ret = SPI_TIMEOUT;
        break;
      }
    }

Here is the SPI/src/SPI,cpp mod

/**
  * @brief  Transfer several bytes. One constant buffer used to send and
  *         one to receive data.
  *         begin() or beginTransaction() must be called at least once before.
  * @param  tx_buf: array of Tx bytes that is filled by the user before starting
  *                 the SPI transfer. If NULL, default dummy 0xFF bytes will be
  *                 clocked out.
  * @param  rx_buf: array of Rx bytes that will be filled by the slave during
  *                 the SPI transfer. If NULL, the received data will be discarded.
  * @param  count: number of bytes to send/receive.
  */
void SPIClass::transfer(const void *tx_buf, void *rx_buf, size_t count)
{
#ifdef OLD_WAY  //========================================
  if (tx_buf) {
    spi_transfer(&_spi, ((const uint8_t *)tx_buf), ((uint8_t *)rx_buf), count);
  } else {
    uint8_t dummy_buf[count];
    memset(dummy_buf, 0XFF, count);
    spi_transfer(&_spi, dummy_buf, ((uint8_t *)rx_buf), count);
  }
#else  // OLD_WAY ========================================
    spi_transfer(&_spi, ((const uint8_t *)tx_buf), ((uint8_t *)rx_buf), count);
#endif  // OLD_WAY  ======================================
}

[fpistm]

I also tested for the case where rx_buf is nullptr. It appears to fail in a way I have seen with other board packages.

Right, as stated in the comment of the patch but did not time to check it yet.

+ /* Use a tmp buffer as the Rx is not properly discarded, under investigation */

I found the skip receive implemented by a contributor some years ago do not work as expected and so have to fix it like this we could pass null ptr for Rx.
I will check your modified code and thanks.