I2C Fix #748
I2C Fix #748
Conversation
When NACK is received by master, instead of immediate re-transmission by twi driver, error is reported. Up to application to manage re-transmission. This fix regression found on STM32F2 and STM32F4 when using I2C_OTHER_FRAME. See PR stm32duino#663
Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
Signed-off-by: Frederic Pillon <frederic.pillon@st.com>
fpistm
added
enhancement
| && (delta < I2C_TIMEOUT_TICK)); | ||
|
|
||
| err = HAL_I2C_GetError(&(obj->handle)); | ||
| if ((delta > I2C_TIMEOUT_TICK) |
There was a problem hiding this comment.
This introduces a new problem: When this timeout is triggered, I2C_OK is triggered. This happens because in the loop above, it checks for (delta < I2C_TIMEOUT_TICK), so it breaks out of the loop as soon as delta == I2C_TIMEOUT_TICK. Then, this if checks for (delta > I2C_TIMEOUT_TICK), which is not yet false. Since the hardware reports no errors either, I2C_OK is returned.
We noticed this when something confused the I2C hardware and prevented it from continuing somehow. We haven't figured out what the underlying cause is, but we did notice that we did not get an I2C error (just invalid data), which I think can be traced back to this problem.
The same applies to i2c_write_master above, I think.
Fixing this is, I think, a matter of replacing > with >= here.
There was a problem hiding this comment.
One more thing: This > and < combination was already present in the old code, but due to the old structure there it would, I think, only be problematic when an ACK failure occured exactly a the timeout, which is of course extremely likely. These refactorings have caused this to happen on every timeout instead.
Fixing this is, I think, a matter of replacing > with >= here.
We just tried this, and it seems to work. @HendryKaak is considering to prepare a PR for this.
Fixes STM32 I2C driver "repeated start" issue #583
When NACK is received by master, instead of immediate re-transmission
by twi driver, error is reported. Up to application to manage re-transmission.