PIC I2C Master Crash

Thread Starter

Robin Mitchell

Joined Oct 25, 2009
819
Hi everyone,

So im using a PIC18F45k20 as a I2C master control and it makes the controller of a laser tag gun. Now I have clips that load into the gun which contain an I2C EEPROM that inform the PIC of the ammo contents.

Here is the problem:

Sometimes if I either put the clip in or take the clip out it causes the PIC to crash and stop entirely leaving SDA and SCL (RC3 and RC4) permanently high. I dont have the code for the PIC on this computer but this happens regardless of any I2C operation. The program looks at an EEPROM located at address 001 but the clip is 000. So even if an operation is occurring with 001 attaching and detaching 000 should make no problem.

Is the removal/addition of the device causing a problem with the bus? Does the PIC I2C Module hate removals and why would it stop?

Any help would be great!

All the best,
Robin
 

Papabravo

Joined Feb 24, 2006
21,228
As long has the PIC has Vcc and an oscillation method, there is virtually no way you can get it to STOP. It will keep executing instructions until hell freezes over. External events can make it execute code that you did not intend which is a great deal more likely.

An I2C master has a great deal of flexibility in dealing with a bus that does not behave such as when a device is being inserted or removed. Too bad you don't have access to the code.
 

Thread Starter

Robin Mitchell

Joined Oct 25, 2009
819
Here is the main.c file

Rich (BB code):
/** C O N F I G U R A T I O N   B I T S ******************************/


/** I N C L U D E S **************************************************/
#include "p18f45k20.h"
#include "delays.h"
#include "IntOSC.h"
#include "math.h"
#include <string.h>
#include "i2c protocol.h"


/** V A R I A B L E S *************************************************/
#pragma udata // declare statically allocated uninitialized variables

char sound        = 0;
char sound_start_over = 0;
unsigned char ammo_tens = 6;
unsigned char ammo_digit= 0;
unsigned char total_ammo =60;
unsigned char temp;

int boot = 800;      
int address = 0;
int cool_down_trigger = 0;
int cool_down_display = 0;
int led_cooldown = 0;

char address_high = 0;
char address_low = 0;     
unsigned char clip_present = 0;
unsigned char ack;



/** D E C L A R A T I O N S *******************************************/
// declare constant data in program memory starting at address 0x180
#define SEG1        LATCbits.LATC5
#define SEG2        LATCbits.LATC6
#define TRIGGER        PORTCbits.RC7
#define NUM0 0b00111111
#define NUM1 0b00000110
#define NUM2 0b01011011
#define NUM3 0b01001111
#define NUM4 0b01100110
#define NUM5 0b01101101
#define NUM6 0b01111101
#define NUM7 0b00000111
#define NUM8 0b01111111
#define NUM9 0b01100111
#define LED            LATCbits.LATC0
     


void display_digit(char number, char port);
void load_ammo();


void main (void)
{
    ANSELH = 0x00;                 // AN8-12 are digital inputs (AN12 on RB0)
    OSCCON = 0x70;              // IRCFx = 110 (8 MHz)
    OSCTUNEbits.PLLEN = 1;      // x4 PLL enabled = 32MHz
    
    address = 0;
    sound_start_over = 0;

    TRISA = 0x00;
    TRISB = 0xFF;
    TRISC = 0b10011000;
    TRISD = 0x00;
    
    TRISBbits.TRISB4 = 0;

    I2CInit();    
    I2CStart();
    I2CSend(0b10100010);    
    I2CSend(0);
    I2CSend(0);
    I2CStop();

        
    do
    {
        I2CWait();
        I2CStart();
           I2CSend(0b10100011);
        I2CWait();
               
        temp = I2CRead() - 0x60;
        LATD = temp;
        I2CWait();
        I2CStop();
        
        
        
        // Check a clip is present
        do

           {
               I2CStart();
               I2CSend(0b10100000);
               
           }while(SSPCON2bits.ACKSTAT == 1);
      
           I2CStop();    
     
    
    
    
        if(address >= 5000)
        {
            address = 0;
            sound = 0;
            LATD = 0;
        }

        
        if(sound_start_over == 1)
        {
            address = 0;
            sound = 1;
            sound_start_over = 0;
        }
        

        if(sound == 1)
        {

            address ++;                    
        }
        
        // Trigger pressed
        if(TRIGGER == 1 && cool_down_trigger == 0 && (total_ammo > 0))
        {
            sound = 1;
            cool_down_trigger = 500;
            sound_start_over = 1;
            led_cooldown = 3;
            
            total_ammo --;        
            
            if(ammo_digit == 0)
            {
                ammo_digit = 9;
                ammo_tens --;
            }
            else
            ammo_digit --;
            

            
        }
        

        if(cool_down_display == 0)
        {
            if(sound == 1)
            cool_down_display = 1;
            else
            cool_down_display = 1;

            display_digit(ammo_digit, 2);
            display_digit(ammo_tens, 1);
            SEG1 = 0;
            SEG2 = 0;
        }
        
        if(cool_down_trigger != 0)
        cool_down_trigger --;
    
        if(cool_down_display != 0)
        cool_down_display --;

        if(led_cooldown!= 0)
        {
            LATCbits.LATC0 = 1;
            led_cooldown --;
        }
        else
        LATCbits.LATC0 = 0;
        
    }while(1);        

}



