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/MSP430F5529: I2C Master Read - First byte read is garbage (Rest of data is read properly)

Part Number: MSP430F5529

Tool/software: Code Composer Studio

I am writing a generic, polling-based I2C master RX/TX driver on the MSP430F5529LP evaluation board (Default Clock Rate). I have an Adafruit metro-mini (Arduino based) board functioning as a slave transmitter sending "Hello" when I request a read.

My write function works properly, but when I attempt to read, I experience strange behavior. The first byte read from RXBUF is garbage, however the the entire message is received in consecutive reads. If I step through the code, the first byte is no longer garbage, and I receive the entire message properly. This makes me think I am not checking for some IFG/CTL flag to be set properly after the start condition, and as a result have a timing issue. The baffling thing is if I read the RXBUF prior to issuing the start condition, all bytes are read correctly.  I didn't see anything obvious in the User guide to make me think this is correct (Seems more like a Hack) so I am hoping you guys see my issue. 

i2c::i2c(I2C_Type port, uint8_t clkDiv)
{
	switch((int16_t)port)
	{
		case USCIB0:

			UCxxCTL1 =	&UCB0CTL1;
			UCxxCTL0 = &UCB0CTL0;
			UCxxBR0  = &UCB0BR0;
			UCxxBR1  = &UCB0BR1;
			UCxxI2CSA = &UCB0I2CSA;
			UCxxIFG   = &UCB0IFG;
			UCxxRXBUF = &UCB0RXBUF;
			UCxxTXBUF = &UCB0TXBUF;

			break;
		case USCIB1:

			UCxxCTL1  = &UCB1CTL1;
			UCxxCTL0  = &UCB1CTL0;
			UCxxBR0   = &UCB1BR0;
			UCxxBR1   = &UCB1BR1;
			UCxxI2CSA = &UCB1I2CSA;
			UCxxIFG   = &UCB1IFG;
			UCxxRXBUF = &UCB1RXBUF;
			UCxxTXBUF = &UCB1TXBUF;

			break;

		default:
			return;
	}

	pinSetup();

	*UCxxCTL1 |= UCSWRST;                      // Enable SW reset
	*UCxxCTL0 = UCMST + UCMODE_3 + UCSYNC;     // I2C Master, synchronous mode
	*UCxxCTL1 = UCSSEL_2 + UCSWRST;            // Use SMCLK, keep SW reset
//	*UCxxI2CSA = 0x04;
	*UCxxBR0  = clkDiv;                        // Apply Clock divider to SMCLK
	*UCxxBR1  = 0;
	*UCxxCTL1 &= ~UCSWRST;                     // Clear SW reset, resume operation

	*UCxxIE   = 0;
	*UCxxIFG  = 0;                             // Clear interrupt flags

}

bool i2c::start(bool isRead)
{
	// Set the I2C r/w bit, Send start condition
	if(isRead == false)
	{
		*UCxxCTL1 |= UCTR|UCTXSTT;
	}
	else
	{
		*UCxxCTL1 |= UCTXSTT;
		*UCxxCTL1 &= ~UCTR;
	}

	return !((*UCxxIFG & UCNACKIFG) == UCNACKIFG);
}

bool i2c::stop(void)
{
	// Wait for the Stop condition to be accepted
	uint16_t count = 5000;

	*UCxxCTL1 |= UCTXSTP;

	while ((*UCxxCTL1 & UCTXSTP)==UCTXSTP)
	{
		count--;
		if(count <= 0)
			return false;
	}

	return true;
}

