MSP430FR2433: UCBBUSY set while communicating with two sensors through I2C

Can you tell which transaction is failing? (Is it always the same one?)

Hello!

I tested two more times, and yes, it is in the same transaction (request to read from CCS811_ALG_RESULT_DATA - the register that stores CO2 and TVOC data); it displaying the same condition too: UCBBUSY set and START bit pending.

I also checked that some times the referred register have no data, so I wonder if the configuration or the circuitry is the main cause.

I'm guessing it's the CCS811 that's stretching the clock -- while it is possible for your code to inadvertently hang the bus, those accidents usually show up in the first few transactions. (Stretching the clock for a "long time" is called "hanging the bus", i.e. it's the same phenomenon on the bus). Neither the datasheet nor AN000369 mention clock stretching, but 5-minutes-with-Google tells me that the CCS811 does it under some circumstances.

I wonder if the sensor is stretching the clock waiting for new data; I have seen this done in another sensor somewhere. There's nothing in the datasheet (nor App Note) that says you can't read register 0x02 if there's no new data -- they even include a copy of the STATUS register so you can read-then-check -- but every example I found only reads register 0x02 if DATA_READY=1. Since you've requested one reading per second, a quick experiment would be to extend your loop (delay at the bottom) from 100ms to 1000ms. Slightly more elaborate: enable then probe the nINT pin along with the bus.

Are you using a commercial breakout board? I notice that the Adafruit board ties nWAKE low, but I don't know about others. You don't want it to float in any case.

One thing that I don't like to do with I2C code is allow for an infinite wait. Whenever I have something like "while(UCB0CTL1 & UCTXSTP);" I include a timeout. For debugging purposes I also tend to return a unique code for each error as that makes locating trouble easier.

If the problem is a hanging start condition at the beginning of the read, I would first check to see that the previous write (where you set the register number to read) finished correctly. Also, the usual way to do this is as a single transaction with a repeated start.

I added a verification code (accessing CCS811_STATUS) before check the data register (CCS811_ALG_RESULT_DATA) and the error persists.

I'll take a look at this. 

About the nWAKE pin, it needs to be grounded. Strangely, I was getting NACKs when reading the CCS address; it came back to work letting RST and INT unconnected. 

I recoded the ISR to implement the repeated START:

#if defined(__TI_COMPILER_VERSION__) || defined(__IAR_SYSTEMS_ICC__)
#pragma vector = USCI_B0_VECTOR
__interrupt void USCIB0_ISR(void)
#elif defined(__GNUC__)
void __attribute__ ((interrupt(USCI_B0_VECTOR))) USCIB0_ISR (void)
#else
#error Compiler not supported!
#endif
{
  switch(__even_in_range(UCB0IV, USCI_I2C_UCBIT9IFG))
  {
    case USCI_I2C_UCRXIFG0:
            if(RX_ByteCtr)
            {
                RX_Data[RX_ByteCtr--] = UCB0RXBUF;  // Get received byte
                UCB0IFG |= UCRXIFG0;
            }
            else
            {
                UCB0CTLW0 |= UCTXSTP;
                UCB0IFG &= ~UCRXIFG0;
                __bic_SR_register_on_exit(CPUOFF);      // Exit LPM0
            }
        break;
    case USCI_I2C_UCTXIFG0:
                if(TX_ByteCtr)
                {
                    TX_Data[TX_ByteCtr--] = UCB0TXBUF;  // Get received byte
                    UCB0IFG |= UCTXIFG0;
                }
                else
                {
                    UCB0CTLW0 |= UCTXSTP;
                    UCB0IFG &= ~UCTXIFG0;
                    __bic_SR_register_on_exit(CPUOFF);      // Exit LPM0
                }
        break;
  }
}

It's worse, because I cannot make a communication, dropping SCL and SDA in the first I2C comm attempt (setting MPU).

Usually you write something to TXBUF in response to a transmit interrupt.

The receive interrupt does redundant and strange things. Redundant because UCRXIFG0 is cleared by reading UCB0IV and UCB0RXBUF so there is no need it clear it explicitly.

