SPI communication between Uno and Pro mini problem [SOLVED] but still unclear!

Hi,

As part of bigger project, I am trying to setup an SPI between a Uno(Master) and a Pro Mini (ATmega328, 5V 16MHz) (Slave).

To test the SPI interface I have written some code to blink an LED connected to the Pro Mini, the period of which can be changed via the Uno over SPI.

I am having trouble with the interface as I cannot seem to change the period and also the data returned over SPI seems to be random. :confused:

Anyone any idea as to why and how to may this interface work.

code for the Pro Mini:

#include <SPI.h>

volatile uint8_t spi,command=0,i=0;
volatile unsigned long oldtime=0,T=300;

// SPI interrupt routine
ISR (SPI_STC_vect)
{
  spi = SPDR;
 
  switch (command)
  {
  // no command? then this is the command
  case 0:
    command = spi;
    SPDR = 0;
    break;

  case 't':
    T = 10*spi;
    command = 0;
    SPDR = spi - 10;
    break;
  } // end of switch

}  // end of interrupt service routine (ISR) SPI_STC_vect

void setup (void)
{

  pinMode(2, OUTPUT); //LED output

  // have to send on master in, *slave out*
  pinMode(MISO, OUTPUT);

  // turn on SPI in slave mode
  SPCR |= _BV(SPE);

  // turn on interrupts
  SPCR |= _BV(SPIE);
  
}  // end of setup

void loop (void)
{
  if(millis()-oldtime>T){
    oldtime=millis();
    i^=0x01; 
    digitalWrite(2,i); //toggle LED
  }
}

code for Uno:

#include <SPI.h>

volatile char req[9];
volatile uint8_t a,i=0,j=0,T,T_Set[5]={0};

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

  digitalWrite(SS, HIGH);  // ensure SS stays high for now

  // Put SCK, MOSI, SS pins into output mode
  // also put SCK, MOSI into LOW state, and SS into HIGH state.
  // Then put SPI hardware into Master mode and turn SPI on
  SPI.begin ();
  
  //Slow down the master a bit (8MHz for 16MHz board)
  SPI.setClockDivider(SPI_CLOCK_DIV2);

  Serial.println ("READY");
  
}  // end of setup

void loop (void)
{
    
  if(Serial.available()){
    req[i] = Serial.read();
    ++i;
  }

  
  if(req[i-1]==';'){
      --i;
      if(req[0]=='t'){
          if(i>0){
              for(j=0;j<i;++j){
                  T_Set[j]= req[j+1];  
              }
  
              T_Set[j]= '\0';
              T = atoi(T_Set);

              Serial.println (T, DEC);
              
              // enable Slave Select
              digitalWrite(SS, LOW);

              a = SPI.transfer ('t');
              Serial.println (a, DEC);
              //delayMicroseconds(10);
              a = SPI.transfer (T);

              // disable Slave Select
              digitalWrite(SS, HIGH);

              Serial.println (a, DEC);
          }
      }
      i=0;
  }

}

Sherzaad

For a comprehensive article on Arduino SPI (master/slave) look at Gammon Forum : Electronics : Microprocessors : SPI - Serial Peripheral Interface - for Arduino

I actually did check that site to write the code. Just not working as it should!

Anyone know what gone wrong in my code? HELP PLEASE :cry:

Why not put some Serial.print() statements in the slave code to see if it is receiving anything from the master, instead of simply relaying on a round trip master --> slave --> master for your debugging.

How have you got master and slave wired together ?

Since it appears that your code is indeed a light adaption of this, with some inexplicable changes like direct manipulation of the SPI registers instead of using library functions, why not get the example code working first, then gradually add in your changes to see what happens and where it breaks?

the purpose of blinking the LED IS for debugging purposes to see if the slave is receiving anything from the master.

the wiring is same as described on website.

I have tried to original code before moving to this one (the website code had direct manipulation of the SPI registers btw!) and that worked fine.

Clearly it's wen I adapted the code that something clearly went wrong. hence me posting the problem here to understand what went wrong with it.

Hi,

This follow my previous post SPI communication between Uno and Pro mini problem

after looking around and some tinkering with the code I finally got it to work!

MASTER code:

#include <SPI.h>

volatile char req[9];
volatile uint8_t a,i=0;

byte transferAndWait (const byte what)
{
  byte x = SPI.transfer (what);
  delayMicroseconds (20); //wait for slave to process and reply
  return x;
} 

void setup (void)
{
  Serial.begin (9600);
  
  digitalWrite(SS, HIGH);  // ensure SS stays high for now

  SPI.begin ();

  SPI.beginTransaction(SPISettings(4000000, MSBFIRST, SPI_MODE0));
  
  Serial.println ("READY"); 
}

void loop (void)
{
uint8_t j;

  if(Serial.available()){
    req[i] = Serial.read();
    ++i;
  }

  
  if(req[i-1]==';'){
      if(req[0]=='o'){ //turn LED OFF
        // enable Slave Select
        digitalWrite(SS, LOW);

        a = transferAndWait(0); //start SPI COMMS
        //Serial.println (String(a,HEX)); 
        a = transferAndWait('o'); //send command
        //Serial.println (String(a,HEX)); 
        a = transferAndWait(0); //end SPI COMMS
        
        if(a == 'O'){
          Serial.print("LED OFF!");
          Serial.println (char(a));          
        }
        else{
          Serial.print("Failed! ");
          Serial.println (String(a,HEX));
          while(a!=0) a = transferAndWait(0); //reset comms 
        }
      
        // disable Slave Select
        digitalWrite(SS, HIGH);
      }
      else if(req[0]=='i'){ //turn LED ON
        // enable Slave Select
        digitalWrite(SS, LOW);

        a = transferAndWait(0); //start SPI COMMS
        //Serial.println (String(a,HEX)); 
        a = transferAndWait('i'); //send command
        //Serial.println (String(a,HEX)); 
        a = transferAndWait(0); //end SPI COMMS
               
        if(a == 'I'){
          Serial.print("LED ON!");
          Serial.println (char(a));
        }
        else{
          Serial.print("Failed! ");
          Serial.println (String(a,HEX));
          while(a!=0) a = transferAndWait(0); //reset comms
        }
                
        // disable Slave Select
        digitalWrite(SS, HIGH);
      }
      else if(req[0]=='b'){ //blink LED with 50% duty
         // enable Slave Select
        digitalWrite(SS, LOW);

        a = transferAndWait(0); //start SPI COMMS
        //Serial.println (String(a,HEX)); 
        a = transferAndWait('b'); //send command
        //Serial.println (String(a,HEX));
        
        j=i-2; //get number of data bytes
        a = transferAndWait(j); //send number of data bytes
        //Serial.println (String(a,HEX));
          
        for(j=1; j<i;++j){
          a = transferAndWait(req[j]); //send data bytes
          //Serial.println (String(a,HEX));                 
        }

        a = transferAndWait(0); //end SPI COMMS

        if(a == 'B'){
          Serial.print("LED Blinking!");
          Serial.println (char(a));
        }
        else{
          Serial.print("Failed! ");
          Serial.println (String(a,HEX));
          while(a!=0) a = transferAndWait(0); //reset comms
        }
      }
      
      i=0;
  }

}

SLAVE code:

#define TGL_BIT(x,y) ((x)^=(1<<(y))) //toggle bit y in byte x

#include <SPI.h>
volatile uint8_t start=0,command=0, blink_en=0,i=0,n;
uint8_t T_char[5]={0};
volatile uint16_t T;
volatile long oldtime=0;

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

  if(spi==0 && start==0){ //start of SPI comms
    if(command!=0xA3){
      start=1;
      SPDR = 0xA4; //acknowledge start of comms
    }
    else{
      command=0;
      SPDR = 0;
    }
  }
  else if(start>0){
      if(start==1){
        command = spi;
        start=2;
      }
    
      switch (command)
      {
        case 'o':
          digitalWrite(2,LOW);
          SPDR = 'O';
          command = 0xA3; //end of command
          blink_en=0; //disable LED blinking
          start=0; //end of comms
        break;

        case 'i':
          digitalWrite(2,HIGH);
          SPDR = 'I';
          command = 0xA3; //end of command
          blink_en=0; //disable LED blinking
          start=0; //end of comms
        break;

        case 'b':
          if(i==0){ //command byte received
             i=1;
             SPDR = 0xA5;
          }
          else if(i==1){
            n=spi; //number of data bytes
            i=2;
            SPDR = spi|0xA0;
          }
          else if(i-2<n){//received data bytes
            T_char[i-2]=spi;
            ++i;
            SPDR = spi|0xA0;     
          }
          else if(i-2==n){ //evalute period from data bytes
            T_char[n]='\0';
            T = atoi( T_char);
            T/=2;
            i=0; //reset counter
            blink_en=1; //enable LED blinking
            command = 0xA3; //end of command
            start=0; //end of comms
            SPDR = 'B'; 
          }
        break;
      }
  }

} 

void setup (void)
{

  pinMode(2, OUTPUT); //LED output
  digitalWrite(2,LOW);
  
  // have to send on master in, *slave out*
  pinMode(MISO, OUTPUT);

  // turn on SPI in slave mode
  SPCR |= _BV(SPE);

  // turn on interrupts
  SPCR |= _BV(SPIE);

  SPDR = 0; //initialise SPI register
  
}  // end of setup

void loop (void)
{
  if(blink_en==1 && ((millis()-oldtime)>T)){
    oldtime=millis();
    TGL_BIT(PORTD,2); //toggle pin2 state         
  }
}

However I'm not really satisfied with the solution as in the MASTER code is need to use a "delaymicroseconds" in the "transferAndWait" routine to allow SLAVE to process and respond.

Is there anyway a non blocking code (ie not using delaymicroseconds) or somehow using a while loop to wait for SLAVE to respond instead.

Also there is a while loop already implemented in SPI.transfer to wait for a reply. so why do we need to add an addtional delay to get the SLAVE's response!!! :confused:

no one got any ideas how to improve the transferAndWait routine...

byte transferAndWait (const byte what)
{
  byte x = SPI.transfer (what);
  delayMicroseconds (20); //would rather have a while loop (even if its a blocking code) as response times from slave can vary!
  return x;
}

I think you misunderstand how SPI works.
8 bits are shifted in at the same time that 8 bits are shifted out.
The delayMicroseconds does nothing - if the slave does not respond to the clock during those 8 clocks, then no data will be coming back.
Sometimes 2 transfers are needed. The first transfer gives the slave a register, the 2nd transfer reads the data back.

Thank for your comment CrossRoads.

That was my understanding as well (hence why it makes no sense to me to use a delay!) before until I did a couple of trials to get a stable communication.

the reason I mentioned in my post for using the delay actually came for here Gammon Forum : Electronics : Microprocessors : SPI - Serial Peripheral Interface - for Arduino