Skip to content

Commit 82d436d

Browse files
tannewttannewt
committed
Add writeto_then_readfrom to I2C API. Deprecate stop kwarg.
writeto_then_readfrom has been added to do a write -> no stop -> repeated start -> read sequence. This is done to match the capabilities of Blinka on Linux. Code that uses stop=False will not work correctly on Blinka. To fix, if stop=False then use writeto_then_readfrom otherwise use writeto then readfrom_into. First step in #2082
1 parent 18f441a commit 82d436d

File tree

2 files changed

+174
-64
lines changed

2 files changed

+174
-64
lines changed

‎shared-bindings/bitbangio/I2C.c

Lines changed: 89 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,23 @@ MP_DEFINE_CONST_FUN_OBJ_1(bitbangio_i2c_unlock_obj, bitbangio_i2c_obj_unlock);
175175
//| :param int start: Index to start writing at
176176
//| :param int end: Index to write up to but not include
177177
//|
178+
// Shared arg parsing for readfrom_into and writeto_then_readfrom.
179+
STATIC void readfrom(bitbangio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffer, int32_t start, mp_int_t end) {
180+
mp_buffer_info_t bufinfo;
181+
mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_WRITE);
182+
183+
uint32_t length = bufinfo.len;
184+
normalize_buffer_bounds(&start, end, &length);
185+
if (length == 0) {
186+
mp_raise_ValueError(translate("Buffer must be at least length 1"));
187+
}
188+
189+
uint8_t status = shared_module_bitbangio_i2c_read(self, address, ((uint8_t*)bufinfo.buf) + start, length);
190+
if (status != 0) {
191+
mp_raise_OSError(status);
192+
}
193+
}
194+
178195
STATIC mp_obj_t bitbangio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
179196
enum { ARG_address, ARG_buffer, ARG_start, ARG_end };
180197
static const mp_arg_t allowed_args[] = {
@@ -185,33 +202,21 @@ STATIC mp_obj_t bitbangio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_a
185202
};
186203
bitbangio_i2c_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
187204
check_for_deinit(self);
205+
check_lock(self);
188206
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
189207
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
190-
check_lock(self);
191-
mp_buffer_info_t bufinfo;
192-
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
193208

194-
int32_t start = args[ARG_start].u_int;
195-
uint32_t length = bufinfo.len;
196-
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
197-
if (length == 0) {
198-
mp_raise_ValueError(translate("Buffer must be at least length 1"));
199-
}
200-
uint8_t status = shared_module_bitbangio_i2c_read(self,
201-
args[ARG_address].u_int,
202-
((uint8_t*)bufinfo.buf) + start,
203-
length);
204-
if (status != 0) {
205-
mp_raise_OSError(status);
206-
}
209+
readfrom(self, args[ARG_address].u_int, args[ARG_buffer].u_obj, args[ARG_start].u_int,
210+
args[ARG_end].u_int);
207211
return mp_const_none;
208212
}
209213
MP_DEFINE_CONST_FUN_OBJ_KW(bitbangio_i2c_readfrom_into_obj, 3, bitbangio_i2c_readfrom_into);
210214

