From 710b5d8aff2de490c5531e5cbc9092d51fb39958 Mon Sep 17 00:00:00 2001 From: Scott Shawcroft Date: Sun, 19 Feb 2017 17:02:29 +0100 Subject: Two I2C fixes: 1) Bus error will be thrown on read/write errors with errno set. (Read didn't used to fail at all.) 2) try_lock correctly returns boolean whether lock was grabbed. Fixes #87 --- atmel-samd/common-hal/nativeio/I2C.c | 19 ++++++++++++++---- esp8266/common-hal/nativeio/I2C.c | 9 +++++---- shared-bindings/bitbangio/I2C.c | 18 +++++++++++------ shared-bindings/bitbangio/I2C.h | 14 ++++++------- shared-bindings/nativeio/I2C.c | 15 ++++++++------ shared-bindings/nativeio/I2C.h | 14 +++++++------ shared-module/bitbangio/I2C.c | 39 +++++++++++++++++++++++------------- 7 files changed, 81 insertions(+), 47 deletions(-) diff --git a/atmel-samd/common-hal/nativeio/I2C.c b/atmel-samd/common-hal/nativeio/I2C.c index fb1080268..00b2a43e5 100644 --- a/atmel-samd/common-hal/nativeio/I2C.c +++ b/atmel-samd/common-hal/nativeio/I2C.c @@ -28,6 +28,7 @@ // module. #include "shared-bindings/nativeio/I2C.h" +#include "py/mperrno.h" #include "py/nlr.h" #include "asf/sam0/drivers/sercom/i2c/i2c_master.h" @@ -135,7 +136,7 @@ void common_hal_nativeio_i2c_unlock(nativeio_i2c_obj_t *self) { i2c_master_unlock(&self->i2c_master_instance); } -bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr, +uint8_t common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr, const uint8_t *data, size_t len, bool transmit_stop_bit) { struct i2c_master_packet packet = { .address = addr, @@ -161,10 +162,15 @@ bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr, break; } } - return status == STATUS_OK; + if (status == STATUS_OK) { + return 0; + } else if (status == STATUS_ERR_BAD_ADDRESS) { + return MP_ENODEV; + } + return MP_EIO; } -bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr, +uint8_t common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr, uint8_t *data, size_t len) { struct i2c_master_packet packet = { .address = addr, @@ -185,5 +191,10 @@ bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr, break; } } - return status == STATUS_OK; + if (status == STATUS_OK) { + return 0; + } else if (status == STATUS_ERR_BAD_ADDRESS) { + return MP_ENODEV; + } + return MP_EIO; } diff --git a/esp8266/common-hal/nativeio/I2C.c b/esp8266/common-hal/nativeio/I2C.c index 4fb05ba29..8addb92cd 100644 --- a/esp8266/common-hal/nativeio/I2C.c +++ b/esp8266/common-hal/nativeio/I2C.c @@ -25,6 +25,7 @@ */ #include "shared-bindings/nativeio/I2C.h" +#include "py/mperrno.h" #include "py/nlr.h" void common_hal_nativeio_i2c_construct(nativeio_i2c_obj_t *self, @@ -51,12 +52,12 @@ bool common_hal_nativeio_i2c_has_lock(nativeio_i2c_obj_t *self) { void common_hal_nativeio_i2c_unlock(nativeio_i2c_obj_t *self) { } -bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr, +uint8_t common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr, const uint8_t * data, size_t len, bool transmit_stop_bit) { - return false; + return MP_EIO; } -bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr, +uint8_t common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr, uint8_t * data, size_t len) { - return false; + return MP_EIO; } diff --git a/shared-bindings/bitbangio/I2C.c b/shared-bindings/bitbangio/I2C.c index 2b547d65c..5b772c9bf 100644 --- a/shared-bindings/bitbangio/I2C.c +++ b/shared-bindings/bitbangio/I2C.c @@ -31,6 +31,7 @@ #include "shared-bindings/microcontroller/Pin.h" #include "lib/utils/context_manager_helpers.h" +#include "py/mperrno.h" #include "py/runtime.h" //| .. currentmodule:: bitbangio //| @@ -129,8 +130,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_i2c_scan_obj, bitbangio_i2c_scan); //| Attempts to grab the I2C lock. Returns True on success. //| STATIC mp_obj_t bitbangio_i2c_obj_try_lock(mp_obj_t self_in) { - shared_module_bitbangio_i2c_try_lock(MP_OBJ_TO_PTR(self_in)); - return self_in; + return mp_obj_new_bool(shared_module_bitbangio_i2c_try_lock(MP_OBJ_TO_PTR(self_in))); } MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_i2c_try_lock_obj, bitbangio_i2c_obj_try_lock); @@ -183,7 +183,13 @@ STATIC mp_obj_t bitbangio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_a } else if (len > bufinfo.len) { len = bufinfo.len; } - shared_module_bitbangio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len); + uint8_t status = shared_module_bitbangio_i2c_read(self, + args[ARG_address].u_int, + ((uint8_t*)bufinfo.buf) + start, + len); + if (status != 0) { + mp_raise_OSError(status); + } return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_KW(bitbangio_i2c_readfrom_into_obj, 3, bitbangio_i2c_readfrom_into); @@ -235,10 +241,10 @@ STATIC mp_obj_t bitbangio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, m } // do the transfer - bool ok = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int, + uint8_t status = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int, ((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool); - if (!ok) { - nlr_raise(mp_obj_new_exception_msg(&mp_type_OSError, "I2C bus error")); + if (status != 0) { + mp_raise_OSError(status); } return mp_const_none; } diff --git a/shared-bindings/bitbangio/I2C.h b/shared-bindings/bitbangio/I2C.h index 451fa4d94..4d01230af 100644 --- a/shared-bindings/bitbangio/I2C.h +++ b/shared-bindings/bitbangio/I2C.h @@ -50,14 +50,14 @@ extern void shared_module_bitbangio_i2c_unlock(bitbangio_i2c_obj_t *self); // Probe the bus to see if a device acknowledges the given address. extern bool shared_module_bitbangio_i2c_probe(bitbangio_i2c_obj_t *self, uint8_t addr); -extern bool shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self, - uint16_t address, - const uint8_t * data, size_t len, - bool stop); +extern uint8_t shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self, + uint16_t address, + const uint8_t * data, size_t len, + bool stop); // Reads memory of the i2c device picking up where it left off. -extern bool shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self, - uint16_t address, - uint8_t * data, size_t len); +extern uint8_t shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self, + uint16_t address, + uint8_t * data, size_t len); #endif // __MICROPY_INCLUDED_SHARED_BINDINGS_BITBANGIO_I2C_H__ diff --git a/shared-bindings/nativeio/I2C.c b/shared-bindings/nativeio/I2C.c index bb827a95d..43cf7b63d 100644 --- a/shared-bindings/nativeio/I2C.c +++ b/shared-bindings/nativeio/I2C.c @@ -141,8 +141,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(nativeio_i2c_scan_obj, nativeio_i2c_scan); //| Attempts to grab the I2C lock. Returns True on success. //| STATIC mp_obj_t nativeio_i2c_obj_try_lock(mp_obj_t self_in) { - common_hal_nativeio_i2c_try_lock(MP_OBJ_TO_PTR(self_in)); - return self_in; + return mp_obj_new_bool(common_hal_nativeio_i2c_try_lock(MP_OBJ_TO_PTR(self_in))); } MP_DEFINE_CONST_FUN_OBJ_1(nativeio_i2c_try_lock_obj, nativeio_i2c_obj_try_lock); @@ -196,7 +195,11 @@ STATIC mp_obj_t nativeio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_ar } else if (len > bufinfo.len) { len = bufinfo.len; } - common_hal_nativeio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len); + uint8_t status = common_hal_nativeio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len); + if (status != 0) { + mp_raise_OSError(status); + } + return mp_const_none; } MP_DEFINE_CONST_FUN_OBJ_KW(nativeio_i2c_readfrom_into_obj, 3, nativeio_i2c_readfrom_into); @@ -248,10 +251,10 @@ STATIC mp_obj_t nativeio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp } // do the transfer - bool ok = common_hal_nativeio_i2c_write(self, args[ARG_address].u_int, + uint8_t status = common_hal_nativeio_i2c_write(self, args[ARG_address].u_int, ((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool); - if (!ok) { - nlr_raise(mp_obj_new_exception_msg(&mp_type_OSError, "I2C bus error")); + if (status != 0) { + mp_raise_OSError(status); } return mp_const_none; } diff --git a/shared-bindings/nativeio/I2C.h b/shared-bindings/nativeio/I2C.h index cf7a9fa60..5f3c89a77 100644 --- a/shared-bindings/nativeio/I2C.h +++ b/shared-bindings/nativeio/I2C.h @@ -57,12 +57,14 @@ extern void common_hal_nativeio_i2c_unlock(nativeio_i2c_obj_t *self); // Probe the bus to see if a device acknowledges the given address. extern bool common_hal_nativeio_i2c_probe(nativeio_i2c_obj_t *self, uint8_t addr); -extern bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t address, - const uint8_t * data, size_t len, - bool stop); +// Write to the device and return 0 on success or an appropriate error code from mperrno.h +extern uint8_t common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t address, + const uint8_t * data, size_t len, + bool stop); -// Reads memory of the i2c device picking up where it left off. -extern bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t address, - uint8_t * data, size_t len); +// Reads memory of the i2c device picking up where it left off and return 0 on +// success or an appropriate error code from mperrno.h +extern uint8_t common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t address, + uint8_t * data, size_t len); #endif // __MICROPY_INCLUDED_SHARED_BINDINGS_NATIVEIO_I2C_H__ diff --git a/shared-module/bitbangio/I2C.c b/shared-module/bitbangio/I2C.c index 9ba68262f..422f7778d 100644 --- a/shared-module/bitbangio/I2C.c +++ b/shared-module/bitbangio/I2C.c @@ -26,6 +26,7 @@ #include "shared-bindings/bitbangio/I2C.h" +#include "py/mperrno.h" #include "py/obj.h" #include "common-hal/microcontroller/types.h" @@ -192,38 +193,48 @@ bool shared_module_bitbangio_i2c_probe(bitbangio_i2c_obj_t *self, uint8_t addr) return ok; } -bool shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self, uint16_t addr, +uint8_t shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self, uint16_t addr, const uint8_t *data, size_t len, bool transmit_stop_bit) { // start the I2C transaction start(self); - bool ok = write_byte(self, addr << 1); + uint8_t status = 0; + if (!write_byte(self, addr << 1)) { + status = MP_ENODEV; + } - for (uint32_t i = 0; i < len; i++) { - ok = ok && write_byte(self, data[i]); - if (!ok) { - break; + if (status == 0) { + for (uint32_t i = 0; i < len; i++) { + if (!write_byte(self, data[i])) { + status = MP_EIO; + break; + } } } if (transmit_stop_bit) { stop(self); } - return ok; + return status; } -bool shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self, uint16_t addr, +uint8_t shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self, uint16_t addr, uint8_t * data, size_t len) { // start the I2C transaction start(self); - bool ok = write_byte(self, (addr << 1) | 1); + uint8_t status = 0; + if (!write_byte(self, (addr << 1) | 1)) { + status = MP_ENODEV; + } - for (uint32_t i = 0; i < len; i++) { - ok = ok && read_byte(self, data + i, i < len - 1); - if (!ok) { - break; + if (status == 0) { + for (uint32_t i = 0; i < len; i++) { + if (!read_byte(self, data + i, i < len - 1)) { + status = MP_EIO; + break; + } } } stop(self); - return ok; + return status; } -- cgit v1.2.3