STM32: Correct I2C master error handling #4012
Conversation
targets/TARGET_STM/i2c_api.c
Outdated
| @@ -904,7 +904,8 @@ void HAL_I2C_ErrorCallback(I2C_HandleTypeDef *hi2c){ | |||
|
|
|||
| #if DEVICE_I2CSLAVE | |||
| /* restore slave address */ | |||
| i2c_slave_address(obj, 0, address, 0); | |||
| if(address != 0) | |||
| i2c_slave_address(obj, 0, address, 0); | |||
There was a problem hiding this comment.
actually, one small remark, code may need small formatting update : space after if and use of braces
if (address != 0) {
i2c_slave_address(obj, 0, address, 0);
}
Note that the same applies to other places of the code that I wrote before but our maintainer Martin usually asks for such changes :-)
|
@monkiineko CLA signed? https://developer.mbed.org/contributor_agreement/ |
If I2C slave support is included, then the I2C error handler would always reset the I2C address, resulting in incorrectly changing the I2C state to listen for a controller configured as I2C master. This change conditionalizes the address setting to only occur if the controller was in slave mode when the error occurred.
|
@LMESTM Thanks for the review. I have updated the formatting as you request (my preferred formatting, actually ; I had copied the format from the slave "if" a few lines above, but I see other "if"s in the file using the improved formatting). I also noticed that i2c_init() would reset the slave field back to 0, so I added a line just before the slave address restore to set slave field to 1, as it should be. |
|
@0xc0170 Yes, I have previously signed the CLA using my mbed.org account, under the name bscott: |
|
@monkiineko good catch - thanks for joining your efforts to ours ! |
|
/morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputTest failed! |
|
Timing test failure due to CI load, restarting... /morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Ports for Upcoming Targets 3841: Add nRf52840 target ARMmbed/mbed-os#3841 3992: Introducing UBLOX_C030 platform. ARMmbed/mbed-os#3992 Fixes and Changes 3951: [NUCLEO_F303ZE] Correct ARDUINO pin ARMmbed/mbed-os#3951 4021: Fixing a macro to detect when RTOS was in use for the NRF52840_DK ARMmbed/mbed-os#4021 3979: KW24D: Add missing SPI defines and Arduino connector definitions ARMmbed/mbed-os#3979 3990: UBLOX_C027: construct a ticker-based wait, rather than calling wait_ms(), in the ARMmbed/mbed-os#3990 4003: Fixed OBOE in async serial tx for NRF52 target, fixes #4002 ARMmbed/mbed-os#4003 4012: STM32: Correct I2C master error handling ARMmbed/mbed-os#4012 4020: NUCLEO_L011K4 remove unsupported tool chain files ARMmbed/mbed-os#4020 4065: K66F: Move bss section to m_data_2 Section ARMmbed/mbed-os#4065 4014: Issue 3763: Reduce heap allocation in the GCC linker file ARMmbed/mbed-os#4014 4030: [STM32L0] reduce IAR heap and stack size for small targets ARMmbed/mbed-os#4030 4109: NUCLEO_L476RG : minor serial pin update ARMmbed/mbed-os#4109 3982: Ticker - kl25z bugfix for handling events in the past ARMmbed/mbed-os#3982
Description
If I2C slave support is included, then the I2C error handler would always reset the I2C address, resulting in incorrectly changing the I2C state to listen for a controller configured as I2C master. This change conditionalizes the address setting to only occur if the controller was in slave mode when the error occurred.
Status
READY
Migrations
NO
Related PRs
None
Todos
None
Deploy notes
None
Steps to test or reproduce
Create a build for an STM32 based board with DEVICE_I2CSLAVE and DEVICE_I2C_ASYNCH enabled (current default). Create an I2C master and issue a .transfer() call to a non-existent I2C device address, so that the transfer will be NACKed. HAL_I2C_ErrorCallback() will be called on the NACK and call i2c_slave_address(), resulting in the state being set to LISTEN. Subsequent I2C transfer attempts to any address will then immediately fail because i2c_active() returns 1 (true) for any state other than READY.