AnsweredAssumed Answered

[BUG] STM32Cube_FW_F4_V1.14.0 HAL_I2C_Mem_Read: handling of last 2 bytes done incorrectly

Question asked by Zotow.Adam on Jan 10, 2017
Latest reply on Aug 22, 2017 by john noyb

Hi,

 

I didn't find any report posted about this issue so here we are.

 

There is a stupid bug in the implementation of HAL_I2C_Mem_Read()@STM32Cube_FW_F4_V1.14.0/Drivers/STM32F4xx_HAL_Driver/Src/stm32f4xx_hal_i2c.c. The "Size" parameter is supposed to be used only to initialize the transfer size, but it is also misused during the transfer to check the number of bytes left for reading (see the marked line in the code below). In result if in the last loop iteration there are 2 bytes left, code block expecting 3 bytes on the bus could be executed and the function hangs (till the timeout event, not shown here).

HAL_StatusTypeDef HAL_I2C_Mem_Read(I2C_HandleTypeDef *hi2c, uint16_t DevAddress,
uint16_t MemAddress, uint16_t MemAddSize, uint8_t *pData, uint16_t Size,
uint32_t Timeout)
{
    /*....*/

    /* Prepare transfer parameters */
    hi2c->pBuffPtr    = pData;
    hi2c->XferCount   = Size;
    hi2c->XferOptions = I2C_NO_OPTION_FRAME;
    hi2c->XferSize    = hi2c->XferCount;

    /*....*/

    while(hi2c->XferSize > 0U)
    {
      if(hi2c->XferSize <= 3U)
      {
        /* One byte */
        if(hi2c->XferSize== 1U)
        {
          /*....*/

          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
        /* Two bytes */
        else if(Size == 2U) /* <----------- CHECK THIS LINE ----------------- */
        {
          /*....*/

          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;

          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
        /* 3 Last bytes */
        else
        {
          /*....*/

          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;

          /*....*/

          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;

          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
      }
      else
      {
        /*....*/

        /* Read data from DR */
        (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
        hi2c->XferSize--;
        hi2c->XferCount--;

        if(__HAL_I2C_GET_FLAG(hi2c, I2C_FLAG_BTF) == SET)
        {
          /* Read data from DR */
          (*hi2c->pBuffPtr++) = hi2c->Instance->DR;
          hi2c->XferSize--;
          hi2c->XferCount--;
        }
      }
    }

    /*....*/
}

To fix the problem the marked line:

        /* Two bytes */
        else if(Size == 2U)

should be changed to:

        /* Two bytes */
        else if(hi2c->XferSize == 2U)

 

Cheers!

A

Outcomes