This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

CCS/MSP430F2112: I2C Reads sometimes return 0x00

Part Number: MSP430F2112

Tool/software: Code Composer Studio

Hi All,

I'm writing an interrupt driven MSP430 I2C Slave that features a realt-time clock and battery monitor for logging purposes. It seems to work "most of the time" but occasionally when I'm reading large bursts of data some registers return zero even though they have valid data. I'm running main loop from DCO at 1MHz. If I increase DCO speed to 8, 12 or 16MHz I always get zeros....???

I've implemented a bunch of registers that can be read or written over I2C.

At the same time, I'm also using LFXTAL to drive a timer to generate an accurate 0.5s tick from a connected 32.768kHz xtal.

Any ideas what might be going on?

Code below.

Cheers,

Mike.

#include <msp430.h>

#define ON    1
#define OFF    0
#define TRUE    1
#define FALSE    0


typedef unsigned char REG_SUBADDR_TYPE;
typedef unsigned char bool;

#define REG_STATUS            (REG_SUBADDR_TYPE) 0x00
#define REG_CONTROL1        (REG_SUBADDR_TYPE) 0x01
#define REG_CONTROL2        (REG_SUBADDR_TYPE) 0x02
#define REG_BATTERY_HIGH    (REG_SUBADDR_TYPE) 0x03
#define REG_BATTERY_LOW        (REG_SUBADDR_TYPE) 0x04
#define REG_SW_VERSION        (REG_SUBADDR_TYPE) 0x05
#define REG_YEAR            (REG_SUBADDR_TYPE) 0x06
#define REG_MONTH            (REG_SUBADDR_TYPE) 0x07
#define REG_DATE            (REG_SUBADDR_TYPE) 0x08
#define REG_HOUR            (REG_SUBADDR_TYPE) 0x09
#define REG_MINUTE            (REG_SUBADDR_TYPE) 0x0A
#define REG_SECOND            (REG_SUBADDR_TYPE) 0x0B
#define REG_ALARM_YEAR        (REG_SUBADDR_TYPE) 0x0C
#define REG_ALARM_MONTH        (REG_SUBADDR_TYPE) 0x0D
#define REG_ALARM_DATE        (REG_SUBADDR_TYPE) 0x0E
#define REG_ALARM_HOUR        (REG_SUBADDR_TYPE) 0x0F
#define REG_ALARM_MINUTE    (REG_SUBADDR_TYPE) 0x10
#define REG_ALARM_SECONDS    (REG_SUBADDR_TYPE) 0x1A


typedef enum
{
    IIC_SLAVE_IDLE,
    IIC_SLAVE_RXED_SLAVE_ADDR,
    IIC_SLAVE_SLAVE_TX_BLOCK,
    IIC_SLAVE_SLAVE_RX_BLOCK
} iic_slave_state_type;

static iic_slave_state_type    iic_slave_state;    //I2C State
REG_SUBADDR_TYPE             iic_reg_address;    // Current register pointer


static unsigned char    battery_level_high = 0x00;
static unsigned char    battery_level_low = 0x00;

static unsigned int    reg_year = 17;
static unsigned int    reg_month = 2;
static unsigned int    reg_date = 28;
static unsigned int    reg_hours = 15;
static unsigned int    reg_minutes = 30;
static unsigned int    reg_seconds = 30;

static unsigned int    reg_alarm_year = 17;
static unsigned int    reg_alarm_month = 2;
static unsigned int    reg_alarm_date = 28;
static unsigned int    reg_alarm_hours = 16;
static unsigned int    reg_alarm_minutes = 30;
static unsigned int    reg_alarm_seconds = 30;


// Declare function prototypes
void delay ( unsigned int );
void Update_Half_Seconds(void);
void Update_Clock_24hr(void);
void Update_Display(void);
void Update_Mode(void);
void Check_Alarm(void);
void Check_Battery(void);
int REG_read_register(REG_SUBADDR_TYPE);
int REG_write_register(REG_SUBADDR_TYPE, int);
REG_SUBADDR_TYPE REG_inc_register(REG_SUBADDR_TYPE);


