Skip to content

Commit 710b5d8

Browse files
tannewttannewt
committed
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 adafruit#87
1 parent e9659e6 commit 710b5d8

7 files changed

Lines changed: 81 additions & 47 deletions

File tree

‎atmel-samd/common-hal/nativeio/I2C.c‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
// module.
2929

3030
#include "shared-bindings/nativeio/I2C.h"
31+
#include "py/mperrno.h"
3132
#include "py/nlr.h"
3233

3334
#include "asf/sam0/drivers/sercom/i2c/i2c_master.h"
@@ -135,7 +136,7 @@ void common_hal_nativeio_i2c_unlock(nativeio_i2c_obj_t *self) {
135136
i2c_master_unlock(&self->i2c_master_instance);
136137
}
137138

138-
bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr,
139+
uint8_t common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr,
139140
const uint8_t *data, size_t len, bool transmit_stop_bit) {
140141
struct i2c_master_packet packet = {
141142
.address = addr,
@@ -161,10 +162,15 @@ bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr,
161162
break;
162163
}
163164
}
164-
return status == STATUS_OK;
165+
if (status == STATUS_OK) {
166+
return 0;
167+
} else if (status == STATUS_ERR_BAD_ADDRESS) {
168+
return MP_ENODEV;
169+
}
170+
return MP_EIO;
165171
}
166172

167-
bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr,
173+
uint8_t common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr,
168174
uint8_t *data, size_t len) {
169175
struct i2c_master_packet packet = {
170176
.address = addr,
@@ -185,5 +191,10 @@ bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr,
185191
break;
186192
}
187193
}
188-
return status == STATUS_OK;
194+
if (status == STATUS_OK) {
195+
return 0;
196+
} else if (status == STATUS_ERR_BAD_ADDRESS) {
197+
return MP_ENODEV;
198+
}
199+
return MP_EIO;
189200
}

‎esp8266/common-hal/nativeio/I2C.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
*/
2626

2727
#include "shared-bindings/nativeio/I2C.h"
28+
#include "py/mperrno.h"
2829
#include "py/nlr.h"
2930

3031
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) {
5152
void common_hal_nativeio_i2c_unlock(nativeio_i2c_obj_t *self) {
5253
}
5354

54-
bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr,
55+
uint8_t common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t addr,
5556
const uint8_t * data, size_t len, bool transmit_stop_bit) {
56-
return false;
57+
return MP_EIO;
5758
}
5859

59-
bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr,
60+
uint8_t common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t addr,
6061
uint8_t * data, size_t len) {
61-
return false;
62+
return MP_EIO;
6263
}

‎shared-bindings/bitbangio/I2C.c‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "shared-bindings/microcontroller/Pin.h"
3232

3333
#include "lib/utils/context_manager_helpers.h"
34+
#include "py/mperrno.h"
3435
#include "py/runtime.h"
3536
//| .. currentmodule:: bitbangio
3637
//|
@@ -129,8 +130,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_i2c_scan_obj, bitbangio_i2c_scan);
129130
//| Attempts to grab the I2C lock. Returns True on success.
130131
//|
131132
STATIC mp_obj_t bitbangio_i2c_obj_try_lock(mp_obj_t self_in) {
132-
shared_module_bitbangio_i2c_try_lock(MP_OBJ_TO_PTR(self_in));
133-
return self_in;
133+
return mp_obj_new_bool(shared_module_bitbangio_i2c_try_lock(MP_OBJ_TO_PTR(self_in)));
134134
}
135135
MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_i2c_try_lock_obj, bitbangio_i2c_obj_try_lock);
136136

@@ -183,7 +183,13 @@ STATIC mp_obj_t bitbangio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_a
183183
} else if (len > bufinfo.len) {
184184
len = bufinfo.len;
185185
}
186-
shared_module_bitbangio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len);
186+
uint8_t status = shared_module_bitbangio_i2c_read(self,
187+
args[ARG_address].u_int,
188+
((uint8_t*)bufinfo.buf) + start,
189+
len);
190+
if (status != 0) {
191+
mp_raise_OSError(status);
192+
}
187193
return mp_const_none;
188194
}
189195
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
235241
}
236242

237243
// do the transfer
238-
bool ok = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int,
244+
uint8_t status = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int,
239245
((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool);
240-
if (!ok) {
241-
nlr_raise(mp_obj_new_exception_msg(&mp_type_OSError, "I2C bus error"));
246+
if (status != 0) {
247+
mp_raise_OSError(status);
242248
}
243249
return mp_const_none;
244250
}

‎shared-bindings/bitbangio/I2C.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,14 @@ extern void shared_module_bitbangio_i2c_unlock(bitbangio_i2c_obj_t *self);
5050
// Probe the bus to see if a device acknowledges the given address.
5151
extern bool shared_module_bitbangio_i2c_probe(bitbangio_i2c_obj_t *self, uint8_t addr);
5252

53-
extern bool shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self,
54-
uint16_t address,
55-
const uint8_t * data, size_t len,
56-
bool stop);
53+
extern uint8_t shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self,
54+
uint16_t address,
55+
const uint8_t * data, size_t len,
56+
bool stop);
5757

5858
// Reads memory of the i2c device picking up where it left off.
59-
extern bool shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self,
60-
uint16_t address,
61-
uint8_t * data, size_t len);
59+
extern uint8_t shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self,
60+
uint16_t address,
61+
uint8_t * data, size_t len);
6262

6363
#endif // __MICROPY_INCLUDED_SHARED_BINDINGS_BITBANGIO_I2C_H__

