MSP430G2955: I2C Communication not working..

> unsigned char Slave_Address1 = 0x17;

My first guess is that your device/board is using address 0x16 rather than 0x17 [Ref BQ78350 TRM (SLUUAN7B) Sec 16.6]. Try instead:

> unsigned char Slave_Address1 = 0x16;

What platform (board set) are you using?

Hey, Thank you for the quick response! 

I did try it with 0x16 as well, but it still is not working as i want it to.

 What platform (board set) are you using?

I am using the MSP-TS430DA38 to communicate with the bq78350 chip through I2C.

What board is the BQ78350 mounted on? (I'm unfamiliar with the term "bms".) Of particular interest here is whether the I2C pullups are supplied, since (as far as I know) the TS430 boards don't supply them.

I should mention here that in this code the "nack" message really reports a timeout, which might or might not result from a NACK.

Hey Bruce. Sorry for the delayed response. 

BMS stands for a Battery Management System which is an in house battery management system board and the bq78350 is mounted onto this board. 

One question, are the pull up resistors necessary for the working I2C communication(I have no clue about the PCB Designing part)??

I should mention here that in this code the "nack" message really reports a timeout, which might or might not result from a NAC.

I did try with the logic analyzer and did a few changes to the original code. Could you please check that as well?

Also we have used a 100 ohms pull up resistors.

I will attach the necessary files here below.

Edited code:

#include <stdio.h>
#include <inttypes.h>
#include <msp430.h>
#include <driverlib.h>
#include <string.h>
#include "drv_i2c.h"
#include "drv_bq76930.h"

#define DELAY_LIMIT1 10
unsigned char *ptr_rx;
unsigned char RXByte[100];
unsigned char Slave_Address1 = 0x0B;

// COMMANDS

#define REMAININGCAP 0X01
#define REMAININGTIME 0X02
#define BATTERYMODE 0X03
#define ATRATE 0X04
#define ATRATETIMETOFULL 0X05
#define ATRATETIMETOEMPTY 0X06
#define ATRATEOK 0X07
#define TEMPERATURE 0X08
unsigned char VOLTAGE = 0x09;
unsigned char CURRENT = 0X0A;
#define AVGCURRENT 0X0B
#define MAXERROR 0X0C
#define RELATIVESOC OXOD
#define REMAININGCAPACITY 0X0E
#define FULLCHARGECAPACITY 0X10
#define RUNTIMETOEMPTY 0X11
#define AVGTTE 0X12
#define AVGTTF 0X13
#define CHARGINGCURRENT 0X14
#define CHARGINGVOLTAGE 0X15
#define BATTERYSTATUS 0X16
#define CYCLECOUNT 0X17
#define DESIGNCAPACITY 0X18
#define DESIGNVOLTAGE 0X19
#define SPECSINFO 0X1A
#define MFGDATA 0X1A
#define SOH 0X4F
#define SAFETYALERT 0X50
#define SAFETYSTATUS 0X51
#define PFALERT 0X52
#define PFSTATUS 0X52

typedef struct
{
int nI2C;
unsigned char nAddress;
} TI2C;

void i2c_init();
int I2CSendByte(unsigned char I2CSlaveAddress, unsigned char data);
int I2CSendBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ByteCount, unsigned int *SentByte);
int I2CWriteRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data);
int I2CWriteRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data);
int I2CWriteRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data);
int I2CWriteBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char StartAddress, unsigned char *Buffer, unsigned char Length);
int I2CWriteRegisterWord(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data);
int I2CReadBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ExpectedByteNumber, unsigned int *NumberOfReceivedBytes);
int I2CReadRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data);
int I2CReadBlock(unsigned char I2CSlaveAddress, unsigned char StartRegisterAddress, unsigned char *Buffer, unsigned int BlockSize, unsigned int *NumberOfBytes);
unsigned char CRC8(unsigned char *ptr, unsigned char len,unsigned char key);
int I2CReadRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data);
int I2CReadRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int *Data);
int I2CReadBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Buffer, unsigned char Length);

int main(void)
{
    printf("entered main \n");
    WDTCTL = WDTPW | WDTHOLD;   // stop watchdog timer
                            // Set DCO to 16MHz

        DCOCTL = 0;                         // Select lowest DCOx and MODx settings
        BCSCTL1 = CALBC1_16MHZ;             // Set DCO to 16MHz
        DCOCTL = CALDCO_16MHZ;

    i2c_init();

    unsigned int curr, voltage;
    unsigned int voltage2;

    printf(I2CReadRegisterWordWithCRC(Slave_Address1, VOLTAGE , &voltage));
        printf("voltage = %d \n",voltage);
   if(I2CReadRegisterWordWithCRC(Slave_Address1, VOLTAGE , &voltage2)<0)
    printf("I2C error \n");
    else
        printf("voltage = %c \n",voltage2);
    printf(I2CReadRegisterWordWithCRC(Slave_Address1, CURRENT, &curr));
    printf("current = %d \n",curr);
    return 0;
}