int main(void)
{
    WDTCTL = WDTPW | WDTHOLD;            // Stop watchdog timer

    if (CALBC1_1MHZ ==0xFF || CALDCO_1MHZ == 0xFF)
    {
      while(1);                               // Halt if calibration constants erased

    }
    //1Mhz
    BCSCTL1 = CALBC1_1MHZ;              // Set range
    DCOCTL = CALDCO_1MHZ;               // Set DCO step + modulation */

    //BCSCTL2 |= DIVS1 + DIVS0;            // SMCLK /8 divider
    BCSCTL3 = 0x0C;                        // 32.768kHz, 12.5pF Load Caps

    P3SEL |= 0x06;                        // Assign I2C pins to USCI_B0
    UCB0CTL1 |= UCSWRST;    // Hold USCI Logic in Reset State, SMCLK
    UCB0CTL0 = UCMODE_3 + UCSYNC;        // USCI=I2C, Synchronous mode
    UCB0I2COA = 0x18;                    // Own Address is 0x18
    UCB0CTL1 &= ~UCSWRST;                // Release USCI Logic Reset
    UCB0I2CIE |= UCSTPIE + UCSTTIE;        // Enable STT and STP interrupt
    IE2 |= UCB0RXIE + UCB0TXIE;            // Enable RX & TX interrupts
    iic_slave_state = IIC_SLAVE_IDLE;    // initialise I2C State Machine

    P1DIR = 0x3D;
    P1OUT = 0x00;

    P2DIR = 0x20;                        //P2.5 as OP
    P2IE |= 0x18;                        // P2.3, P2.4 Interrupt enabled
    P2IES |= 0x18;                        // Hi/lo edge
    P2IFG &= ~0x18;                        // IFG cleared
    P2OUT |= BIT5;                        //DEBUG - Switch on 5V_AUX_EN to enable DC-DC

    P3DIR |= 0xF9;
    P3OUT = 0x00;

    TA1CTL = TASSEL_1 + MC_1;            // ACLK (32.678kHz), upmode
    TA1CCR0 = 0x3FFF;                    // 3FFF equates to 0.5s at 32.768kHz
    TA1CCTL0 = CCIE;                    // TACCR0 interrupt enabled

    __enable_interrupt();
    //__bis_SR_register(CPUOFF + GIE);        // Enter LPM0 w/ interrupts

    while(1);
}


//------------------------------------------------------------------------------
// I2C Bus State-Change Interrupts are handled here
// Tx IFG is cleared when UCB0TXBUF is loaded with a byte
// Rx IFG is cleared upon reading UCB0RXBUF
//------------------------------------------------------------------------------
#pragma vector = USCIAB0TX_VECTOR
__interrupt void USCIAB0TX_ISR(void)
{
  if (IFG2 & UCB0RXIFG)    // Master is writing to us
  {
      if (iic_slave_state == IIC_SLAVE_IDLE)            //If we were idle will be register host wants to read or write
      {
          iic_reg_address = UCB0RXBUF;                    //Will be Sub-Address
          iic_slave_state = IIC_SLAVE_RXED_SLAVE_ADDR;    //Move to next state
      }
      else if (iic_slave_state == IIC_SLAVE_RXED_SLAVE_ADDR)
      {
          REG_write_register(iic_reg_address, UCB0RXBUF);
          iic_reg_address = (REG_SUBADDR_TYPE) REG_inc_register(iic_reg_address);     //Calculate next address
      }
  }
  else    //Else, Master wants to read data
  {
      UCB0TXBUF = REG_read_register(iic_reg_address);                            //Load the contents of previously set sub-address
      iic_reg_address = (REG_SUBADDR_TYPE) REG_inc_register(iic_reg_address);     //Calculate next address
  }
}


//------------------------------------------------------------------------------
// I2C Bus State-Change Interrupts are handled here
//------------------------------------------------------------------------------
#pragma vector = USCIAB0RX_VECTOR
__interrupt void USCIAB0RX_ISR(void)
{
    if (UCB0STAT & UCSTPIFG) //If STOP INT
    {
        iic_slave_state = IIC_SLAVE_IDLE;
        iic_reg_address = 0x00;
        UCB0STAT &= ~UCSTPIFG;
    }

    else if (UCB0STAT & UCSTTIFG) //If START INT
    {
        UCB0STAT &= ~UCSTTIFG;
    }
}



void delay(unsigned int ms)
{
 while (ms--)
    {
        __delay_cycles(1000); // set for 16Mhz change it to 1000 for 1 Mhz
    }
}



// Timer1_A0 interrupt service routine
#pragma vector=TIMER1_A0_VECTOR
__interrupt void Timer1_A0 (void)
{
  Update_Half_Seconds();
}



/*This function should get called every 1/512th of a second from an interupt*/
/*All functions inside this interupt should be completed within 1ms and therefore interupts are not disabled!*/
void Update_Half_Seconds(void)
{
    static char half_seconds = 0;

    P3OUT ^= BIT6;    //DEBUG: Flash Green LED

    half_seconds++;

    if (half_seconds>=2)
    {
        Update_Clock_24hr();
        half_seconds=0;
    }
}