void display_digit(char number, char port)
{
    SEG1 = 0;
    SEG2 = 0;

    switch(number)
    {
        case 0:
        LATA = NUM0;
        break;
        
        case 1:
          LATA =  NUM1;
        break;

        case 2:
        LATA = NUM2;
        break;

        case 3:
        LATA = NUM3;
        break;

        case 4:
        LATA = NUM4;
        break;
        
        case 5:
        LATA = NUM5;
        break;
        
        case 6:
        LATA = NUM6;
        break;

        case 7:
        LATA = NUM7;
        break;

        case 8:
        LATA = NUM8;
        break;

        case 9:
        LATA = NUM9;
        break;
        
        default:
        LATA = NUM0;
        break;
    }    
    
        if(port == 1)
    {
        SEG1 = 1;
    }            

    if(port == 2)
    {
        SEG2 = 1;
    }            

}


void load_ammo()
{
    // Set address pointer
     I2CStart();
    I2CSend(0b10100000);    
    I2CSend(0);
    I2CSend(0);
    I2CStop();
    
    I2CWait();
    I2CStart();
       I2CSend(0b10100011);
    I2CWait();
        
 
    total_ammo = I2CRead();
    I2CWait();
    ammo_tens  = I2CRead();
    I2CWait();
    ammo_digit = I2CRead();
    I2CWait(); 
    I2CStop();    
    
}
Here is the I2C Protocol header file

Rich (BB code):
#ifndef I2CPROTOCOL_H
#define I2CPROTOCOL_H
#include "p18f45k20.h"

// Function Prototypes

#define SCL_OUTPUT        TRISCbits.TRISC3 = 0;
#define SCL_INPUT        TRISCbits.TRISC3 = 1;
#define SDA_OUTPUT        TRISCbits.TRISC4 = 0;
#define SDA_INPUT        TRISCbits.TRISC4 = 1;
#define SCL_ON            LATCbits.LATC3 = 1;
#define SCL_OFF            LATCbits.LATC3 = 0;
#define SDA_ON            LATCbits.LATC4 = 1;
#define SDA_OFF            LATCbits.LATC4 = 0;
#define SDA_IN            PORTCbits.RC4;

void I2CInit(void);
void I2CStart();
void I2CStop();
void I2CRestart();
void I2CAck();
void I2CNak();
void I2CWait();
void I2CSend(unsigned char dat);
unsigned char I2CRead(void);


/*
Function: I2CInit
Return:
Arguments:
Description: Initialize I2C in master mode, Sets the required baudrate
*/
void I2CInit(void){
        TRISCbits.TRISC3 = 1;      /* SDA and SCL as input pin */
        TRISCbits.TRISC4 = 1;      /* these pins can be configured either i/p or o/p */
        SSPSTAT |= 0x80; /* Slew rate disabled */
        SSPCON1 = 0x28;   /* SSPEN = 1, I2C Master mode, clock = FOSC/(4 * (SSPADD + 1)) */
        SSPADD = 0x27;   /* 100Khz @ 4Mhz Fosc */
}

/*
Function: I2CStart
Return:
Arguments:
Description: Send a start condition on I2C Bus
*/
void I2CStart(){
        SSPCON2bits.SEN = 1;         /* Start condition enabled */
        while(SSPCON2bits.SEN);      /* automatically cleared by hardware */
                     /* wait for start condition to finish */
}