void i2c_init()
{
    printf("entering i2c init \n");

    P3SEL |= BIT1;
    P3SEL |= BIT2;
    P3SEL2 &= (~BIT1);
    P3SEL2 &= (~BIT2);

    UCB0CTL1 |= UCSWRST;       // SWRST SET


    UCB0CTL0 |= UCMODE_3;      // CHOOSE I2C MODE
    UCB0CTL0 |= UCMST;         //CHOOSE MASTER MODE
    UCB0CTL0 |= UCSYNC;        //enable synchronous mode

    UCB0CTL1 = UCSSEL_2 | UCSWRST;       //CHOOSE SMCLK

    UCB0BR0 = 100;                //CHOOSE PRESCALER VALUEwhere SMCLK =16MHz, 16000000/16 = 1Mhz
    UCB0BR1 = 0;

    //UCB0CTL1 |= UCTR;          //CHOOSE TRANSMITTER MODE
    UCB0I2CSA |= Slave_Address1;          //set slave address

    UCB0CTL1 &= ~UCSWRST;        //CLEAR SWRST
    UCB0I2CIE |= UCSTTIE;
    IE2 |= UCB0TXIE;
    IE2 |= UCB0RXIE;

    printf("i2c init done \n");
}

int I2CSendByte(unsigned char I2CSlaveAddress, unsigned char data)
{
    unsigned long int DelayCounter = 0;

    UCB0CTL0 |= UCMST;
    UCB0I2CSA = I2CSlaveAddress;

     UCB0CTL1 |= UCTR; //data in transmit direction
     UCB0CTL1 |= UCTXSTT; //Generate Start Condition
                          //Send Start Byte

     while(!(IFG2 & UCB0TXIFG))  //if UCB0TXIFG != 0, wait here
     {
         DelayCounter ++;
         if (DelayCounter >= DELAY_LIMIT)
             break;
     }

     if (DelayCounter >= DELAY_LIMIT)
     {
         return -1;
     }

     UCB0TXBUF = data;
     // send the data

     DelayCounter = 0;

     while(DelayCounter < DELAY_LIMIT && !(IFG2 & UCB0TXIFG))
     {
         DelayCounter++;
     }

     if (DelayCounter >= DELAY_LIMIT)
     {
         return -1;
     }

     UCB0CTL1 |= UCTXSTP;           //send stop bit

     DelayCounter = 0;

     while(DelayCounter < DELAY_LIMIT && (UCB0CTL1 & UCTXSTP))
     {
         DelayCounter++;
     }

     if (DelayCounter >= DELAY_LIMIT)
     {//check if NACK condition occurred
         return -1;

     }

     else
         return 0;

}

int I2CSendBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ByteCount, unsigned int *SentByte)
{
    unsigned long int DelayCounter = 0;
    unsigned int NumberOfBytesSent = 0;
    unsigned char *DataPointer;

    UCB0CTL0 |= UCMST;
    UCB0I2CSA = I2CSlaveAddress;
    IE2 |= UCB0TXIE;

     DataPointer = DataBuffer;
     UCB0CTL1 |= UCTR; //data in transmit direction
     UCB0CTL1 |= UCTXSTT; //Generate Start Condition
                          //Send Start Byte

     while(!(IFG2 & UCB0TXIFG))  //if UCTXSTT != 0, wait here
     {
         DelayCounter ++;
         if (DelayCounter > DELAY_LIMIT)
             break;
     }

     if (DelayCounter >= DELAY_LIMIT)   //check if NACK condition occurred
     {
         *SentByte = NumberOfBytesSent;
         printf(" number of bytes sent %d \n", NumberOfBytesSent);
         UCB0CTL1 |= UCTXSTP;
         return -1;
     }

     for(NumberOfBytesSent = 0; NumberOfBytesSent < ByteCount; NumberOfBytesSent++)
     {
         UCB0TXBUF= *DataPointer;
         printf(" data pointer %c \n", DataPointer);
         DelayCounter = 0;

         while(DelayCounter < DELAY_LIMIT && (!(IFG2 & UCB0TXIFG) || (UCB0CTL1 & UCTXSTT))) //check if the byte has been sent
         {
             DelayCounter++;
         }
         printf("byte sent %ld 1 \n",DelayCounter);
         if (DelayCounter >= DELAY_LIMIT)   //check if NACK condition occurred
         {
             printf("nack occured delay counter %ld \n",DelayCounter);

             *SentByte = NumberOfBytesSent;
             printf("Sent Byte %d \n", SentByte);
             UCB0CTL1 |= UCTXSTP;
             printf("nack occured \n");
             //send stop condition
             return -1;
         }

         DataPointer++;
     }

     IFG2 &= ~UCB0TXIFG;
     UCB0CTL1 |= UCTXSTP;       //send stop bit

     DelayCounter = 0;

     while(DelayCounter < DELAY_LIMIT && ((UCB0CTL1 & UCTXSTP)))
     {
         DelayCounter++;
     }

     *SentByte =  NumberOfBytesSent;

     if (DelayCounter >= DELAY_LIMIT)   //check if NACK condition occurred
     {
         UCB0CTL1 |= UCSWRST;
         return -1;
     }
     else
         return 0;
}

int I2CWriteRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data)
{
    unsigned char DataBuffer[2];
    unsigned int SentByte = 0;


    DataBuffer[0] = Register;
    DataBuffer[1] = Data;

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer, 2, &SentByte));
}

