Calling SPI.writeBytes or SPI.transferBytes after calling a SPI.transfer doesn't send correct data

[Makuna]

Basic Infos

• [ X] This issue complies with the issue POLICY doc.
• [X ] I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git). (sorry, but this is too much work to ask, make it easier to do and I would)
• [X ] I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• [ X] I have filled out all fields below.

Platform

• Hardware: ESP-12
• Core Version: 2.5.0-beta2
• Development Env: Arduino IDE
• Operating System: Windows

Settings in IDE

• Module: Wemos D1 R1
• Flash Mode: option not exposed
• Flash Size: 4MB/no spiffs
• lwip Variant: v2 Lower Memory
• Reset Method: option not exposed
• Flash Frequency: option not exposed
• CPU Frequency: 80Mhz
• Upload Using: SERIAL
• Upload Speed: 921600

Problem Description

(updated with new findings)
Calling SPI.writeBytes or SPI.transferBytes after calling a SPI.transfer doesn't send correct data and just sends 0xFF. Calling before SPI.transfer works correctly.
Further, once SPI.transferBytes is called and wrong data is sent, SPI.transfer may also fail.

To reproduce:

• run the following sketch with the Serial Monitor open and a logic probe attached to the CLK, MOSI, and SS pins (SS pin is useful to set the probe to trigger on; but otherwise really isn't needed).

• Set to probe to capture, then in the Serial Monitor type lower case b and hit enter to send. This will cause the sketch to call SPI.transferBytes with a copy of the global buffer. The results will show that correct data was sent.
  

• Set to probe to capture, then in the Serial Monitor type lower case t and hit enter to send. This will cause the sketch to call SPI.transfer with a copy of the global buffer. The results will show that correct was sent.
  

• Set to probe to capture, then in the Serial Monitor type lower case b and hit enter to send. This will cause the sketch to call SPI.transferBytes with a copy of the global buffer. The results will show that all 0xFF were sent.
  

• Set the probe to capture again, then in the Serial Monitor type lower case w and hit enter to send. This will cause the sketch to call SPI.writeBytes with a copy of the global buffer. The results will show that all 0xFF were sent.
  (same as the above, so I didn't include another image)

The buffer addresses are as follows, these instructions are using the copied buffer.
global = 0x3FFE84EC
copied = 0x3FFEF6E4

MCVE Sketch

Below is a testing sketch, it exposes the ability for the Serial Monitor to trigger different actions, including ones that will test that your test rig is setup correctly (use lower case t).

#include <SPI.h>

const uint8_t PIN_SS = 2;

uint8_t c_sendBuffer[] = {0xf0, 0x55, 0x55, 0x55, 0x55, 0x55, 0x0f, 0x00};
uint32_t c_frequencyTests[] = {4000000L, 6000000L, 8000000L, 12000000L, 16000000L, 20000000L };

uint8_t indexFrequency = 0;
uint8_t* sendBuffer;
uint8_t* recieveBuffer;

#define countof(a)  (sizeof(a)/sizeof(a[0]))

void InitSPI() {
  SPI.begin();

  pinMode(PIN_SS, OUTPUT);
  digitalWrite(PIN_SS, HIGH);
}

void setup() {
  // put your setup code here, to run once:
    Serial.begin(115200);
    while (!Serial); // wait for serial attach

    Serial.println();
    Serial.println("Initializing...");
    Serial.flush();

    InitSPI();

    sendBuffer = new uint8_t[sizeof(c_sendBuffer)];
    memcpy(sendBuffer, c_sendBuffer, sizeof(c_sendBuffer));

    recieveBuffer = new uint8_t[sizeof(c_sendBuffer)];
    memset(recieveBuffer, 0, sizeof(c_sendBuffer));
    
    Serial.print("0x");
    Serial.println((uint32_t)(&c_sendBuffer[0]), HEX);

    Serial.print("0x");
    Serial.println((uint32_t)(sendBuffer), HEX);
    
    Serial.println();
    Serial.println("Running...");

}

class SpiTransaction
{
  public:
      SpiTransaction(uint32_t frequency) {
        SPI.beginTransaction(SPISettings(frequency, MSBFIRST, SPI_MODE0));
        digitalWrite(PIN_SS, LOW);
      }
      ~SpiTransaction() {
        digitalWrite(PIN_SS, HIGH);
        SPI.endTransaction();
      }
};


void loop() {
  if (Serial.available()) {
    int cmd = Serial.read();
    if (cmd != -1) {
      bool frequencyChanged = false;
      
      switch ((char)cmd) {
        case '+':
        indexFrequency = (indexFrequency+1) % countof(c_frequencyTests);
        frequencyChanged = true;
        break;
        
        case '-':
        indexFrequency = indexFrequency ? (indexFrequency-1) : (countof(c_frequencyTests) - 1);
        frequencyChanged = true;
        break;

        case 'T':
        {
          SpiTransaction spiTransaction(c_frequencyTests[indexFrequency]);
          SPI.transfer(c_sendBuffer, sizeof(c_sendBuffer));
          Serial.println("used Transfer");
        }
        break;
        
        case 't':
        {
          SpiTransaction spiTransaction(c_frequencyTests[indexFrequency]);
          SPI.transfer(sendBuffer, sizeof(c_sendBuffer));
          Serial.println("used transfer");
        }
        break;

        case 'W':
#if defined(ARDUINO_ARCH_ESP8266) || defined(ARDUINO_ARCH_ESP32)       
        {
          SpiTransaction spiTransaction(c_frequencyTests[indexFrequency]);
          SPI.writeBytes(c_sendBuffer, sizeof(c_sendBuffer));
          Serial.println("used WriteBytes");
        } 
#endif        
        break;
        
        case 'w':
#if defined(ARDUINO_ARCH_ESP8266) || defined(ARDUINO_ARCH_ESP32)      
        {
          SpiTransaction spiTransaction(c_frequencyTests[indexFrequency]);
          SPI.writeBytes(sendBuffer, sizeof(c_sendBuffer));
          Serial.println("used writeBytes");
        }   
#endif        
        break;

        case 'B':
#if defined(ARDUINO_ARCH_ESP8266) || defined(ARDUINO_ARCH_ESP32)        
        {
          SpiTransaction spiTransaction(c_frequencyTests[indexFrequency]);
          SPI.transferBytes(c_sendBuffer, recieveBuffer, sizeof(c_sendBuffer));
          Serial.println("used TransferBytes");
        } 
#endif        
        break;
        
        case 'b':
#if defined(ARDUINO_ARCH_ESP8266) || defined(ARDUINO_ARCH_ESP32)        
        {
          SpiTransaction spiTransaction(c_frequencyTests[indexFrequency]);
          SPI.transferBytes(sendBuffer, recieveBuffer, sizeof(c_sendBuffer));
          Serial.println("used transferBytes");
        } 
#endif        
        break;
        
        default:
        break;
      }
      if (frequencyChanged) {
        Serial.println();
        Serial.print("requency set to ");
        Serial.println(c_frequencyTests[indexFrequency]);
      }
    }
  }
}

[earlephilhower]

@Makuna did you want to close #5670 ?

transferBytes requires a 4-byte aligned input buffer and a multiple of 4-bytes length. You've got a 7-byte array which, even if aligned, may give you some odd behavior or overwrite other RAM.

[Makuna]

@earlephilhower nope, two different problems. This one maybe resolved by your comment above, will test that now.

[Makuna]

@earlephilhower The buffers are already aligned, extended the data to be 8 bytes and the problem is still reproduceable. So this remains active (updating the sketch above so this detail can be ignored).
I still believe that the data should NOT be mulitples of 4 bytes in length (alignment of the buffer pointer is valid and ok since this call is not standard Arduino). If you can only handle 4 byte mulitples, then the function should be transferUint32 not transferByte.

[earlephilhower]

I still believe that the data should NOT be mulitples of 4 bytes in length (alignment of the buffer pointer is valid and ok). If you can only handle 4 byte mulitples, then the function should be transferUint32 not transferByte.

Agreed. It bit me when using the new SDFS and SDFat library (greiman/SdFat#125). Our bug is in #4967. For me, sdfat occasionally uses misaligned buffers which resulted in HW exceptions.

[Makuna]

@earlephilhower Looking at the code for transferBytes_, it does seem to assume the data is in 4 byte lengths. This seems all wrong as the size of data is device specific and may NEVER be 4 byte rounded lengths and sending extra data can mess things up. But again, this is not the problem this issue is tracking.

[Makuna]

OK, this will be closed shortly. This is due to user error. transfer will store the received data back into the buffer that was used to send; thus overwriting the original data with 0xFF.

[Makuna]

Confirmed, issue was the API overwrites the out data buffer as documented. User error.