‎shared-bindings/nativeio/I2C.c‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(nativeio_i2c_scan_obj, nativeio_i2c_scan);
141141
//| Attempts to grab the I2C lock. Returns True on success.
142142
//|
143143
STATIC mp_obj_t nativeio_i2c_obj_try_lock(mp_obj_t self_in) {
144-
common_hal_nativeio_i2c_try_lock(MP_OBJ_TO_PTR(self_in));
145-
return self_in;
144+
return mp_obj_new_bool(common_hal_nativeio_i2c_try_lock(MP_OBJ_TO_PTR(self_in)));
146145
}
147146
MP_DEFINE_CONST_FUN_OBJ_1(nativeio_i2c_try_lock_obj, nativeio_i2c_obj_try_lock);
148147

@@ -196,7 +195,11 @@ STATIC mp_obj_t nativeio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_ar
196195
} else if (len > bufinfo.len) {
197196
len = bufinfo.len;
198197
}
199-
common_hal_nativeio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len);
198+
uint8_t status = common_hal_nativeio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, len);
199+
if (status != 0) {
200+
mp_raise_OSError(status);
201+
}
202+
200203
return mp_const_none;
201204
}
202205
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
248251
}
249252

250253
// do the transfer
251-
bool ok = common_hal_nativeio_i2c_write(self, args[ARG_address].u_int,
254+
uint8_t status = common_hal_nativeio_i2c_write(self, args[ARG_address].u_int,
252255
((uint8_t*) bufinfo.buf) + start, len, args[ARG_stop].u_bool);
253-
if (!ok) {
254-
nlr_raise(mp_obj_new_exception_msg(&mp_type_OSError, "I2C bus error"));
256+
if (status != 0) {
257+
mp_raise_OSError(status);
255258
}
256259
return mp_const_none;
257260
}

‎shared-bindings/nativeio/I2C.h‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,14 @@ extern void common_hal_nativeio_i2c_unlock(nativeio_i2c_obj_t *self);
5757
// Probe the bus to see if a device acknowledges the given address.
5858
extern bool common_hal_nativeio_i2c_probe(nativeio_i2c_obj_t *self, uint8_t addr);
5959

60-
extern bool common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t address,
61-
const uint8_t * data, size_t len,
62-
bool stop);
60+
// Write to the device and return 0 on success or an appropriate error code from mperrno.h
61+
extern uint8_t common_hal_nativeio_i2c_write(nativeio_i2c_obj_t *self, uint16_t address,
62+
const uint8_t * data, size_t len,
63+
bool stop);
6364

64-
// Reads memory of the i2c device picking up where it left off.
65-
extern bool common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t address,
66-
uint8_t * data, size_t len);
65+
// Reads memory of the i2c device picking up where it left off and return 0 on
66+
// success or an appropriate error code from mperrno.h
67+
extern uint8_t common_hal_nativeio_i2c_read(nativeio_i2c_obj_t *self, uint16_t address,
68+
uint8_t * data, size_t len);
6769

6870
#endif // __MICROPY_INCLUDED_SHARED_BINDINGS_NATIVEIO_I2C_H__

‎shared-module/bitbangio/I2C.c‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626

2727
#include "shared-bindings/bitbangio/I2C.h"
2828

29+
#include "py/mperrno.h"
2930
#include "py/obj.h"
3031

3132
#include "common-hal/microcontroller/types.h"
@@ -192,38 +193,48 @@ bool shared_module_bitbangio_i2c_probe(bitbangio_i2c_obj_t *self, uint8_t addr)
192193
return ok;
193194
}
194195

195-
bool shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self, uint16_t addr,
196+
uint8_t shared_module_bitbangio_i2c_write(bitbangio_i2c_obj_t *self, uint16_t addr,
196197
const uint8_t *data, size_t len, bool transmit_stop_bit) {
197198
// start the I2C transaction
198199
start(self);
199-
bool ok = write_byte(self, addr << 1);
200+
uint8_t status = 0;
201+
if (!write_byte(self, addr << 1)) {
202+
status = MP_ENODEV;
203+
}
200204

201-
for (uint32_t i = 0; i < len; i++) {
202-
ok = ok && write_byte(self, data[i]);
203-
if (!ok) {
204-
break;
205+
if (status == 0) {
206+
for (uint32_t i = 0; i < len; i++) {
207+
if (!write_byte(self, data[i])) {
208+
status = MP_EIO;
209+
break;
210+
}
205211
}
206212
}
207213

208214
if (transmit_stop_bit) {
209215
stop(self);
210216
}
211-
return ok;
217+
return status;
212218
}
213219

214-
bool shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self, uint16_t addr,
220+
uint8_t shared_module_bitbangio_i2c_read(bitbangio_i2c_obj_t *self, uint16_t addr,
215221
uint8_t * data, size_t len) {
216222
// start the I2C transaction
217223
start(self);
218-
bool ok = write_byte(self, (addr << 1) | 1);
224+
uint8_t status = 0;
225+
if (!write_byte(self, (addr << 1) | 1)) {
226+
status = MP_ENODEV;
227+
}
219228

220-
for (uint32_t i = 0; i < len; i++) {
221-
ok = ok && read_byte(self, data + i, i < len - 1);
222-
if (!ok) {
223-
break;
229+
if (status == 0) {
230+
for (uint32_t i = 0; i < len; i++) {
231+
if (!read_byte(self, data + i, i < len - 1)) {
232+
status = MP_EIO;
233+
break;
234+
}
224235
}
225236
}
226237

227238
stop(self);
228-
return ok;
239+
return status;
229240
}

0 commit comments

Comments
 (0)