Add timeout handling for I2C in Wire

libraries/Wire/Wire.cpp

@@ -20,6 +20,8 @@
   Modified 2017 by Chuck Todd ([email protected]) to correct Unconfigured Slave Mode reboot
 
   Version 2022 for Renesas RA4 by Daniele Aimo ([email protected])
+
+  Version 2025 by Hanz Häger ([email protected]) added timeout interface
 */
 
 extern "C" {
@@ -28,6 +30,7 @@ extern "C" {
   #include <inttypes.h>
 }
 
+#include "Arduino.h"
 #include "Wire.h"
 
 TwoWire *TwoWire::g_SCIWires[TWOWIRE_MAX_SCI_CHANNELS] = {nullptr};
@@ -69,13 +72,13 @@ void TwoWire::WireSCIMasterCallback(i2c_master_callback_args_t *arg) {
       ptr->setBusStatus(WIRE_STATUS_TX_COMPLETED);
     }
   }
-
 }
 
 /* -------------------------------------------------------------------------- */
 void TwoWire::WireMasterCallback(i2c_master_callback_args_t *arg) {
 /* -------------------------------------------------------------------------- */  
   /* +++++ MASTER I2C not SCI Callback ++++++ */
+
   i2c_master_cfg_t *cfg = (i2c_master_cfg_t *)arg->p_context;
 
   TwoWire *ptr = nullptr; 
@@ -191,7 +194,9 @@ TwoWire::TwoWire(int scl, int sda, WireAddressMode_t am /*= ADDRESS_MODE_7_BITS*
   is_master(true),
   is_sci(false),
   address_mode(am),
-  timeout(1000),
+  timeout_us(25000),
+  timed_out_flag(false),
+  do_reset_on_timeout(false),
   transmission_begun(false),
   data_too_long(false),
   rx_index(0),
@@ -329,7 +334,7 @@ void TwoWire::_begin(void) {
         m_i2c_cfg.p_callback                  = WireMasterCallback;
 
         m_i2c_extend.timeout_mode             = IIC_MASTER_TIMEOUT_MODE_SHORT;
-        m_i2c_extend.timeout_scl_low          = IIC_MASTER_TIMEOUT_SCL_LOW_DISABLED;
+        m_i2c_extend.timeout_scl_low          = IIC_MASTER_TIMEOUT_SCL_LOW_ENABLED;
       }
 
       m_i2c_cfg.channel                     = channel;
@@ -465,7 +470,7 @@ void TwoWire::end(void) {
 
 
 /* -------------------------------------------------------------------------- */
-uint8_t TwoWire::read_from(uint8_t address, uint8_t* data, uint8_t length, unsigned int timeout_ms, bool sendStop) {
+uint8_t TwoWire::read_from(uint8_t address, uint8_t* data, uint8_t length, uint32_t timeout_us, bool sendStop) {
 /* -------------------------------------------------------------------------- */  
   /* ??? does this function make sense only for MASTER ???? */
 
@@ -476,13 +481,18 @@ uint8_t TwoWire::read_from(uint8_t address, uint8_t* data, uint8_t length, unsig
     }
     if(err == FSP_SUCCESS) {
       if(m_read != nullptr) {
-        bus_status = WIRE_STATUS_UNSET;
+        setBusStatus(WIRE_STATUS_UNSET);
         err = m_read(&m_i2c_ctrl,data,length,!sendStop);
       }
     }
-    uint32_t const start = millis();
-    while(((millis() - start) < timeout_ms) && bus_status == WIRE_STATUS_UNSET && err == FSP_SUCCESS) {
 
+    uint32_t const start = micros();
+    while (((timeout_us == 0ul) || ((micros() - start) < timeout_us)) &&
+                bus_status == WIRE_STATUS_UNSET && err == FSP_SUCCESS) {
+    }
+    if ((err == FSP_SUCCESS) && (bus_status == WIRE_STATUS_UNSET)) {
+      handleTimeout(do_reset_on_timeout);
+      return 0;
     }
   }
 
@@ -494,38 +504,57 @@ uint8_t TwoWire::read_from(uint8_t address, uint8_t* data, uint8_t length, unsig
 }
 
 /* -------------------------------------------------------------------------- */    
-uint8_t TwoWire::write_to(uint8_t address, uint8_t* data, uint8_t length, unsigned int timeout_ms, bool sendStop) {
+uint8_t TwoWire::write_to(uint8_t address, uint8_t* data, uint8_t length, uint32_t timeout_us , bool sendStop) {
 /* -------------------------------------------------------------------------- */  
   uint8_t rv = END_TX_OK;
   fsp_err_t err = FSP_ERR_ASSERTION;
   if(init_ok) {
     if(m_setSlaveAdd != nullptr) {
+      setBusStatus(WIRE_STATUS_UNSET);
       err = m_setSlaveAdd(&m_i2c_ctrl, address, m_i2c_cfg.addr_mode);
     }
-    if(err == FSP_SUCCESS) {
+    if((err == FSP_SUCCESS) && (bus_status != WIRE_STATUS_TRANSACTION_ABORTED)) {
+
       if(m_write != nullptr) {
-        bus_status = WIRE_STATUS_UNSET;
+        setBusStatus(WIRE_STATUS_UNSET);
         err = m_write(&m_i2c_ctrl,data,length,!sendStop);
       }
-    }
-    uint32_t const start = millis();
-    while(((millis() - start) < timeout_ms) && bus_status == WIRE_STATUS_UNSET && err == FSP_SUCCESS) {
+      if (err==FSP_ERR_INVALID_SIZE) {
+        rv = END_TX_DATA_TOO_LONG;
+        Serial.println(F("Invalid size when trying to write"));
+      } 
+
+    } else  // No FSP_SUCCESS in m_setSlaveAdd or WIRE_STATUS_TRANSACTION_ABORTED 
+      if (err == FSP_ERR_IN_USE) {
+        Serial.println(F("An I2C Transaction is in progress. when setting slave address"));
+      } else
+        if (bus_status == WIRE_STATUS_TRANSACTION_ABORTED){
+          rv = END_TX_NACK_ON_ADD;
+          Serial.println(F("Transaction aborted -> NACK on ADDR"));
+      }
 
+    // If FSP_SUCCESS, wait for change in bus_status or timeout
+    uint32_t const start = micros();
+    while (((timeout_us == 0ul) || ((micros() - start) < timeout_us)) && 
+		bus_status == WIRE_STATUS_UNSET && err == FSP_SUCCESS) {
     }
 
     if(err != FSP_SUCCESS) {
       rv = END_TX_ERR_FSP;
     }
     else if(data_too_long) {
       rv = END_TX_DATA_TOO_LONG;
+      Serial.println(F("Trying to write more than II2C_BUFFER_LENGTH, buffer truncated before written"));
     }
     else if(bus_status == WIRE_STATUS_UNSET) {
       rv = END_TX_TIMEOUT;
+      handleTimeout(do_reset_on_timeout);	
     }
     /* as far as I know is impossible to distinguish between NACK on ADDRESS and
       NACK on DATA */
     else if(bus_status == WIRE_STATUS_TRANSACTION_ABORTED) {
-      rv = END_TX_NACK_ON_ADD;
+      if (length==0) rv = END_TX_NACK_ON_ADD;
+      if (rv != END_TX_NACK_ON_ADD) rv = END_TX_NACK_ON_DATA;
     }
   }
   else {
@@ -579,13 +608,13 @@ void TwoWire::setClock(uint32_t freq) {
           m_i2c_extend.clock_settings.brh_value = 15;
           m_i2c_extend.clock_settings.cks_value = 0 + clock_divisor;
           break;
-#if BSP_FEATURE_IIC_FAST_MODE_PLUS
         case I2C_MASTER_RATE_FASTPLUS:
+#if BSP_FEATURE_IIC_FAST_MODE_PLUS
           m_i2c_extend.clock_settings.brl_value = 6;
           m_i2c_extend.clock_settings.brh_value = 5;
           m_i2c_extend.clock_settings.cks_value = 0;
-          break;
 #endif
+          break;
       }
     }
   }
@@ -602,6 +631,93 @@ void TwoWire::setClock(uint32_t freq) {
   }
 }
 
+/***
+ * Sets the I2C timeout.
+ *
+ * This limits the maximum time to wait for the I2C hardware. If more time passes, the bus is assumed
+ * to have locked up (e.g. due to noise-induced glitches or faulty slaves) and the transaction is aborted.
+ * Optionally, the I2C hardware is also reset, which can be required to allow subsequent transactions to
+ * succeed in some cases (in particular when noise has made the I2C hardware think there is a second
+ * master that has claimed the bus).
+ *
+ * When a timeout is triggered, a flag is set that can be queried with `getWireTimeoutFlag()` and is cleared
+ * when `clearWireTimeoutFlag()` or `setWireTimeoutUs()` is called.
+ *
+ * Note that this timeout can also trigger while waiting for clock stretching or waiting for a second master
+ * to complete its transaction. So make sure to adapt the timeout to accommodate for those cases if needed.
+ * A typical timeout would be 25ms (which is the maximum clock stretching allowed by the SMBus protocol),
+ * but (much) shorter values will usually also work.
+ *
+ * In the future, a timeout will be enabled by default, so if you require the timeout to be disabled, it is
+ * recommended you disable it by default using `setWireTimeoutUs(0)`, even though that is currently
+ * the default.
+ *
+ * @param timeout a timeout value in microseconds, if zero then timeout checking is disabled
+ * @param reset_with_timeout if true then I2C interface will be automatically reset on timeout
+ *                           if false then I2C interface will not be reset on timeout
+ */
+/* -------------------------------------------------------------------------- */
+void TwoWire::setWireTimeout(uint32_t timeout, bool reset_with_timeout){
+/* -------------------------------------------------------------------------- */
+  timed_out_flag = false;
+  timeout_us = timeout;
+  do_reset_on_timeout = reset_with_timeout;
+}
+
+/***
+ * Returns the timeout flag.
+ *
+ * @return true if timeout has occurred since the flag was last cleared.
+ */
+bool TwoWire::getWireTimeoutFlag(void){
+  return(timed_out_flag);
+}
+
+/***
+ * Clears the timeout flag.
+ */
+/* -------------------------------------------------------------------------- */
+void TwoWire::clearWireTimeoutFlag(void){
+/* -------------------------------------------------------------------------- */
+  timed_out_flag = false;
+}
+
+/* 
+ * Function handleTimeout
+ * Desc     this gets called whenever a while loop here has lasted longer than
+ *          timeout_us microseconds. always sets timed_out_flag
+ * Input    reset: true causes this function to reset the hardware interface
+ * Output   none
+ */
+/* -------------------------------------------------------------------------- */
+void TwoWire::handleTimeout(bool reset){
+/* -------------------------------------------------------------------------- */
+  timed_out_flag = true;
+  Serial.println(F("Handling TwoWire::handleTimeout()"));
+
+  if (reset) { //TBD; What do we do here? like fixHungWire()?
+    // TBD, Is this the way to go to reset the bus? 
+    // Do we need more to handle devices that hangs the bus?
+    Serial.print(F("with reset Abort result: "));
+    if(m_abort != nullptr) {
+      //setBusStatus(WIRE_STATUS_TRANSACTION_ABORTED);
+      fsp_err_t err = m_abort(&m_i2c_ctrl);
+      Serial.println(err);
+    }
+    // TDB, Is this the right way to get back after reset?
+    if(m_open != nullptr) {
+      fsp_err_t err = m_open(&m_i2c_ctrl,&m_i2c_cfg);
+      if(FSP_SUCCESS == err) {
+         init_ok &= true;
+      }
+      Serial.print(F(" Open result: "));
+      Serial.println(err);
+    }
+    // Is it neccesarry to do the open after the abort?
+    // Is that more to be done after the abort to get back to same settings
+  } 
+}
+
 /*  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  *                           TRANSMISSION BEGIN
  *  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ */
@@ -642,7 +758,7 @@ void TwoWire::beginTransmission(int address) {
 /* -------------------------------------------------------------------------- */
 uint8_t TwoWire::endTransmission(bool sendStop) {
 /* -------------------------------------------------------------------------- */  
-  uint8_t ret = write_to(master_tx_address, tx_buffer, tx_index, timeout, sendStop);
+  uint8_t ret = write_to(master_tx_address, tx_buffer, tx_index, timeout_us, sendStop);
   transmission_begun = false;
   return ret;
 }
@@ -687,7 +803,7 @@ size_t TwoWire::requestFrom(uint8_t address, size_t quantity, uint32_t iaddress,
       quantity = I2C_BUFFER_LENGTH;
     }
     // perform blocking read into buffer
-    uint8_t read = read_from(address, rx_buffer, quantity, timeout, sendStop);
+    uint8_t read = read_from(address, rx_buffer, quantity, timeout_us, sendStop);
     // set rx buffer iterator vars
     rx_index = read;
     rx_extract_index = 0;

libraries/Wire/Wire.h

@@ -69,6 +69,14 @@ using I2C_onTxCallback_f        = void (*)(void);
 // WIRE_HAS_END means Wire has end()
 #define WIRE_HAS_END 1
 
+// WIRE_HAS_TIMEOUT means Wire has setWireTimeout(), getWireTimeoutFlag
+// and clearWireTimeoutFlag()
+#define WIRE_HAS_TIMEOUT 1
+
+// When not configured, these settings are used for the timeout
+#define WIRE_DEFAULT_TIMEOUT 25000
+#define WIRE_DEFAULT_RESET_WITH_TIMEOUT 0
+
 #define END_TX_OK      0
 #define END_TX_DATA_TOO_LONG 1
 #define END_TX_NACK_ON_ADD  2
@@ -104,6 +112,10 @@ class TwoWire : public arduino::HardwareI2C {
     void end();
     void setClock(uint32_t);
 
+    void setWireTimeout(uint32_t timeout = 25000, bool reset_with_timeout = false);
+    bool getWireTimeoutFlag(void);
+    void clearWireTimeoutFlag(void);
+
     void beginTransmission(uint32_t);
     void beginTransmission(uint16_t);
     void beginTransmission(uint8_t);
@@ -169,7 +181,12 @@ class TwoWire : public arduino::HardwareI2C {
     bool is_sci;
     WireAddressMode_t address_mode;
 
-    unsigned int timeout;
+    uint32_t timeout_us;
+    volatile bool timed_out_flag;
+    bool do_reset_on_timeout;
+
+    void handleTimeout(bool reset);
+
     bool transmission_begun;
     bool data_too_long; 
 
@@ -211,8 +228,8 @@ class TwoWire : public arduino::HardwareI2C {
 
     bool require_sci;
 
-    uint8_t read_from(uint8_t, uint8_t*, uint8_t, unsigned int, bool);
-    uint8_t write_to(uint8_t, uint8_t*, uint8_t, unsigned int, bool);
+    uint8_t read_from(uint8_t, uint8_t*, uint8_t, uint32_t, bool);
+    uint8_t write_to(uint8_t, uint8_t*, uint8_t, uint32_t, bool);
 
     bool cfg_pins(int max_index);
 
@@ -235,4 +252,4 @@ extern TwoWire Wire3;
 #endif
 
 #endif
-#endif
+#endif