SPI sending data errors

[robertocordon]

Hello!

I am currently trying to use Arduino as an SPI Slave (using a Raspberry Pi as Master). I should mention this is the only Slave and I have no others sharing the bus.
I have a very simple program on the Master which sends a 9-byte buffer (MOSI), and then prints out whatever was received during that 9 byte transmission (MISO).
On the Slave (Ardunio) side, I've set it up to just print out whatever it receives (MOSI), and it always sends back 0x00 on every byte (MISO). The code is below:

#include <SPI.h>

byte readBuffer[9];
bool isReady = false;
int currentPosition = 0;

void setup() {
  Serial.begin(9600);

  pinMode(MISO, OUTPUT);
  SPCR |= _BV(SPE);
  SPI.attachInterrupt();
}

void loop() {
  if(isReady) {
    isReady = false;

    for(int i = 0; i < 9; i++) {
      Serial.print(readBuffer[i], HEX);
      Serial.print(" ");
    }
    Serial.println("");
  }
}

ISR(SPI_STC_vect) {
  readBuffer[currentPosition] = SPDR;
  SPDR = 0x00;
  currentPosition++;

  if(currentPosition == 9) {
    isReady = true;
    currentPosition = 0;
  }
}

The above works flawlessly. Both on the master and slave, I can see the correct data being sent and received.

The problem comes in when I modify the line SPDR = 0x00; to any other value. If I try to send, for example, 0xAA then my Master sees a bunch of garbled data. Interestingly enough, the Arduino is also having the data it receives corrupted. I'm not sure why that'd happen if they're running on separate lines.

A couple of interesting things I noticed. If I set SDPR to all zeroes or ones (0x00 or 0xFF), or the value it just received (readBuffer[currentPosition]), then the issue isn't there.

Does anyone have any experience transmitting both ways in SPI using Arduino, who might know what's going on here?

Happy to answer any questions, and thank you!

[GolamMostafa]

Please, post your SPI-Master's sketch. Are you using UNO as Slave? What is data transmission rate from Master to Slave? In the meantime, re-try by putting volatile keyword with all the global variables.

[alto777]

@Delta_G

marked as volatile.

Yes. And any access to the int (a multi-byte variable) outside the ISR must be done with interrupts off.

Ppl typically grab a copy to use, viz:

    noInterrupts();
    int myCopy = currentPosition;
    interrupts();

and use myCopy everywhere after.

Name it something better.

a7

[robertocordon]

I was using an UNO, but now I'm using a Mega2650. Same issue on both. The transmission rate is currently 32kHz, but I've tried with 10kHz and 1MHz and it hasn't made a difference.

I just tried adding volatile to the 3 variables, but it didn't make a difference. I'm not surprised because the way the code is set up, it would be very rare that they'd get overwritten while being read. That being said I'll be keeping the volatile keyword as good practice, and especially as my program gets larger it will certainly be necessary. So thank you for that advice.

The master's sketch is part of a large program, but here are the relevant parts. Friendly reminder this is in an RPi, and I am using the PiGPIO library to interface with the GPIO pins with C++

#include <pigpiod_if2.h>

//to set up
_spiAnalogControllerHandle = spiOpen(channel, 32000, 0);

//to xfer
spiXfer(_spiAnalogControllerHandle, writeBuffer, readBuffer, 9);

I also have a writeBuffer that's set once at the start, and readBuffer comes back populated from spiXfer, and then I just print it out.

[robertocordon]

Unfortunately volatile didn't fix it. Also, I only modify currentPosition inside the ISR, is it possible for an interrupt to come in while servicing another interrupt? Either way I tried that just in case and it also didn't make a difference

[alto777]

@robertocordon

Unfortunately volatile didn't fix it

You must protect access, not just modification. So that may still be your, or a, problem.

Use the pattern I quoted above to perform a protected access of a variable whose value may be changed in the background by the ISR interrupt service routine.

If you think you have done, post the code that shows your attempt. If you have not, try it just for fun and see if it helps. If it does not, post the code that shows your attempt.

a7

[robertocordon]

ok so here's my new code

#include <SPI.h>

volatile byte readBuffer[9];
volatile bool isReady = false;
volatile byte currentPosition = 0;

void setup() {
  Serial.begin(9600);

  pinMode(MISO, OUTPUT);
  SPCR |= _BV(SPE);
  SPI.attachInterrupt();
}