/*This is the only function where the actual real-time is updated*/
/*This function should be called once every second*/
void Update_Clock_24hr(void)
{
    reg_seconds++;
    Check_Alarm();    //Check for alarm if bit set

    if (reg_seconds >= 60)
    {
        reg_seconds =0;        //Reset reg_seconds

        reg_minutes++;

        if (reg_minutes >= 60)
        {
            reg_minutes = 0;
            reg_hours++;
        }
        if (reg_hours >= 24)
        {
            reg_hours = 0;
        }

        Check_Battery();    //Battery voltage is updated every minute
    }
}


/*This function is called every time the reg_minutes counter is updated
 *
 */
void Check_Battery(void)
{
    static long ADC_Temp = 0.0;

    ADC10CTL1 = INCH_1;                                              //CH1
    ADC10CTL0 = SREF_1 + ADC10SHT_2 + REFON + REF2_5V + ADC10ON;    //VREF, 16 ADC CLK, 2.5V REF ON,
    P3OUT|= BIT3;                                                    //Switch on battery ADC path
    delay(5);                                                        //Settling time
    ADC10CTL0 |= ENC + ADC10SC;                                     //Sampling and conversion start
    while (ADC10CTL1 & BUSY);                                        //Wait for ADC10 to finish
    ADC_Temp = ADC10MEM;                                            //Assigns the value held in ADC10MEM
    P3OUT &= ~BIT3;                                                    //Switch off battery ADC path

    battery_level_low = (ADC_Temp & 0x00FF);        //Store Lower Bits
    battery_level_high = (ADC_Temp >> 8);            //Shift right and store Upper bits

    if (ADC_Temp >= 302)                                 //4.2V
    {
        ;
    }
    else if ( (ADC_Temp >= 259) && (ADC_Temp <= 301 ) ) //3.6V-->4.2V
    {
        ;
    }
    else if ( (ADC_Temp >= 223) && (ADC_Temp <= 258) ) //3.1V-->3.6V
    {
        ;
    }
    else if ( ADC_Temp <= 216)                            //<3.1V
    {
        ;
    }
}



/*This function is called every time the reg_minutes counter is updated
 *
 */
void Check_Alarm(void)
{
    if ( (reg_hours == reg_alarm_hours) && (reg_minutes == reg_alarm_minutes) &&
          (reg_seconds == reg_alarm_seconds))
    {
        ;
    }
}


/*I2C READ Register Function
 *
 */
int REG_read_register(REG_SUBADDR_TYPE    subaddr)
{
    switch (subaddr)
    {
        case REG_BATTERY_LOW:
            return battery_level_low;

        case REG_BATTERY_HIGH:
            return battery_level_high;

        case REG_SW_VERSION:
            return 0x01;

        case REG_YEAR:
            return reg_year;

        case REG_MONTH:
            return reg_month;

        case REG_DATE:
            return reg_date;

        case REG_HOUR:
            return reg_hours;

        case REG_MINUTE:
            return reg_minutes;

        case REG_SECOND:
            return reg_seconds;

        case REG_ALARM_YEAR:
            return reg_alarm_year;

        case REG_ALARM_MONTH:
            return reg_alarm_month;

        case REG_ALARM_DATE:
            return reg_alarm_date;

        case REG_ALARM_HOUR:
            return reg_alarm_hours;

        case REG_ALARM_MINUTE:
            return reg_alarm_minutes;

        case REG_ALARM_SECONDS:
            return reg_alarm_seconds;

        default:
            return 0;
    }
}


/* I2C WRITE Register Function
 *
 */
int REG_write_register(REG_SUBADDR_TYPE subaddr, int value)
{
    switch (subaddr)
    {
        case REG_BATTERY_LOW:
            return FALSE;

        case REG_BATTERY_HIGH:
            return FALSE;

        case REG_SW_VERSION:
            return FALSE;

        case REG_YEAR:
            reg_year = value;
            return TRUE;

        case REG_MONTH:
            reg_month = value;
            return TRUE;

        case REG_DATE:
            reg_date = value;
            return TRUE;

        case REG_HOUR:
            reg_hours = value;
            return TRUE;

        case REG_MINUTE:
            reg_minutes = value;
            return TRUE;

        case REG_SECOND:
            reg_seconds = value;
            return TRUE;

        case REG_ALARM_YEAR:
            reg_alarm_year = value;
            return TRUE;

        case REG_ALARM_MONTH:
            reg_alarm_month = value;
            return TRUE;

        case REG_ALARM_DATE:
            reg_alarm_date = value;
            return TRUE;

        case REG_ALARM_HOUR:
            reg_alarm_hours = value;
            return TRUE;

        case REG_ALARM_MINUTE:
            reg_alarm_minutes = value;
            return TRUE;

        case REG_ALARM_SECONDS:
            reg_alarm_seconds = value;
            return TRUE;

        default:
            return FALSE;
    }
}


