I2C protocol fails between Raspberry PI and MCU

Discussion in 'Digital Circuit Design' started by zaferaltun, Jun 25, 2017.

  1. zaferaltun

    Thread Starter Member

    Aug 31, 2016
    65
    1
    Hello

    I have Raspberry Pi 3 and a custom circuit designed and coded by me. The circuit has 18F87K22 MCU, they are talking with I2C protocol. I developed the MCU's code by looking to the datasheet in C, using XC8 compiler. It is working in Proteus simulator as well but the real word is different as you know.

    The problem is sometimes I2C protocol is just disconnecting in a way as I understood. I 'm using smbus library in Python code on RP side and getting Input/Output Error (Error 5) sometimes but totally random. if I disconnect the circuit, I 'm getting that error always, so it means the connection is losing but I don't know how?

    I 'm giving latency between operations but it does not matter the latency is 1ms or 1 second, as I told before, it is totally random. I updated the RP kernel to 4.4 but nothing changed. I tried decreasing baudrate down to 9600 but nothing changed. There is no difference either 100000 or 9600. I tried to catch the problem like overflow etc on circuit side but it seems there is no overflow or something like that as I tested. Also I checked if the MCU is resetting but no, it is not resetting. How can I fix this problem? Actually is the problem about RP's I2C protocol or smbus library or my own circuit? There are lots of prorability but here is where the experince talks, please help me. I 'm not using external pull ups since RP has already 1.8k internal pull up resistors, I2C connection cable is about 50cm length. I can share the codes if needed.

    I 'm awaiting urgent replies please.
    Best
     
  2. Papabravo

    Expert

    Feb 24, 2006
    12,040
    2,590
    There are a couple of things that are unclear from your original post. Does the PIC18F87K22 have builtin hardware support for the I2C protocol or is it bit banging? [EDIT: yes it does have two MSSPs that can be used for I2C] Does the Pi 3 have I2C hardware or is it bit banging? The reason I ask is that I2C has no concept of connection or disconnection. It is not a connection oriented protocol in the sense that most network protocols use that concept. If SDA and SCL are connected and have proper voltage levels and pullups then the two ends are connected, period, full stop.

    It is true that a misbehaving transmitter can force the slave device to quit responding(ACK'ing). It is also true that in any error condition the master can RESET the slave by issuing a STOP followed by a valid START. I think it much more likely that your code is at fault, rather than the hardware in the Pi 3 or the microchip part.

    There are other possible recovery mechanisms for dealing with a slave device that holds SDA low preventing the master from issuing a STOP condition.
     
    Last edited: Jun 25, 2017
  3. zaferaltun

    Thread Starter Member

    Aug 31, 2016
    65
    1

    Well, I 'm in doubt about my code since I 'm developing something with I2c protocol for the first time, but I read somethings in RP forums about this error although there is no coded MCU, just a EEPROM or some sensors. Yet, here is my code:

    Code (Text):
    1. #define _XTAL_FREQ              16000000
    2.  
    3. #define ADC_RANGE               4095                    // 2 ^ 12 (4096) bit resolution - 1
    4. #define ADC_VREF                5000                    // 5V
    5. #define I2C_DATA_BUFFER_LEN     10
    6. #define I2C_CMD_BUFFER_LEN      10
    7.  
    8. #include <xc.h>
    9. #include <stdio.h>
    10. #include <stdlib.h>
    11. #include <string.h>
    12.  
    13. #pragma config FOSC             = INTIO2    // Oscillator (Internal RC oscillator)
    14. #pragma config PLLCFG           = OFF       // PLL x4 Enable bit (Disabled)
    15. #pragma config FCMEN            = OFF       // Fail-Safe Clock Monitor (Disabled)
    16. #pragma config IESO             = OFF       // Internal External Oscillator Switch Over Mode (Disabled)
    17. #pragma config WDTEN            = OFF       // Watchdog Timer (WDT disabled in hardware; SWDTEN bit disabled)
    18. #pragma config XINST            = OFF       // Extended Instruction Set (Disabled) VERY IMPORTANT FOR UNEXPECTED BEHAVIORS
    19. #pragma config SOSCSEL          = DIG       // SOSC Power Selection and mode Configuration bits (Digital (SCLKI) mode)
    20. #pragma config RETEN            = OFF       // VREG Sleep Enable bit (Disabled - Controlled by SRETEN bit)
    21.  
    22. int SLAVE_ADDR                  = 0x76;     // TEMP
    23. i2cInProgress             = 0;
    24.  
    25. void init(void);
    26. void delayUS(int us);
    27. void delayMS(int ms);
    28.  
    29. void main(void) {
    30.  
    31.     init();
    32.  
    33.     delayMS(500);
    34.  
    35.     while (1) {
    36.      
    37.         if (!i2cInProgress && SSP1STATbits.S) {
    38.             i2cInProgress = 1;
    39.          
    40.             // Enable automatic baud rate calculation
    41.             BAUDCON1bits.ABDEN = 1;
    42.         }
    43.      
    44.         if (SLAVE_ADDR && i2cInProgress && PIR1bits.SSP1IF){
    45.          
    46.             // Receive overflow. A byte is received while the SSPxBUF register is still holding the previous byte (must be cleared insoftware)
    47.             if (SSP1CON1bits.SSPOV){
    48.              
    49.                 // Clear
    50.                 unsigned char prevByte = SSP1BUF;
    51.                 unsigned int bf = SSP1STATbits.BF;  // clear
    52.                 SSP1CON1bits.SSPOV = 0;
    53.              
    54.                 // Release the clock if strecting enabled
    55.                 if (SSP1CON2bits.SEN) SSP1CON1bits.CKP = 1;
    56.              
    57.                 // Clear interrup bit
    58.                 PIR1bits.SSP1IF = 0;
    59.              
    60.             } else if (BAUDCON1bits.ABDOVF){
    61.                 BAUDCON1bits.ABDOVF = 0;    // clear
    62.             } else {
    63.              
    64.                 if (!SSP1STATbits.RW){                  // Master is writing sometihng
    65.              
    66.                     if (!SSP1STATbits.DA){              // Last byte was the address
    67.  
    68.                         // Get address
    69.                         unsigned char addr = SSP1BUF;
    70.  
    71.                         // Some synchronous codes here...
    72.  
    73.                         // Release the clock if strecting enabled
    74.                         if (SSP1CON2bits.SEN) SSP1CON1bits.CKP = 1;
    75.                      
    76.                         // Clear interrup bit
    77.                         PIR1bits.SSP1IF = 0;
    78.                      
    79.                     } else if (i2cAddressMatched) {     // Otherwise data
    80.  
    81.                         // Get data
    82.                         unsigned char data = SSP1BUF;
    83.                      
    84.                         // Some synchronous codes here...
    85.  
    86.                         // Release the clock if strecting enabled
    87.                         if (SSP1CON2bits.SEN) SSP1CON1bits.CKP = 1;
    88.                      
    89.                         // Clear interrup bit
    90.                         PIR1bits.SSP1IF = 0;
    91.  
    92.                     }
    93.  
    94.                 } else if (SSP1STATbits.RW) {           // Master is trying to reading
    95.                  
    96.                     // Clear BF by reading
    97.                     unsigned int bf = SSP1STATbits.BF;
    98.                  
    99.                     // Process if any command suitable got before reading
    100.                     if (i2cCmd[0] && !i2cCmdProcessed){
    101.                      
    102.                         // Get address
    103.                         unsigned char addr = SSP1BUF;
    104.  
    105.             // Some synchronous codes here...
    106.  
    107.                         // Transmit data byte by byte from starting MSB
    108.                         for (int i = 0; i < I2C_DATA_BUFFER_LEN; i++){
    109.                             while(!PIR1bits.SSP1IF);
    110.                             // Clear BF by reading
    111.                             unsigned int bf = SSP1STATbits.BF;
    112.                             SSP1BUF = i2cData[i];
    113.                          
    114.                             // Release the clock if strecting enabled
    115.                             if (SSP1CON2bits.SEN) SSP1CON1bits.CKP = 1;
    116.                          
    117.                             PIR1bits.SSP1IF = 0;
    118.  
    119.                        // Some synchronous codes here...
    120.                         }
    121.                      
    122.                         i2cCmdProcessed = 1;
    123.                      
    124.                         SSP1BUF = '\0'; // EOF transmit
    125.                      
    126.                         // Release the clock if strecting enabled
    127.                         if (SSP1CON2bits.SEN) SSP1CON1bits.CKP = 1;
    128.  
    129.                         PIR1bits.SSP1IF = 0;
    130.  
    131.                     } else {
    132.                         SSP1BUF = '\0';
    133.                     }
    134.                  
    135.                     // delayMS(1); causing write error after reading
    136.                  
    137.                     // Release the clock if strecting enabled
    138.                     if (SSP1CON2bits.SEN) SSP1CON1bits.CKP = 1;
    139.                  
    140.                     // Clear interrup bit
    141.                     PIR1bits.SSP1IF = 0;
    142.  
    143.                 }
    144.              
    145.             }
    146.          
    147.         }
    148.      
    149.         if (!SSP1STATbits.S && SSP1STATbits.P){
    150.             i2cInProgress = 0;
    151.         }
    152.      
    153.     }
    154.  
    155.     return;
    156. }
    157.  
    158. void init() {
    159.  
    160.     // Ultra-low power on sleep
    161.     WDTCONbits.SRETEN = 0;
    162.  
    163.     // OSC
    164.     OSCCONbits.IRCF0 = 1;
    165.     OSCCONbits.IRCF1 = 1;
    166.     OSCCONbits.IRCF2 = 1;
    167.     OSCCONbits.OSTS = 1;
    168.     OSCCONbits.HFIOFS = 1;
    169.     OSCCONbits.SCS0 = 0;
    170.     OSCCONbits.SCS1 = 0;
    171.  
    172.     // Disable comparators
    173.     CM1CONbits.CON = 0;
    174.     CM2CONbits.CON = 0;
    175.     CM3CONbits.CON = 0;
    176.  
    177.     // ANALOG INPUTS
    178.  
    179.     // Enable all analog inputs
    180.     ANCON0 = 0xFF;
    181.     ANCON1 = 0xFF;
    182.     ANCON2 = 0xFF;
    183.  
    184.     // Set conversion as left justified and vref as AVREF+ and AVSS
    185.     ADCON1bits.VCFG = 0b00;
    186.     ADCON1bits.VNCFG = 0b0;
    187.     ADCON2bits.ADFM = 0;
    188.  
    189.     // Set acquisition time for minimum requirements (14 TAD, ~16 TOSC)
    190.     ADCON2bits.ADCS = 0b101;
    191.     ADCON2bits.ACQT = 0b010;
    192.  
    193.     // I2C CONFIGURATION
    194.  
    195.     // Set as slave (SSPM(3:0) = 0110) as interrupt enabled
    196.     SSP1CON1 = 0b00101110;
    197.  
    198.     // Set stretching enable
    199.     SSP1CON2bits.SEN = 1;
    200.  
    201.     // Configure Input Levels and slew rate as I2C Standard Levels (Slew Rate control (SMP) set for 100kHz)
    202.     SSP1STAT = 0b10000000;
    203.  
    204.     // Set i2c address to SLAVE_ADDR variable. Note that last bit of SSPADD is not used in 7 bit addressing mode
    205.     SSP1ADD = SLAVE_ADDR << 1;
    206.  
    207.     // PORTS
    208.  
    209.     // Set address switch input port
    210.     TRISB = 0b11111111;
    211.     PORTB = 0;
    212.     LATB = 0;
    213.  
    214.     // Set analog inputs ports. Note that sharp numbers shows input indices as starting from 1
    215.     TRISAbits.TRISA1 = 1;   // #1
    216.     PORTA = 0;
    217.     LATA = 0;
    218.  
    219.     TRISF = 0b11111111;     // #2, #7, #8, #9, #10, #11, #12
    220.     PORTF = 0;
    221.     LATF = 0;
    222.  
    223.     TRISG = 0b11111111;     // #13, #14, #15, #16
    224.     PORTG = 0;
    225.     LATG = 0;
    226.  
    227.     TRISH = 0b11111111;     // #3, #4, #5, #6, #17, #18, #19, #20
    228.     PORTH = 0;
    229.     LATH = 0;
    230.  
    231.     // Set digital output ports
    232.     TRISAbits.TRISA4 = 0;   // #7
    233.     TRISAbits.TRISA5 = 0;   // #8
    234.     PORTA = 0;
    235.     LATA = 0;
    236.  
    237.     TRISCbits.TRISC1 = 0;   // #6
    238.     TRISCbits.TRISC2 = 0;   // #4
    239.     TRISCbits.TRISC7 = 0;   // #5
    240.     PORTC = 0;
    241.     LATC = 0;
    242.  
    243.     TRISDbits.TRISD4 = 0;   // #9
    244.     TRISDbits.TRISD5 = 0;   // #10
    245.     TRISDbits.TRISD6 = 0;   // #11
    246.     TRISDbits.TRISD7 = 0;   // #12
    247.     PORTD = 0;
    248.     LATD = 0;
    249.  
    250.     TRISEbits.TRISE4 = 0;   // #20
    251.     TRISEbits.TRISE5 = 0;   // #19
    252.     TRISEbits.TRISE6 = 0;   // #18
    253.     TRISEbits.TRISE7 = 0;   // #17
    254.     PORTE = 0;
    255.     LATE = 0;
    256.  
    257.     TRISJ = 0b00000000;     // #1, #2, #3, #13, #14, #15, #16 and activity led pin
    258.     PORTJ = 0;
    259.     LATJ = 0;
    260. }
    261.  
    262. void delayUS(int us){
    263.     while(us-- >= 0){
    264.         __delay_us(1);
    265.     }
    266. }
    267.  
    268. void delayMS(int ms){
    269.     while(ms-- >= 0){
    270.         __delay_ms(1);
    271.     }
    272. }
    273.  
     
    Last edited: Jun 25, 2017
  4. Papabravo

    Expert

    Feb 24, 2006
    12,040
    2,590
    Three observations:
    1. You're using the && operator. You do realize this is probably the wrong thing to do with operands that are single bits -- don't you? Since the operands to && are .TRUE. and .FALSE. you don't know how the compiler will treat numerical quantities and bit quantities as it evaluates these expressions. Conside using the single & operator which does a bitwise .AND. operation. I'd have to look at the compiled code to find the problems.
    2. You're not using interrupts, and I think you should.
    3. The 500 msec delay is suspicious. Plenty of time for the RPi to muck things up while nobody is paying attention. What is that for? Is there some reason you can't be ready to roll when initialization is complete?
    What you want to do is get through your initialization as fast as possible and be ready to respond to any incoming data.
     
    Last edited: Jun 25, 2017
  5. zaferaltun

    Thread Starter Member

    Aug 31, 2016
    65
    1

    Hi again, the answers below, thanks for replying:

    1. I 'm thinking that I 'm already using interrupts, please see the init function in my code, there is a line like this:

    // Set as slave (SSPM(3:0) = 0110) as interrupt enable
    SSP1CON1 = 0b00101110;

    Is not it enough? Or shoudl I set another interrupt bit? I 'm not sure, I 'm not expert about it.

    2. The 500 msec delay is just a one-time delay as inital, it is outside of while loop as you can see, so I dont think it affects the protocol.
     
  6. Papabravo

    Expert

    Feb 24, 2006
    12,040
    2,590
    You are not using interrupts since you have not created an interrupt service routine. You have not configured the MSSP interrupt to be enabled and you have not set the global interrupt enable flag. Check bits PIR1<3> and PIR3<7> depending on which MSSP you are using.

    The 500 msec delay serves no useful function. See my other comment about the use of && to do bit comparisons.

    New question. Why are you enabling automatic baudrate detection for one of the asynchronous serial ports?

    You need to rewrite your code to avoid the && operators -- they being used incorrectly.

    I see now that you are polling the interrupt flag. Not sure that is a good idea.
     
    Last edited: Jun 25, 2017
  7. geekoftheweek

    Active Member

    Oct 6, 2013
    201
    26
    I'll agree with the use of && and &... it's bitten me before.

    I've never used interrupts with I2C in my projects. While it serves a purpose generaly you won't need them and it tends to make programming a bit more difficult. SSP1CON1 = 0b00100110 would work with the way your code is heading. You really don't need start and stop interrupts if you're doing basic I2C communication as it will only interrupt if the address matches and then after every byte until the stop condition. PIR1bits.SSP1IF will be set regardless of if you have interrupts enabled or not.

    I would suggest adding a way to detect if the I2C fails and keeps repeating the 'while(!PIR1bits.SSP1IF);' line. Normally the problems I have are because for some reason the next byte didn't get sent or read and it just keeps looping and waiting.
    If you decide not to use start and stop interrupts (I recommend if possible) you will need to add a test for the stop condition also in your while section otherwise it won't get detected.

    Reading the SSPSTAT doesn't clear the BF flag. BF gets set and cleared by the PIC itself when it's time. You can eliminate those lines.

    To save some instruction cycles you should eliminate the if (SSP1CON2bits.SEN) and just use SSP1CON1bits.CKP = 1. The SEN bit does not change unless you change it. Since you set it during initialization it will be set until you change it.

    You could also eliminate the test for overflow to simplify things until it works properly. In theory as long as you always handle the I2C when needed you won't have an overflow condition. Actually since you are using clock stretching it won't send or receive until you release the clock anyways so as long as you read SSPBUF everytime (when the address is received and after every byte received) you won't have an overflow.

    As Papa said the automatic baud rate has nothing to do with I2C.

    If you don't have it already I would suggest to blink and led or something outside of the I2C code just to make sure the PIC is still doing something. Normally when I2C fails the program will get stuck waiting for the interrupt flag to be set.

    I've found that it seems certain PICs I've messed with are particular about the order in which the clock is released, the interrupt flag is cleared, and data is written or read in SSPBUF. It seems like everything is in order, but check out the waveforms in the datasheet to confirm.

    When it fails check voltages on SCL and SDA. If one of them are low disconnect your slave device and see if it changes. That will give you an idea if your master or slave is the one with the issue.
     
    Last edited: Jun 25, 2017
    zaferaltun likes this.
  8. geekoftheweek

    Active Member

    Oct 6, 2013
    201
    26
    Just an idea:

    Code (Text):
    1.  
    2. // I2C initialization
    3.      SSP1CON1 = 0b00000110;  // slave mode 7 bit address
    4.     SSP1CON2bits.SEN = 1;  // enable clock stretching
    5.     SSP1STAT = 0b10000000;
    6.     SSP1ADD = SLAVE_ADDR << 1;
    7.     SSP1CON1bits.SSPEN =1; // save enable of module until everything set up
    8.  
    9.  
    10. // I2C slave polling
    11.   if (!PIR1bits.SSP1IF) return; // no address match has occured
    12.  
    13.   // Get address
    14.   unsigned char addr = SSP1BUF;
    15.  
    16.   if (SSP1STATbits.RW) { // master is reading
    17.       while (!SSP1STATbits.P) {
    18.           // move data to SSP1BUF
    19.           SSP1BUF = data;
    20.           // Release the clock
    21.           SSP1CON1bits.CKP = 1;
    22.           // Clear interrup bit
    23.          PIR1bits.SSP1IF = 0;
    24.  
    25.          while (!PIR1bits.SSP1IF) {
    26.               // increment counter or something here to check for infinite loop due to error
    27.          }
    28.      }
    29.   }
    30.  
    31.   else { // master is writing
    32.       while (!SSP1STATbits.P) {
    33.           // move data to SSP1BUF
    34.           data = SSP1BUF;
    35.           // Release the clock
    36.           SSP1CON1bits.CKP = 1;
    37.           // Clear interrup bit
    38.           PIR1bits.SSP1IF = 0;
    39.  
    40.           while (!PIR1bits.SSP1IF) {
    41.               // increment counter or something here to check for infinite loop due to error
    42.           }
    43.       }
    44.   }
    45.   return;
    46. }
    47.  
    Kind of a basic idea of what needs to happen at minimum.
     
    zaferaltun likes this.
  9. Papabravo

    Expert

    Feb 24, 2006
    12,040
    2,590
    Thanks for the contribution; you clearly have more experience with this specific part than I do. Most of my previous projects were an I2C master talking to a peripheral device, and interrupts were of limited usefulness. Same for an SPI Master, and a UART transmitter. When doing an I2C slave device it seems like responding to incoming data in an interrupt service routine is the thing to do -- IMHO.
     
  10. zaferaltun

    Thread Starter Member

    Aug 31, 2016
    65
    1
    I really thank both of you, especially thank you @geekoftheweek for very long explanation and sample code. I will check and try all thing both you said. I 'm suprised about && AND operator, interesting. Sometimes I 'm getting in doubt about the compiler strange behaviours, indeed.

    I will write here the results after I check the things you said. Thanks again.

    Best
     
  11. geekoftheweek

    Active Member

    Oct 6, 2013
    201
    26
    Best[/QUOTE]
    You're probably right in ways about using an interrupt service routine, but for what I've done so far it has always worked fine without it. If your slave device has a really long program loop it will hold up the master until the slave gets around to checking if something happened... an interrupt driven routine would probably be better in that instance. I program in asm so it gets ugly quickly trying to keep track of what happened last and what needs to happen next. It is possible to do and I do have a LCD controller program that uses interrupts for I2C.

    Thanks for the kind words. I went back and looked at what I put on here last night and realized I forgot something that will cause problems. You will also want to add test for a stop condition while waiting for an interrupt and add a goto or something to break the program out of the loop. I normally use asm so it didn't hit me until now.

    while ((!PIR1bits.SSP1IF) && (!SSP1STATbits.P)) would probably do the trick.
     
    Last edited: Jun 26, 2017
    zaferaltun likes this.
  12. zaferaltun

    Thread Starter Member

    Aug 31, 2016
    65
    1
    I tried all things both you said and now it is working, most of them like fine tuning. && AND operator is required, it did not work after I changed all && to &, it was surprising to me already, now I did not surprised. Simply, only disabling I2C interrupt setting is enough, you are right @geekoftheweek, it is working fine without interrupt already. The solution is hidden on this line:

    SSP1CON1 = 0b00100110;

    Thank you very much.
     
  13. geekoftheweek

    Active Member

    Oct 6, 2013
    201
    26
    Glad to hear you made it work.
     
    Last edited: Jun 28, 2017
Loading...