/*
Function: I2CStop
Return:
Arguments:
Description: Send a stop condition on I2C Bus
*/
void I2CStop(){
        SSPCON2bits.PEN = 1;         /* Stop condition enabled */
        while(SSPCON2bits.PEN);      /* Wait for stop condition to finish */
                     /* PEN automatically cleared by hardware */
}

/*
Function: I2CRestart
Return:
Arguments:
Description: Sends a repeated start condition on I2C Bus
*/
void I2CRestart(){
        SSPCON2bits.RSEN = 1;        /* Repeated start enabled */
        while(SSPCON2bits.RSEN);     /* wait for condition to finish */
}

/*
Function: I2CAck
Return:
Arguments:
Description: Generates acknowledge for a transfer
*/
void I2CAck(){
        SSPCON2bits.ACKDT = 0;       /* Acknowledge data bit, 0 = ACK */
        SSPCON2bits.ACKEN = 1;       /* Ack data enabled */
        while(SSPCON2bits.ACKEN);    /* wait for ack data to send on bus */
}

/*
Function: I2CNck
Return:
Arguments:
Description: Generates Not-acknowledge for a transfer
*/
void I2CNak(){
        SSPCON2bits.ACKDT = 1;       /* Acknowledge data bit, 1 = NAK */
        SSPCON2bits.ACKEN = 1;       /* Ack data enabled */
        while(SSPCON2bits.ACKEN);    /* wait for ack data to send on bus */
}

/*
Function: I2CWait
Return:
Arguments:
Description: wait for transfer to finish
*/
void I2CWait(){
      while ( ( SSPCON2 & 0x1F ) || ( SSPSTAT & 0x04 ) );
    /* wait for any pending transfer */
}

/*
Function: I2CSend
Return:
Arguments: dat - 8-bit data to be sent on bus
           data can be either address/data byte
Description: Send 8-bit data on I2C bus
*/
void I2CSend(unsigned char dat){
    
        SSPBUF = dat;    /* Move data to SSPBUF */
        while(SSPSTATbits.BF);       /* wait till complete data is sent from buffer */
        I2CWait();       /* wait for any pending transfer */
}

/*
Function: I2CRead
Return:    8-bit data read from I2C bus
Arguments:
Description: read 8-bit data from I2C bus
*/
unsigned char I2CRead(void){
        unsigned char temp;
/* Reception works if transfer is initiated in read mode */
        SSPCON2bits.RCEN = 1;        /* Enable data reception */
        while(!SSPSTATbits.BF);      /* wait for buffer full */
        temp = SSPBUF;   /* Read serial buffer and store in temp register */
        I2CWait();       /* wait to check any pending transfer */
        return temp;     /* Return the read data from bus */
}
 

#endif
 

Thread Starter

Robin Mitchell

Joined Oct 25, 2009
819
I should note that the crash occurs regardless of the code below in main.c
Rich (BB code):
        // Check a clip is present
        do

           {
               I2CStart();
               I2CSend(0b10100000);
               
           }while(SSPCON2bits.ACKSTAT == 1);
      
           I2CStop();
 

Thread Starter

Robin Mitchell

Joined Oct 25, 2009
819
Something interesting I just found out.

The same problem can be recreated if you temporarily connect the SDA line and SCL together.

ALso found out that the SCL is constantly held low
 
Last edited:

Papabravo

Joined Feb 24, 2006
21,228
You have a loop that depends on only one condition. This is the road to Embedded Software Perdition. Everytime you have a condition that is false you need to check for other conditions that might be true and take appropriate action. I would expect to see code that would do an I2C bus STOP whenever any suspicious behavior is detected. Don't forget to observe the minimum Bus Free time between a STOP and a subsequent START (4.7 μsec in Standard Mode)

I understand you are trying to get something working; just don't expect it to be bulletproof out of the gate. Do you have a copy of the I2C specification? Mine is version 2.1 dated January 2000(NXP Semiconductors, formerly Philips). Search for Application Notes published by device manufacturers with I2C code for specific devices. You will learn a ton by reading them.
 

ErnieM

Joined Apr 24, 2011
8,377
Something interesting I just found out.

The same problem can be recreated if you temporarily connect the SDA line and SCL together.

ALso found out that the SCL is constantly held low

I believe I've had similar problems as you when doing some debugging of I2C code. What I believe was happening was I would occasionally stop the debugger during an I2C transaction, and if I hit the wrong place it would stall the bus.

