search

STMicroelectronics / STM32CubeH7

STM32Cube MCU Full Package for the STM32H7 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
https://www.st.com/en/embedded-software/stm32cubeh7.html
Other
498 stars 305 forks source link

A bug in the HAL_I2C_IsDeviceReady function in version 1.11.3 #289

Closed engycz closed 2 months ago

engycz commented 4 months ago

I have found a bug in the HAL_I2C_IsDeviceReady function in version 1.11.3 fec141c.

The problem is in the function HAL_I2C_IsDeviceReady on Line 3332. There is a call to the I2C_WaitOnFlagUntilTimeout function . There is a new test for I2C_IsErrorOccurred and if so (the device is not ready and sent NACK), I2C_IsErrorOccurred waits for STOP and returns HAL_ERROR to I2C_WaitOnFlagUntilTimeout. I2C_WaitOnFlagUntilTimeout returns HAL_ERROR to HAL_I2C_IsDeviceReady and it immediately returns HAL_ERROR to the upcoming function without considering the number of Trials from the function parameter.

How To Reproduce

  1. Write a data block into the EEPROM using the HAL_I2C_Mem_Write function
  2. Call HAL_I2C_IsDeviceReady with the Trials parameter set to 1000
  3. HAL_I2C_IsDeviceReady return value is HAL_ERROR, only one busy test is performed

Version 1.11.2 - multiple attempts to test if the device is ready (not busy) obrazek

obrazek

Version 1.11.3 - only one attempt testing if the device is ready (not busy) obrazek

KRASTM commented 4 months ago

Hello @engycz,

Thank you for the report.

KRASTM commented 4 months ago

ST Internal Reference: 181377

KRASTM commented 2 months ago

Fixed in commit 946eb47

degermann commented 1 month ago

The change that's checked in does not seem to make sense with respect to the new local variable "status". "Status" is set in several places, but it is not returned and it does not seem to affect the logic. The only use of "status" that I can see is in the comparison on line 3370, where it is compared to HAL_ERROR and reset to HAL_OK if the current number of trials is less than the maximum number of trials. But this doesn't affect anything because "status" isn't used anywhere else. Am I missing something?