uint8_t i2c::read(uint8_t address, uint8_t* data, uint8_t length, bool repeated)
{
	// Holds number of bytes read
	uint8_t br = 0;

	 // Assign the Slave Address
	*UCxxI2CSA = address;

	{
		// A junk byte is read if I don't clear the RXBUF
		uint8_t junk = *UCxxRXBUF;
	}

	// Issue Start Request
	if(start(true) == false)
	{
		// Regardless of repeated status, issue stop and terminate
		stop();
		return br;
	}

	for(uint8_t idx = 0; idx < length; idx++)
	{
		int16_t counts=10;

		// Wait for previous rx to complete.
		while (!(*UCxxIFG & UCRXIFG))
		{
			_delay_cycles(100);
			counts--;
			if(counts <= 0)
			{
				// Regardless of repeated status, issue stop and terminate
				stop();
				return 0;
			}
		}

		data[br++] = *UCxxRXBUF;
	}

	// Calling function will issue a repeated start, don't issue a STOP
	if(repeated == false)
		stop();

	return br;
}

  • Have you tried waiting for the start condition to finish sending (while(*UCxxCTL1 & UCTXSTT)==UCTXSTT);) before continuing operation?

    Regards,
    Ryan
  • Thanks for the suggestion. I will give this a try and let  you know  how it turns out.

  • While I feel like this line is good to have (I will keep it in my start function), it didn't solve my problem. I guess this isn't surprising because in my source code the hack of reading the RXBUF occurred before a start condition. 

  • It appears that the RXIFG is already set before starting the I2C sequence, this could be due to a previous sequence where the UCTXSTP bit was not set after receiving the second to last data byte (refer to code example MSP430F55xx_uscib0_i2c_10.c). At any rate there is nothing wrong in continuing with the workaround/hack you've already implemented. You could try using oscilloscope or logic analyzer screenshots of the I2C lines to further understand the issue.

    Regards,
    Ryan
  • Thanks for the response. It looks like there is a flaw in my understanding of how I2C works. I have been sending the STOP bit after receiving the last byte of information, when it should have been the second to last byte. Correct me if I'm wrong, but the following code:

    uint8_t i2c::read(uint8_t address, uint8_t* data, uint8_t length, bool repeated)
    {
    	// Holds number of bytes read
    	uint8_t br = 0;
    
    	 // Assign the Slave Address
    	*UCxxI2CSA = address;
    
    	// A junk byte is read before actual data if RXBUF isn't cleared. (Hack)
    	uint8_t junk = *UCxxRXBUF;
    
    	// Issue Start Request
    	if(start(true) == false)
    	{
    		// Regardless of repeated status, issue stop and terminate
    		stop();
    		return br;
    	}
    
    	for(uint8_t idx = 0; idx < length; idx++)
    	{
    		int16_t counts=10;
    
    		// Wait for previous rx to complete.
    		while (!(*UCxxIFG & UCRXIFG))
    		{
    			_delay_cycles(100);
    			counts--;
    			if(counts <= 0)
    			{
    				// Regardless of repeated status, issue stop and terminate
    				stop();
    				return 0;
    			}
    		}
    
    		data[br++] = *UCxxRXBUF;
    	}
    
    	// Calling function will issue a repeated start, don't issue a STOP
    	if(repeated == false)
    		stop();
    
    	return br;
    }

    Should look more like this:

    uint8_t i2c::read(uint8_t address, uint8_t* data, uint8_t length, bool repeated)
    {
    	// Holds number of bytes read
    	uint8_t br = 0;
    
    	 // Assign the Slave Address
    	*UCxxI2CSA = address;
    
    	// A junk byte is read before actual data if RXBUF isn't cleared. (Hack)
    	uint8_t junk = *UCxxRXBUF;
    
    	// Issue Start Request
    	if(start(true) == false)
    	{
    		// Regardless of repeated status, issue stop and terminate
    		stop();
    		return br;
    	}
    
    	for(uint8_t idx = 0; idx < length; idx++)
    	{
    		int16_t counts=10;
    
    		// Wait for previous rx to complete.
    		while (!(*UCxxIFG & UCRXIFG))
    		{
    			_delay_cycles(100);
    			counts--;
    			if(counts <= 0)
    			{
    				// Regardless of repeated status, issue stop and terminate
    				stop();
    				return 0;
    			}
    		}
    
    		data[br++] = *UCxxRXBUF;
    
                    // Check for last byte (length - 2 accounts for 0 index offset and second to last byte)
                    if(idx == length - 2)
    	              *UCxxCTL1 |= UCTXSTP;
    	}
    
    	// Calling function will issue a repeated start, don't issue a STOP
    	if(repeated == false)
    		stop();      // Checks that Stop condition has been successfully issued
    
    	return br;
    }

  • Yes, please try using this code instead and see if it resolves the issue.

    Regards,
    Ryan
  • Ryan, this fixed my issue with the extra garbage byte. Thanks for the input.
  • Just a disclaimer for anyone that plans on dangerously copying this into their project: There are a lot of things I haven't ironed out, especially for things like combining write+read operations. Also the boundary condition where read = 1 will not work. Finally, delay_cycles shouldn't be used for a timeout because adjusting the clock settings will result in a variable amount of time before timeouts occur.


    Just a warning!

**Attention** This is a public forum