211215
//| .. method:: writeto(address, buffer, *, start=0, end=None, stop=True)
212216
//|
213-
//| Write the bytes from ``buffer`` to the slave specified by ``address``.
214-
//| Transmits a stop bit if ``stop`` is set.
217+
//| Write the bytes from ``buffer`` to the slave specified by ``address`` and then transmits a
218+
//| stop bit. Use `writeto_then_readfrom` when needing a write, no stop and repeated start
219+
//| before a read.
215220
//|
216221
//| If ``start`` or ``end`` is provided, then the buffer will be sliced
217222
//| as if ``buffer[start:end]``. This will not cause an allocation like
@@ -224,9 +229,27 @@ MP_DEFINE_CONST_FUN_OBJ_KW(bitbangio_i2c_readfrom_into_obj, 3, bitbangio_i2c_rea
224229
//| :param bytearray buffer: buffer containing the bytes to write
225230
//| :param int start: Index to start writing from
226231
//| :param int end: Index to read up to but not include
227-
//| :param bool stop: If true, output an I2C stop condition after the
228-
//| buffer is written
232+
//| :param bool stop: If true, output an I2C stop condition after the buffer is written.
233+
//| Deprecated. Will be removed in 6.x and act as stop=True.
229234
//|
235+
// Shared arg parsing for writeto and writeto_then_readfrom.
236+
STATIC void writeto(bitbangio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffer, int32_t start, mp_int_t end, bool stop) {
237+
// get the buffer to write the data from
238+
mp_buffer_info_t bufinfo;
239+
mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_READ);
240+
241+
uint32_t length = bufinfo.len;
242+
normalize_buffer_bounds(&start, end, &length);
243+
244+
// do the transfer
245+
uint8_t status = shared_module_bitbangio_i2c_write(self, address,
246+
((uint8_t*) bufinfo.buf) + start, length,
247+
stop);
248+
if (status != 0) {
249+
mp_raise_OSError(status);
250+
}
251+
}
252+
230253
STATIC mp_obj_t bitbangio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
231254
enum { ARG_address, ARG_buffer, ARG_start, ARG_end, ARG_stop };
232255
static const mp_arg_t allowed_args[] = {
@@ -242,23 +265,55 @@ STATIC mp_obj_t bitbangio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, m
242265
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
243266
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
244267

245-
// get the buffer to write the data from
246-
mp_buffer_info_t bufinfo;
247-
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
268+
writeto(self, args[ARG_address].u_int, args[ARG_buffer].u_obj, args[ARG_start].u_int,
269+
args[ARG_end].u_int, args[ARG_stop].u_bool);
270+
return mp_const_none;
271+
}
272+
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bitbangio_i2c_writeto_obj, 1, bitbangio_i2c_writeto);
248273

249-
int32_t start = args[ARG_start].u_int;
250-
uint32_t length = bufinfo.len;
251-
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
252274

253-
// do the transfer
254-
uint8_t status = shared_module_bitbangio_i2c_write(self, args[ARG_address].u_int,
255-
((uint8_t*) bufinfo.buf) + start, length, args[ARG_stop].u_bool);
256-
if (status != 0) {
257-
mp_raise_OSError(status);
258-
}
275+
//| .. method:: writeto_then_readfrom(address, out_buffer, in_buffer, *, out_start=0, out_end=None, in_start=0, in_end=None)
276+
//|
277+
//| Write the bytes from ``out_buffer`` to the slave specified by ``address``, generate no stop
278+
//| bit, generate a repeated start and read into ``in_buffer``.
279+
//|
280+
//| If ``start`` or ``end`` is provided, then the corresponding buffer will be sliced
281+
//| as if ``buffer[start:end]``. This will not cause an allocation like ``buf[start:end]``
282+
//| will so it saves memory.
283+
//|
284+
//| :param int address: 7-bit device address
285+
//| :param bytearray out_buffer: buffer containing the bytes to write
286+
//| :param bytearray in_buffer: buffer to write into
287+
//| :param int out_start: Index to start writing from
288+
//| :param int out_end: Index to read up to but not include. Defaults to ``len(buffer)``
289+
//| :param int in_start: Index to start writing at
290+
//| :param int in_end: Index to write up to but not include. Defaults to ``len(buffer)``
291+
//|
292+
STATIC mp_obj_t bitbangio_i2c_writeto_then_readfrom(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
293+
enum { ARG_address, ARG_out_buffer, ARG_in_buffer, ARG_out_start, ARG_out_end, ARG_in_start, ARG_in_end };
294+
static const mp_arg_t allowed_args[] = {
295+
{ MP_QSTR_address, MP_ARG_REQUIRED | MP_ARG_INT, {.u_int = 0} },
296+
{ MP_QSTR_out_buffer, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
297+
{ MP_QSTR_in_buffer, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
298+
{ MP_QSTR_out_start, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0} },
299+
{ MP_QSTR_out_end, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = INT_MAX} },
300+
{ MP_QSTR_in_start, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0} },
301+
{ MP_QSTR_in_end, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = INT_MAX} },
302+
};
303+
bitbangio_i2c_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
304+
check_for_deinit(self);
305+
check_lock(self);
306+
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
307+
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
308+
309+
writeto(self, args[ARG_address].u_int, args[ARG_out_buffer].u_obj, args[ARG_out_start].u_int,
310+
args[ARG_out_end].u_int, false);
311+
readfrom(self, args[ARG_address].u_int, args[ARG_in_buffer].u_obj, args[ARG_in_start].u_int,
312+
args[ARG_in_end].u_int);
313+
259314
return mp_const_none;
260315
}
261-
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bitbangio_i2c_writeto_obj, 1, bitbangio_i2c_writeto);
316+
MP_DEFINE_CONST_FUN_OBJ_KW(bitbangio_i2c_writeto_then_readfrom_obj, 3, bitbangio_i2c_writeto_then_readfrom);
262317