int I2CWriteRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data)
{
    unsigned char DataBuffer[4];
    unsigned int SentByte = 0;

    DataBuffer[0] = I2CSlaveAddress << 1;
    DataBuffer[1] = Register;
    DataBuffer[2] = Data;
    DataBuffer[3] = CRC8(DataBuffer, 3, CRC_KEY);

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer + 1, 3, &SentByte));
}

int I2CWriteRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data)
{
    unsigned char DataBuffer[6];
    unsigned int SentByte = 0;

    DataBuffer[0] = I2CSlaveAddress << 1;
    DataBuffer[1] = Register;
    DataBuffer[2] = LOW_BYTE(Data);
    DataBuffer[3] = CRC8(DataBuffer, 3, CRC_KEY);
    DataBuffer[4] = HIGH_BYTE(Data);
    DataBuffer[5] = CRC8(DataBuffer + 4, 1, CRC_KEY);

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer + 1, 5, &SentByte));
}

int I2CWriteBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char StartAddress, unsigned char *Buffer, unsigned char Length)
{
    unsigned char *BufferCRC, *Pointer;
    int i;
    unsigned int SentByte = 0;
    int result;

    BufferCRC = (unsigned char*)malloc(2*Length + 2);
    if (NULL == BufferCRC)
        return -1;

    Pointer = BufferCRC;
    *Pointer = I2CSlaveAddress << 1;
    Pointer++;
    *Pointer = StartAddress;
    Pointer++;
    *Pointer = *Buffer;
    Pointer++;
    *Pointer = CRC8(BufferCRC, 3, CRC_KEY);

    for(i = 1; i < Length; i++)
    {
        Pointer++;
        Buffer++;
        *Pointer = *Buffer;
        *(Pointer + 1) = CRC8(Pointer, 1, CRC_KEY);
        Pointer++;
    }

    result = I2CSendBytes(I2CSlaveAddress, BufferCRC + 1, 2*Length + 1, &SentByte);

    free(BufferCRC);
    BufferCRC = NULL;

    return result;
}

int I2CWriteRegisterWord(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data)
{
    unsigned char DataBuffer[3];
    unsigned int SentByte = 0;

    DataBuffer[0] = Register;
    DataBuffer[1] = LOWBYTE(Data);
    DataBuffer[2] = HIGHBYTE(Data);

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer, 3, &SentByte));
}

int I2CReadBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ExpectedByteNumber, unsigned int *NumberOfReceivedBytes)
{
    unsigned long int DelayCounter = 0;
    unsigned char *DataPointer;
    unsigned int *NumberOfReceivedBytesPointer;

    NumberOfReceivedBytesPointer = NumberOfReceivedBytes;
    *NumberOfReceivedBytesPointer = 0;

    UCB0CTL0 |= UCMST;
    DataPointer = DataBuffer;
    UCB0I2CSA = I2CSlaveAddress;

    UCB0CTL1 &= ~(UCTR); //data in receive direction

    UCB0CTL1 |= UCTXSTT; //Generate Start Condition

     while((UCB0CTL1 & UCTXSTT)
             )  //if UCTXSTT != 0, wait here
     {
         DelayCounter ++;
         if (DelayCounter >= DELAY_LIMIT)
             break;
     }

     if (DelayCounter >= DELAY_LIMIT || UCB0STAT & UCNACKIFG)   //check if NACK condition occurred
         return -1;

     for(*NumberOfReceivedBytesPointer = 0; *NumberOfReceivedBytesPointer < ExpectedByteNumber; (*NumberOfReceivedBytesPointer)++)
     {
         if(*NumberOfReceivedBytesPointer + 1 == ExpectedByteNumber)
             UCB0CTL1 |= UCTXSTP;

         DelayCounter = 0;

         while(DelayCounter < DELAY_LIMIT && !(IFG2 & UCB0RXIFG))
         {
             DelayCounter++;
         }

         if(DelayCounter == DELAY_LIMIT)
         {
             UCB0CTL1 |= UCSWRST;   //if I2C overtime condition occurred, reset I2C engine
             return -1;
         }

         *DataPointer = UCB0RXBUF;

         DataPointer++;
     }

     DelayCounter = 0;
     while(DelayCounter < DELAY_LIMIT && (UCB0CTL1 & UCTXSTP))
     {
         DelayCounter++;
     }

     if(DelayCounter >= DELAY_LIMIT)
     {
         UCB0CTL1 |= UCSWRST;
         return -1;
     }

     return 0;

}

int I2CReadRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned int ReadDataCount = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, Data, 1, &ReadDataCount);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }

    return 0;
}

int I2CReadBlock(unsigned char I2CSlaveAddress, unsigned char StartRegisterAddress, unsigned char *Buffer, unsigned int BlockSize, unsigned int *NumberOfBytes)
{
    unsigned char TargetRegister = StartRegisterAddress;
    unsigned int SentByte = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, Buffer, BlockSize, NumberOfBytes);

    if(ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }

    return 0;
}

unsigned char CRC8(unsigned char *ptr, unsigned char len,unsigned char key)
{
    unsigned char i;
    unsigned char crc=0;
    while(len--!=0)
    {
        for(i=0x80; i!=0; i/=2)
        {
            if((crc & 0x80) != 0)
            {
                crc *= 2;
                crc ^= key;
            }
            else
                crc *= 2;

            if((*ptr & i)!=0)
                crc ^= key;
        }
        ptr++;
    }
    return(crc);
}

