Rethink STM32 I2C v2 HAL (#78)

* Initial attempt at rethinking the STM32 I2C v2 HAL.  Makes single-byte work properly and adds a new 'state' variable to track what the hardware is doing.

* Fix some initial test failures

* Fix incorrect logic

* Fix more incorrect logic

* Tabs to spaces

* Fix repeated starts with single-byte API

* Fix race condition causing stop() after nacked address to sometimes break things

* Fix missed i2c structs that should have been removed

* Fix doing a repeated start from single-byte to transaction API causing I2C peripheral to lock up

* Fix xferOperation being set wrong for repeated starts, causing the peripheral to hang

* Fix race condition with repeated start after single-byte operation

* Fix compilation for targets that use I2C IP v1

* Fix initialization of XferOperation for API v1, optimize stop()

* Remove unneeded line

* Add docs for I2C events

Author: Jamie Smith

features/frameworks/unity/source/unity.c

@@ -17,7 +17,7 @@ int UNITY_OUTPUT_FLUSH(void);
 #endif
 
 /* Helpful macros for us to use here */
-#define UNITY_FAIL_AND_BAIL   { UNITY_OUTPUT_CHAR('\n'); utest_unity_assert_failure(); }
+#define UNITY_FAIL_AND_BAIL   { UNITY_OUTPUT_CHAR('\n'); utest_unity_assert_failure(); Unity.CurrentTestFailed = 1; }
 #define UNITY_IGNORE_AND_BAIL { UNITY_OUTPUT_CHAR('\n'); utest_unity_ignore_failure(); }
 
 /* return prematurely if we are already in failure or ignore state */

hal/include/hal/i2c_api.h

@@ -35,10 +35,36 @@
  *
  * @{
  */
+
+/**
+ * Indicates that an unspecified error has occurred in the transfer.  This usually means
+ * either an internal error in the Mbed MCU's I2C module, or something like an arbitration loss.
+ * Does not indicate a NACK.
+ */
 #define I2C_EVENT_ERROR               (1 << 1)
+
+/**
+ * Indicates that the slave did not respond to the address byte of the transfer.
+ */
 #define I2C_EVENT_ERROR_NO_SLAVE      (1 << 2)
+
+/**
+ * Indicates that the transfer completed successfully.
+ */
 #define I2C_EVENT_TRANSFER_COMPLETE   (1 << 3)
+
+/**
+ * Indicates that a NACK was received after the address byte, but before the requested number of bytes
+ * could be transferred.
+ *
+ * Note: Not every manufacturer HAL is able to make a distinction between this flag and #I2C_EVENT_ERROR_NO_SLAVE.
+ * On a NACK, you might conceivably get one or both of these flags.
+ */
 #define I2C_EVENT_TRANSFER_EARLY_NACK (1 << 4)
+
+/**
+ * Use this macro to request all possible I2C events.
+ */
 #define I2C_EVENT_ALL                 (I2C_EVENT_ERROR |  I2C_EVENT_TRANSFER_COMPLETE | I2C_EVENT_ERROR_NO_SLAVE | I2C_EVENT_TRANSFER_EARLY_NACK)
 
 /**@}*/
@@ -61,7 +87,8 @@ typedef struct i2c_s i2c_t;
 
 enum {
     I2C_ERROR_NO_SLAVE = -1,
-    I2C_ERROR_BUS_BUSY = -2
+    I2C_ERROR_BUS_BUSY = -2,
+    I2C_ERROR_INVALID_USAGE = -3 // Invalid usage of the I2C API, e.g. by mixing single-byte and transactional function calls.
 };
 
 typedef struct {
@@ -229,7 +256,7 @@ int i2c_byte_read(i2c_t *obj, int last);
  *
  *  @param obj The I2C object
  *  @param data Byte to be written
- *  @return 0 if NAK was received, 1 if ACK was received, 2 for timeout.
+ *  @return 0 if NAK was received, 1 if ACK was received, 2 for timeout, or 3 for other error.
  */
 int i2c_byte_write(i2c_t *obj, int data);
 

targets/TARGET_STM/TARGET_STM32F0/objects.h

@@ -77,39 +77,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct analogin_s {
     ADC_HandleTypeDef handle;
     PinName pin;

targets/TARGET_STM/TARGET_STM32F3/objects.h

@@ -90,41 +90,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    int sda_func;
-    int scl_func;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct dac_s {
     DACName dac;
     PinName pin;

targets/TARGET_STM/TARGET_STM32F7/objects.h

@@ -108,39 +108,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct analogin_s {
     ADC_HandleTypeDef handle;
     PinName pin;

targets/TARGET_STM/TARGET_STM32G0/objects.h

@@ -89,41 +89,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    int sda_func;
-    int scl_func;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct flash_s {
     /*  nothing to be stored for now */
     uint32_t dummy;

targets/TARGET_STM/TARGET_STM32G4/objects.h

@@ -88,41 +88,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    int sda_func;
-    int scl_func;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct dac_s {
     DACName dac;
     PinName pin;

targets/TARGET_STM/TARGET_STM32H7/objects.h

@@ -97,39 +97,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct analogin_s {
     ADC_HandleTypeDef handle;
     PinName pin;

targets/TARGET_STM/TARGET_STM32L0/objects.h

@@ -91,41 +91,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    int sda_func;
-    int scl_func;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 #if DEVICE_FLASH
 struct flash_s {
     /*  nothing to be stored for now */

targets/TARGET_STM/TARGET_STM32L4/objects.h

@@ -87,41 +87,6 @@ struct serial_s {
 #endif
 };
 
-struct i2c_s {
-    /*  The 1st 2 members I2CName i2c
-     *  and I2C_HandleTypeDef handle should
-     *  be kept as the first members of this struct
-     *  to ensure i2c_get_obj to work as expected
-     */
-    I2CName  i2c;
-    I2C_HandleTypeDef handle;
-    uint8_t index;
-    int hz;
-    PinName sda;
-    PinName scl;
-    int sda_func;
-    int scl_func;
-    IRQn_Type event_i2cIRQ;
-    IRQn_Type error_i2cIRQ;
-    uint32_t XferOperation;
-    volatile uint8_t event;
-    volatile int pending_start;
-    int current_hz;
-#if DEVICE_I2CSLAVE
-    uint8_t slave;
-    volatile uint8_t pending_slave_tx_master_rx;
-    volatile uint8_t pending_slave_rx_maxter_tx;
-    uint8_t *slave_rx_buffer;
-    volatile uint16_t slave_rx_buffer_size;
-    volatile uint16_t slave_rx_count;
-#endif
-#if DEVICE_I2C_ASYNCH
-    uint32_t address;
-    uint8_t stop;
-    uint8_t available_events;
-#endif
-};
-
 struct flash_s {
     /*  nothing to be stored for now */
     uint32_t dummy;