263318
STATIC const mp_rom_map_elem_t bitbangio_i2c_locals_dict_table[] = {
264319
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&bitbangio_i2c_deinit_obj) },
@@ -271,6 +326,7 @@ STATIC const mp_rom_map_elem_t bitbangio_i2c_locals_dict_table[] = {
271326

272327
{ MP_ROM_QSTR(MP_QSTR_writeto), MP_ROM_PTR(&bitbangio_i2c_writeto_obj) },
273328
{ MP_ROM_QSTR(MP_QSTR_readfrom_into), MP_ROM_PTR(&bitbangio_i2c_readfrom_into_obj) },
329+
{ MP_ROM_QSTR(MP_QSTR_writeto_then_readfrom), MP_ROM_PTR(&bitbangio_i2c_writeto_then_readfrom_obj) },
274330
};
275331

276332
STATIC MP_DEFINE_CONST_DICT(bitbangio_i2c_locals_dict, bitbangio_i2c_locals_dict_table);

‎shared-bindings/busio/I2C.c

Lines changed: 85 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,23 @@ MP_DEFINE_CONST_FUN_OBJ_1(busio_i2c_unlock_obj, busio_i2c_obj_unlock);
189189
//| :param int start: Index to start writing at
190190
//| :param int end: Index to write up to but not include. Defaults to ``len(buffer)``
191191
//|
192+
// Shared arg parsing for readfrom_into and writeto_then_readfrom.
193+
STATIC void readfrom(busio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffer, int32_t start, mp_int_t end) {
194+
mp_buffer_info_t bufinfo;
195+
mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_WRITE);
196+
197+
uint32_t length = bufinfo.len;
198+
normalize_buffer_bounds(&start, end, &length);
199+
if (length == 0) {
200+
mp_raise_ValueError(translate("Buffer must be at least length 1"));
201+
}
202+
203+
uint8_t status = common_hal_busio_i2c_read(self, address, ((uint8_t*)bufinfo.buf) + start, length);
204+
if (status != 0) {
205+
mp_raise_OSError(status);
206+
}
207+
}
208+
192209
STATIC mp_obj_t busio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
193210
enum { ARG_address, ARG_buffer, ARG_start, ARG_end };
194211
static const mp_arg_t allowed_args[] = {
@@ -203,29 +220,18 @@ STATIC mp_obj_t busio_i2c_readfrom_into(size_t n_args, const mp_obj_t *pos_args,
203220
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
204221
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
205222

206-
mp_buffer_info_t bufinfo;
207-
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_WRITE);
208-
209-
int32_t start = args[ARG_start].u_int;
210-
uint32_t length = bufinfo.len;
211-
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
212-
if (length == 0) {
213-
mp_raise_ValueError(translate("Buffer must be at least length 1"));
214-
}
215-
216-
uint8_t status = common_hal_busio_i2c_read(self, args[ARG_address].u_int, ((uint8_t*)bufinfo.buf) + start, length);
217-
if (status != 0) {
218-
mp_raise_OSError(status);
219-
}
220-
223+
readfrom(self, args[ARG_address].u_int, args[ARG_buffer].u_obj, args[ARG_start].u_int,
224+
args[ARG_end].u_int);
221225
return mp_const_none;
222226
}
223227
MP_DEFINE_CONST_FUN_OBJ_KW(busio_i2c_readfrom_into_obj, 3, busio_i2c_readfrom_into);
224228