I never tracked it down totally but as I recall it was the SDA line (not SCL) that was stuck low on the slave end: I believe a NAK was in progress when the PIC was reset. So next time the PIC tries to talk, it sees SDA low, assumes another master is talking, and patiently waits till the transaction ends.

Of course, with no other master on line the transaction never ends, and the program hangs.

My workaround was on start up to always manually toggle the SCL line 9 times: that guarantees a transaction NAK is well in the past. By "manually" I mean toggle the port bit directly, not thru the I2C module.

The only time I know a slave can hold SCL low is when it needs some extra time to process a transaction, but that is a momentary and very fast event.
 

Papabravo

Joined Feb 24, 2006
21,228
If you know you are the only master it helps considerably. You need to crawl and walk before you try to run a marathon. If you are the master you need to establish that you are in control of the bus lines EXCEPT when you are in the middle of an ACK or a DATA transfer. If the bus lines are not in the state you think they should be in, you need to force them by sending STOP commands, and then waiting.
 

THE_RB

Joined Feb 11, 2008
5,438
I2C is a mongrel for plug and play. Not a great choice.

You are much better off with SPI (although that takes 3 lines) and you can source SPI EEPROMS just as easy.

Since this is a low datarate application my preference would be asynchronous serial (TTL USART serial). That would mean instead of an EEPROM in the clips you could use a really cheap PIC like a 10F series (or the slightly more costly 12F series if you prefer).
 

ErnieM

Joined Apr 24, 2011
8,377
TELL ME WHERE IS IT! I SHALL DESTROY IT!

Thanks for all of this great feedback :)

All the best,
MitchElectronics
Consider the last line of this function:

Rich (BB code):
void I2CAck(){
         SSPCON2bits.ACKDT = 0;       /* Acknowledge data bit, 0 = ACK */
         SSPCON2bits.ACKEN = 1;       /* Ack data enabled */
         while(SSPCON2bits.ACKEN);    /* wait for ack data to send on bus*/ 
}
If the I2C breaks you get stuck there forever.

Such patterns are common in your code, and most I2C code I've seen. It was what I was working on when I was debugging that I2C code (which was for the PIC32 series so isn't very useful to you).
 

Papabravo

Joined Feb 24, 2006
21,228
I2C is a mongrel for plug and play. Not a great choice.

You are much better off with SPI (although that takes 3 lines) and you can source SPI EEPROMS just as easy.

Since this is a low datarate application my preference would be asynchronous serial (TTL USART serial). That would mean instead of an EEPROM in the clips you could use a really cheap PIC like a 10F series (or the slightly more costly 12F series if you prefer).
I don't necessarily agree with this. Like anything else there are pros and cons. I've not seen any devices that I couldn't make work after a period of investigation, careful coding, and insightful debugging. They are certainly easier than USB and Ethernet.
 

THE_RB

Joined Feb 11, 2008
5,438
I don't necessarily agree with this. Like anything else there are pros and cons. I've not seen any devices that I couldn't make work after a period of investigation, careful coding, and insightful debugging. They are certainly easier than USB and Ethernet.
I don't think I2C was ever meant for plug and play. It's critical for things like its start and stop conditions which means holding one line in a particular state while the other line goes from lo->hi or hi->lo. That's all bad juju for plug and play systems, even if you mess with pullup resistors etc.

Another issue for I2C is that comms must be bidirectional and send device addresses etc, and both master and slave need to be able to pull the same wires low etc. That's just a heap more stuff to go wrong.

SPI is better as the only critical transition is the clock pulse \ edge etc, so with pullup resistors on both master and slave devices you can be pretty safe knowing the clock line stays hi at all times before/during/after plugging a slave device in (until data starts). It's also simpler with less data needing to be sent (lower error rates at x errors per million bits, etc).

Asynch TTL USART serial is simpler and better still, only needing one wire (or two IF you need bidirectional data) and being a fixed baudrate can be low pass RC filtered through a schmidt trigger, to even give total immunity to things like high speed digital noise and dropout noise from bad connections etc.

Sure you could make I2C work for plug n play ammo clips but I still think async TTL USART serial is a far better choice, assuming only small amounts of data need to be sent.
 

ErnieM

Joined Apr 24, 2011
8,377
There is simply no inherent reason I2C could not do plug and play. There is nothing critical about the protocol, and each and every transaction is limited to a single well defined byte of data, and that byte is always marked with an acknowledge signal. That means if someone unplugs in the middle you will know it soon enough.