int I2CReadRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char ReadData[2];
    unsigned int ReadDataCount = 0;
    unsigned char CRCInput[2];
    unsigned char CRC = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;
    printf("CRC Read \n");
    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1,  &SentByte);
    printf("write status %d \n",WriteStatus);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 2, &ReadDataCount);
    printf("read status %d \n", ReadStatus);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        printf("Reading error \n");
        return -1;
    }

    printf("readdata %c %c \n", ReadData[0], ReadData[1]);

    CRCInput[0] = (I2CSlaveAddress << 1) + 1;

    printf("crcinput0 %c \n", CRCInput[0]);

    CRCInput[1] = ReadData[0];
    printf("crcinput 1 %c \n",CRCInput[1]);

    CRC = CRC8(CRCInput, 2, CRC_KEY);
    printf("CRC = %c \n ", CRC);

    if (CRC != ReadData[1])
    {
        printf("CRC Error \n");
        return -1;

    }


    *Data = ReadData[0];
    return 0;
}

int I2CReadRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char ReadData[4];
    unsigned int ReadDataCount = 0;
    unsigned char CRCInput[2];
    unsigned char CRC = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 4, &ReadDataCount);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }

    CRCInput[0] = (I2CSlaveAddress << 1) + 1;
    CRCInput[1] = ReadData[0];

    CRC = CRC8(CRCInput, 2, CRC_KEY);

    if (CRC != ReadData[2])
        return -1;

    CRC = CRC8(ReadData + 2, 1, CRC_KEY);

    if (CRC != ReadData[3])
        return -1;

    *Data = ReadData[0];

    *Data = (*Data << 8) + ReadData[1];

    return 0;
}

int I2CReadBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Buffer, unsigned char Length)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char *ReadData = NULL, *StartData = NULL;
    unsigned int ReadDataCount = 0;
    unsigned char CRCInput[2];
    unsigned char CRC = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;
    int i;

    StartData = (unsigned char *)malloc(2 * Length);

    if (NULL == StartData)
        return -1;

    ReadData = StartData;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 2 * Length, &ReadDataCount);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        free(StartData);
        StartData = NULL;

        return -1;
    }

    CRCInput[0] = (I2CSlaveAddress << 1) + 1;
    CRCInput[1] = *ReadData;

    CRC = CRC8(CRCInput, 2, CRC_KEY);

    ReadData++;
    if (CRC != *ReadData)
    {
        free(StartData);
        StartData = NULL;
        return -1;
    }
    else
        *Buffer = *(ReadData - 1);

    for(i = 1; i < Length; i++)
    {
        ReadData++;
        CRC = CRC8(ReadData, 1, CRC_KEY);
        ReadData++;
        Buffer++;

        if (CRC != *ReadData)
        {
            free(StartData);
            StartData = NULL;

            return -1;
        }
        else
            *Buffer = *(ReadData - 1);
    }

    free(StartData);
    StartData = NULL;

    return 0;
}

Screenshots of the logic analyzer:

I don't understanding why I'm getting this:

Its this sequence that is repeated afterwards throughout the recording time:

1) What is the value of DELAY_LIMIT? I see DELAY_LIMIT1[=10], but that doesn't appear to be used. 

From your earlier console output, it looks like DELAY_LIMIT was =255, which is probably too aggressive. Each bit takes UCB0BR0/1 SMCLKs, so a byte  would take (minimum) (9*100)=900 SMCLKs. Software counting loops are notoriously difficult to calibrate, but with DELAY_LIMIT=255 that requires at least 4 CPU clocks per loop to avoid timing out prematurely.

Looking at your analyzer trace, it appears the I2C is actually running at more like 50kHz (rather than 160kHz), which would mean 20us*16*9=2880 CPU clocks/byte. Allowing 10 CPU clocks/loop, that implies a DELAY_LIMIT of at least 288. Timeouts are (presumed) rare, so I suggest you multiply this by at least *10 to add some cushion, so maybe DELAY_LIMIT=3000 would be reasonable.

2) It looks like, in the case of an Rx timeout (possibly premature), the code puts the I2C unit into reset (UCSWRST), and I don't see anything that un-does this. That might help explain the last 2 analyzer traces.

3) In main() there are two malformed printf(missing format string) calls -- the compiler even noticed them. I'm not quite sure what those calls are doing to you.