/* Determine next I2C register in a group
 *
 */
REG_SUBADDR_TYPE REG_inc_register(REG_SUBADDR_TYPE subaddr)
{
    switch(subaddr)
    {
        case 0x00: return(REG_SUBADDR_TYPE)(0x03);  // Reserved
        case 0x01: return(REG_SUBADDR_TYPE)(0x03);  // Reserved
        case 0x02: return(REG_SUBADDR_TYPE)(0x03);  // Reserved

        case 0x03: return(REG_SUBADDR_TYPE)(0x04);  // Battery High
        case 0x04: return(REG_SUBADDR_TYPE)(0x03);  // Battery Low

        case 0x05: return(REG_SUBADDR_TYPE)(0x05);  // FW Version

        case 0x06: return(REG_SUBADDR_TYPE)(0x07);  // Year
        case 0x07: return(REG_SUBADDR_TYPE)(0x08);  // Month
        case 0x08: return(REG_SUBADDR_TYPE)(0x09);  // Date
        case 0x09: return(REG_SUBADDR_TYPE)(0x0A);  // Hour
        case 0x0A: return(REG_SUBADDR_TYPE)(0x0B);  // Minute
        case 0x0B: return(REG_SUBADDR_TYPE)(0x06);  // Second

        case 0x0C: return(REG_SUBADDR_TYPE)(0x0D);  // Alarm_Year
        case 0x0D: return(REG_SUBADDR_TYPE)(0x0E);  // Alarm_Month
        case 0x0E: return(REG_SUBADDR_TYPE)(0x0F);  // Alarm_Date
        case 0x0F: return(REG_SUBADDR_TYPE)(0x10);     // Alarm_Hour
        case 0x10: return(REG_SUBADDR_TYPE)(0x11);     // Alarm_Minute
        case 0x11: return(REG_SUBADDR_TYPE)(0x0C);     // Alarm_Second
     }
     return(REG_SUBADDR_TYPE)(subaddr);  /* Else we return the same register! */
}



  • Hi Mike,

    A quick look over your code and there is nothing glaring I can see. Can you be sure that you are receiving a valid register address from the master? If you are not, it would make sense you get 0, since in REG_read_register if it falls through the switch statement, then it always returns 0. Maybe as a quick test change that to return 0xbeef and see if you get that instead. That would at least point you in the direction the iic_reg_address not having the correct value for some reason.
  • Hi Chris - many thanks for the feedback. I tried a value of 0xAA as a default return value but still getting the 0x00 returned. I'm performing the I2C read testing with a basic Python script on a Pi 3 which has it's I2C running at 100kHz. Waveforms all look good on the scope (the pull-ups are pretty low on each board combining to around 3k3).

    I've been testing it with discrete read / write operations for each register, for example : -
    year = i2cBus.read_byte_data(MSP430_ADDR, REG_ALARM_YEAR)

    However, if I perform a block read of all registers at once on the Pi it works all of the time (I ran a test of 5000 block reads and all were OK): -
    data_buffer = i2cbus.read_i2c_block_data(MSP430_ADDR, REG_STATUS, 18)

    I believe the I2C block read uses a repeated start, so I think I need to look at how well I'm handling the STOP interrupt next... :-)

    Many thanks for your help.

    Cheers,
    Mike.
  • Hi Chris - Many thanks for taling the time to look over my code.
    I tried your suggestion and I can see that it never gets the default value (I set it to 0xAA), which has led me to run some similar tests and I'm looking into the handling of the STOP interrupt as I've found that if I read all the registers at once using a block read it works 100%. I think the 0x00 may be creeping in at my handling of the STOP interrupt as I set iic_reg_address = 0x00; at that point. The block read uses a repeated start, so only 1 stop interrupt is generated, whereas discrete reads will always generate a stop.
    Cheers,
    Mike.
  • Yeah I think you got it figured out. Resetting the register address when you receive a STOP command will definitely result in bad behaviour. I don't think you need to reset the register address at all in fact. Its not uncommon for a slave device to track the address across multiple transactions - for example an EEPROM. You might read a specific address, but then there is often a read current, which tracks the last accessed memory location even across transactions. The master will just do a read with no address (so 1 byte) and the EEPROM will just return the next byte of data. I think you could just remove that all together, and just make sure to initialise the register to a valid address at the beginning. If the pointer gets to the end of the valid address range, you could either have it wrap back to the beginning (not my favourite solution) or just NACK the request. Good luck!