With no limits on clock rates you can run this slow to extend the range. Transceiver amplifiers (self-sensing bidirectional buffers) are available to clean up signals if you have a long signal path; these can also do level translations if the power supply voltage doesn't match between sections.

The worst that I've even seen happen is a slave will stall during an ACK on the data line (forcing it low), but that isn't an issue when UNPLUGGING a device, and can be fixed just by sending some extra clock signals.

There are many other equally workable solutions, but the best one here is the one you already built: I2C.
 

THE_RB

Joined Feb 11, 2008
5,438
Hi ErnieM, the O.P. said;

...
Is the removal/addition of the device causing a problem with the bus? Does the PIC I2C Module hate removals and why would it stop?
...
I think if he has used a single USART serial wire, with the clip sending one byte (remaining ammo capacity) he would not be struggling with these problems.

Code would be extremely simple, ie just check the PIC USART if a byte had been received, then that byte tells the ammo capacity. The clip can just send that byte a few times a second. Error checking is as simple as reading 2 or 3 same values in a row means data is good.

Besides the code benefits there are hardware benefits in that it only needs a single comms wire and can tolerate noise issues much better.

So you can say "equally workable solutions" if you like, but they are NOT equally workable. I2C is a poorer choice for plug and play and has a lot of issues. It was developed by Philips to use in chip to chip comms within a single appliance, being worse than many other systems but has an advantage of multiple devices on the same 2-wire bus.

We don't even know if the O.P. is making use of that ONE advantage, he's definitely coming up against ALL of I2C's disadvantages. :)
 

Thread Starter

Robin Mitchell

Joined Oct 25, 2009
819
Hi THE_RB

I have two EEPROMS on the bus. Address 001 is sound data so when the trigger is pressed it is accessed to generate the gunshot and the clip is on address 000. This means I can use both on the same bus with the onboard PIC I2C Module (MSSP module).

USART sounds good but im unfamiliar with any memory chips using this method. As of now i have a work around. I have an external 4013 as the "Been shot" flag and if the PIC stalls and crashes the watchdog timer resets the pic. Now the pic tests the external flip flop to see if the player was previously hit.
 

ErnieM

Joined Apr 24, 2011
8,377
Mitch: Look, if you want you can toss most of your existing work out the window and follow suggestions to enormously complicate your clip module from a simple EEPROM to an EEPROM, controller and probably a mess of other things, plus rewrite out much of your existing code, well, you go right ahead and do that.

There is absolutely no reason something on the I2C bus could stall your processor, assuming the signals are in some reasonable nominal range. So if you are not sending in large glitch signals that upset the power supply it's not strictly an I2C issue. The statements to the contrary gave no supporting facts, just a (seemingly biased) opinion along with other disparaging terms.

An oscilloscope would be incredibly helpful here to observe the connection event so you can see if anything unexpected happens.

Using the watchdog to recover is ok (and should be there) but if this was my work I'd get the plug/unplug not to depend on the watchdog.

How sure are you of your power supply when you plug in? You do have bypass caps about your chips, correct?

Some isolation resistors may be useful here; 100 ohms in series with power (followed by bypass cap on EEPROM side), and then 100 ohms in series with the I2C lines.

Is there any arrangement to get power to the EEPROM before the bus connects? That's what USB does to power the remote device befor it connects to the computer.
 

THE_RB

Joined Feb 11, 2008
5,438
...
I have two EEPROMS on the bus. Address 001 is sound data so when the trigger is pressed it is accessed to generate the gunshot and the clip is on address 000. This means I can use both on the same bus with the onboard PIC I2C Module (MSSP module).
...
I see that as a potential source of problems. If you get mud/dirt/noise/static/spikes etc on your removable clip contacts they are shared totally, those two wires in parallel with your critical sound memory EEPROM! What if a clip is faulty (shorted?).

Myself and ErnieM obviously have different design philosophies, but my personal preference would be for the plug'n'play clip to use a separate sytem optimised for its reliability, and the sound playback system separate to the potentially problematic clip connections and optimised for its own reliability.

Re using EEPROMs in the clips, I suggested before maybe going to a tiny cheap PIC like a 10F series. That can output any signal you like, BUT can also offer performance features like clips that slowly recharge their capacity over time (mimicing some games and SciFi standards).
 
Top