[Edit: (4) 100 Ohm is very strong for I2C pullups; 3-5KOhm is more typical. This probably isn't causing your current troubles.]

From your earlier console output, it looks like DELAY_LIMIT was =255

Yes its is set to 0xFF.

Looking at your analyzer trace, it appears the I2C is actually running at more like 50kHz (rather than 160kHz),

Yes it is running at around 50kHz. I am not understanding how to set the prescaler value. I tried various values for the UCBR0 bit, i did not get the required response for a set of values. could you help me out with setting up the prescaler value? I thought it would be according to the formula given below which i found in https://www.ti.com/lit/ug/slau144k/slau144k.pdf?ts=1710323770953&ref_url=https%253A%252F%252Fwww.ti.com%252Fproduct%252FMSP430G2955.

I suggest you multiply this by at least *10 to add some cushion, so maybe DELAY_LIMIT=3000 would be reasonable.

Ok will try with it once. 

In main() there are two malformed printf(missing format string) calls -- the compiler even noticed them. I'm not quite sure what those calls are doing to you

I will edit this accordingly. Thanks for rectifying the error.

Also the NACK Bit is occurring after the register address being sent. Could this be due to the previous errors in the code?

Are you fairly certain you're seeing a NACK and not a (premature) timeout? My thought is to get the other things under control, so you can be sure of what you're seeing. Running the I2C at 50kHz is fine for now, as long as you're conscious of it.

Could you suggest me some more changes that could be done in my code to make the situation better?

What have you changed so far, and what results are you getting now? I expect the timeout change will have an effect, even if it doesn't fix everything.

I don't have one of your devices, so you may need to experiment. It's possible the device has its own oddities.

Also, I noticed that the device has a bus-timeout mechanism (roughly 1 second?) so avoid stopping in the debugger in the middle of a transaction.

Hey Bruce, I have changed the delay limit value and some other changes are done. However we found out today that the IC what was used was faulty. So we did change it and now it is able to retrieve data and there are a few errors:

1. The first time we run the code, it works and the data retrieved is also correct and matches the multimeter value. For example we read voltage and the multimeter showed 24.196V and the logic analyzer showed 0x5EE8(24.29V). Then the next execution just doesn't work and gives a NACK bit in the first write to the slave itself. 

2. I am unable to come up with the code to retrieve the WORD and can just retrieve BYTE.

Could you please guide me with these two queries. 

I will attach the current code and the relevant logic analyzer screenshots int his thread. 

Thanks in advance! 

This is the most recent code:

#include <stdio.h>
#include <inttypes.h>
#include <msp430.h>
#include <driverlib.h>
#include <string.h>
#include "drv_i2c.h"
#include "drv_bq76930.h"

#define DELAYLIMIT 0x0BB8
unsigned char *ptr_rx;
unsigned char RXByte[100];
unsigned char Slave_Address1 = 0x0B;

// COMMANDS

void delay_cycles(int delay)
{
    int i=0;
    for(i=0;i<delay;i++)
    {

    }
}

#define REMAININGCAP 0X01
#define REMAININGTIME 0X02
#define BATTERYMODE 0X03
#define ATRATE 0X04
#define ATRATETIMETOFULL 0X05
#define ATRATETIMETOEMPTY 0X06
#define ATRATEOK 0X07
#define TEMPERATURE 0X08
unsigned char VOLTAGE = 0x09;
unsigned char CURRENT = 0X0A;
#define AVGCURRENT 0X0B
#define MAXERROR 0X0C
#define RELATIVESOC OXOD
#define REMAININGCAPACITY 0X0E
#define FULLCHARGECAPACITY 0X10
#define RUNTIMETOEMPTY 0X11
#define AVGTTE 0X12
#define AVGTTF 0X13
#define CHARGINGCURRENT 0X14
#define CHARGINGVOLTAGE 0X15
#define BATTERYSTATUS 0X16
#define CYCLECOUNT 0X17
#define DESIGNCAPACITY 0X18
#define DESIGNVOLTAGE 0X19
#define SPECSINFO 0X1A
#define MFGDATA 0X1A
#define SOH 0X4F
#define SAFETYALERT 0X50
#define SAFETYSTATUS 0X51
#define PFALERT 0X52
#define PFSTATUS 0X52

typedef struct
{
int nI2C;
unsigned char nAddress;
} TI2C;

void i2c_init();
int I2CSendByte(unsigned char I2CSlaveAddress, unsigned char data);
int I2CSendBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ByteCount, unsigned int *SentByte);
int I2CWriteRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data);
int I2CWriteRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data);
int I2CWriteRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data);
int I2CWriteBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char StartAddress, unsigned char *Buffer, unsigned char Length);
int I2CWriteRegisterWord(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data);
int I2CReadBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ExpectedByteNumber, unsigned int *NumberOfReceivedBytes);
int I2CReadRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data);
int I2CReadRegisterWord(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int *Data);
int I2CReadBlock(unsigned char I2CSlaveAddress, unsigned char StartRegisterAddress, unsigned char *Buffer, unsigned int BlockSize, unsigned int *NumberOfBytes);
unsigned char CRC8(unsigned char *ptr, unsigned char len,unsigned char key);
int I2CReadRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data);
int I2CReadRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int *Data);
int I2CReadBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Buffer, unsigned char Length);

int main(void)
{
    printf("entered main \n");
    WDTCTL = WDTPW | WDTHOLD;   // stop watchdog timer
                            // Set DCO to 16MHz

        DCOCTL = 0;                         // Select lowest DCOx and MODx settings
        BCSCTL1 = CALBC1_16MHZ;             // Set DCO to 16MHz
        DCOCTL = CALDCO_16MHZ;

    i2c_init();
    delay_cycles(1000);
    unsigned char voltage;
    unsigned int curr;

        I2CReadRegisterByte(Slave_Address1, VOLTAGE , &voltage);
        printf("voltage = %d \n", voltage);
        delay_cycles(1000);
        I2CReadRegisterWord(Slave_Address1, CURRENT, &curr);
        printf("Current = %d \n", curr);


    return 0;
}

