SPI between two arduino's

Hi, i'm new to programming with SPI. I red a lot about it and want to give it a try. So here is the setup; 2 rgb leds, each led is connected to an arduino (blue - pin 7 / green - pin 6 / red - pin 5), arduino's are connected trough spi with each other. Goal i try to archive; first the blue led goes on of one arduino for 1 second, then the blue led goes off and the other blue led is turned on for 1 second, blue led goes off and the green led is turned on for 1 second on the first arduino, led is turned of and green led of second arduino is turned on for one second, and so on. This is my code but don't understand why it doesn't work. The blue colors are OK but then they start to blink together sometimes and the flow of the program isn't right. Thanks in advance!

master:
#include <SPI.h>;
byte Mastersend, Mastereceive, MastereceiveOld;
int counter;
void setup (void) {
pinMode(7, OUTPUT);
pinMode(6, OUTPUT);
pinMode(5, OUTPUT);
Serial.begin(115200); //set baud rate to 115200 for usart
digitalWrite(SS, HIGH); // disable Slave Select
SPI.begin ();
SPI.setClockDivider(SPI_CLOCK_DIV4);//divide the clock by 8
MastereceiveOld = 256;
}
void loop (void) {
digitalWrite(SS, LOW); // enable Slave Select
Serial.println(Mastereceive);
if (Mastereceive == 0) {
digitalWrite(7, HIGH);
digitalWrite(6, LOW);
digitalWrite(5, LOW);
} else if (Mastereceive == 2) {
digitalWrite(6, HIGH);
digitalWrite(5, LOW);
digitalWrite(7, LOW);
} else if (Mastereceive == 4) {
digitalWrite(5, HIGH);
digitalWrite(7, LOW);
digitalWrite(6, LOW);
} else {
digitalWrite(5, LOW);
digitalWrite(7, LOW);
digitalWrite(6, LOW);
}
delay(1000);
if (Mastereceive != MastereceiveOld) {
Mastersend = counter++;
MastereceiveOld = Mastersend;
}
Serial.println(Mastersend);
Mastereceive = SPI.transfer(Mastersend);
counter = Mastereceive;
digitalWrite(SS, HIGH); // disable Slave Select
delay(1000);
}

slave:
#include <SPI.h>
char buff [50];
volatile byte indx;
volatile boolean process;
volatile byte Slavereceived,Slavesend,SlavereceiveOld;
void setup (void) {
pinMode(7,OUTPUT); //Sets pin 2 as input
pinMode(6,OUTPUT); //Sets pin 7 as Output
pinMode(5,OUTPUT); //Sets pin 7 as Output

Serial.begin (1000000);
pinMode(MISO, OUTPUT); // have to send on master in so it set as output
SPCR |= _BV(SPE); // turn on SPI in slave mode
indx = 0; // buffer empty
process = false;
SPI.attachInterrupt(); // turn on interrupt
SlavereceiveOld = 255;
}
ISR (SPI_STC_vect){ // SPI interrupt routine {
byte Slavereceived = SPDR;
}
void loop (void) {

if(Slavereceived == 1) {
digitalWrite(7,HIGH);
digitalWrite(6,LOW);
digitalWrite(5,LOW);
} else if (Slavereceived == 3) {
digitalWrite(6,HIGH);
digitalWrite(5,LOW);
digitalWrite(7,LOW);
} else if (Slavereceived == 5) {
digitalWrite(5,HIGH);
digitalWrite(7,LOW);
digitalWrite(6,LOW);
Slavereceived = 0;
} else {
digitalWrite(5,LOW);
digitalWrite(7,LOW);
digitalWrite(6,LOW);
}
delay(1000);
if (Slavereceived != SlavereceiveOld) {
Slavesend = Slavereceived++;
SlavereceiveOld = Slavesend;
}

SPDR = Slavesend;

}

Is this line what you intended?

  MastereceiveOld = 256;

MastereceiveOld is defined as byte which can only be in the range 0 to 255. Setting to 256 will cause it to be 0.

Also, please put your code in code tags as indicated here: Read this before posting a programming question ...

Thanks for the reply! Silly mistake, the intension was to be that the start value never would be zero but in this case thats axactly what happend. However its better now my leds are still blinking at the same time sometimes, and the order is still not right

First of you had the following coding error in your slave interrupt:

ISR (SPI_STC_vect){ // SPI interrupt routine {
   byte Slavereceived = SPDR;
}

You declared Slavereceived local to the interrupt routine. This is NOT the same variable as the global Slavereceived and therefore the global variable Slavereceived was NEVER getting set!

Your strategy for coding this is making it unnecessarily complicated. You are trying to make the slave too smart and it is complicating your code. You should have the master completely control the sequencing. If it were me I would just send the slave command bytes.

I would code the slave like this and then all of your sequencing logic can be in the master:

#include <SPI.h>
volatile byte Slavereceived;
byte SlavereceiveOld;

const int redLedPin = 5;
const int greenLedPin = 6;
const int blueLedPin = 7;

const byte allLedsOff = 0;
const byte redLedOn = 1;
const byte greenLedOn = 2;
const byte blueLedOn = 3;

void setup (void) 
{
  pinMode(redLedPin, OUTPUT);  
  pinMode(greenLedPin, OUTPUT);
  pinMode(blueLedPin, OUTPUT); 
  digitalWrite(redLedPin, LOW);
  digitalWrite(greenLedPin, LOW);
  digitalWrite(blueLedPin, LOW);

  Serial.begin (115200);
  pinMode(MISO, OUTPUT); // have to send on master in so it set as output
  SPCR |= _BV(SPE); // turn on SPI in slave mode
  SPI.attachInterrupt(); // turn on interrupt
  SlavereceiveOld = 255;
}

ISR (SPI_STC_vect) 
{ 
  Slavereceived = SPDR;
}

void loop (void) 
{
  if (Slavereceived != SlavereceiveOld) 
  {
    SlavereceiveOld = Slavereceived;
    switch (Slavereceived)
    {
      case allLedsOff:
        digitalWrite(redLedPin, LOW);
        digitalWrite(greenLedPin, LOW);
        digitalWrite(blueLedPin, LOW);
        break;
        
      case redLedOn:
        digitalWrite(redLedPin, HIGH);
        break;

      case greenLedOn:
        digitalWrite(greenLedPin, HIGH);
        break;

      case blueLedOn:
        digitalWrite(blueLedPin, HIGH);
        break;
    }
  }
}

Hi, thanks for your code that was very helpfull. I will defenitly check them out an dig into that a little bit deeper. The reason because I made it so complicated is because I needed a quite simple example to test how the arduino communicatates between master and slave and reversed. So I thought it was a really good example to move a variable reverse and forward between these two arduino's so I can fully understand how these communicate. Thats why I programmed it in this way