225229
//| .. method:: writeto(address, buffer, *, start=0, end=None, stop=True)
226230
//|
227231
//| Write the bytes from ``buffer`` to the slave specified by ``address``.
228-
//| Transmits a stop bit if ``stop`` is set.
232+
//| Transmits a stop bit when stop is True. Setting stop=False is deprecated and stop will be
233+
//| removed in CircuitPython 6.x. Use `writeto_then_readfrom` when needing a write, no stop and
234+
//| repeated start before a read.
229235
//|
230236
//| If ``start`` or ``end`` is provided, then the buffer will be sliced
231237
//| as if ``buffer[start:end]``. This will not cause an allocation like
@@ -238,9 +244,26 @@ MP_DEFINE_CONST_FUN_OBJ_KW(busio_i2c_readfrom_into_obj, 3, busio_i2c_readfrom_in
238244
//| :param bytearray buffer: buffer containing the bytes to write
239245
//| :param int start: Index to start writing from
240246
//| :param int end: Index to read up to but not include. Defaults to ``len(buffer)``
241-
//| :param bool stop: If true, output an I2C stop condition after the
242-
//| buffer is written
247+
//| :param bool stop: If true, output an I2C stop condition after the buffer is written.
248+
//| Deprecated. Will be removed in 6.x and act as stop=True.
243249
//|
250+
// Shared arg parsing for writeto and writeto_then_readfrom.
251+
STATIC void writeto(busio_i2c_obj_t *self, mp_int_t address, mp_obj_t buffer, int32_t start, mp_int_t end, bool stop) {
252+
// get the buffer to write the data from
253+
mp_buffer_info_t bufinfo;
254+
mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_READ);
255+
256+
uint32_t length = bufinfo.len;
257+
normalize_buffer_bounds(&start, end, &length);
258+
259+
// do the transfer
260+
uint8_t status = common_hal_busio_i2c_write(self, address, ((uint8_t*) bufinfo.buf) + start,
261+
length, stop);
262+
if (status != 0) {
263+
mp_raise_OSError(status);
264+
}
265+
}
266+
244267
STATIC mp_obj_t busio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
245268
enum { ARG_address, ARG_buffer, ARG_start, ARG_end, ARG_stop };
246269
static const mp_arg_t allowed_args[] = {
@@ -256,24 +279,54 @@ STATIC mp_obj_t busio_i2c_writeto(size_t n_args, const mp_obj_t *pos_args, mp_ma
256279
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
257280
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
258281

259-
// get the buffer to write the data from
260-
mp_buffer_info_t bufinfo;
261-
mp_get_buffer_raise(args[ARG_buffer].u_obj, &bufinfo, MP_BUFFER_READ);
282+
writeto(self, args[ARG_address].u_int, args[ARG_buffer].u_obj, args[ARG_start].u_int,
283+
args[ARG_end].u_int, args[ARG_stop].u_bool);
284+
return mp_const_none;
285+
}
286+
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(busio_i2c_writeto_obj, 1, busio_i2c_writeto);
262287

288+
//| .. method:: writeto_then_readfrom(address, out_buffer, in_buffer, *, out_start=0, out_end=None, in_start=0, in_end=None)
289+
//|
290+
//| Write the bytes from ``out_buffer`` to the slave specified by ``address``, generate no stop
291+
//| bit, generate a repeated start and read into ``in_buffer``.
292+
//|
293+
//| If ``start`` or ``end`` is provided, then the corresponding buffer will be sliced
294+
//| as if ``buffer[start:end]``. This will not cause an allocation like ``buf[start:end]``
295+
//| will so it saves memory.
296+
//|
297+
//| :param int address: 7-bit device address
298+
//| :param bytearray out_buffer: buffer containing the bytes to write
299+
//| :param bytearray in_buffer: buffer to write into
300+
//| :param int out_start: Index to start writing from
301+
//| :param int out_end: Index to read up to but not include. Defaults to ``len(buffer)``
302+
//| :param int in_start: Index to start writing at
303+
//| :param int in_end: Index to write up to but not include. Defaults to ``len(buffer)``
304+
//|
305+
STATIC mp_obj_t busio_i2c_writeto_then_readfrom(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
306+
enum { ARG_address, ARG_out_buffer, ARG_in_buffer, ARG_out_start, ARG_out_end, ARG_in_start, ARG_in_end };
307+
static const mp_arg_t allowed_args[] = {
308+
{ MP_QSTR_address, MP_ARG_REQUIRED | MP_ARG_INT, {.u_int = 0} },
309+
{ MP_QSTR_out_buffer, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
310+
{ MP_QSTR_in_buffer, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} },
311+
{ MP_QSTR_out_start, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0} },
312+
{ MP_QSTR_out_end, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = INT_MAX} },
313+
{ MP_QSTR_in_start, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0} },
314+
{ MP_QSTR_in_end, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = INT_MAX} },
315+
};
316+
busio_i2c_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
317+
check_for_deinit(self);
318+
check_lock(self);
319+
mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
320+
mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
263321