void i2c_init()
{
    printf("entering i2c init \n");

    P3SEL |= BIT1;
    P3SEL |= BIT2;
    P3SEL2 &= ~(BIT1);
    P3SEL2 &= ~(BIT2);

    UCB0CTL1 |= UCSWRST;       // SWRST SET
    UCB0CTL0 = UCMST + UCMODE_3+ UCSYNC;         //CHOOSE MASTER MODE
    UCB0CTL1 = UCSSEL_2 + UCSWRST;       //CHOOSE SMCLK
    UCB0BR0 = 80;                //CHOOSE PRESCALER VALUEwhere SMCLK =16MHz, 16000000/16 = 1Mhz
    UCB0BR1 = 0;
    UCB0I2CSA |= Slave_Address1;          //set slave address

    UCB0CTL1 |= UCSSEL_2;
    UCB0CTL1 &= ~UCSWRST;        //CLEAR SWRST
    UCB0I2CIE |= UCSTTIE;
    IE2 |= UCB0TXIE;
    IE2 |= UCB0RXIE;
    UCB0I2CIE |= UCNACKIE;

    printf("i2c init done \n");
}

int I2CSendByte(unsigned char I2CSlaveAddress, unsigned char data)
{
    unsigned long int DelayCounter = 0;

    UCB0CTL0 |= UCMST;
    UCB0I2CSA = I2CSlaveAddress;

     UCB0CTL1 |= UCTR; //data in transmit direction
     UCB0CTL1 |= UCTXSTT; //Generate Start Condition
                          //Send Start Byte

     while(!(IFG2 & UCB0TXIFG))  //if UCB0TXIFG != 0, wait here
     {
         DelayCounter ++;
         if (DelayCounter >= DELAYLIMIT)
             break;
     }

     if (DelayCounter >= DELAYLIMIT)
     {
         return -1;
     }

     UCB0TXBUF = data;
     // send the data

     DelayCounter = 0;

     while(DelayCounter < DELAYLIMIT && !(IFG2 & UCB0TXIFG))
     {
         DelayCounter++;
     }

     if (DelayCounter >= DELAYLIMIT)
     {
         return -1;
     }

     UCB0CTL1 |= UCTXSTP;           //send stop bit

     DelayCounter = 0;

     while(DelayCounter < DELAYLIMIT && (UCB0CTL1 & UCTXSTP))
     {
         DelayCounter++;
     }

     if (DelayCounter >= DELAYLIMIT)
     {//check if NACK condition occurred
         return -1;

     }

     else
         return 0;

}

int I2CSendBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ByteCount, unsigned int *SentByte)
{
    unsigned long int DelayCounter = 0;
    unsigned int NumberOfBytesSent = 0;
    unsigned char *DataPointer;

    UCB0CTL0 |= UCMST;
    UCB0I2CSA = I2CSlaveAddress;
    IE2 |= UCB0TXIE;

     DataPointer = DataBuffer;
     UCB0CTL1 |= UCTR; //data in transmit direction
     UCB0CTL1 |= UCTXSTT; //Generate Start Condition
                          //Send Start Byte

     while(!(IFG2 & UCB0TXIFG))  //if UCTXSTT != 0, wait here
     {
         DelayCounter ++;
         if (DelayCounter > DELAYLIMIT)
             break;
     }

     if (DelayCounter >= DELAYLIMIT)   //check if NACK condition occurred
     {
         *SentByte = NumberOfBytesSent;
         printf(" number of bytes sent %d \n", NumberOfBytesSent);
         UCB0CTL1 |= UCTXSTP;
         return -1;
     }

     for(NumberOfBytesSent = 0; NumberOfBytesSent < ByteCount; NumberOfBytesSent++)
     {
         UCB0TXBUF= *DataPointer;
         DelayCounter = 0;

         while(DelayCounter < DELAYLIMIT && (!(IFG2 & UCB0TXIFG) || (UCB0CTL1 & UCTXSTT))) //check if the byte has been sent
         {
             DelayCounter++;
         }

         if (DelayCounter >= DELAYLIMIT)   //check if NACK condition occurred
         {
             printf("nack occured delay counter %ld \n",DelayCounter);

             *SentByte = NumberOfBytesSent;
             printf("Sent Byte %d \n", SentByte);
             UCB0CTL1 |= UCTXSTP;
             printf("nack occured \n");
             //send stop condition
             return -1;
         }

         DataPointer++;
     }

     IFG2 &= ~UCB0TXIFG;
     UCB0CTL1 |= UCTXSTP;       //send stop bit

     DelayCounter = 0;

     while(DelayCounter < DELAYLIMIT && ((UCB0CTL1 & UCTXSTP)))
     {
         DelayCounter++;
     }

     *SentByte =  NumberOfBytesSent;

     if (DelayCounter >= DELAYLIMIT)   //check if NACK condition occurred
     {
         UCB0CTL1 |= UCSWRST;
         return -1;
     }
     else
         return 0;
}

int I2CWriteRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data)
{
    unsigned char DataBuffer[2];
    unsigned int SentByte = 0;


    DataBuffer[0] = Register;
    DataBuffer[1] = Data;

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer, 2, &SentByte));
}