Strange in that there is an easier way to deal with the stop condition when reading data. When the ISR begins, the I2C hardware is waiting and one of two things can happen. You can read RXBUF and it will begin receiving the next byte. Or you can set TXSTP and it will generate a stop. As written, it receives an extra byte that it just ignores.

In the TX part I made a huge mistake, too stressed.

That's the new version:

#if defined(__TI_COMPILER_VERSION__) || defined(__IAR_SYSTEMS_ICC__)
#pragma vector = USCI_B0_VECTOR
__interrupt void USCIB0_ISR(void)
#elif defined(__GNUC__)
void __attribute__ ((interrupt(USCI_B0_VECTOR))) USCIB0_ISR (void)
#else
#error Compiler not supported!
#endif
{
  switch(__even_in_range(UCB0IV, USCI_I2C_UCBIT9IFG))
  {
      case USCI_I2C_UCRXIFG0:
          RX_Data[--RX_ByteCtr] = UCB0RXBUF;  // Get received byte
          if(!RX_ByteCtr)
          {
              if(restart)
              {
                  UCB0CTL1 |= UCTXSTT;
                  while(UCB0CTL1 & UCTXSTT);          // Ensure START sent

                  restart = 0;
              }
              else
              {
                  UCB0CTL1 |= UCTXSTP;
                  while(UCB0CTL1 & UCTXSTP);          // Ensure STOP sent
              }
              __bic_SR_register_on_exit(CPUOFF);  // Exit LPM0
          }
          break;
      case USCI_I2C_UCTXIFG0:
          UCB0TXBUF = TX_Data[--TX_ByteCtr];  // Passes byte to transmit buffer
          if(!TX_ByteCtr)
          {
              if(restart)
              {
                  UCB0CTL1 |= UCTXSTT;
                  while(UCB0CTL1 & UCTXSTT);          // Ensure START sent

                  restart = 0;
              }
              else
              {
                  UCB0CTL1 |= UCTXSTP;
                  while(UCB0CTL1 & UCTXSTP);          // Ensure STOP sent
              }
              __bic_SR_register_on_exit(CPUOFF);  // Exit LPM0
          }
          break;
  }
}

The hanging bus issue was resolved! Now the other problem is that its readings are wrong.

Seems like the counter is being messed up.

In the following block of code, 3 writes should be performed.

slaveAddress = CCS811_ADDR;         // Make transition from boot to application mode
TX_Data[0] = CCS811_APP_START;
TX_ByteCtr = 1;
restart = 1;
i2c_wr(slaveAddress);
__delay_cycles(22000);
TX_Data[1] = CCS811_MEAS_MODE;
TX_Data[0] = 0x10;                  // Put CCS to normal mode, no interrupt enable
TX_ByteCtr = 2;
i2c_wr(slaveAddress);

But this is being done:

So much wrong I don't think I can cover it all.

First, when doing a restart, you have to switch the I2C port from write to read. (Clear UCTR) Otherwise it sends the device address with the write bit set. Again.

Example:

  IFG2 &= ~UCB0RXIFG;         // make sure no previous interrupt
  UCB0CTL1 &= ~UCTR;          // restart as receiver

  UCB0CTL1 |= UCTXSTT;        // I2C repeated start condition
  IE2 |= UCB0RXIE;                // enable receive interrupt

(For a G series part so details are different.)

Not sure why the receive code checks the restart flag. You never start a receive and switch to transmit.

That command sending APP_START seems wrong. You usually have to send a register address/ value pair. But looking at the device data sheet, that seems correct. Sort of. Have you loaded an application? Did you read the status register to check that as recommended? Why did you set the restart flag? That will cause no end of trouble. (Think about what value your byte counter will have on the next transmit interrupt.)

Hello!

Well, I had many issues with the TX interrupt, triggering in wrong time (before enter low power mode) and not triggering when it should; this caused some bytes to be read more than expected.

I switched back to using automatic STOP, and now the communication seems more stable. So I'll keep it this way for a while.

Regarding APP_START, you're right, it's not ecessary. When I first coded the gas sensor interface, I thought it was necessary to boot it up.