264-
int32_t start = args[ARG_start].u_int;
265-
uint32_t length = bufinfo.len;
266-
normalize_buffer_bounds(&start, args[ARG_end].u_int, &length);
322+
writeto(self, args[ARG_address].u_int, args[ARG_out_buffer].u_obj, args[ARG_out_start].u_int,
323+
args[ARG_out_end].u_int, false);
324+
readfrom(self, args[ARG_address].u_int, args[ARG_in_buffer].u_obj, args[ARG_in_start].u_int,
325+
args[ARG_in_end].u_int);
267326

268-
// do the transfer
269-
uint8_t status = common_hal_busio_i2c_write(self, args[ARG_address].u_int,
270-
((uint8_t*) bufinfo.buf) + start, length, args[ARG_stop].u_bool);
271-
if (status != 0) {
272-
mp_raise_OSError(status);
273-
}
274327
return mp_const_none;
275328
}
276-
STATIC MP_DEFINE_CONST_FUN_OBJ_KW(busio_i2c_writeto_obj, 1, busio_i2c_writeto);
329+
MP_DEFINE_CONST_FUN_OBJ_KW(busio_i2c_writeto_then_readfrom_obj, 3, busio_i2c_writeto_then_readfrom);
277330

278331
STATIC const mp_rom_map_elem_t busio_i2c_locals_dict_table[] = {
279332
{ MP_ROM_QSTR(MP_QSTR_deinit), MP_ROM_PTR(&busio_i2c_deinit_obj) },
@@ -286,6 +339,7 @@ STATIC const mp_rom_map_elem_t busio_i2c_locals_dict_table[] = {
286339

287340
{ MP_ROM_QSTR(MP_QSTR_readfrom_into), MP_ROM_PTR(&busio_i2c_readfrom_into_obj) },
288341
{ MP_ROM_QSTR(MP_QSTR_writeto), MP_ROM_PTR(&busio_i2c_writeto_obj) },
342+
{ MP_ROM_QSTR(MP_QSTR_writeto_then_readfrom), MP_ROM_PTR(&busio_i2c_writeto_then_readfrom_obj) },
289343
};
290344

291345
STATIC MP_DEFINE_CONST_DICT(busio_i2c_locals_dict, busio_i2c_locals_dict_table);

0 commit comments

Comments
 (0)