Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HAL_I2C_IsDeviceReady() NACK Bug #15

Open
bencowperthwaite opened this issue Nov 1, 2024 · 1 comment
Open

HAL_I2C_IsDeviceReady() NACK Bug #15

bencowperthwaite opened this issue Nov 1, 2024 · 1 comment
Assignees
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request. i2c I2C-related issue or pull-request spotted before customer Spotted and fixed internally before being pointed out by users but not published yet

Comments

@bencowperthwaite
Copy link

bencowperthwaite commented Nov 1, 2024

Changes introduced in V1.5 of stm32u5xx_hal_i2c.c now mean that HAL_I2C_IsDeviceReady() will always return a HAL_ERROR immediately if a slave NACKs, without any repeated trials or a timeout.

After the START is generated (line #3480), a subsequent NACK will cause execution to drop out of the while loop and the intention is then to wait until the STOPF flag is reset (line #3535). Due to the if statement at #3513 we know that the NACK flag is definitely set when entering I2C_WaitOnFlagUntilTimeout().

A change to I2C_WaitOnFlagUntilTimeout for v1.5 at line #7292 now means that I2C_WaitOnFlagUntilTimeout() will call I2C_IsErrorOccurred(), which returns HAL_ERROR if the NACK Flag is set. We already know that the NACKF flag is set, that's why we've called I2C_WaitOnFlagUntilTimeout().

The above causes I2C_WaitOnFlagUntilTimeout() to return HAL_ERROR, which is treated as fatal at line #3535 which returns HAL_ERROR immediately without re-trying.

I don't think this is intended behaviour, as calling a function which return HAL_ERROR on a NACKF because you've detected a NACKF is pointless.

Also, it is normal for NACKs to be used by I2C devices to signal that they are busy as part of an ACK polling regime. See https://ww1.microchip.com/downloads/en/devicedoc/21713m.pdf for one such example, where ACK polling is expected following a write to detect the end of EEPROM activity before the next transaction.

We're currently stuck in limbo between HAL versions, as V1.6 fixes some other bugs whilst introducing this one.
Can you confirm that it is safe to selectively downgrade stm32u5xx_hal_i2c.c and stm32u5xx_hal_i2c.h to v1.4 whilst upgrading the rest of HAL? I assume the API presented by stm32u5xx_hal_i2c.h is intended to be unchanged across HAL revisions?
Otherwise can you advise a fix?

@ALABSTM ALABSTM added bug Something isn't working hal HAL-LL driver-related issue or pull-request. i2c I2C-related issue or pull-request labels Nov 4, 2024
@KRASTM KRASTM added the spotted before customer Spotted and fixed internally before being pointed out by users but not published yet label Nov 5, 2024
@KRASTM
Copy link
Contributor

KRASTM commented Nov 5, 2024

Hello @bencowperthwaite,

Thank you for the report.

Actually, this issue already fixed internally, and it will be available within the next release ASAP on ST.com and/or GitHub.
I will keep you informed.

With regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hal HAL-LL driver-related issue or pull-request. i2c I2C-related issue or pull-request spotted before customer Spotted and fixed internally before being pointed out by users but not published yet
Projects
Development

No branches or pull requests

3 participants