STMicroelectronics / stm32l4xx-hal-driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32L4 series.
BSD 3-Clause "New" or "Revised" License
37 stars 15 forks source link

HAL_I2C_IsDeviceReady broken since v1.13.5 #20

Closed didi1357 closed 2 months ago

didi1357 commented 3 months ago

Describe the set-up

Describe the bug Commit 0844cb8 introduces a check in I2C_WaitOnFlagUntilTimeout which in turn breaks timeout/wait functionality of HAL_I2C_IsDeviceReady. The problem was initially reported for H7 family and already fixed there.

Maybe consider a common library to overcome such issues in future?

How To Reproduce

  1. Indicate the global behavior of your application project

I tried to compile our code one time against v1.13.4 and afterwards against v1.13.5. Writing to an EEPROM I2C IC after the upgrade to v1.13.5 fails after the first page. We test if the EEPROM is ready using HAL_I2C_IsDeviceReady and since it is writing the first page at that point in time, it reports that it isn't ready yet when I try to send the second one. Unfortunately the library does not retry or wait the timeout we supplied using the parameters.

  1. The modules that you suspect to be the cause of the problem (Driver, BSP, MW ...)

HAL_I2C

  1. The use case that generates the problem

An I2C device which reports that it is not ready yet.

  1. How we can reproduce the problem

Take an evaluation board, connect an I2C EEPROM and test if HAL_I2C_IsDeviceReady can be used to wait for the EEPROM to finish writing using the trials/timeout parameters.

Additional context

Screenshots

KRASTM commented 3 months ago

Hello @didi1357,

Thank you for the report. As you said, it's detected on H7 and fixed, also internally fixed on L4. It's a matter of updating on L4.

With regards,

KRASTM commented 3 months ago

ST Internal Reference: 181377

degermann commented 2 months ago

The change that's checked in for the H7 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?

RJMSTM commented 2 months ago

Fixed in commit : d98b0721c4ab37985758d19a849471ed50b8594d