int I2CWriteRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char Data)
{
    unsigned char DataBuffer[4];
    unsigned int SentByte = 0;

    DataBuffer[0] = I2CSlaveAddress << 1;
    DataBuffer[1] = Register;
    DataBuffer[2] = Data;
    DataBuffer[3] = CRC8(DataBuffer, 3, CRC_KEY);

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer + 1, 3, &SentByte));
}

int I2CWriteRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data)
{
    unsigned char DataBuffer[6];
    unsigned int SentByte = 0;

    DataBuffer[0] = I2CSlaveAddress << 1;
    DataBuffer[1] = Register;
    DataBuffer[2] = LOW_BYTE(Data);
    DataBuffer[3] = CRC8(DataBuffer, 3, CRC_KEY);
    DataBuffer[4] = HIGH_BYTE(Data);
    DataBuffer[5] = CRC8(DataBuffer + 4, 1, CRC_KEY);

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer + 1, 5, &SentByte));
}

int I2CWriteBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char StartAddress, unsigned char *Buffer, unsigned char Length)
{
    unsigned char *BufferCRC, *Pointer;
    int i;
    unsigned int SentByte = 0;
    int result;

    BufferCRC = (unsigned char*)malloc(2*Length + 2);
    if (NULL == BufferCRC)
        return -1;

    Pointer = BufferCRC;
    *Pointer = I2CSlaveAddress << 1;
    Pointer++;
    *Pointer = StartAddress;
    Pointer++;
    *Pointer = *Buffer;
    Pointer++;
    *Pointer = CRC8(BufferCRC, 3, CRC_KEY);

    for(i = 1; i < Length; i++)
    {
        Pointer++;
        Buffer++;
        *Pointer = *Buffer;
        *(Pointer + 1) = CRC8(Pointer, 1, CRC_KEY);
        Pointer++;
    }

    result = I2CSendBytes(I2CSlaveAddress, BufferCRC + 1, 2*Length + 1, &SentByte);

    free(BufferCRC);
    BufferCRC = NULL;

    return result;
}

int I2CWriteRegisterWord(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int Data)
{
    unsigned char DataBuffer[3];
    unsigned int SentByte = 0;

    DataBuffer[0] = Register;
    DataBuffer[1] = LOWBYTE(Data);
    DataBuffer[2] = HIGHBYTE(Data);

    return(I2CSendBytes(I2CSlaveAddress, DataBuffer, 3, &SentByte));
}

int I2CReadBytes(unsigned char I2CSlaveAddress, unsigned char *DataBuffer, unsigned int ExpectedByteNumber, unsigned int *NumberOfReceivedBytes)
{
    unsigned long int DelayCounter = 0;
    unsigned char *DataPointer;
    unsigned int *NumberOfReceivedBytesPointer;

    NumberOfReceivedBytesPointer = NumberOfReceivedBytes;
    *NumberOfReceivedBytesPointer = 0;

    UCB0CTL0 |= UCMST;
    DataPointer = DataBuffer;
    UCB0I2CSA = I2CSlaveAddress;

    UCB0CTL1 &= ~(UCTR); //data in receive direction

    UCB0CTL1 |= UCTXSTT; //Generate Start Condition

     while((UCB0CTL1 & UCTXSTT)
             )  //if UCTXSTT != 0, wait here
     {
         DelayCounter ++;
         if (DelayCounter >= DELAYLIMIT)
             break;
     }

     if (DelayCounter >= DELAYLIMIT || UCB0STAT & UCNACKIFG)   //check if NACK condition occurred
         return -1;

     for(*NumberOfReceivedBytesPointer = 0; *NumberOfReceivedBytesPointer < ExpectedByteNumber; (*NumberOfReceivedBytesPointer)++)
     {
         if(*NumberOfReceivedBytesPointer + 1 == ExpectedByteNumber)
             UCB0CTL1 |= UCTXSTP;

         DelayCounter = 0;

         while(DelayCounter < DELAYLIMIT && !(IFG2 & UCB0RXIFG))
         {
             DelayCounter++;
         }

         if(DelayCounter == DELAYLIMIT)
         {
             UCB0CTL1 |= UCSWRST;   //if I2C overtime condition occurred, reset I2C engine
             return -1;
         }

         *DataPointer = UCB0RXBUF;

         DataPointer++;
     }

     DelayCounter = 0;
     while(DelayCounter < DELAYLIMIT && (UCB0CTL1 & UCTXSTP))
     {
         DelayCounter++;
     }

     if(DelayCounter >= DELAYLIMIT)
     {
         UCB0CTL1 |= UCSWRST;
         return -1;
     }

     return 0;

}

int I2CReadRegisterByte(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned int ReadDataCount = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, Data, 1, &ReadDataCount);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }

    return 0;
}

int I2CReadRegisterWord(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char ReadData[2];
    unsigned int ReadDataCount = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;
    delay_cycles(1000);
    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);
    delay_cycles(1000);
    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 2, &ReadDataCount);
    delay_cycles(1000);
    if (ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }


    *Data = ReadData[1];

    *Data = (*Data << 8) + ReadData[0];

    return 0;
}