void loop() {
  if(isReady) {
    noInterrupts();

    isReady = false;

    for(int i = 0; i < 9; i++) {
      Serial.print(readBuffer[i], HEX);
      Serial.print(" ");
    }
    Serial.println("-");

    interrupts();
  }
}

ISR(SPI_STC_vect) {
  noInterrupts();

  readBuffer[currentPosition] = SPDR;
  SPDR = 0xAA;
  currentPosition++;

  if(currentPosition == 9) {
    isReady = true;
    currentPosition = 0;
  }

  interrupts();
}

A few changes there:

1. Made all 3 variables volatile
2. Changed currentPosition from 16bit to 8bit, to avoid the issue you mentioned. It should be fine since I never expect any buffer to be bigger than 256 bytes.
3. I wrapped both the ISR and the inside of the loop inside the noInterrupt/interrupt code. I decided that even though the top level if inside the loop reads the variable written in the ISR, it would be damaging to disable interrupts pretty much the entire time. Worst case if isReady gets written in the ISR and after I read it in the loop, I'll just have to wait until the next loop, which is acceptable.

Unfortunately, the code above still doesn't work. I tried disabling interrupts during the whole loop but as predicted, I was skipping bytes.

[robertocordon]

Also, just want to re-highlight a crucial thing here. When I set SDPR to 0 or 0xFF, or just don't set it, there is no problem. It's only when it's not all ones or all zeroes (e.g. 0xAA above) that both the MISO and MOSI get screwed up.

Under the hypothesis that the changes to the MISO line is causing some issues, I tried the following values: 0x80, 0xC0, 0xE0, 0xF0, 0xF8, 0xFC, 0xFE, 0xFF. They all worked fine too. If you look at these numbers in binary, you'll see why I chose them. Interestingly enough, the following values do cause the issue: 0x01, 0x03, 0x07, 0x0F ... 0xEF.

From the MISO line perspective, I don't know what the difference would be between 0x01 and 0x80, yet one works and the other doesn't. I'm starting to go a bit crazy here. Feels like some hardware issue, but I've experienced this in both my UNO and my Mega.

[robertocordon]

So after typing my above comment, I figured this MUST be a hardware issue. To rule that out, I decided to write a quick Master program and run it on the UNO, and run the above on the Mega, and hook them up together. They work fine.

So I think my code above is correct. So thank you to everyone who gave their feedback, and even if this doesn't fix the problem at hand, I definitely learned about using volatile and being mindful of accesses between the ISR and main thread, which I hadn't considered before and will surely come in handy.

For next steps, either my problem is the RPi, or (and my bet is on this one) I have a problem with my level converter that sits between my RPi and Arduino... maybe causing interference or something like that. I'd hope any problem with that would have been addressed by running at a low enough baud rate, but who knows... the one I'm using is supposed to be able to handle up to 2MHz, and I'm at 32kHz

[robertocordon]

Confirmed the level converter is to blame. Not sure why exactly though.

I was running all 4 SPI lines through it, but now I'm running CLK, SS and MOSI directly and it works. Since CLK, CS and MOSI go from Pi->Arduino, and Arduino can handle 3.3V, it's no problem. Since MISO is Pi<-Arduino and Pi can't handle 5V, then I'm leaving that one going through the converter.

Interestingly, regardless of which of the 4 "ports" I use on the converter, it works fine, so there are no shorts in the headers I added as far as I can tell. Maybe the converter isn't great and it interferes with itself (although this is the purpose of it), or something else is going on, but I don't need to figure it out, thankfully. So I'll close this out.

Thanks once again to those of you who helped!

[robertocordon]

oh nice! yeah, that's great, I see why you did that. I tried to minimize the time by keeping it just inside the if, but your memcpy solution is definitely better!

As I just posted, the issue was hardware. But again, your code above will come in very handy as my program grows beyond this sample code.

All these considerations you've posted... ISR/main variable access, disabling interrupts for short amounts of times, will come in super helpful. Thanks so much @Delta_G!

[robertocordon]

@Delta_G

then the compiler may decide that the value is constant since it only changes in a function that's never called

oof great point. I was thinking it would do something like if it read it from memory into a register at the top of the function, it wouldn't reload it later down the line, and if it changed in between, it wouldn't "refresh" it. But I assumed it'd be ok when the function runs again (and hence re-reads from memory).

Didn't think about what you're saying above. Thanks!