From bf3eb9dc3911b8e4e7da3054ffece61ae5e2ef64 Mon Sep 17 00:00:00 2001 From: "David (Pololu)" Date: Wed, 12 Apr 2023 18:58:44 -0700 Subject: [PATCH] rp2/machine_i2c: Add timeout parameter for machine.I2C(). This commit adds support for the `timeout` keyword argument to machine.I2C on the rp2 port, following how it's done on other ports. The main motivation here is avoid the interpreter crashing due to infinite loops when SDA is stuck low, which is quite common if the board gets reset while reading from an I2C device. A default timeout of 50ms is chosen because it's consistent with: - Commit a707fe50b085ec83722106609f6fd219faf9f030 which used a timeout of 50,000us for zero-length writes on the rp2 port. - The machine.SoftI2C class which uses 50,000us as the default timeout. - The stm32 port's hardware I2C, which uses 50,000us for I2C_POLL_DEFAULT_TIMEOUT_US. This commit also fixes the default timeout on the esp32 port to be consistent with the above, and updates the documentation for machine.I2C to document this keyword argument. --- docs/library/machine.I2C.rst | 4 +++- ports/esp32/machine_i2c.c | 2 +- ports/rp2/machine_i2c.c | 16 ++++++++++------ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/library/machine.I2C.rst b/docs/library/machine.I2C.rst index 0eb1b67f58..bfc9f7ebcc 100644 --- a/docs/library/machine.I2C.rst +++ b/docs/library/machine.I2C.rst @@ -52,7 +52,7 @@ Example usage:: Constructors ------------ -.. class:: I2C(id, *, scl, sda, freq=400000) +.. class:: I2C(id, *, scl, sda, freq=400000, timeout=50000) Construct and return a new I2C object using the following parameters: @@ -62,6 +62,8 @@ Constructors - *sda* should be a pin object specifying the pin to use for SDA. - *freq* should be an integer which sets the maximum frequency for SCL. + - *timeout* is the maximum time in microseconds to allow for I2C + transactions. This parameter is not allowed on some ports. Note that some ports/boards will have default values of *scl* and *sda* that can be changed in this constructor. Others will have fixed values diff --git a/ports/esp32/machine_i2c.c b/ports/esp32/machine_i2c.c index c805dab87e..9ec39e5649 100644 --- a/ports/esp32/machine_i2c.c +++ b/ports/esp32/machine_i2c.c @@ -62,7 +62,7 @@ #error "unsupported I2C for ESP32 SoC variant" #endif -#define I2C_DEFAULT_TIMEOUT_US (10000) // 10ms +#define I2C_DEFAULT_TIMEOUT_US (50000) // 50ms typedef struct _machine_hw_i2c_obj_t { mp_obj_base_t base; diff --git a/ports/rp2/machine_i2c.c b/ports/rp2/machine_i2c.c index 85d12c7713..fc4c0723a5 100644 --- a/ports/rp2/machine_i2c.c +++ b/ports/rp2/machine_i2c.c @@ -33,6 +33,7 @@ #include "hardware/i2c.h" #define DEFAULT_I2C_FREQ (400000) +#define DEFAULT_I2C_TIMEOUT (50000) #ifndef MICROPY_HW_I2C0_SCL #if PICO_DEFAULT_I2C == 0 @@ -65,6 +66,7 @@ typedef struct _machine_i2c_obj_t { uint8_t scl; uint8_t sda; uint32_t freq; + uint32_t timeout; } machine_i2c_obj_t; STATIC machine_i2c_obj_t machine_i2c_obj[] = { @@ -74,17 +76,18 @@ STATIC machine_i2c_obj_t machine_i2c_obj[] = { STATIC void machine_i2c_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { machine_i2c_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_printf(print, "I2C(%u, freq=%u, scl=%u, sda=%u)", - self->i2c_id, self->freq, self->scl, self->sda); + mp_printf(print, "I2C(%u, freq=%u, scl=%u, sda=%u, timeout=%u)", + self->i2c_id, self->freq, self->scl, self->sda, self->timeout); } mp_obj_t machine_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { - enum { ARG_id, ARG_freq, ARG_scl, ARG_sda }; + enum { ARG_id, ARG_freq, ARG_scl, ARG_sda, ARG_timeout }; static const mp_arg_t allowed_args[] = { { MP_QSTR_id, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_freq, MP_ARG_INT, {.u_int = DEFAULT_I2C_FREQ} }, { MP_QSTR_scl, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} }, { MP_QSTR_sda, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} }, + { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = DEFAULT_I2C_TIMEOUT} }, }; // Parse args. @@ -121,6 +124,7 @@ mp_obj_t machine_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n self->freq = args[ARG_freq].u_int; i2c_init(self->i2c_inst, self->freq); self->freq = i2c_set_baudrate(self->i2c_inst, self->freq); + self->timeout = args[ARG_timeout].u_int; gpio_set_function(self->scl, GPIO_FUNC_I2C); gpio_set_function(self->sda, GPIO_FUNC_I2C); gpio_set_pulls(self->scl, true, 0); @@ -135,14 +139,14 @@ STATIC int machine_i2c_transfer_single(mp_obj_base_t *self_in, uint16_t addr, si int ret; bool nostop = !(flags & MP_MACHINE_I2C_FLAG_STOP); if (flags & MP_MACHINE_I2C_FLAG_READ) { - ret = i2c_read_blocking(self->i2c_inst, addr, buf, len, nostop); + ret = i2c_read_timeout_us(self->i2c_inst, addr, buf, len, nostop, self->timeout); } else { if (len == 0) { // Workaround issue with hardware I2C not accepting zero-length writes. mp_machine_soft_i2c_obj_t soft_i2c = { .base = { &mp_machine_soft_i2c_type }, .us_delay = 500000 / self->freq + 1, - .us_timeout = 50000, + .us_timeout = self->timeout, .scl = self->scl, .sda = self->sda, }; @@ -157,7 +161,7 @@ STATIC int machine_i2c_transfer_single(mp_obj_base_t *self_in, uint16_t addr, si gpio_set_function(self->sda, GPIO_FUNC_I2C); return ret; } else { - ret = i2c_write_blocking(self->i2c_inst, addr, buf, len, nostop); + ret = i2c_write_timeout_us(self->i2c_inst, addr, buf, len, nostop, self->timeout); } } if (ret < 0) {