STM32 I2C Hal Driver issue HAL_I2C_Mem_Read and HAL_I2C_Master_Receive

There's an issue in the STM32 I2C Hal Driver, observed in HAL_I2C_Mem_Read but the same issue might exist in other read commands.

I observed the issue that sometimes the I2C master fails to acknowledge and send a stop condition, after which the SDA is stuck low and HAL_BUSY is returned on consecutive I2C reads. I can reproduce this issue while stressing I2C read (reading 7 values of an accelero sensor) in combination with running a httpd and refreshing the webpage in my browser every 5 seconds.

I have noticed that the issue occurs when I get in the case where only 2 remaining bytes are left

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

The problem is that ACK is not disabled in this case (CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_ACK); )

Typically, when reading multiple data, the MCU will read 1 byte at a time from hi2c->Instance->DR, and eventually get in the case where 3 bytes are remaining (where the behavior is correct). However, if "I2C_FLAG_BTF" is set, the MCU will read out an extra byte. If after that one, only 2 bytes are remaining, the ACK is never disabled.

As a solution, something like below works for me: (check the "//NEW" comment)

{
 /* Wait until RXNE flag is set */
 if (I2C_WaitOnRXNEFlagUntilTimeout(hi2c, Timeout, tickstart) != HAL_OK)
 {
 return HAL_ERROR;
 }
 
 /* Read data from DR */
 *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;
 
 /* Increment Buffer pointer */
 hi2c->pBuffPtr++;
 
 /* Update counter */
 hi2c->XferSize--;
 hi2c->XferCount--;
 
 if (__HAL_I2C_GET_FLAG(hi2c, I2C_FLAG_BTF) == SET)
 {
 if(hi2c->XferSize == 3U) {
 //NEW -> clear ACK (must be done before reading DR). After this read, only 2 bytes are remaining.
 CLEAR_BIT(hi2c->Instance->CR1, I2C_CR1_ACK);
 }
 /* Read data from DR */
 *hi2c->pBuffPtr = (uint8_t)hi2c->Instance->DR;
 
 /* Increment Buffer pointer */
 hi2c->pBuffPtr++;
 
 /* Update counter */
 hi2c->XferSize--;
 hi2c->XferCount--;
 }
 }
 }

However, I don't like this "I2C_FLAG_BTF" checking at all, there is no need for it since it doesn't really speed up the process and only (as you noticed) can create more bugs and more complex code. I think the function can work perfectly without the IF case but I might be wrong. Maybe it was added for another reason?

The same issue probably exists in the "HAL_I2C_Master_Receive" function, but I have not verified it. The code looks all the same.

I'm having even another doubt if this code (HAL_I2C_Mem_Read) is 100% correct, since I'm not sure if the "I2C_WaitOnFlagUntilTimeout(hi2c, I2C_FLAG_BTF, RESET, Timeout, tickstart)" (when only 2 bytes remaining) guarantees that effectively that exactly the last 2 bytes have been received. It seems possible to me that this condition could be raised also when the 2 first of the 3 last bytes have been received. But I might be wrong in that and it's harder to explain.