int I2CReadBlock(unsigned char I2CSlaveAddress, unsigned char StartRegisterAddress, unsigned char *Buffer, unsigned int BlockSize, unsigned int *NumberOfBytes)
{
    unsigned char TargetRegister = StartRegisterAddress;
    unsigned int SentByte = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, Buffer, BlockSize, NumberOfBytes);

    if(ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }

    return 0;
}

unsigned char CRC8(unsigned char *ptr, unsigned char len,unsigned char key)
{
    unsigned char i;
    unsigned char crc=0;
    while(len--!=0)
    {
        for(i=0x80; i!=0; i/=2)
        {
            if((crc & 0x80) != 0)
            {
                crc *= 2;
                crc ^= key;
            }
            else
                crc *= 2;

            if((*ptr & i)!=0)
                crc ^= key;
        }
        ptr++;
    }
    return(crc);
}

int I2CReadRegisterByteWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char ReadData[2];
    unsigned int ReadDataCount = 0;
    unsigned char CRCInput[2];
    unsigned char CRC = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;
    printf("CRC Read \n");
    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1,  &SentByte);
    printf("write status %d \n",WriteStatus);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 2, &ReadDataCount);
    printf("read status %d \n", ReadStatus);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        printf("Reading error \n");
        return -1;
    }

    printf("readdata %c %c \n", ReadData[0], ReadData[1]);

    CRCInput[0] = (I2CSlaveAddress << 1) + 1;

    printf("crcinput0 %c \n", CRCInput[0]);

    CRCInput[1] = ReadData[0];
    printf("crcinput 1 %c \n",CRCInput[1]);

    CRC = CRC8(CRCInput, 2, CRC_KEY);
    printf("CRC = %c \n ", CRC);

    if (CRC != ReadData[1])
    {
        printf("CRC Error \n");
        return -1;

    }


    *Data = ReadData[0];
    return 0;
}

int I2CReadRegisterWordWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned int *Data)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char ReadData[4];
    unsigned int ReadDataCount = 0;
    unsigned char CRCInput[2];
    unsigned char CRC = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;
    delay_cycles(1000);
    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);
    delay_cycles(1000);
    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 4, &ReadDataCount);
    delay_cycles(1000);
    if (ReadStatus != 0 || WriteStatus != 0)
    {
        return -1;
    }

    CRCInput[0] = (I2CSlaveAddress << 1) + 1;
    CRCInput[1] = ReadData[0];

    CRC = CRC8(CRCInput, 2, CRC_KEY);

    if (CRC != ReadData[2])
        return -1;

    CRC = CRC8(ReadData + 2, 1, CRC_KEY);

    if (CRC != ReadData[3])
        return -1;

    *Data = ReadData[0];

    *Data = (*Data << 8) + ReadData[1];

    return 0;
}

int I2CReadBlockWithCRC(unsigned char I2CSlaveAddress, unsigned char Register, unsigned char *Buffer, unsigned char Length)
{
    unsigned char TargetRegister = Register;
    unsigned int SentByte = 0;
    unsigned char *ReadData = NULL, *StartData = NULL;
    unsigned int ReadDataCount = 0;
    unsigned char CRCInput[2];
    unsigned char CRC = 0;
    int ReadStatus = 0;
    int WriteStatus = 0;
    int i;

    StartData = (unsigned char *)malloc(2 * Length);

    if (NULL == StartData)
        return -1;

    ReadData = StartData;

    WriteStatus = I2CSendBytes(I2CSlaveAddress, &TargetRegister, 1, &SentByte);

    ReadStatus = I2CReadBytes(I2CSlaveAddress, ReadData, 2 * Length, &ReadDataCount);

    if (ReadStatus != 0 || WriteStatus != 0)
    {
        free(StartData);
        StartData = NULL;

        return -1;
    }

    CRCInput[0] = (I2CSlaveAddress << 1) + 1;
    CRCInput[1] = *ReadData;

    CRC = CRC8(CRCInput, 2, CRC_KEY);

    ReadData++;
    if (CRC != *ReadData)
    {
        free(StartData);
        StartData = NULL;
        return -1;
    }
    else
        *Buffer = *(ReadData - 1);

    for(i = 1; i < Length; i++)
    {
        ReadData++;
        CRC = CRC8(ReadData, 1, CRC_KEY);
        ReadData++;
        Buffer++;

        if (CRC != *ReadData)
        {
            free(StartData);
            StartData = NULL;

            return -1;
        }
        else
            *Buffer = *(ReadData - 1);
    }

    free(StartData);
    StartData = NULL;

    return 0;
}

Screenshots:

1. After the first execution

2. First execution: I do not have a picture of the console for this one.

What I see in the second trace is:

a) A successful write(0x09) followed by a successful read of 4 bytes, which isn't what your code appears to do.

b) Another successful write(0x09) followed by a read which gets a NACK. Here again, I don't see in your code where the second sequence originates. I also don't know why the device would NACK the read request, but once it does the code doesn't issue a Stop, and things seem to fail after that.

Unsolicited: This code sets a number of interrupt enables (UCB0RXIE, e,g.) but I don't see an ISR. I also don't see anything that sets GIE, so I expect it all works by accident. If you ever enable global interrupts (GIE=1, __enable_interrupt()) your program will malfunction, so I suggest you remove the I2C interrupt enables.