Skip to content

SPI.transfer using global byte array doesn't send correct data #5670

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
2 tasks
Makuna opened this issue Jan 25, 2019 · 6 comments
Closed
2 tasks

SPI.transfer using global byte array doesn't send correct data #5670

Makuna opened this issue Jan 25, 2019 · 6 comments

Comments

@Makuna
Copy link
Collaborator

Makuna commented Jan 25, 2019

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

Using a global array passed into SPI.transfer sends all 0xFF. If the same array is copied into a buffer and then that buffer is used, it is correctly sent.

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 upper case T and hit enter to send. This will cause the sketch to call SPI.transfer with the global buffer. The results will show that all 0xFF were sent.
    image

  • Set the probe to capture again, 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 the correct data was sent.
    image

In the debug output, you will also see the addresses of the global buffer and copied buffer. These were
global = 0x3FFE84E4
copy = 0x3FFEF6E4

MCVE Sketch

#include <SPI.h>

const uint8_t PIN_SS = 2;

uint8_t c_sendBuffer[] = {0xf0, 0x55, 0x55, 0x55, 0x55, 0x55, 0x0f};
uint8_t* sendBuffer;

#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));
    
    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) {
      switch ((char)cmd) {
        case 'T':
        {
          SpiTransaction spiTransaction(4000000L);
          SPI.transfer(c_sendBuffer, sizeof(c_sendBuffer));
          Serial.println("used Transfer");
        }
        break;
        
        case 't':
        {
          SpiTransaction spiTransaction(4000000L);
          SPI.transfer(sendBuffer, sizeof(c_sendBuffer));
          Serial.println("used transfer");
        }
        break;
        
        default:
        break;
      }
    }
  }
}
@earlephilhower
Copy link
Collaborator

I'm not at my bench, but the code and HW itself can't differentiate between a heap allocated bunch of RAM or a global allocation (i.e. just above the heap in the same RAM). It's all just some bytes in the 8-bit addressable RAM space.

There may be some kind of hidden state, though, where it fails the first run and passes the 2nd (i.e. HW register not set up right until one pass, etc.).

If it's easy, have you tried reversing the order of your test? Is it always the 1st run or is it always the heap run, etc.?

@Makuna
Copy link
Collaborator Author

Makuna commented Jan 25, 2019

The test sketch, its driven by the developer using the Serial Monitor, so you can have it call what ever order you want and as often as you want with what ever delays between the sends it takes you type T and then enter, or even TTTTT and then enter (runs five in row quickly).
I just called it with T, then T, then T, in all cases it sends invalid data. Also tried TTTTT and all calls sent invalid data.

@Makuna
Copy link
Collaborator Author

Makuna commented Jan 25, 2019

Also just tried t, then t, then t, and guess what, the first is sent correctly, the others fail. So ignoring the global completely causes an issue. Also note, I extended the data to full 8 bytes to see if this was causing an issue; it does not effect the outcome.

@earlephilhower
Copy link
Collaborator

It will take a while before I get a chance to run your sketch, but I'll give it a go as soon as I can.

SPI hasn't been touched in ~6months per the git logs, and I've personally been using it lots while developing SDFS (and anyone using the existing SD lib would also be using it), so I'm pretty sure basic SPI functionality is OK.

I'm not an SPI expert (8266 or otherwise), but I have the feeling there's some issue with the MCVE code here and need to sit down and look at it for a bit. Breaking basic functionality normally gets yells from users fast, and it'd be broken 6 months ago given the logs...

@earlephilhower
Copy link
Collaborator

Same issue as #5762?

@Makuna
Copy link
Collaborator Author

Makuna commented Jan 25, 2019

User error. SPI.transfer will overwrite the out buffer with what is received.

@Makuna Makuna closed this as completed Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants