Jump to content
mityeltu

Odd Spi Communication Error

Recommended Posts

I gave up on the I2C com for a while and started tinkering with SPI istead. Figured if I can get the simpler one to work, I'll be able to get the harder one later. The master program has a heartbeat on timer0 and periodically gets an a/d conversion on AN0. I convert this to a single byte and transmit this to my slave which then displays the byte on port b leds. Both master and slave are 18F452, master sdi to slave sdo, master sdi to slave sdo, sck connected together. 20MHz resonator for clock source, sck set for Fosc/64 (just to make sure I'm not overclocking the slave).

 

The wierd part is that it works... sometimes. The a/d conversion works every time. My scope is not good enough to show everything going on, but the fact that it works aperiodically just doesn't make sense.

 

I moved my sdi/sdo lines as far away from the resonators as possible (approximately 1 inch) and the sck line away from the sdi/sdo lines by about an inch. I just don't know if this is a hardware or firmware problem. Coupld one of you with alot more expoerience please check my code to see if it SHOULD be working correctly? I'd really appreciate it. Thank you.

 

Here's the master code:

#include <system.h>

/*
SPI Master
18F452
20MHz resonator
*/

#pragma DATA _CONFIG1H, _HS_OSC & _OSCS_OFF_1H
#pragma DATA _CONFIG2L, _PWRT_OFF_2L & _BOR_OFF_2L
#pragma DATA _CONFIG2H, _WDT_OFF_2H
#pragma DATA _CONFIG3H, _CCP2MX_OFF
#pragma DATA _CONFIG4L, _LVP_OFF_4L & _DEBUG_OFF_4L
#pragma DATA _CONFIG5L, _CP0_OFF_5L & _CP1_OFF_5L & _CP2_OFF_5L & _CP3_OFF_5L
#pragma DATA _CONFIG5H, _CPB_OFF_5H
#pragma DATA _CONFIG6L, _WRT0_OFF_6L & _WRT1_OFF_6L & _WRT2_OFF_6L & _WRT3_OFF_6L
#pragma DATA _CONFIG6H, _WRTC_OFF_6H & _WRTB_OFF_6H & _WRTD_OFF_6H
#pragma DATA _CONFIG7L, _EBTR0_OFF_7L & _EBTR1_OFF_7L & _EBTR2_OFF_7L & _EBTR3_OFF_7L
#pragma DATA _CONFIG7H, _EBTRB_OFF_7H

void interrupt(void);
void setup(void);


//Global Variables
char flags, int_cnt;
short adr;

void main()
{
	setup();

	while(1)
	{
		if (test_bit(flags,0)) // tmr0 int -> heart beat
		{
			clear_bit(flags,0);
			if (test_bit(porta,5))
			{
				clear_bit(porta,5);
				tmr0h = 0xe0;
				tmr0l = 0x00;
			}
			else
			{
				set_bit(porta,5);
			}
			if (int_cnt == 50)
			{
				set_bit(adcon0,2); // start a/d
				int_cnt = 0;
			}
		}
		
		if (test_bit(flags,1)) // a/d int
		{
			delay_ms(255);
			clear_bit(portd,0);
			delay_ms(255);
			set_bit(portd,0);
			delay_ms(255);
			clear_bit(portd,0);
			delay_ms(255);
			
			clear_bit(flags,1);
			MAKESHORT (adr, adresl, adresh);
			adr = adr/256;
			portb = adr;
			clear_bit(pir1,3); // clear spi flag
			set_bit(portd,1); // select slave
			delay_us(20);
			clear_bit(pie1,6); // disable a/d int
			sspbuf = adr; // send byte
			
			while (!test_bit(pir1,3))
			{} // wait till send complete
			clear_bit(portd,1);
			clear_bit(pir1,6); // clear a/d int flag
			set_bit(pie1,6); // re enable a/d int

			delay_ms(255);
			set_bit(portd,0);
			delay_ms(255);
			clear_bit(portd,0);
			delay_ms(255);
		}
	}
}


void interrupt()
{
	clear_bit(intcon,7);
	clear_bit(intcon,6);
	
	if (test_bit(intcon,2)) // tmr0 int
	{
		clear_bit(intcon,2);
		set_bit(flags,0);
		int_cnt++;
	}
	
	if (test_bit(pir1,6)) // a/d int
	{
		clear_bit(pir1,6);
		set_bit(flags,1);
		set_bit(portd,0);
	}
	
	set_bit(intcon,7);
	set_bit(intcon,6);
}


void setup()
{
	portd = 0x00;
	portc = 0x00;
	
	intcon = 0xe0; // tmr0 int enabled,
	intcon2 = 0x80; // interrupt on rising edge, pull ups disabled
	intcon3 = 0x00; // int1, int2 enabled
	
	ccp1con = 0x00; // capture/compare off
	sspcon1 = 0x00; // spi/i2c off
	
	adcon0 = 0x01; // a/d on, channel 0
	adcon1 = 0x0e; // only an0 analog
	set_bit(pie1,6); // a/d int enabled

	trisa = 0x0f; // high nibble = output, low nibble = input
	trisb = 0x00; 
	trisc = 0x00; 
	trisd = 0x00; // d0 = slave select pin
	trise = 0x07;
	
	portb = 0x00;
	portc = 0x00;
	portd = 0x00;
	
	t0con = 0x82; // tmr0 on, 16bit, 1:256 prescaler
	
	flags = 0x00;
	int_cnt = 0x00;
	adr = 0;
	
	// SPI Configuration
	sspcon1 = 0x22; // spi enabled, master mode, fosc/64
	sspstat = 0x40; // data sampled in middle, tx on rosing edge
	trisc = 0x10; // set per book page 216
	portc = 0x00;
	clear_bit(trisd,0); // slave select pin
	clear_bit(rcon,7); //disabled int priority
}

Slave code:

#include <system.h>

#pragma DATA _CONFIG1H, _HS_OSC & _OSCS_OFF_1H
#pragma DATA _CONFIG2L, _PWRT_OFF_2L & _BOR_OFF_2L
#pragma DATA _CONFIG2H, _WDT_OFF_2H
#pragma DATA _CONFIG3H, _CCP2MX_OFF
#pragma DATA _CONFIG4L, _LVP_OFF_4L & _DEBUG_OFF_4L
#pragma DATA _CONFIG5L, _CP0_OFF_5L & _CP1_OFF_5L & _CP2_OFF_5L & _CP3_OFF_5L
#pragma DATA _CONFIG5H, _CPB_OFF_5H
#pragma DATA _CONFIG6L, _WRT0_OFF_6L & _WRT1_OFF_6L & _WRT2_OFF_6L & _WRT3_OFF_6L
#pragma DATA _CONFIG6H, _WRTC_OFF_6H & _WRTB_OFF_6H & _WRTD_OFF_6H
#pragma DATA _CONFIG7L, _EBTR0_OFF_7L & _EBTR1_OFF_7L & _EBTR2_OFF_7L & _EBTR3_OFF_7L
#pragma DATA _CONFIG7H, _EBTRB_OFF_7H

void interrupt(void);
void setup(void);

//Global Variables
char flags;
unsigned char ssp_dat;


void main()
{
	setup();
	
	while(1)
	{
		if(test_bit(flags,0)) // data received
		{
			ssp_dat = sspbuf;
			clear_bit(pir1,3);
			portb = ssp_dat;
			clear_bit(flags,0);
		}
	}
}



void interrupt()
{
	clear_bit(intcon,7);
	clear_bit(intcon,6);
	
	if (test_bit(pir1,3)) // spi int flag
	{
		clear_bit(pir1,3);
		set_bit(flags,0);
	}
	
	set_bit(intcon,6);
	set_bit(intcon,7);
}



void setup()
{
	portb = 0x00;
	portc = 0x00;
	portd = 0x00;
	porte = 0x00;
	
	intcon = 0x00; // ints disabled
	intcon2 = 0x80; // pull ups disabled
	intcon3 = 0x00; // int disabled
	
	ccp1con = 0x00; // capture/compare off
	
	adcon0 = 0x00; // a/d off
	adcon1 = 0x06; // all digital io

	trisa = 0xff; // trisa5 must be input for slave select bit
	trisb = 0x00; 
	trisc = 0x18; // set spi io and clk pins page 216
	trisd = 0x00;
	trise = 0x07;
	
	flags = 0x00;
	
	//set spi for slave
	sspcon1 = 0x24; // spi enabled, slve mode
	sspstat = 0x40; // tx on rising edge
	
	set_bit(intcon,6);
	set_bit(intcon,7);
	clear_bit(rcon,7); // priority level int disabled
	set_bit(pie1,3); // spi int enabled
}

Share this post


Link to post
Share on other sites

OK, well, dead silence again.

 

Well, I modified my code to remove most of the fluff and just bit bang the whole thing out and managed to fix everything (almost). All this does now is decrement port B on the master and send the updated port B value to the slave to display on the slave's port B - not exactly glamorous, but I'm juet trying to figure this out right now.

 

Apparently my bit timing was off. I added a longer delay between the slave selct bit going low before loading the sspbuf register. I also discovered that one of my 20MHz resonators had died and was no longer timing anywhere near 20MHz. I had to switch to 16MHz resonators since I didn't have a replacement. The master slave are now communicating regularly, but the slave still drops about 1 out of every 100 messages. Is there something that someone can recomment in either hardware or firmware that might account for the 1-2% dropped bytes?

 

I really want to get this to work 100%, but I don;t know what else to try. Any suggestion will be appreciated.

 

The code follows:

 

Master Code:

#include <system.h>

/*
SPI Master
18F452
16MHz resonator
*/

#pragma DATA _CONFIG1H, _HS_OSC & _OSCS_OFF_1H
#pragma DATA _CONFIG2L, _PWRT_OFF_2L & _BOR_OFF_2L
#pragma DATA _CONFIG2H, _WDT_OFF_2H
#pragma DATA _CONFIG3H, _CCP2MX_OFF
#pragma DATA _CONFIG4L, _LVP_OFF_4L & _DEBUG_OFF_4L
#pragma DATA _CONFIG5L, _CP0_OFF_5L & _CP1_OFF_5L & _CP2_OFF_5L & _CP3_OFF_5L
#pragma DATA _CONFIG5H, _CPB_OFF_5H
#pragma DATA _CONFIG6L, _WRT0_OFF_6L & _WRT1_OFF_6L & _WRT2_OFF_6L & _WRT3_OFF_6L
#pragma DATA _CONFIG6H, _WRTC_OFF_6H & _WRTB_OFF_6H & _WRTD_OFF_6H
#pragma DATA _CONFIG7L, _EBTR0_OFF_7L & _EBTR1_OFF_7L & _EBTR2_OFF_7L & _EBTR3_OFF_7L
#pragma DATA _CONFIG7H, _EBTRB_OFF_7H

void interrupt(void);
void setup(void);


//Global Variables
char flags, int_cnt;

void main()
{
    osccon = 0x00;
    setup();

    while(1)
    {        
        delay_ms(50);
        int_cnt++;
        
        if (int_cnt == 1)
        {
            if(test_bit(porta,5))
            {
                clear_bit(porta,5);
            }
            else
            {
                set_bit(porta,5);
            }
            set_bit(flags,1);
            portb --;
        }
        
        if (test_bit(flags,1))
        {
            delay_ms(200);
            clear_bit(portd,0);
            delay_ms(200);
            set_bit(portd,0);
            delay_ms(200);
            clear_bit(portd,0);
            delay_ms(200);
            
            clear_bit(flags,1);

            clear_bit(pir1,3); // clear spi flag
            clear_bit(portd,1); // select slave
            delay_ms(255);
            sspbuf = portb; // send byte
            
            while (!test_bit(pir1,3))
            {} // wait till send complete
            delay_ms(255);
            set_bit(portd,1);

            delay_ms(200);
            set_bit(portd,0);
            delay_ms(200);
            clear_bit(portd,0);
            delay_ms(200);
            int_cnt = 0x00;
        }
    }
}


void interrupt()
{

}


void setup()
{
    portd = 0x00;
    portc = 0x00;
    
    intcon = 0x00;
    intcon2 = 0x00; // interrupt on rising edge, pull ups enabled
    intcon3 = 0x00; // int1, int2 enabled
    
    ccp1con = 0x00; // capture/compare off
    sspcon1 = 0x00; // spi/i2c off
    
    adcon0 = 0x00; // a/d off
    adcon1 = 0x06; // digital io

    trisa = 0x0f; // high nibble = output, low nibble = input
    trisb = 0x00;
    trisc = 0x00;
    trisd = 0x00; // d1 = slave select pin
    trise = 0x07;
    
    portb = 0x00;
    portc = 0x00;
    portd = 0x00;
    
    flags = 0x00;
    int_cnt = 0x00;
    
    // SPI Configuration
    set_bit(portd,1); // slave select is active low
    sspcon1 = 0x20; // spi enabled, master mode, fosc/64
    sspstat = 0x40; // data sampled in middle, tx on rosing edge
    trisc = 0x10; // set per book page 216
    portc = 0x00;
    clear_bit(trisd,0); // slave select pin
    clear_bit(rcon,7); //disabled int priority
}

Slave Code:

#include <system.h>

/*
18F452
16MHz resonator
*/

#pragma DATA _CONFIG1H, _HS_OSC & _OSCS_OFF_1H
#pragma DATA _CONFIG2L, _PWRT_OFF_2L & _BOR_OFF_2L
#pragma DATA _CONFIG2H, _WDT_OFF_2H
#pragma DATA _CONFIG3H, _CCP2MX_OFF
#pragma DATA _CONFIG4L, _LVP_OFF_4L & _DEBUG_OFF_4L
#pragma DATA _CONFIG5L, _CP0_OFF_5L & _CP1_OFF_5L & _CP2_OFF_5L & _CP3_OFF_5L
#pragma DATA _CONFIG5H, _CPB_OFF_5H
#pragma DATA _CONFIG6L, _WRT0_OFF_6L & _WRT1_OFF_6L & _WRT2_OFF_6L & _WRT3_OFF_6L
#pragma DATA _CONFIG6H, _WRTC_OFF_6H & _WRTB_OFF_6H & _WRTD_OFF_6H
#pragma DATA _CONFIG7L, _EBTR0_OFF_7L & _EBTR1_OFF_7L & _EBTR2_OFF_7L & _EBTR3_OFF_7L
#pragma DATA _CONFIG7H, _EBTRB_OFF_7H

void interrupt(void);
void setup(void);

//Global Variables
char flags;
unsigned char ssp_dat;


void main()
{
    osccon = 0x00;
    setup();
    
    while(1)
    {
        if(test_bit(flags,0)) // data received
        {
            ssp_dat = sspbuf;
            clear_bit(pir1,3);
            portb = ssp_dat;
            clear_bit(flags,0);
            clear_bit(sspcon1,7);
            clear_bit(sspstat,0);
            clear_bit(sspcon1,6);
            
            set_bit(portd,1);
            delay_ms(100);
            clear_bit(portd,1);
            delay_ms(100);
            set_bit(portd,1);
            delay_ms(100);
            clear_bit(portd,1);
            delay_ms(100);
        }
    }
}



void interrupt()
{
    clear_bit(intcon,7);
    clear_bit(intcon,6);
    
    if (test_bit(pir1,3)) // spi int flag
    {
        clear_bit(pir1,3);
        set_bit(flags,0);
    }
    
    set_bit(intcon,6);
    set_bit(intcon,7);
}



void setup()
{
    portb = 0x00;
    portc = 0x00;
    portd = 0x00;
    porte = 0x00;
    
    intcon = 0x00; // ints disabled
    intcon2 = 0x00; // pull ups enabled
    intcon3 = 0x00; // int disabled
    
    ccp1con = 0x00; // capture/compare off
    
    adcon0 = 0x00; // a/d off
    adcon1 = 0x06; // all digital io

    trisa = 0xff; // trisa5 must be input for slave select bit
    trisb = 0x00;
    trisc = 0x18; // set spi io and clk pins page 216
    trisd = 0x00;
    trise = 0x07;
    
    flags = 0x00;
    
    //set spi for slave
    sspcon1 = 0x24; // spi enabled, slve mode
    sspstat = 0x40; // tx on rising edge
    
    set_bit(intcon,6);
    set_bit(intcon,7);
    set_bit(rcon,7); // priority level int enabled
    set_bit(ipr1,3); // spi int high priority
    set_bit(pie1,3); // spi int enabled
}



			
		

Share this post


Link to post
Share on other sites

mityeltu,

 

Here are a few basic problems, particularly with the slave:

 

Do not globally disable or re-enable interrupts within the interrupt routine. Doing so is pointless and can cause problems. The global interrupt is

disabled and re-enabled automatically. In addition why are you disabling pheripheral interrupts within the interrupt?

 

You are you using the SPI interrupt to set a flag which you then monitor and clear in main (). This is totally pointless, you may as well not use an

interrupt at all but just monitor the interrupt flag and clear it in main () when required. However, this will not work properly either because you are

using massive delays in main ().

 

Don't use in-line delays. These simply waste processor time and are blocking. Depending on the timing this is likely to be the main cause of your

problem. The SPI input is asynchronous with your slave clock therefore you could easily miss reading an SPI input before another occurs while you

are stuck in one of your delay loops.

 

A few basic suggestions for your simply case:

 

Setup a timer to interrupt regularly, say every 1ms, to give a base frequency. Within this handler decrement a counter and when this reaches

zero set a flag and reset the count. This flag can then be monitored and cleared in main () to do whatever is required at the right time.
A number of counters of different sizes and associated flags can be used to produce a range of 'clocks' which can be monitored in main () to carry out different tasks. This method avoids using any long delays and allows the main () code to run as fast as possible.

 

Use the SPI interrupt but read the input directly within the interrupt. If required a flag could be set to indicate that a new value is available.

Regards

 

davidb

Share this post


Link to post
Share on other sites

davidb,

 

I appreciate the guidance. I'll attempt to implement these ideas this week.

 

I disable the interrupts to prevent additional interrupts from occurring while in the ISR. I guess from your post this is superfluous. I didn't realize this. I will change that today.

 

The reason for the massive delays is that it would not operate without them. I originally had very short delays (20uS), but the slave would only pickup about every 10th or 15th transmission. Once I added the delays, it began only missing a few. Then with these massive delays it misses about 1 or 2 out of 100.

 

As to the interrupts, I am trying to keep the runtime out of interrupt as much as possible, I see your point on scanning as opposed to interrupt though. I also didn't know the delays were blocking. I assume you mean they block the interrupts the firware is looking for.

 

I don't understand what you mean by the following:

The SPI input is asynchronous with your slave clock therefore you could easily miss reading an SPI input before another occurs while you are stuck in one of your delay loops.

 

I'll modify the ISR to read directly and remove the delays. A possible problem is that the timer may interrupt in the middle of my spi transmission. Will that not cause a dropped byte or possible bus collision or overflow?

Share this post


Link to post
Share on other sites

mityeltu,

 

I would suggest that you thoroughly read the data sheet for the part you are using as well as checking out application notes

and sample code so that you properly understand how the interrupts work.

 

It is only my opinion but I believe using delay loops is bad programming practice hence my suggestion of using an interrupt

driven task timer. You can also use similar techniques to produce one shot timers, PWM etc. Delays are a quick fix for

simple applications but become a pain when trying to expand the program to do something useful.

 

While it is generally true that interrupt code should run as quickly as possible it depends on your application. I have

produced applications where the main () loop has very little code in it with everything being done with interrupts.

 

What I meant when I said the SPI input is asynchronous is that there will always be a frequency difference and also a varying

phase between the master and slave clocks. The effect would be that the SPI input occurs at a different point in your slave

main loop every time. Because of your long delays and depending on the data rate the SPI data buffer could be overwritten

before you have a chance to read it the first time. This is why you should read the buffer in an interrupt.

 

I don't think you need to worry too much about dropping a byte since you are using the hardware SPI which has a buffered

output and you are only transferring one byte at a time at a very slow rate. The interrupt and main code will easily keep up. However, if you

are worried the 18F452 supports priority level interrupts so put the SPI on a high level interrupt and the timer on a low level interrupt.

At high data rates you may also need to implement a circular buffer.

 

Regards

 

davidb

Share this post


Link to post
Share on other sites

Allright, well I took out the delays and replaced them with timer 3 interrupt routines and now it neither sends nor receives. Timer 3 is using 1:8 prescaler and an interrupt count of 5 giving me about the same delay as the delay_ms() routnes. Any other ideas?

Share this post


Link to post
Share on other sites

Hi mityeltu,

 

Sorry it didn't work but this is all pretty basic stuff. If your master is not transmitting how do you know that the slave is not working?

 

It is difficult to comment without seeing your code but have you checked that your basic timing is working? Use a scope and a spare output port for debugging to check the timing and then check that the SPI clock and data outputs from your master are correct. Do this before worrying about the slave.

 

Regards

 

davidb

Share this post


Link to post
Share on other sites

Hi

 

Here you have a piece of scrap code extracted from one of my applications that can ilustrate the basic structure of an interrupt based delay using an internal timer.

 

In this example TIMER0 is used as a master time base (aka system timer). It generates periodic interrupts that are used to decrement the application delay counters.

In this code scrap there is only one application delay that is used to control de interval betwen periodic ADC samples.

Each time the delay gets down to zero, one ADC sample is obtained and the delay is restarted.

The interrupt routine decrements the delay until it reaches "0".

 

Note that there are no active delays (closed loops) neither for the ADC operation intervals, neither for waiting for ADC /DONE flag, so the PIC is free to keep processing other stuff.

void interrupt(void)
{
        // Process TIMER0 interrupt
        if(intcon.TMR0IE && intcon.TMR0IF)
            {
            // Timer 0 interrupt ocurred
            intcon.T0IF=0;
            // Process ADC delay
            if(AdcDelayCount)
                {
                AdcDelayCount--;
                } // if(AdcDelayCount)
            // Process other TIMER0 interrupt dependent stuff
            // ..................
            } // if(intcon.TMR0IE && intcon.TMR0IF)
            
        // Process other interrupts
        // ................
} // void interrupt(void)


void main(void)
{
        // Configure I/O lines
        INIT_ANALOG_IO();            // Configure analog pins (AN0 and AN1)
        INIT_LCD_IO();                // Configure LCD I/O pins

        // Configure on chip peripherals
        ADC_Config();                // Configure ADC
        Timer0_Config();            // Configure Timer 0                
        Init_ADC_AN0();                // Initial configuration of ADC

        // Initialize application
        ADC_On();                    // Activate ADC module
        AdcDelayCount=ADC_DELAY;    // Start delay before first ADC reading
        Enable_Tmr0_Int();            // Enable Timer0 interrupts (system time base)
        Enable_Interrupts();        // Enable global interrupts to start timing
        
        // Start application timing
        Timer0_On();                // Start Timer0

        // Main loop
        while(1)
            {
            // Process ADC operation
            if(AdcStarted)
                {
                if(ADC_Done)
                    {
                    // Init delay for next ADC sample
                    AdcStarted = 0;
                    AdcDelayCount = ADC_DELAY;
                    // Save most recent ADC result
                    Adc0_Result = adresh;
                    // Adjust PWM update interval
                    PwmStepValue = ((WORD)Adc0_Result << 3) + CYCLE_PERIOD_MIN;
                    // Update AN0 display on LCD
                    BCDDisplay = PwmStepValue;
                    Bin2Bcd();
                    LCD_Cursor(LCD_LINE_1,1);
                    for(int i=3; i>=0; i--)
                        {
                        LCD_SendData(BCDBuff[i]);
                        }
                    }    // if(ADC_Done()
                }  // if(AdcStarted)
            else
                {
                if(!AdcDelayCount)
                    {
                    // Delay finished - start ADC convertion
                    AdcStarted = 1;
                    ADC_Start();
                    }    //    if(!AdcDelayCount)
                } // else if(AdcStarted)
                
            // Process XXX

            // Process YYY
               
            // Process ZZZ
               
            // And so on .....

            } // while(1)
} // void main(void)

Please note that most of the statements in this sample code are macros that I created as part of the project from were the code was extracted, so don't try to compile this sample or find the macros/functions in some standard library.

 

HIH

 

Best regards

Jorge

Edited by JorgeF

Share this post


Link to post
Share on other sites

Join the conversation

You are posting as a guest. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...

×
×
  • Create New...