While writing the simplest example I could come up for using the HAL I2C routines, I came across what seems like a clear-cut bug. This is for the latest version of STM32CubeF4 (1.24.0), identified in stm32f4xx_hal.c as "STM32F4xx HAL Driver version number V1.7.5".
To be clear, I only want to write some data to an I2C device using HAL_I2C_Master_Transmit_IT(). I configured the I2Cx EV interrupt and I2Cx_EV_IRQHandler() simply calls HAL_I2C_EV_IRQHandler(), passing in the proper I2C handle.
Running the code with a logic analyzer hooked up, I notice only the slave address is sent. After that, SCL is pulled low indefinitely, the next bytes aren't transmitted, and the I2C stop signal isn't sent.
Debugging the code shows that the code is stuck rerunning HAL_I2C_EV_IRQHandler() endlessly -- just as it leaves the ISR, it immediately comes back there.
Cutting the story short, here is the issue:
In the I2C_Master_SB() function (inside the file stm32f4xx_hal_i2c.c, line 5452), there is the following check:
if ((hi2c->hdmatx->XferCpltCallback != NULL) || (hi2c->hdmarx->XferCpltCallback != NULL))
{
/* Enable DMA Request */
SET_BIT(hi2c->Instance->CR2, I2C_CR2_DMAEN);
}
In my case, since I'm using interrupts and not DMA, it follows that both hdmatx and hdmarx are NULL. However, numerically NULL equals 0, which is a valid memory address, and contains the interrupt vector. Therefore, the access to hi2c->hdmatx or hi2c->hdmarx is a valid memory access which, given the struct offset of the XferCpltCallback field, corresponds to the SysTick handler, which isn't NULL. So, the DMAEN bit gets inadvertently set.
Back in the HAL_I2C_EV_IRQHandler() (line 4568 of stm32f4xx_hal_i2c.c), we have the following code:
/* I2C in mode Transmitter -----------------------------------------------*/
else if (I2C_CHECK_FLAG(sr2itflags, I2C_FLAG_TRA) != RESET)
{
/* Do not check buffer and BTF flag if a Xfer DMA is on going */
if (READ_BIT(hi2c->Instance->CR2, I2C_CR2_DMAEN) != I2C_CR2_DMAEN)
{
...
}
The code inside (which I replaced by ...) is what needs to run for the transmission to occur. However, because DMAEN was previously set, the if() condition is false and the code inside never runs.
In my opinion, the fix is to check if hi2c->hdmatx and hi2c->hdmarx are not NULL prior to checking if the callbacks are NULL. Concretely, replacing line 5452 with the following solves the issue, which I propose as a fix:
if ((hi2c->hdmatx != NULL && hi2c->hdmatx->XferCpltCallback != NULL) || (hi2c->hdmarx != NULL && hi2c->hdmarx->XferCpltCallback != NULL))
